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://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?
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://github.com/google/packetdrill/pull/96/changes/fe4c7293ea640a4c81178b6c88744d7a5d209fd6
> >
> > 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://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
