[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<PAXPR07MB79840B8A0D8FDC3778D79539A38BA@PAXPR07MB7984.eurprd07.prod.outlook.com>
Date: Sun, 18 Jan 2026 22:56:18 +0000
From: "Chia-Yu Chang (Nokia)" <chia-yu.chang@...ia-bell-labs.com>
To: Neal Cardwell <ncardwell@...gle.com>
CC: "pabeni@...hat.com" <pabeni@...hat.com>, "edumazet@...gle.com"
<edumazet@...gle.com>, "parav@...dia.com" <parav@...dia.com>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>, "corbet@....net"
<corbet@....net>, "horms@...nel.org" <horms@...nel.org>, "dsahern@...nel.org"
<dsahern@...nel.org>, "kuniyu@...gle.com" <kuniyu@...gle.com>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "dave.taht@...il.com" <dave.taht@...il.com>,
"jhs@...atatu.com" <jhs@...atatu.com>, "kuba@...nel.org" <kuba@...nel.org>,
"stephen@...workplumber.org" <stephen@...workplumber.org>,
"xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>, "jiri@...nulli.us"
<jiri@...nulli.us>, "davem@...emloft.net" <davem@...emloft.net>,
"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "donald.hunter@...il.com"
<donald.hunter@...il.com>, "ast@...erby.net" <ast@...erby.net>,
"liuhangbin@...il.com" <liuhangbin@...il.com>, "shuah@...nel.org"
<shuah@...nel.org>, "linux-kselftest@...r.kernel.org"
<linux-kselftest@...r.kernel.org>, "ij@...nel.org" <ij@...nel.org>, "Koen De
Schepper (Nokia)" <koen.de_schepper@...ia-bell-labs.com>,
"g.white@...lelabs.com" <g.white@...lelabs.com>,
"ingemar.s.johansson@...csson.com" <ingemar.s.johansson@...csson.com>,
"mirja.kuehlewind@...csson.com" <mirja.kuehlewind@...csson.com>, cheshire
<cheshire@...le.com>, "rs.ietf@....at" <rs.ietf@....at>,
"Jason_Livingood@...cast.com" <Jason_Livingood@...cast.com>, Vidhi Goel
<vidhi_goel@...le.com>, Willem de Bruijn <willemb@...gle.com>
Subject: RE: [PATCH net-next 1/1] selftests/net: Add packetdrill packetdrill
cases
> -----Original Message-----
> From: Neal Cardwell <ncardwell@...gle.com>
> Sent: Sunday, January 18, 2026 5:11 PM
> To: Chia-Yu Chang (Nokia) <chia-yu.chang@...ia-bell-labs.com>
> Cc: pabeni@...hat.com; edumazet@...gle.com; parav@...dia.com; linux-doc@...r.kernel.org; corbet@....net; horms@...nel.org; dsahern@...nel.org; kuniyu@...gle.com; bpf@...r.kernel.org; netdev@...r.kernel.org; dave.taht@...il.com; jhs@...atatu.com; kuba@...nel.org; stephen@...workplumber.org; xiyou.wangcong@...il.com; jiri@...nulli.us; davem@...emloft.net; andrew+netdev@...n.ch; donald.hunter@...il.com; ast@...erby.net; liuhangbin@...il.com; shuah@...nel.org; linux-kselftest@...r.kernel.org; ij@...nel.org; Koen De Schepper (Nokia) <koen.de_schepper@...ia-bell-labs.com>; g.white@...lelabs.com; ingemar.s.johansson@...csson.com; mirja.kuehlewind@...csson.com; cheshire <cheshire@...le.com>; rs.ietf@....at; Jason_Livingood@...cast.com; Vidhi Goel <vidhi_goel@...le.com>; Willem de Bruijn <willemb@...gle.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 Thu, Jan 8, 2026 at 5:46 PM Neal Cardwell <ncardwell@...gle.com> wrote:
> >
> > On Thu, Jan 8, 2026 at 10:58 AM <chia-yu.chang@...ia-bell-labs.com> wrote:
> > >
> > > From: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> > >
> > > 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@...nel.org>
> > > Signed-off-by: Ilpo Järvinen <ij@...nel.org>
> > > Co-developed-by: Neal Cardwell <ncardwell@...gle.com>
> > > Signed-off-by: Neal Cardwell <ncardwell@...gle.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://github.com/google/packetdrill/pull/96
>
> (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://github.com/google/packetdrill/pull/96/changes/f6861f888bc7f1e08026de4825519a95504d1047
>
> (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://github.com/google/packetdrill/pull/96/changes/f43649c87a2aa79a33a78111d3d7e5f027d13a7f
>
> 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?
And I will update the comment of tcpi_received_ce, thanks for the comments.
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://github.com/google/packetdrill/pull/96/changes/fe4c7293ea640a4c81178b6c88744d7a5d209fd6
>
> Thanks!
> neal
Chia-Yu
-----Original Message-----
From: Neal Cardwell <ncardwell@...gle.com>
Sent: Sunday, January 18, 2026 5:11 PM
To: Chia-Yu Chang (Nokia) <chia-yu.chang@...ia-bell-labs.com>
Cc: pabeni@...hat.com; edumazet@...gle.com; parav@...dia.com; linux-doc@...r.kernel.org; corbet@....net; horms@...nel.org; dsahern@...nel.org; kuniyu@...gle.com; bpf@...r.kernel.org; netdev@...r.kernel.org; dave.taht@...il.com; jhs@...atatu.com; kuba@...nel.org; stephen@...workplumber.org; xiyou.wangcong@...il.com; jiri@...nulli.us; davem@...emloft.net; andrew+netdev@...n.ch; donald.hunter@...il.com; ast@...erby.net; liuhangbin@...il.com; shuah@...nel.org; linux-kselftest@...r.kernel.org; ij@...nel.org; Koen De Schepper (Nokia) <koen.de_schepper@...ia-bell-labs.com>; g.white@...lelabs.com; ingemar.s.johansson@...csson.com; mirja.kuehlewind@...csson.com; cheshire <cheshire@...le.com>; rs.ietf@....at; Jason_Livingood@...cast.com; Vidhi Goel <vidhi_goel@...le.com>; Willem de Bruijn <willemb@...gle.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 Thu, Jan 8, 2026 at 5:46 PM Neal Cardwell <ncardwell@...gle.com> wrote:
>
> On Thu, Jan 8, 2026 at 10:58 AM <chia-yu.chang@...ia-bell-labs.com> wrote:
> >
> > From: Chia-Yu Chang <chia-yu.chang@...ia-bell-labs.com>
> >
> > 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@...nel.org>
> > Signed-off-by: Ilpo Järvinen <ij@...nel.org>
> > Co-developed-by: Neal Cardwell <ncardwell@...gle.com>
> > Signed-off-by: Neal Cardwell <ncardwell@...gle.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://github.com/google/packetdrill/pull/96
(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://github.com/google/packetdrill/pull/96/changes/f6861f888bc7f1e08026de4825519a95504d1047
(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://github.com/google/packetdrill/pull/96/changes/f43649c87a2aa79a33a78111d3d7e5f027d13a7f
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 */
(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://github.com/google/packetdrill/pull/96/changes/fe4c7293ea640a4c81178b6c88744d7a5d209fd6
Thanks!
neal
Powered by blists - more mailing lists