Skip to content

Commit b84b33c

Browse files
committed
Addressed copilot's comments
1 parent 348e7d7 commit b84b33c

3 files changed

Lines changed: 131 additions & 42 deletions

File tree

docs/API.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ struct wolfIP_ll_dev {
2727
uint8_t mac[6]; // Device MAC address
2828
char ifname[16]; // Interface name
2929
uint8_t non_ethernet; // L3-only link (no Ethernet header/ARP when set)
30-
uint32_t mtu; // Optional max frame size, defaults to LINK_MTU
30+
uint32_t mtu; // Optional internal frame budget, defaults to LINK_MTU
3131
int (*poll)(struct wolfIP_ll_dev *ll, void *buf, uint32_t len); // Receive function
3232
int (*send)(struct wolfIP_ll_dev *ll, void *buf, uint32_t len); // Transmit function
3333
};
3434
```
3535
wolfIP maintains an array of these descriptors sized by `WOLFIP_MAX_INTERFACES` (default `1`). Call `wolfIP_getdev_ex()` to access a specific slot; the legacy `wolfIP_getdev()` helper targets the first hardware slot (index `0` normally, or `1` when the optional loopback interface is enabled).
3636

3737
When `non_ethernet` is set, the interface is treated as L3-only point-to-point: the stack skips ARP/neighbor resolution, omits Ethernet headers on transmit, and expects receive buffers to begin at the IP header.
38+
The `mtu` field still describes wolfIP's internal frame budget including Ethernet headroom, so on non-Ethernet links the payload passed to `ll->send()` is effectively capped at `mtu - ETH_HEADER_LEN` on Ethernet-enabled builds.
3839

3940
### IP Configuration
4041
```c
@@ -209,6 +210,7 @@ int wolfIP_mtu_get(struct wolfIP *s, unsigned int if_idx, uint32_t *mtu);
209210
```
210211
Access the link-layer descriptor(s) that should be wired to hardware drivers. `_ex` returns `NULL` if `if_idx` exceeds `WOLFIP_MAX_INTERFACES`.
211212
`wolfIP_mtu_set()` updates the effective per-interface MTU, treating `0` as the default `LINK_MTU` and clamping to `[LINK_MTU_MIN, LINK_MTU]`. `wolfIP_mtu_get()` returns the effective MTU currently used by the stack.
213+
For `non_ethernet` devices this value remains the internal frame budget; the maximum IP bytes handed to the driver are reduced by `ETH_HEADER_LEN` when Ethernet support is compiled in.
212214
213215
## DHCP Client Functions
214216

src/test/unit/unit.c

Lines changed: 75 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9233,7 +9233,7 @@ START_TEST(test_arp_queue_packet_uses_empty_slot)
92339233
}
92349234
END_TEST
92359235

9236-
START_TEST(test_arp_queue_packet_truncates_len)
9236+
START_TEST(test_arp_queue_packet_drops_oversize_len)
92379237
{
92389238
struct wolfIP s;
92399239
uint8_t ip_buf[LINK_MTU];
@@ -9245,7 +9245,8 @@ START_TEST(test_arp_queue_packet_truncates_len)
92459245
memset(ip_buf, 0, sizeof(ip_buf));
92469246

92479247
arp_queue_packet(&s, TEST_PRIMARY_IF, 0x0A0000A3U, ip, len);
9248-
ck_assert_uint_eq(s.arp_pending[0].len, LINK_MTU);
9248+
ck_assert_uint_eq(s.arp_pending[0].len, 0U);
9249+
ck_assert_uint_eq(s.arp_pending[0].dest, IPADDR_ANY);
92499250
}
92509251
END_TEST
92519252

@@ -10544,6 +10545,24 @@ START_TEST(test_ll_send_frame_non_ethernet_short_len)
1054410545
}
1054510546
END_TEST
1054610547

10548+
START_TEST(test_ll_send_frame_drops_oversize)
10549+
{
10550+
struct wolfIP s;
10551+
uint8_t buf[ETH_HEADER_LEN + IP_HEADER_LEN + 32];
10552+
10553+
wolfIP_init(&s);
10554+
mock_link_init(&s);
10555+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_PRIMARY_IF,
10556+
(uint32_t)(sizeof(buf) - 1U)), 0);
10557+
10558+
memset(buf, 0, sizeof(buf));
10559+
last_frame_sent_size = 0;
10560+
10561+
wolfIP_ll_send_frame(&s, TEST_PRIMARY_IF, buf, (uint32_t)sizeof(buf));
10562+
ck_assert_uint_eq(last_frame_sent_size, 0U);
10563+
}
10564+
END_TEST
10565+
1054710566
START_TEST(test_ll_helpers_invalid_inputs)
1054810567
{
1054910568
struct wolfIP s;
@@ -13816,7 +13835,7 @@ START_TEST(test_tcp_connect_syn_advertises_interface_mss)
1381613835

1381713836
wolfIP_init(&s);
1381813837
mock_link_init(&s);
13819-
wolfIP_getdev(&s)->mtu = 640U;
13838+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_PRIMARY_IF, 640U), 0);
1382013839
wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0);
1382113840

1382213841
tcp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
@@ -13843,6 +13862,49 @@ START_TEST(test_tcp_connect_syn_advertises_interface_mss)
1384313862
}
1384413863
END_TEST
1384513864

13865+
START_TEST(test_tcp_connect_syn_limits_options_to_small_mtu)
13866+
{
13867+
struct wolfIP s;
13868+
int tcp_sd;
13869+
struct tsocket *ts;
13870+
struct wolfIP_sockaddr_in sin;
13871+
struct pkt_desc *desc;
13872+
struct wolfIP_tcp_seg *syn;
13873+
struct tcp_parsed_opts po;
13874+
uint32_t mtu = 0;
13875+
13876+
wolfIP_init(&s);
13877+
mock_link_init(&s);
13878+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_PRIMARY_IF, 64U), 0);
13879+
ck_assert_int_eq(wolfIP_mtu_get(&s, TEST_PRIMARY_IF, &mtu), 0);
13880+
ck_assert_uint_eq(mtu, 64U);
13881+
wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0);
13882+
13883+
tcp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
13884+
ck_assert_int_gt(tcp_sd, 0);
13885+
ts = &s.tcpsockets[SOCKET_UNMARK(tcp_sd)];
13886+
13887+
memset(&sin, 0, sizeof(sin));
13888+
sin.sin_family = AF_INET;
13889+
sin.sin_port = ee16(5006);
13890+
sin.sin_addr.s_addr = ee32(0x0A000002U);
13891+
ck_assert_int_eq(wolfIP_sock_connect(&s, tcp_sd,
13892+
(struct wolfIP_sockaddr *)&sin, sizeof(sin)), -WOLFIP_EAGAIN);
13893+
13894+
desc = fifo_peek(&ts->sock.tcp.txbuf);
13895+
ck_assert_ptr_nonnull(desc);
13896+
ck_assert_uint_le(desc->len, mtu);
13897+
syn = (struct wolfIP_tcp_seg *)(ts->txmem + desc->pos + sizeof(*desc));
13898+
13899+
memset(&po, 0, sizeof(po));
13900+
tcp_parse_options(syn, desc->len, &po);
13901+
ck_assert_int_eq(po.mss_found, 1);
13902+
ck_assert_int_eq(po.ws_found, 1);
13903+
ck_assert_int_eq(po.sack_permitted, 0);
13904+
ck_assert_int_eq(po.ts_found, 0);
13905+
}
13906+
END_TEST
13907+
1384613908
START_TEST(test_sock_sendto_tcp_respects_negotiated_peer_mss)
1384713909
{
1384813910
struct wolfIP s;
@@ -14018,7 +14080,7 @@ START_TEST(test_sock_sendto_tcp_respects_interface_mtu)
1401814080

1401914081
wolfIP_init(&s);
1402014082
mock_link_init(&s);
14021-
wolfIP_getdev(&s)->mtu = 320U;
14083+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_PRIMARY_IF, 320U), 0);
1402214084
wolfIP_ipconfig_set(&s, 0x0A000001U, 0xFFFFFF00U, 0);
1402314085

1402414086
tcp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_STREAM, WI_IPPROTO_TCP);
@@ -17409,7 +17471,7 @@ START_TEST(test_wolfip_ll_frame_mtu_enforces_minimum)
1740917471
ll = wolfIP_getdev(&s);
1741017472
ck_assert_ptr_nonnull(ll);
1741117473

17412-
ll->mtu = LINK_MTU_MIN - 1U;
17474+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_PRIMARY_IF, LINK_MTU_MIN - 1U), 0);
1741317475
ck_assert_uint_eq(wolfIP_ll_frame_mtu(ll), LINK_MTU_MIN);
1741417476
}
1741517477
END_TEST
@@ -17507,7 +17569,7 @@ START_TEST(test_wolfip_loopback_send_paths)
1750717569
}
1750817570
END_TEST
1750917571

17510-
START_TEST(test_wolfip_loopback_send_truncates)
17572+
START_TEST(test_wolfip_loopback_send_drops_oversize)
1751117573
{
1751217574
struct wolfIP s;
1751317575
struct wolfIP_ll_dev *loop;
@@ -17518,10 +17580,10 @@ START_TEST(test_wolfip_loopback_send_truncates)
1751817580
ck_assert_ptr_nonnull(loop);
1751917581

1752017582
memset(frame, 0xAB, sizeof(frame));
17521-
ck_assert_int_eq(wolfIP_loopback_send(loop, frame, (uint32_t)sizeof(frame)), LINK_MTU);
17583+
ck_assert_int_eq(wolfIP_loopback_send(loop, frame, (uint32_t)sizeof(frame)), 0);
1752217584

17523-
loop->mtu = LINK_MTU_MIN - 1U;
17524-
ck_assert_int_eq(wolfIP_loopback_send(loop, frame, (uint32_t)sizeof(frame)), LINK_MTU_MIN);
17585+
ck_assert_int_eq(wolfIP_mtu_set(&s, TEST_LOOPBACK_IF, LINK_MTU_MIN - 1U), 0);
17586+
ck_assert_int_eq(wolfIP_loopback_send(loop, frame, (uint32_t)sizeof(frame)), 0);
1752517587
}
1752617588
END_TEST
1752717589

@@ -18994,7 +19056,7 @@ Suite *wolf_suite(void)
1899419056
#if WOLFIP_ENABLE_LOOPBACK
1899519057
tcase_add_test(tc_utils, test_wolfip_loopback_defaults);
1899619058
tcase_add_test(tc_utils, test_wolfip_loopback_send_paths);
18997-
tcase_add_test(tc_utils, test_wolfip_loopback_send_truncates);
19059+
tcase_add_test(tc_utils, test_wolfip_loopback_send_drops_oversize);
1899819060
tcase_add_test(tc_utils, test_wolfip_loopback_send_null_container);
1899919061
#endif
1900019062
tcase_add_test(tc_utils, test_wolfip_ipconfig_ex_per_interface);
@@ -19217,6 +19279,7 @@ Suite *wolf_suite(void)
1921719279
tcase_add_test(tc_utils, test_forward_packet_filter_drop_udp_icmp);
1921819280
tcase_add_test(tc_utils, test_ll_send_frame_non_ethernet_strips);
1921919281
tcase_add_test(tc_utils, test_ll_send_frame_non_ethernet_short_len);
19282+
tcase_add_test(tc_utils, test_ll_send_frame_drops_oversize);
1922019283
tcase_add_test(tc_utils, test_ll_helpers_invalid_inputs);
1922119284
tcase_add_test(tc_utils, test_non_ethernet_recv_oversize_dropped);
1922219285
#endif
@@ -19311,6 +19374,7 @@ Suite *wolf_suite(void)
1931119374
tcase_add_test(tc_utils, test_tcp_input_peer_rwnd_growth_sets_writable);
1931219375
tcase_add_test(tc_utils, test_tcp_input_synack_negotiates_peer_mss);
1931319376
tcase_add_test(tc_utils, test_tcp_connect_syn_advertises_interface_mss);
19377+
tcase_add_test(tc_utils, test_tcp_connect_syn_limits_options_to_small_mtu);
1931419378
tcase_add_test(tc_utils, test_sock_sendto_tcp_respects_negotiated_peer_mss);
1931519379
tcase_add_test(tc_utils, test_sock_sendto_tcp_defaults_to_rfc_mss_when_unset_by_peer);
1931619380
tcase_add_test(tc_utils, test_sock_sendto_tcp_respects_interface_mtu);
@@ -19414,7 +19478,7 @@ Suite *wolf_suite(void)
1941419478
tcase_add_test(tc_proto, test_arp_queue_packet_reuses_existing_slot);
1941519479
tcase_add_test(tc_proto, test_arp_queue_packet_same_dest_different_if);
1941619480
tcase_add_test(tc_proto, test_arp_queue_packet_uses_empty_slot);
19417-
tcase_add_test(tc_proto, test_arp_queue_packet_truncates_len);
19481+
tcase_add_test(tc_proto, test_arp_queue_packet_drops_oversize_len);
1941819482
tcase_add_test(tc_proto, test_arp_queue_packet_slot_fallback_zero);
1941919483
tcase_add_test(tc_proto, test_arp_flush_pending_no_neighbor);
1942019484
tcase_add_test(tc_proto, test_arp_flush_pending_len_zero_clears);

src/wolfip.c

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,10 @@ struct wolfIP_icmp_packet;
114114

115115
#define NO_TIMER 0
116116

117-
#define WI_IP_MTU (LINK_MTU - ETH_HEADER_LEN)
118-
#define TCP_MSS (WI_IP_MTU - (IP_HEADER_LEN + TCP_HEADER_LEN))
117+
#define IP_MTU_MAX 1500U
118+
#define WI_IP_STD_MTU IP_MTU_MAX
119+
#define WI_IP_MTU WI_IP_STD_MTU
120+
#define TCP_MSS (WI_IP_STD_MTU - (IP_HEADER_LEN + TCP_HEADER_LEN))
119121
#define TCP_DEFAULT_MSS 536U
120122
#define TCP_CTRL_RTO_MAXRTX 6U
121123
#define TCP_RTO_MAX_BACKOFF 15U /* Max retries before closing; also clamps shift */
@@ -1248,7 +1250,10 @@ static inline uint32_t wolfIP_ip_mtu(struct wolfIP *s, unsigned int if_idx)
12481250

12491251
if (mtu <= ETH_HEADER_LEN)
12501252
return 0;
1251-
return mtu - ETH_HEADER_LEN;
1253+
mtu -= ETH_HEADER_LEN;
1254+
if (mtu > IP_MTU_MAX)
1255+
mtu = IP_MTU_MAX;
1256+
return mtu;
12521257
}
12531258

12541259
static inline uint32_t wolfIP_socket_ip_mtu(const struct tsocket *t)
@@ -1297,7 +1302,7 @@ static int wolfIP_loopback_send(struct wolfIP_ll_dev *ll, void *buf, uint32_t le
12971302
if (!s)
12981303
return -1;
12991304
if (copy > wolfIP_ll_frame_mtu(ll))
1300-
copy = wolfIP_ll_frame_mtu(ll);
1305+
return 0;
13011306
memcpy(frame, buf, copy);
13021307
wolfIP_recv_on(s, WOLFIP_LOOPBACK_IF_IDX, frame, copy);
13031308
return (int)copy;
@@ -1330,7 +1335,7 @@ static inline void wolfIP_ll_send_frame(struct wolfIP *s, unsigned int if_idx,
13301335
return;
13311336
frame_mtu = wolfIP_ll_frame_mtu(ll);
13321337
if (len > frame_mtu)
1333-
len = frame_mtu;
1338+
return;
13341339
if (ll->non_ethernet) {
13351340
if (len <= ETH_HEADER_LEN)
13361341
return;
@@ -2250,6 +2255,8 @@ static void tcp_send_syn(struct tsocket *t, uint8_t flags)
22502255
uint8_t include_sack = 0;
22512256
uint8_t include_ts = 0;
22522257
uint8_t opt_len = 0;
2258+
uint8_t max_opt_len = 0;
2259+
uint32_t ip_mtu = wolfIP_socket_ip_mtu(t);
22532260
tcp = (struct wolfIP_tcp_seg *)buffer;
22542261
memset(tcp, 0, sizeof(buffer));
22552262
if (flags & TCP_FLAG_SYN) {
@@ -2265,6 +2272,12 @@ static void tcp_send_syn(struct tsocket *t, uint8_t flags)
22652272
include_ts = t->sock.tcp.ts_offer;
22662273
}
22672274
}
2275+
if (ip_mtu > (IP_HEADER_LEN + TCP_HEADER_LEN)) {
2276+
uint32_t opt_budget = ip_mtu - (IP_HEADER_LEN + TCP_HEADER_LEN);
2277+
if (opt_budget > TCP_MAX_OPTIONS_LEN)
2278+
opt_budget = TCP_MAX_OPTIONS_LEN;
2279+
max_opt_len = (uint8_t)opt_budget;
2280+
}
22682281
tcp->src_port = ee16(t->src_port);
22692282
tcp->dst_port = ee16(t->dst_port);
22702283
tcp->seq = ee32(t->sock.tcp.seq);
@@ -2274,37 +2287,41 @@ static void tcp_send_syn(struct tsocket *t, uint8_t flags)
22742287
tcp->csum = 0;
22752288
tcp->urg = 0;
22762289
opt = tcp->data;
2277-
if (include_ts) {
2278-
ts = (struct tcp_opt_ts *)opt;
2279-
ts->opt = TCP_OPTION_TS;
2280-
ts->len = TCP_OPTION_TS_LEN;
2281-
ts->val = ee32(t->S->last_tick & 0xFFFFFFFFU);
2282-
ts->ecr = t->sock.tcp.last_ts;
2283-
ts->pad = TCP_OPTION_NOP;
2284-
ts->eoo = TCP_OPTION_NOP;
2285-
opt += sizeof(*ts);
2286-
opt_len += sizeof(*ts);
2287-
}
2288-
mss = (struct tcp_opt_mss *)opt;
2289-
mss->opt = TCP_OPTION_MSS;
2290-
mss->len = TCP_OPTION_MSS_LEN;
2291-
mss->mss = ee16((uint16_t)wolfIP_socket_tcp_mss(t));
2292-
opt += sizeof(*mss);
2293-
opt_len += sizeof(*mss);
2294-
if (include_ws) {
2290+
if (max_opt_len >= sizeof(*mss)) {
2291+
mss = (struct tcp_opt_mss *)opt;
2292+
mss->opt = TCP_OPTION_MSS;
2293+
mss->len = TCP_OPTION_MSS_LEN;
2294+
mss->mss = ee16((uint16_t)wolfIP_socket_tcp_mss(t));
2295+
opt += sizeof(*mss);
2296+
opt_len += sizeof(*mss);
2297+
}
2298+
if (include_ws &&
2299+
((uint8_t)((opt_len + sizeof(*ws) + 3U) & ~3U) <= max_opt_len)) {
22952300
ws = (struct tcp_opt_ws *)opt;
22962301
ws->opt = TCP_OPTION_WS;
22972302
ws->len = TCP_OPTION_WS_LEN;
22982303
ws->shift = t->sock.tcp.rcv_wscale;
22992304
opt += sizeof(*ws);
23002305
opt_len += sizeof(*ws);
23012306
}
2302-
if (include_sack) {
2307+
if (include_sack &&
2308+
((uint8_t)((opt_len + TCP_OPTION_SACK_PERMITTED_LEN + 3U) & ~3U) <= max_opt_len)) {
23032309
*opt++ = TCP_OPTION_SACK_PERMITTED;
23042310
*opt++ = TCP_OPTION_SACK_PERMITTED_LEN;
23052311
opt_len += TCP_OPTION_SACK_PERMITTED_LEN;
23062312
}
2307-
while ((opt_len % 4) != 0 && opt_len < TCP_MAX_OPTIONS_LEN) {
2313+
if (include_ts && (uint8_t)(opt_len + sizeof(*ts)) <= max_opt_len) {
2314+
ts = (struct tcp_opt_ts *)opt;
2315+
ts->opt = TCP_OPTION_TS;
2316+
ts->len = TCP_OPTION_TS_LEN;
2317+
ts->val = ee32(t->S->last_tick & 0xFFFFFFFFU);
2318+
ts->ecr = t->sock.tcp.last_ts;
2319+
ts->pad = TCP_OPTION_NOP;
2320+
ts->eoo = TCP_OPTION_NOP;
2321+
opt += sizeof(*ts);
2322+
opt_len += sizeof(*ts);
2323+
}
2324+
while ((opt_len % 4) != 0 && opt_len < max_opt_len) {
23082325
*opt++ = TCP_OPTION_NOP;
23092326
opt_len++;
23102327
}
@@ -5253,9 +5270,9 @@ static void arp_queue_packet(struct wolfIP *s, unsigned int if_idx, ip4 dest,
52535270
if (!s || len == 0)
52545271
return;
52555272
if (len > LINK_MTU)
5256-
len = LINK_MTU;
5273+
return;
52575274
if (len > wolfIP_frame_mtu(s, if_idx))
5258-
len = wolfIP_frame_mtu(s, if_idx);
5275+
return;
52595276

52605277
for (i = 0; i < WOLFIP_ARP_PENDING_MAX; i++) {
52615278
if (s->arp_pending[i].dest == dest && s->arp_pending[i].if_idx == if_idx) {
@@ -5292,8 +5309,11 @@ static void arp_flush_pending(struct wolfIP *s, unsigned int if_idx, ip4 ip)
52925309
pending->dest = IPADDR_ANY;
52935310
continue;
52945311
}
5295-
if (pending->len > wolfIP_frame_mtu(s, if_idx))
5296-
pending->len = wolfIP_frame_mtu(s, if_idx);
5312+
if (pending->len > wolfIP_frame_mtu(s, if_idx)) {
5313+
pending->dest = IPADDR_ANY;
5314+
pending->len = 0;
5315+
continue;
5316+
}
52975317
{
52985318
struct wolfIP_ip_packet *pkt =
52995319
(struct wolfIP_ip_packet *)pending->frame;
@@ -6178,11 +6198,14 @@ int wolfIP_poll(struct wolfIP *s, uint64_t now)
61786198
continue;
61796199
do {
61806200
if (ll->non_ethernet) {
6201+
uint32_t frame_mtu = wolfIP_ll_frame_mtu(ll);
6202+
if (frame_mtu <= ETH_HEADER_LEN)
6203+
break;
61816204
#if ETH_HEADER_LEN > 0
61826205
memset(buf, 0, ETH_HEADER_LEN);
61836206
#endif
61846207
len = ll->poll(ll, buf + ETH_HEADER_LEN,
6185-
wolfIP_ll_frame_mtu(ll) - ETH_HEADER_LEN);
6208+
frame_mtu - ETH_HEADER_LEN);
61866209
if (len > 0)
61876210
len += ETH_HEADER_LEN;
61886211
} else {

0 commit comments

Comments
 (0)