RE: [PATCH net-next 1/1] selftests/net: Add packetdrill packetdrill cases

-----Original Message-----
From: Neal Cardwell <ncardwell@google.com> 
Sent: Monday, January 19, 2026 1:06 AM
To: Chia-Yu Chang (Nokia) <redacted>
Cc: pabeni@redhat.com; edumazet@google.com; parav@nvidia.com; linux-doc@vger.kernel.org; corbet@lwn.net; horms@kernel.org; dsahern@kernel.org; kuniyu@google.com; bpf@vger.kernel.org; netdev@vger.kernel.org; dave.taht@gmail.com; jhs@mojatatu.com; kuba@kernel.org; stephen@networkplumber.org; xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net; andrew+netdev@lunn.ch; donald.hunter@gmail.com; ast@fiberby.net; liuhangbin@gmail.com; shuah@kernel.org; linux-kselftest@vger.kernel.org; ij@kernel.org; Koen De Schepper (Nokia) <redacted>; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire <redacted>; rs.ietf@gmx.at; Jason_Livingood@comcast.com; Vidhi Goel <redacted>; Willem de Bruijn <willemb@google.com>
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) [off-list ref] wrote:

quoted

quoted

-----Original Message-----
From: Neal Cardwell <ncardwell@google.com>
Sent: Sunday, January 18, 2026 5:11 PM
To: Chia-Yu Chang (Nokia) <redacted>
Cc: pabeni@redhat.com; edumazet@google.com; parav@nvidia.com; 
linux-doc@vger.kernel.org; corbet@lwn.net; horms@kernel.org; 
dsahern@kernel.org; kuniyu@google.com; bpf@vger.kernel.org; 
netdev@vger.kernel.org; dave.taht@gmail.com; jhs@mojatatu.com; 
kuba@kernel.org; stephen@networkplumber.org; 
xiyou.wangcong@gmail.com; jiri@resnulli.us; davem@davemloft.net; 
andrew+netdev@lunn.ch; donald.hunter@gmail.com; ast@fiberby.net; 
liuhangbin@gmail.com; shuah@kernel.org; 
linux-kselftest@vger.kernel.org; ij@kernel.org; Koen De Schepper 
(Nokia) [off-list ref]; 
g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; 
mirja.kuehlewind@ericsson.com; cheshire [off-list ref]; 
rs.ietf@gmx.at; Jason_Livingood@comcast.com; Vidhi Goel 
[off-list ref]; Willem de Bruijn [off-list ref]
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 [off-list ref] wrote:

quoted

On Thu, Jan 8, 2026 at 10:58 AM [off-list ref] wrote:

quoted

From: Chia-Yu Chang <redacted>

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 <ij@kernel.org>
Signed-off-by: Ilpo Järvinen <ij@kernel.org>
Co-developed-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---

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?

quoted

And I will update the comment of tcpi_received_ce, thanks for the comments.

Great. Thanks!

neal

quoted

Chia-Yu

quoted

(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 <ncardwell@google.com>
Sent: Sunday, January 18, 2026 5:11 PM
To: Chia-Yu Chang (Nokia) <redacted>
Cc: pabeni@redhat.com; edumazet@google.com; parav@nvidia.com; 
linux-doc@vger.kernel.org; corbet@lwn.net; horms@kernel.org; 
dsahern@kernel.org; kuniyu@google.com; bpf@vger.kernel.org; 
netdev@vger.kernel.org; dave.taht@gmail.com; jhs@mojatatu.com; 
kuba@kernel.org; stephen@networkplumber.org; xiyou.wangcong@gmail.com; 
jiri@resnulli.us; davem@davemloft.net; andrew+netdev@lunn.ch; 
donald.hunter@gmail.com; ast@fiberby.net; liuhangbin@gmail.com; 
shuah@kernel.org; linux-kselftest@vger.kernel.org; ij@kernel.org; Koen 
De Schepper (Nokia) [off-list ref]; 
g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; 
mirja.kuehlewind@ericsson.com; cheshire [off-list ref]; 
rs.ietf@gmx.at; Jason_Livingood@comcast.com; Vidhi Goel 
[off-list ref]; Willem de Bruijn [off-list ref]
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 [off-list ref] wrote:

quoted

On Thu, Jan 8, 2026 at 10:58 AM [off-list ref] wrote:

quoted

From: Chia-Yu Chang <redacted>

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 <ij@kernel.org>
Signed-off-by: Ilpo Järvinen <ij@kernel.org>
Co-developed-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---

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 */