On Mon, Jan 19, 2026 at 1:34 PM Chia-Yu Chang (Nokia) <[email protected]> wrote: > > > -----Original Message----- > > From: Neal Cardwell <[email protected]> > > Sent: Monday, January 19, 2026 1:06 AM > > To: Chia-Yu Chang (Nokia) <[email protected]> > > Cc: [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; > > [email protected]; [email protected]; [email protected]; Koen De > > Schepper (Nokia) <[email protected]>; > > [email protected]; [email protected]; > > [email protected]; cheshire <[email protected]>; > > [email protected]; [email protected]; Vidhi Goel > > <[email protected]>; Willem de Bruijn <[email protected]> > > Subject: Re: [PATCH net-next 1/1] selftests/net: Add packetdrill > > packetdrill cases > > > > > > CAUTION: This is an external email. Please be very careful when clicking > > links or opening attachments. See the URL nok.it/ext for additional > > information. > > > > > > > > On Sun, Jan 18, 2026 at 5:56 PM Chia-Yu Chang (Nokia) > > <[email protected]> wrote: > > > > > > > > > > -----Original Message----- > > > > From: Neal Cardwell <[email protected]> > > > > Sent: Sunday, January 18, 2026 5:11 PM > > > > To: Chia-Yu Chang (Nokia) <[email protected]> > > > > Cc: [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; > > > > [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; [email protected]; > > > > [email protected]; [email protected]; > > > > [email protected]; [email protected]; Koen De Schepper > > > > (Nokia) <[email protected]>; > > > > [email protected]; [email protected]; > > > > [email protected]; cheshire <[email protected]>; > > > > [email protected]; [email protected]; Vidhi Goel > > > > <[email protected]>; Willem de Bruijn <[email protected]> > > > > Subject: Re: [PATCH net-next 1/1] selftests/net: Add packetdrill > > > > packetdrill cases > > > > > > > > > > > > CAUTION: This is an external email. Please be very careful when > > > > clicking links or opening attachments. See the URL nok.it/ext for > > > > additional information. > > > > > > > > > > > > > > > > On Thu, Jan 8, 2026 at 5:46 PM Neal Cardwell <[email protected]> > > > > wrote: > > > > > > > > > > On Thu, Jan 8, 2026 at 10:58 AM <[email protected]> > > > > > wrote: > > > > > > > > > > > > From: Chia-Yu Chang <[email protected]> > > > > > > > > > > > > Linux Accurate ECN test sets using ACE counters and AccECN > > > > > > options to cover several scenarios: Connection teardown, > > > > > > different ACK conditions, counter wrapping, SACK space grabbing, > > > > > > fallback schemes, negotiation retransmission/reorder/loss, > > > > > > AccECN option drop/loss, different handshake reflectors, data with > > > > > > marking, and different sysctl values. > > > > > > > > > > > > Co-developed-by: Ilpo Järvinen <[email protected]> > > > > > > Signed-off-by: Ilpo Järvinen <[email protected]> > > > > > > Co-developed-by: Neal Cardwell <[email protected]> > > > > > > Signed-off-by: Neal Cardwell <[email protected]> > > > > > > --- > > > > > > > > > > Chia-Yu, thank you for posting the packetdrill tests. > > > > > > > > > > A couple thoughts: > > > > > > > > > > (1) These tests are using the experimental AccECN packetdrill > > > > > support that is not in mainline packetdrill yet. Can you please > > > > > share the github URL for the version of packetdrill you used? I > > > > > will work on merging the appropriate experimental AccECN > > > > > packetdrill support into the Google packetdrill mainline branch. > > > > > > > > An update on the 3 patches at: > > > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > thub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96&data=05%7C02%7Cchia-yu.c > > > > hang%40nokia-bell-labs.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d47 > > > > 17519675428d917b70f44f9630b0%7C0%7C0%7C639043779630263465%7CUnknown% > > > > 7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW > > > > 4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=l6PAGPTELL > > > > T3KwEWsuLQnLaxQ0BUixhbrFp3n6dA950%3D&reserved=0 > > > > > > > > (1) I have merged the following patch into the google packetdrill repo > > > > to facilitate testing of the AccECN patch series: > > > > > > > > "net-test: packetdrill: add Accurate ECN (AccECN) option support" > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > thub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96%2Fchanges%2Ff6861f888bc7 > > > > f1e08026de4825519a95504d1047&data=05%7C02%7Cchia-yu.chang%40nokia-be > > > > ll-labs.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d4717519675428d917 > > > > b70f44f9630b0%7C0%7C0%7C639043779630288017%7CUnknown%7CTWFpbGZsb3d8e > > > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT > > > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=wct5GueZNjBZHCaxpbokNBLXh > > > > pGpDFjKKUiIbCMiKco%3D&reserved=0 > > > > > > > > (2) The following patch I did not yet merge, because it proposes to add > > > > an odd number of u32 fields to tcp_info, so AFAICT leaves a 4-byte > > > > padding hole at the end of tcp_info: > > > > > > > > net-test: packetdrill: Support AccECN counters through tcpi > > > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > thub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96%2Fchanges%2Ff43649c87a2a > > > > a79a33a78111d3d7e5f027d13a7f&data=05%7C02%7Cchia-yu.chang%40nokia-be > > > > ll-labs.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d4717519675428d917 > > > > b70f44f9630b0%7C0%7C0%7C639043779630306235%7CUnknown%7CTWFpbGZsb3d8e > > > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT > > > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=HpI3cgq0qf1%2FrTCJACRAQZY > > > > ZRs473MS3p%2BUB0VWATKE%3D&reserved=0 > > > > > > > > I think we'll need to tweak the AccECN kernel patch series so that it > > > > does not leave a 4-byte padding hole at the end of tcp_info, then > > > > update this packetdrill patch to match the kernel patch. > > > > > > > > Let's come up with another useful u32 field we can add to the tcp_info > > > > struct, so that the kernel patch doesn't add a padding hole at the end > > > > of tcp_info. > > > > > > > > One idea would be to add another field to represent newer options and > > > > connection features that are enabled. AFAICT all 8 bits of the > > > > tcpi_options field have been used, so we can't use more bits in that > > > > field. I'd suggest we add a u32 tcpi_more_options field before the > > > > tcpi_received_ce field, so we can encode other useful info, like: > > > > > > > > + 1 bit to indicate whether AccECN was negotiated (this can go in a > > > > separate patch) > > > > > > > > + 1 bit to indicate whether TCP_NODELAY was set (since forgetting to > > > > use TCP_NODELAY is a classic cause of performance problems; again > > > > this can go in a separate patch) > > > > > > > > (And there will be future bits of info we want to add...) > > > > > > > > Also, regarding the comment in this line: > > > > __u32 tcpi_received_ce; /* # of CE marks received */ > > > > > > > > That comment is ambiguous, since it doesn't indicate whether it's > > > > counting (potentially LRO/GRO) skbs or TCP segments. I would suggest > > > > clarifying that this is counting segments: > > > > > > > > __u32 tcpi_received_ce; /* # of CE marked segments received */ > > > > > > > > > > Hi Neal, > > > > > > Related to these 32-bit hole, two extra entries are added into > > > b40671b5ee588c8a61b2d0eacbad32ffc57e9a8f of net-next, and one > > > straightforward way is to apply these changes also in tcp.h of > > > packetdrill (This is my miss). > > > > > > + __u16 tcpi_accecn_fail_mode; > > > + __u16 tcpi_accecn_opt_seen; > > > > > > But I would prefer to update this, because tcpi_accecn_fail_mode and > > > tcpi_accecn_opt_seen overall just needs 8 bits (i.e., 4 bits for > > > tcpi_accecn_fail_mode and 2 bits for tcpi_accecn_opt_seen). > > > > > > So, maybe we could add u16 tcpi_more_options before tcpi_received_ce and > > > change tcpi_accecn_fail_mode and tcpi_accecn_opt_seen both into u8. > > > Within tcpi_more_options, add one bit related to TCP_NODELAY as you said. > > > And within tcpi_accecn_opt_seen, add one bit related to whether AccECN > > > was negotiated as you said, then we can leave more unused bits in > > > tcpi_more_options. > > > > > > Another thought is to use a single u32 before tcpi_received_ce, in which > > > 4 bits for tcpi_accecn_fail_mode, 2 bits for tcpi_accecn_opt_seen, 26 > > > bits for tcpi_more_options. > > > > > > What do you think? > > > > I would suggest something like your last suggestion, where there is a > > u32 before tcpi_received_ce, with bit fields for tcpi_accecn_fail_mode > > (4 bits) and tcpi_accecn_opt_seen (2 bits), and a tcpi_options2 for the > > remaining unused bits in the u32. > > > > I am leaning toward tcpi_options2 rather than tcpi_more_options, because I > > guess in the future we might want yet another options bit field, in which > > case it would be better to have {tcpi_options, tcpi_options2, and > > tcpi_options3}, rather than having {tcpi_options, tcpi_more_options, and > > tcpi_yet_more_options}. :-) > > > > And rather than a single bit indicating whether AccECN was negotiated, it > > occurs to me that it would probably be better to have a 2-bit enum with 4 > > values, corresponding to the modes in tcp_ecn.h: > > tcp_ecn_disabled(), tcp_ecn_mode_rfc3168(), tcp_ecn_mode_accecn(), and > > tcp_ecn_mode_pending(). > > > > We also need to keep in mind that since the tcpi_accecn_fail_mode (4 > > bits) and tcpi_accecn_opt_seen (2 bits) enums are exported to user-space, > > they will become part of the kernel API to userspace, so should be moved > > out of tcp_ecn.h and instead be declared in include/uapi/linux/tcp.h. We > > declare constant values exported to user space in that file: (a) to make it > > easier for maintainers to remember not to change the values for these, so > > kernel changes don't break user-space apps; (b) to make it easier for > > application developers to find the #define values they need to decode the > > values exported in struct tcp_info. :-) > > > > So how about something like: > > > > --- in include/uapi/linux/tcp.h: > > > > /* Values for tcpi_ecn_mode */ > > #define TCPI_ECN_DISABLED 0x0 > > #define TCPI_ECN_RFC3168 0x1 > > #define TCPI_ECN_ACCECN 0x2 > > #define TCPI_ECN_PENDING 0x3 > > > > /* Values for tcpi_accecn_opt_seen */ > > #define TCP_ACCECN_OPT_NOT_SEEN 0x0 > > #define TCP_ACCECN_OPT_EMPTY_SEEN 0x1 > > #define TCP_ACCECN_OPT_COUNTER_SEEN 0x2 > > #define TCP_ACCECN_OPT_FAIL_SEEN 0x3 > > > > /* Values for tcpi_accecn_fail_mode */ > > #define TCP_ACCECN_ACE_FAIL_SEND BIT(0) > > #define TCP_ACCECN_ACE_FAIL_RECV BIT(1) > > #define TCP_ACCECN_OPT_FAIL_SEND BIT(2) > > #define TCP_ACCECN_OPT_FAIL_RECV BIT(3) > > > > ... > > __u32 tcpi_ecn_mode:2, > > tcpi_accecn_opt_seen: 2, > > tcpi_accecn_fail_mode: 4, > > tcpi_options2:24; > > __u32 tcpi_received_ce; /* # of CE marked segments received */ > > ... > > > > --- in tcp_get_info() in net/ipv4/tcp.c: > > > > if (tcp_ecn_disabled(tp)) > > info-> tcpi_ecn_mode = TCPI_ECN_DISABLED; else if > > (tcp_ecn_mode_rfc3168(tp)) > > info-> tcpi_ecn_mode = TCPI_ECN_RFC3168; else if (tcp_ecn_mode_accecn(tp)) > > info-> tcpi_ecn_mode = TCPI_ECN_ACCECN; else if (tcp_ecn_mode_pending(tp)) > > info-> tcpi_ecn_mode = TCPI_ECN_PENDING; > > > > WDYT? > > > > > And I will update the comment of tcpi_received_ce, thanks for the > > > comments. > > > > Great. Thanks! > > > > neal > > > > > Chia-Yu > > > > > > > (3) The following patch I did not merge, because I'd like to migrate > > > > to having all packetdrill tests for the Linux kernel reside in one > > > > place, in the Linux kernel source tree (not the Google packetdrill > > > > repo): > > > > > > > > net-test: add TCP Accurate ECN cases > > > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > > > > thub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96%2Fchanges%2Ffe4c7293ea64 > > > > 0a4c81178b6c88744d7a5d209fd6&data=05%7C02%7Cchia-yu.chang%40nokia-be > > > > ll-labs.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d4717519675428d917 > > > > b70f44f9630b0%7C0%7C0%7C639043779630323311%7CUnknown%7CTWFpbGZsb3d8e > > > > yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiT > > > > WFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=kIDQalqUreGDjFAmUljiaodEq > > > > kcxrypAa5YQBXsOvOE%3D&reserved=0 > > > > > > > > Thanks! > > > > neal > > > Chia-Yu > > > > > > -----Original Message----- > > > From: Neal Cardwell <[email protected]> > > > Sent: Sunday, January 18, 2026 5:11 PM > > > To: Chia-Yu Chang (Nokia) <[email protected]> > > > Cc: [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; > > > [email protected]; [email protected]; [email protected]; Koen > > > De Schepper (Nokia) <[email protected]>; > > > [email protected]; [email protected]; > > > [email protected]; cheshire <[email protected]>; > > > [email protected]; [email protected]; Vidhi Goel > > > <[email protected]>; Willem de Bruijn <[email protected]> > > > Subject: Re: [PATCH net-next 1/1] selftests/net: Add packetdrill > > > packetdrill cases > > > > > > > > > CAUTION: This is an external email. Please be very careful when clicking > > > links or opening attachments. See the URL nok.it/ext for additional > > > information. > > > > > > > > > > > > On Thu, Jan 8, 2026 at 5:46 PM Neal Cardwell <[email protected]> wrote: > > > > > > > > On Thu, Jan 8, 2026 at 10:58 AM <[email protected]> > > > > wrote: > > > > > > > > > > From: Chia-Yu Chang <[email protected]> > > > > > > > > > > Linux Accurate ECN test sets using ACE counters and AccECN options > > > > > to cover several scenarios: Connection teardown, different ACK > > > > > conditions, counter wrapping, SACK space grabbing, fallback > > > > > schemes, negotiation retransmission/reorder/loss, AccECN option > > > > > drop/loss, different handshake reflectors, data with marking, and > > > > > different sysctl values. > > > > > > > > > > Co-developed-by: Ilpo Järvinen <[email protected]> > > > > > Signed-off-by: Ilpo Järvinen <[email protected]> > > > > > Co-developed-by: Neal Cardwell <[email protected]> > > > > > Signed-off-by: Neal Cardwell <[email protected]> > > > > > --- > > > > > > > > Chia-Yu, thank you for posting the packetdrill tests. > > > > > > > > A couple thoughts: > > > > > > > > (1) These tests are using the experimental AccECN packetdrill > > > > support that is not in mainline packetdrill yet. Can you please > > > > share the github URL for the version of packetdrill you used? I will > > > > work on merging the appropriate experimental AccECN packetdrill > > > > support into the Google packetdrill mainline branch. > > > > > > An update on the 3 patches at: > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > > ub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96&data=05%7C02%7Cchia-yu.chang > > > %40nokia-bell-labs.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d47175196 > > > 75428d917b70f44f9630b0%7C0%7C0%7C639043779630340275%7CUnknown%7CTWFpbG > > > Zsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFO > > > IjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=9cEIWqpEDSW4fyo2CclKk1 > > > eNZ5c5C0lOADuSrE7lWFA%3D&reserved=0 > > > > > > (1) I have merged the following patch into the google packetdrill repo to > > > facilitate testing of the AccECN patch series: > > > > > > "net-test: packetdrill: add Accurate ECN (AccECN) option support" > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > > ub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96%2Fchanges%2Ff6861f888bc7f1e0 > > > 8026de4825519a95504d1047&data=05%7C02%7Cchia-yu.chang%40nokia-bell-lab > > > s.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d4717519675428d917b70f44f9 > > > 630b0%7C0%7C0%7C639043779630357016%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1 > > > hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUI > > > joyfQ%3D%3D%7C0%7C%7C%7C&sdata=V9Pt4nNHTV%2BcoQrb%2BMNQKOW%2FJARzk3DfY > > > LvhuVZ3ii4%3D&reserved=0 > > > > > > (2) The following patch I did not yet merge, because it proposes to add > > > an odd number of u32 fields to tcp_info, so AFAICT leaves a 4-byte > > > padding hole at the end of tcp_info: > > > > > > net-test: packetdrill: Support AccECN counters through tcpi > > > > > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith > > > ub.com%2Fgoogle%2Fpacketdrill%2Fpull%2F96%2Fchanges%2Ff43649c87a2aa79a > > > 33a78111d3d7e5f027d13a7f&data=05%7C02%7Cchia-yu.chang%40nokia-bell-lab > > > s.com%7C227b6e8cfe084ca9279708de56ee858b%7C5d4717519675428d917b70f44f9 > > > 630b0%7C0%7C0%7C639043779630374376%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1 > > > hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUI > > > joyfQ%3D%3D%7C0%7C%7C%7C&sdata=vy8TGIgoQum%2FFSN%2FnuaGT3rcZsTVjKHVx5G > > > dIDWTR7A%3D&reserved=0 > > > > > > I think we'll need to tweak the AccECN kernel patch series so that it > > > does not leave a 4-byte padding hole at the end of tcp_info, then update > > > this packetdrill patch to match the kernel patch. > > > > > > Let's come up with another useful u32 field we can add to the tcp_info > > > struct, so that the kernel patch doesn't add a padding hole at the end of > > > tcp_info. > > > > > > One idea would be to add another field to represent newer options and > > > connection features that are enabled. AFAICT all 8 bits of the > > > tcpi_options field have been used, so we can't use more bits in that > > > field. I'd suggest we add a u32 tcpi_more_options field before the > > > tcpi_received_ce field, so we can encode other useful info, like: > > > > > > + 1 bit to indicate whether AccECN was negotiated (this can go in a > > > separate patch) > > > > > > + 1 bit to indicate whether TCP_NODELAY was set (since forgetting to > > > use TCP_NODELAY is a classic cause of performance problems; again this > > > can go in a separate patch) > > > > > > (And there will be future bits of info we want to add...) > > > > > > Also, regarding the comment in this line: > > > __u32 tcpi_received_ce; /* # of CE marks received */ > > > > > > That comment is ambiguous, since it doesn't indicate whether it's > > > counting (potentially LRO/GRO) skbs or TCP segments. I would suggest > > > clarifying that this is counting segments: > > > > > > __u32 tcpi_received_ce; /* # of CE marked segments received */ > Hi Neal, > > I've done doing the patch on both net-next and packetdrill, and all AccECN > cases can still pass. > > Would it be possible for me to send patches via email rather than creating PR > (due to our internal rules)?
Yes, that's fine if you want to email the packetdrill patches to the [email protected] mailing list. thanks, neal
