[PATCH v5 net-next 0/2] ECN offload handling series

[PATCH v5 net-next 0/2] ECN offload handling series

[chia-yu.chang]

From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

Hello,

Please find the v5 ECN offload handling series.
It clarifies the flags of SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN and fixs
the usage of SKB_GSO_TCP_ECN of hns3 in the RX path.

This corresponds to discussions in virtio mailing list:
https://lore.kernel.org/all/20250814120118.81787-1-chia-yu.chang@nokia-bell-labs.com/

Best regards,
Chia-Yu

---
Chia-Yu Chang (2):
  net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
  net: hns3: fix GSO_ECN flag setting in the RX path

 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c |  2 --
 include/linux/skbuff.h                          | 15 ++++++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[chia-yu.chang]

From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

This patch updates the documentation of ECN‑related GSO flags, it
clarifies the limitations of SKB_GSO_TCP_ECN and explains how to preserve
the CWR flag (part of the ACE signal) in the Rx path.

For Tx, SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN are used respectively for
RFC3168 ECN and AccECN (RFC9768). SKB_GSO_TCP_ECN indicates that the
first segment has CWR set, while subsequent segments have CWR cleared.
In contrast, SKB_GSO_TCP_ACCECN means that the segment uses AccECN and
therefore its CWR flag must not be modified during segmentation.

For RX, SKB_GSO_TCP_ECN shall NOT be used, because the stack cannot know
whether the connection uses RFC3168 ECN or AccECN, whereas RFC3168 ECN
offload may clear CWR flag and thus corrupts the ACE signal. Instead, any
segment that arrives with CWR set must use the SKB_GSO_TCP_ACCECN flag
to prevent RFC3168 ECN offload logic from clearing the CWR flag.

This corresponds to discussions in virtio mailing list:
https://lore.kernel.org/all/20250814120118.81787-1-chia-yu.chang@nokia-bell-labs.com/
And it was suggested to clarify SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN.

Co-developed-by: Ilpo Järvinen <ij@kernel.org>
Signed-off-by: Ilpo Järvinen <ij@kernel.org>
Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
Cc: Parav Pandit <parav@nvidia.com>

---
v4:
- Update commit message

v3:
- Update commit messages and documentation for clarity
---
 include/linux/skbuff.h | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 22eda1d54a0e..6c0d725facf0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -671,7 +671,13 @@ enum {
 	/* This indicates the skb is from an untrusted source. */
 	SKB_GSO_DODGY = 1 << 1,
 
-	/* This indicates the tcp segment has CWR set. */
+	/* For TX, this indicates that the first TCP segment has CWR set, and
+	 * any subsequent segment in the same skb has CWR cleared. This flag
+	 * must not be used in RX, because the connection to which the segment
+	 * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
+	 * offload may clear CWR and corrupt ACE signal (CWR is part of it).
+	 * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
+	 */
 	SKB_GSO_TCP_ECN = 1 << 2,
 
 	__SKB_GSO_TCP_FIXEDID = 1 << 3,
@@ -706,6 +712,13 @@ enum {
 
 	SKB_GSO_FRAGLIST = 1 << 18,
 
+	/* For TX, this indicates that the TCP segment uses the CWR flag as part
+	 * of the ACE signal, and the CWR flag must not be modified in the skb.
+	 * For RX, any incoming segment with CWR set must use this flag so that
+	 * no RFC3168 ECN offload can clear the CWR flag. This is required to
+	 * preserve ACE signal correctness (CWR is part of it) in a forwarding
+	 * scenario, e.g., from one netdevice RX to other netdevice TX
+	 */
 	SKB_GSO_TCP_ACCECN = 1 << 19,
 
 	/* These indirectly map onto the same netdev feature.
-- 
2.34.1

Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Jakub Kicinski]

On Tue,  4 Aug 2026 23:35:09 +0200 chia-yu.chang@nokia-bell-labs.com
wrote:
> This corresponds to discussions in virtio mailing list:
> https://lore.kernel.org/all/20250814120118.81787-1-chia-yu.chang@nokia-bell-labs.com/
> And it was suggested to clarify SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN.

> -	/* This indicates the tcp segment has CWR set. */
> +	/* For TX, this indicates that the first TCP segment has CWR set, and
> +	 * any subsequent segment in the same skb has CWR cleared. This flag
> +	 * must not be used in RX, because the connection to which the segment
> +	 * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> +	 * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> +	 * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> +	 */

I still can't wrap my head around this TBH.

SKB_GSO_TCP_ECN means RFC3168
SKB_GSO_TCP_ACCECN means AccECN

If the HW can correctly detect cwr on first frame and then no cwr 
and report that as ECN/RFC3168 - what's the problem? TSO will produce
the exact expected segment sequence.

Is the program that if we re-GRO that frame in SW we end up with
ECN+ACCECN on the same skb?

RE: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Chia-Yu Chang (Nokia)]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org> 
> Sent: Wednesday, August 12, 2026 2:30 AM
> To: Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>
> Cc: shaojijie@huawei.com; shenjian15@huawei.com; linux-rdma@vger.kernel.org; eperezma@redhat.com; jasowang@redhat.com; virtualization@lists.linux.dev; mst@redhat.com; xuanzhuo@linux.alibaba.com; pabeni@redhat.com; edumazet@google.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; 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; ncardwell@google.com; Koen De Schepper (Nokia) <koen.de_schepper@nokia-bell-labs.com>; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com; Parav Pandit <parav@nvidia.com>
> Subject: Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
> 
> 
> 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 Tue,  4 Aug 2026 23:35:09 +0200 chia-yu.chang@nokia-bell-labs.com
> wrote:
> > This corresponds to discussions in virtio mailing list:
> > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2F20250814120118.81787-1-chia-yu.chang%40nokia-bell-
> > labs.com%2F&data=05%7C02%7Cchia-yu.chang%40nokia-bell-labs.com%7C150b5
> > 1c3733640dd2cde08def808e48f%7C5d4717519675428d917b70f44f9630b0%7C0%7C0
> > %7C639220914236399114%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUs
> > IlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7
> > C0%7C%7C%7C&sdata=jGSBo1wXJOnjulVOJKW3PAEbrUAq7X30Zax4SBJ6VRQ%3D&reser
> > ved=0 And it was suggested to clarify SKB_GSO_TCP_ECN and 
> > SKB_GSO_TCP_ACCECN.
> 
> > -     /* This indicates the tcp segment has CWR set. */
> > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > +      * must not be used in RX, because the connection to which the segment
> > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > +      */
> 
> I still can't wrap my head around this TBH.
> 
> SKB_GSO_TCP_ECN means RFC3168
> SKB_GSO_TCP_ACCECN means AccECN
> 
> If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> 
> Is the program that if we re-GRO that frame in SW we end up with
> ECN+ACCECN on the same skb?

Yes, this is the problem.
The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:

+===================+==========+=================+================+
|     Packet id     | CWR flag |       Flag      | Flushed as SKB |
+===================+==========+=================+================+
|         0         |     1    | SKB_GSO_TCP_ECN |        0       |
|         1         |     0    |         -       |        0       |
|         2         |     1    |         -       |        0       |
|         3         |     1    |         -       |        1       |
+===================+==========+=================+================+

If the aggregated skb is forwarded through a device using GSO, e.g., HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168 semantics.
This means that only the 1st segment keeps the CWR flag while all subsequent segments have CWR cleared:

+===================+==========+
|     Packet id     | CWR flag |
+===================+==========+
|         0         |     1    |
|         1         |     0    |
|         2         |     0    |
|         3         |     0    |
+===================+==========+

This behavior is ok for RFC3168, since CWR is expected to appear only once.
However, for AccECN, CWR is part of the ACE signal and must be preserved across all segments.

In the example above, the original CWR sequence was 1,0,1,1.
But after re-segmentation it becomes: 1,0,0,0.
This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.

--
Chia-Yu

Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Jakub Kicinski]

On Wed, 12 Aug 2026 10:33:19 +0000 Chia-Yu Chang (Nokia) wrote:
> > > -     /* This indicates the tcp segment has CWR set. */
> > > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > > +      * must not be used in RX, because the connection to which the segment
> > > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > > +      */  
> > 
> > I still can't wrap my head around this TBH.
> > 
> > SKB_GSO_TCP_ECN means RFC3168
> > SKB_GSO_TCP_ACCECN means AccECN
> > 
> > If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> > 
> > Is the program that if we re-GRO that frame in SW we end up with
> > ECN+ACCECN on the same skb?  
> 
> Yes, this is the problem.
> The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
> For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:
> 
> +===================+==========+=================+================+
> |     Packet id     | CWR flag |       Flag      | Flushed as SKB |
> +===================+==========+=================+================+
> |         0         |     1    | SKB_GSO_TCP_ECN |        0       |
> |         1         |     0    |         -       |        0       |
> |         2         |     1    |         -       |        0       |
> |         3         |     1    |         -       |        1       |
> +===================+==========+=================+================+
> 
> If the aggregated skb is forwarded through a device using GSO, e.g.,
> HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168
> semantics. This means that only the 1st segment keeps the CWR flag
> while all subsequent segments have CWR cleared:
> 
> +===================+==========+
> |     Packet id     | CWR flag |
> +===================+==========+
> |         0         |     1    |
> |         1         |     0    |
> |         2         |     0    |
> |         3         |     0    |
> +===================+==========+
> 
> This behavior is ok for RFC3168, since CWR is expected to appear only
> once. However, for AccECN, CWR is part of the ACE signal and must be
> preserved across all segments.

But this would be obviously a buggy HW-GRO implementation.
The rules for HW-GRO RFC3168 are -- ignore CWR on first segment
(host responsible for populating SKB_GSO_TCP_ECN), and CWR
_must be 0_ for all subsequent segments.

> In the example above, the original CWR sequence was 1,0,1,1.
> But after re-segmentation it becomes: 1,0,0,0.
> This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.

We have extensive gro tests under 
tools/testing/selftests/drivers/net/gro.py

If you want to catch bad devices - add appropriate test cases there.

The comment as stated seems to be misleading - there's nothing wrong
with using the flag if the device follows the RFC3168 semantics
correctly.

And of course, adding a comment and hoping people will find it is much
weaker than adding tests.

Again, maybe I'm missing what _actually_ doesn't work here.
You mention veth but veth does not participate in GRO directly, 
it's not a HW driver either.

RE: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Chia-Yu Chang (Nokia)]

> On Wed, 12 Aug 2026 10:33:19 +0000 Chia-Yu Chang (Nokia) wrote:
> > > > -     /* This indicates the tcp segment has CWR set. */
> > > > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > > > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > > > +      * must not be used in RX, because the connection to which the segment
> > > > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > > > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > > > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > > > +      */
> > >
> > > I still can't wrap my head around this TBH.
> > >
> > > SKB_GSO_TCP_ECN means RFC3168
> > > SKB_GSO_TCP_ACCECN means AccECN
> > >
> > > If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> > >
> > > Is the program that if we re-GRO that frame in SW we end up with
> > > ECN+ACCECN on the same skb?
> >
> > Yes, this is the problem.
> > The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
> > For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:
> >
> > +===================+==========+=================+================+
> > |     Packet id     | CWR flag |       Flag      | Flushed as SKB |
> > +===================+==========+=================+================+
> > |         0         |     1    | SKB_GSO_TCP_ECN |        0       |
> > |         1         |     0    |         -       |        0       |
> > |         2         |     1    |         -       |        0       |
> > |         3         |     1    |         -       |        1       |
> > +===================+==========+=================+================+
> >
> > If the aggregated skb is forwarded through a device using GSO, e.g., 
> > HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168 
> > semantics. This means that only the 1st segment keeps the CWR flag 
> > while all subsequent segments have CWR cleared:
> >
> > +===================+==========+
> > |     Packet id     | CWR flag |
> > +===================+==========+
> > |         0         |     1    |
> > |         1         |     0    |
> > |         2         |     0    |
> > |         3         |     0    |
> > +===================+==========+
> >
> > This behavior is ok for RFC3168, since CWR is expected to appear only 
> > once. However, for AccECN, CWR is part of the ACE signal and must be 
> > preserved across all segments.
> 
> But this would be obviously a buggy HW-GRO implementation.
> The rules for HW-GRO RFC3168 are -- ignore CWR on first segment (host responsible for populating SKB_GSO_TCP_ECN), and CWR _must be 0_ for all subsequent segments.
> 
> > In the example above, the original CWR sequence was 1,0,1,1.
> > But after re-segmentation it becomes: 1,0,0,0.
> > This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.
> 
> We have extensive gro tests under
> tools/testing/selftests/drivers/net/gro.py
> 
> If you want to catch bad devices - add appropriate test cases there.
>

I added a test case in patch 6f74bc8b6e8d related to the CWR flag.
In that case, there are 5 packets with CWR values of 0, 1, 1, 0, and 0, and packets are flushed after the 1st, 3rd, and 5th packets.
The gro.py uses this case in tools/testing/selftests/net/lib/gro.c to verify CWR behavior.
But indeed, that does not cover whether SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN shall be set during the GRO.
So, a test might be added to verify the SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN flags (if there is another suggested way, please let me know)?

> The comment as stated seems to be misleading - there's nothing wrong with using the flag if the device follows the RFC3168 semantics correctly.
> 
> And of course, adding a comment and hoping people will find it is much weaker than adding tests.
>
> Again, maybe I'm missing what _actually_ doesn't work here.

Before adding an extra test, we need to clarify the definition and usages of these flags.
At the TX path, in tcp_gso_segment() of net/ipv4/tcp_offload.c, the SKB_GSO_TCP_ACCECN flag is used to preserve the CWR flags for AccECN flows.
Otherwise, when without SKB_GSO_TCP_ACCECN (RFC3168 ECN or Non-ECN flows), cwr will be cleared from the following packets.

For the RX path, unfortunately I do not find a clear rule of when SKB_GSO_TCP_ECN shall be set except in include/linux/skbuff.h.
Plus, the device usually does not track packets belonging to RFC3168 ECN or ACCECN flows.
So, my previous thought is to always use SKB_GSO_TCP_ACCECN in the RX path to avoid any potential CWR bleaching.

And FYI we will further include the related changes on the virtio-net after we get an approval in the virtio-spec.

Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Willem de Bruijn]

On Thu, Aug 13, 2026 at 6:33 AM Chia-Yu Chang (Nokia)
<chia-yu.chang@nokia-bell-labs.com> wrote:
>
> > On Wed, 12 Aug 2026 10:33:19 +0000 Chia-Yu Chang (Nokia) wrote:
> > > > > -     /* This indicates the tcp segment has CWR set. */
> > > > > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > > > > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > > > > +      * must not be used in RX, because the connection to which the segment
> > > > > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > > > > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > > > > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > > > > +      */
> > > >
> > > > I still can't wrap my head around this TBH.
> > > >
> > > > SKB_GSO_TCP_ECN means RFC3168
> > > > SKB_GSO_TCP_ACCECN means AccECN
> > > >
> > > > If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> > > >
> > > > Is the program that if we re-GRO that frame in SW we end up with
> > > > ECN+ACCECN on the same skb?
> > >
> > > Yes, this is the problem.
> > > The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
> > > For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:
> > >
> > > +===================+==========+=================+================+
> > > |     Packet id     | CWR flag |       Flag      | Flushed as SKB |
> > > +===================+==========+=================+================+
> > > |         0         |     1    | SKB_GSO_TCP_ECN |        0       |
> > > |         1         |     0    |         -       |        0       |
> > > |         2         |     1    |         -       |        0       |
> > > |         3         |     1    |         -       |        1       |
> > > +===================+==========+=================+================+
> > >
> > > If the aggregated skb is forwarded through a device using GSO, e.g.,
> > > HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168
> > > semantics. This means that only the 1st segment keeps the CWR flag
> > > while all subsequent segments have CWR cleared:
> > >
> > > +===================+==========+
> > > |     Packet id     | CWR flag |
> > > +===================+==========+
> > > |         0         |     1    |
> > > |         1         |     0    |
> > > |         2         |     0    |
> > > |         3         |     0    |
> > > +===================+==========+
> > >
> > > This behavior is ok for RFC3168, since CWR is expected to appear only
> > > once. However, for AccECN, CWR is part of the ACE signal and must be
> > > preserved across all segments.
> >
> > But this would be obviously a buggy HW-GRO implementation.
> > The rules for HW-GRO RFC3168 are -- ignore CWR on first segment (host responsible for populating SKB_GSO_TCP_ECN), and CWR _must be 0_ for all subsequent segments.
> >
> > > In the example above, the original CWR sequence was 1,0,1,1.
> > > But after re-segmentation it becomes: 1,0,0,0.
> > > This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.
> >
> > We have extensive gro tests under
> > tools/testing/selftests/drivers/net/gro.py
> >
> > If you want to catch bad devices - add appropriate test cases there.
> >
>
> I added a test case in patch 6f74bc8b6e8d related to the CWR flag.
> In that case, there are 5 packets with CWR values of 0, 1, 1, 0, and 0, and packets are flushed after the 1st, 3rd, and 5th packets.
> The gro.py uses this case in tools/testing/selftests/net/lib/gro.c to verify CWR behavior.
> But indeed, that does not cover whether SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN shall be set during the GRO.
> So, a test might be added to verify the SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN flags (if there is another suggested way, please let me know)?
>
> > The comment as stated seems to be misleading - there's nothing wrong with using the flag if the device follows the RFC3168 semantics correctly.
> >
> > And of course, adding a comment and hoping people will find it is much weaker than adding tests.
> >
> > Again, maybe I'm missing what _actually_ doesn't work here.
>
> Before adding an extra test, we need to clarify the definition and usages of these flags.
> At the TX path, in tcp_gso_segment() of net/ipv4/tcp_offload.c, the SKB_GSO_TCP_ACCECN flag is used to preserve the CWR flags for AccECN flows.
> Otherwise, when without SKB_GSO_TCP_ACCECN (RFC3168 ECN or Non-ECN flows), cwr will be cleared from the following packets.
>
> For the RX path, unfortunately I do not find a clear rule of when SKB_GSO_TCP_ECN shall be set except in include/linux/skbuff.h.
> Plus, the device usually does not track packets belonging to RFC3168 ECN or ACCECN flows.
> So, my previous thought is to always use SKB_GSO_TCP_ACCECN in the RX path to avoid any potential CWR bleaching.

This would be a case where AccECN support causes a regression for
regular ECN handling, if that is no longer allowed to be coalesced.

Most HW-GRO hardware out there today likely only supports ECN. In
which case they can set SKB_GSO_TCP_ECN fine.

If AccECN flows cannot be differentiated from ECN flows, on such
devices, does the admin have to disable HW-GRO with ECN if they care
about preserving AccECN signals?

What does SW GRO do here?

RE: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Chia-Yu Chang (Nokia)]

> From: Mirja Kuehlewind <mirja.kuehlewind@ericsson.com> 
> Sent: Friday, August 14, 2026 10:57 AM
> To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>; Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>
> Cc: Jakub Kicinski <kuba@kernel.org>; shaojijie@huawei.com; shenjian15@huawei.com; linux-rdma@vger.kernel.org; eperezma@redhat.com; jasowang@redhat.com; virtualization@lists.linux.dev; mst@redhat.com; xuanzhuo@linux.alibaba.com; pabeni@redhat.com; edumazet@google.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; 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; ncardwell@google.com; Koen De Schepper (Nokia) <koen.de_schepper@nokia-bell-labs.com>; g.white@cablelabs.com; Ingemar Johansson S <ingemar.s.johansson@ericsson.com>; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com; Parav Pandit <parav@nvidia.com>; Willem de Bruijn <willemb@google.com>
> Subject: Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
> 
> Hi Willem,
> 
> The current function of is SKB_GSO_TCP_ECN wrong. Fixing this causes the regression.
> 
> Mirja
> 
> 
> 
> From: Willem de Bruijn <mailto:willemdebruijn.kernel@gmail.com>
> Subject: Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
> 
> On Thu, Aug 13, 2026 at 6:33 AM Chia-Yu Chang (Nokia)
> <mailto:chia-yu.chang@nokia-bell-labs.com> wrote:
> 
> > On Wed, 12 Aug 2026 10:33:19 +0000 Chia-Yu Chang (Nokia) wrote:
> > > > > -     /* This indicates the tcp segment has CWR set. */
> > > > > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > > > > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > > > > +      * must not be used in RX, because the connection to which the segment
> > > > > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > > > > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > > > > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > > > > +      */
> > > >
> > > > I still can't wrap my head around this TBH.
> > > >
> > > > SKB_GSO_TCP_ECN means RFC3168
> > > > SKB_GSO_TCP_ACCECN means AccECN
> > > >
> > > > If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> > > >
> > > > Is the program that if we re-GRO that frame in SW we end up with
> > > > ECN+ACCECN on the same skb?
> > >
> > > Yes, this is the problem.
> > > The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
> > > For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:
> > >
> > > +===================+==========+=================+================+
> > > |     Packet id     | CWR flag |       Flag      | Flushed as SKB |
> > > +===================+==========+=================+================+
> > > |         0         |     1    | SKB_GSO_TCP_ECN |        0       |
> > > |         1         |     0    |         -       |        0       |
> > > |         2         |     1    |         -       |        0       |
> > > |         3         |     1    |         -       |        1       |
> > > +===================+==========+=================+================+
> > >
> > > If the aggregated skb is forwarded through a device using GSO, e.g.,
> > > HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168
> > > semantics. This means that only the 1st segment keeps the CWR flag
> > > while all subsequent segments have CWR cleared:
> > >
> > > +===================+==========+
> > > |     Packet id     | CWR flag |
> > > +===================+==========+
> > > |         0         |     1    |
> > > |         1         |     0    |
> > > |         2         |     0    |
> > > |         3         |     0    |
> > > +===================+==========+
> > >
> > > This behavior is ok for RFC3168, since CWR is expected to appear only
> > > once. However, for AccECN, CWR is part of the ACE signal and must be
> > > preserved across all segments.
> >
> > But this would be obviously a buggy HW-GRO implementation.
> > The rules for HW-GRO RFC3168 are -- ignore CWR on first segment (host responsible for populating SKB_GSO_TCP_ECN), and CWR _must be 0_ for all subsequent segments.
> >
> > > In the example above, the original CWR sequence was 1,0,1,1.
> > > But after re-segmentation it becomes: 1,0,0,0.
> > > This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.
> >
> > We have extensive gro tests under
> > tools/testing/selftests/drivers/net/gro.py
> >
> > If you want to catch bad devices - add appropriate test cases there.
> >
> 
> I added a test case in patch 6f74bc8b6e8d related to the CWR flag.
> In that case, there are 5 packets with CWR values of 0, 1, 1, 0, and 0, and packets are flushed after the 1st, 3rd, and 5th packets.
> The gro.py uses this case in tools/testing/selftests/net/lib/gro.c to verify CWR behavior.
> But indeed, that does not cover whether SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN shall be set during the GRO.
> So, a test might be added to verify the SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN flags (if there is another suggested way, please let me know)?
> 
> > The comment as stated seems to be misleading - there's nothing wrong with using the flag if the device follows the RFC3168 semantics correctly.
> >
> > And of course, adding a comment and hoping people will find it is much weaker than adding tests.
> >
> > Again, maybe I'm missing what _actually_ doesn't work here.
> 
> Before adding an extra test, we need to clarify the definition and usages of these flags.
> At the TX path, in tcp_gso_segment() of net/ipv4/tcp_offload.c, the SKB_GSO_TCP_ACCECN flag is used to preserve the CWR flags for AccECN flows.
> Otherwise, when without SKB_GSO_TCP_ACCECN (RFC3168 ECN or Non-ECN flows), cwr will be cleared from the following packets.
> 
> For the RX path, unfortunately I do not find a clear rule of when SKB_GSO_TCP_ECN shall be set except in include/linux/skbuff.h.
> Plus, the device usually does not track packets belonging to RFC3168 ECN or ACCECN flows.
> So, my previous thought is to always use SKB_GSO_TCP_ACCECN in the RX path to avoid any potential CWR bleaching.
> 
> This would be a case where AccECN support causes a regression for
> regular ECN handling, if that is no longer allowed to be coalesced.
> 
> Most HW-GRO hardware out there today likely only supports ECN. In
> which case they can set SKB_GSO_TCP_ECN fine.
> 
> If AccECN flows cannot be differentiated from ECN flows, on such
> devices, does the admin have to disable HW-GRO with ECN if they care
> about preserving AccECN signals?
> 
> What does SW GRO do here?

Hi Willem,

Current SW GRO sets SKB_GSO_TCP_ACCECN when the flushed skb carries CWR in tcp_gro_complete():
if (th->cwr)
    shinfo->gso_type |= SKB_GSO_TCP_ACCECN;

For HW GRO of a legacy device that implementing RFC3168 semantics, setting SKB_GSO_TCP_ECN seems reasonable.
However, such a device would not be able to preserve ACCECN signaling across the GRO/GSO.
In that case, if preserving AccECN signaling is required, disabling HW GRO may indeed be necessary.

And I still think the SKB_GSO_TCP_ECN comment could be clarified.
For example, by stating that "RX GRO implementations which need to preserve CWR information across re-segmentation should use SKB_GSO_TCP_ACCECN."?

Thanks!
Chia-Yu

RE: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Willem de Bruijn]

Chia-Yu Chang (Nokia) wrote:
> > From: Mirja Kuehlewind <mirja.kuehlewind@ericsson.com> 
> > Sent: Friday, August 14, 2026 10:57 AM
> > To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>; Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>; shaojijie@huawei.com; shenjian15@huawei.com; linux-rdma@vger.kernel.org; eperezma@redhat.com; jasowang@redhat.com; virtualization@lists.linux.dev; mst@redhat.com; xuanzhuo@linux.alibaba.com; pabeni@redhat.com; edumazet@google.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; 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; ncardwell@google.com; Koen De Schepper (Nokia) <koen.de_schepper@nokia-bell-labs.com>; g.white@cablelabs.com; Ingemar Johansson S <ingemar.s.johansson@ericsson.com>; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com; Parav Pandit <parav@nvidia.com>; Willem de Bruijn <willemb@google.com>
> > Subject: Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
> > 
> > Hi Willem,
> > 
> > The current function of is SKB_GSO_TCP_ECN wrong. Fixing this causes the regression.
> > 
> > Mirja
> > 
> > 
> > 
> > From: Willem de Bruijn <mailto:willemdebruijn.kernel@gmail.com>
> > Subject: Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
> > 
> > On Thu, Aug 13, 2026 at 6:33 AM Chia-Yu Chang (Nokia)
> > <mailto:chia-yu.chang@nokia-bell-labs.com> wrote:
> > 
> > > On Wed, 12 Aug 2026 10:33:19 +0000 Chia-Yu Chang (Nokia) wrote:
> > > > > > -     /* This indicates the tcp segment has CWR set. */
> > > > > > +     /* For TX, this indicates that the first TCP segment has CWR set, and
> > > > > > +      * any subsequent segment in the same skb has CWR cleared. This flag
> > > > > > +      * must not be used in RX, because the connection to which the segment
> > > > > > +      * belongs is not tracked to use RFC3168 or AccECN. Using RFC3168 ECN
> > > > > > +      * offload may clear CWR and corrupt ACE signal (CWR is part of it).
> > > > > > +      * Instead, SKB_GSO_TCP_ACCECN shall be used to avoid CWR corruption.
> > > > > > +      */
> > > > >
> > > > > I still can't wrap my head around this TBH.
> > > > >
> > > > > SKB_GSO_TCP_ECN means RFC3168
> > > > > SKB_GSO_TCP_ACCECN means AccECN
> > > > >
> > > > > If the HW can correctly detect cwr on first frame and then no cwr and report that as ECN/RFC3168 - what's the problem? TSO will produce the exact expected segment sequence.
> > > > >
> > > > > Is the program that if we re-GRO that frame in SW we end up with
> > > > > ECN+ACCECN on the same skb?
> > > >
> > > > Yes, this is the problem.
> > > > The HW does not know whether the received packets belong to an RFC3168 ECN flow or an AccECN flow on the RX path.
> > > > For example, HW GRO may set SKB_GSO_TCP_ECN after observing that the first packet has CWR=1:
> > > >
> > > > +===================+==========+=================+================+
> > > > |     Packet id     | CWR flag |       Flag      | Flushed as SKB |
> > > > +===================+==========+=================+================+
> > > > |         0         |     1    | SKB_GSO_TCP_ECN |        0       |
> > > > |         1         |     0    |         -       |        0       |
> > > > |         2         |     1    |         -       |        0       |
> > > > |         3         |     1    |         -       |        1       |
> > > > +===================+==========+=================+================+
> > > >
> > > > If the aggregated skb is forwarded through a device using GSO, e.g.,
> > > > HW RX (GRO) -> veth TX (GSO), the SKB_GSO_TCP_ECN applies RFC3168
> > > > semantics. This means that only the 1st segment keeps the CWR flag
> > > > while all subsequent segments have CWR cleared:
> > > >
> > > > +===================+==========+
> > > > |     Packet id     | CWR flag |
> > > > +===================+==========+
> > > > |         0         |     1    |
> > > > |         1         |     0    |
> > > > |         2         |     0    |
> > > > |         3         |     0    |
> > > > +===================+==========+
> > > >
> > > > This behavior is ok for RFC3168, since CWR is expected to appear only
> > > > once. However, for AccECN, CWR is part of the ACE signal and must be
> > > > preserved across all segments.
> > >
> > > But this would be obviously a buggy HW-GRO implementation.
> > > The rules for HW-GRO RFC3168 are -- ignore CWR on first segment (host responsible for populating SKB_GSO_TCP_ECN), and CWR _must be 0_ for all subsequent segments.
> > >
> > > > In the example above, the original CWR sequence was 1,0,1,1.
> > > > But after re-segmentation it becomes: 1,0,0,0.
> > > > This is why SKB_GSO_TCP_ECN should not be used in RX/GRO paths.
> > >
> > > We have extensive gro tests under
> > > tools/testing/selftests/drivers/net/gro.py
> > >
> > > If you want to catch bad devices - add appropriate test cases there.
> > >
> > 
> > I added a test case in patch 6f74bc8b6e8d related to the CWR flag.
> > In that case, there are 5 packets with CWR values of 0, 1, 1, 0, and 0, and packets are flushed after the 1st, 3rd, and 5th packets.
> > The gro.py uses this case in tools/testing/selftests/net/lib/gro.c to verify CWR behavior.
> > But indeed, that does not cover whether SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN shall be set during the GRO.
> > So, a test might be added to verify the SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN flags (if there is another suggested way, please let me know)?
> > 
> > > The comment as stated seems to be misleading - there's nothing wrong with using the flag if the device follows the RFC3168 semantics correctly.
> > >
> > > And of course, adding a comment and hoping people will find it is much weaker than adding tests.
> > >
> > > Again, maybe I'm missing what _actually_ doesn't work here.
> > 
> > Before adding an extra test, we need to clarify the definition and usages of these flags.
> > At the TX path, in tcp_gso_segment() of net/ipv4/tcp_offload.c, the SKB_GSO_TCP_ACCECN flag is used to preserve the CWR flags for AccECN flows.
> > Otherwise, when without SKB_GSO_TCP_ACCECN (RFC3168 ECN or Non-ECN flows), cwr will be cleared from the following packets.
> > 
> > For the RX path, unfortunately I do not find a clear rule of when SKB_GSO_TCP_ECN shall be set except in include/linux/skbuff.h.
> > Plus, the device usually does not track packets belonging to RFC3168 ECN or ACCECN flows.
> > So, my previous thought is to always use SKB_GSO_TCP_ACCECN in the RX path to avoid any potential CWR bleaching.
> > 
> > This would be a case where AccECN support causes a regression for
> > regular ECN handling, if that is no longer allowed to be coalesced.
> > 
> > Most HW-GRO hardware out there today likely only supports ECN. In
> > which case they can set SKB_GSO_TCP_ECN fine.
> > 
> > If AccECN flows cannot be differentiated from ECN flows, on such
> > devices, does the admin have to disable HW-GRO with ECN if they care
> > about preserving AccECN signals?
> > 
> > What does SW GRO do here?
> 
> Hi Willem,
> 
> Current SW GRO sets SKB_GSO_TCP_ACCECN when the flushed skb carries CWR in tcp_gro_complete():
> if (th->cwr)
>     shinfo->gso_type |= SKB_GSO_TCP_ACCECN;

And I suppose it follows correct AccECN rules for coalescing.

That is a performance regression from RFC 3168 ECN, as it allows for
less effective coalescing. I have no intuition how much it will
differ in practice.

> 
> For HW GRO of a legacy device that implementing RFC3168 semantics, setting SKB_GSO_TCP_ECN seems reasonable.
> However, such a device would not be able to preserve ACCECN signaling across the GRO/GSO.
> In that case, if preserving AccECN signaling is required, disabling HW GRO may indeed be necessary.

Right. And there currently is no kernel API to disable only ECN
coalescing. NETIF_F_GRO_HW enables or disables HW-GRO entirely.
Or even to signal whether a HW-GRO implementation is AccECN
capable.

Disabling HW-GRO can be a huge efficiency regression. I suspect
many users will prioritize the efficiency over preserving the AccECN
signal.

That said, some devices may have other ways to configure such
finer details of their HW-GRO, even though not available through
Ethtool.

> And I still think the SKB_GSO_TCP_ECN comment could be clarified.
> For example, by stating that "RX GRO implementations which need to preserve CWR information across re-segmentation should use SKB_GSO_TCP_ACCECN."?
> 
> Thanks!
> Chia-Yu

Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Jakub Kicinski]

On Fri, 14 Aug 2026 09:34:01 -0400 Willem de Bruijn wrote:
> > Current SW GRO sets SKB_GSO_TCP_ACCECN when the flushed skb carries CWR in tcp_gro_complete():
> > if (th->cwr)
> >     shinfo->gso_type |= SKB_GSO_TCP_ACCECN;  
> 
> And I suppose it follows correct AccECN rules for coalescing.
> 
> That is a performance regression from RFC 3168 ECN, as it allows for
> less effective coalescing. I have no intuition how much it will
> differ in practice.
> 
> > For HW GRO of a legacy device that implementing RFC3168 semantics, setting SKB_GSO_TCP_ECN seems reasonable.
> > However, such a device would not be able to preserve ACCECN signaling across the GRO/GSO.
> > In that case, if preserving AccECN signaling is required, disabling HW GRO may indeed be necessary.  
> 
> Right.

I'm still not following.. Maybe Willem can ELI5 what the problem is.

_SW_ GRO follows only the AccECN rules.
But if HW GRO follows RFC 3168 and we mark the aggregate as
SKB_GSO_TCP_ECN - TSO will also abide, and segmented output 
will be identical to pre-GRO input.

Are we trying to ban RFC 3168 behavior in HW purely to match SW?

> And there currently is no kernel API to disable only ECN
> coalescing. NETIF_F_GRO_HW enables or disables HW-GRO entirely.
> Or even to signal whether a HW-GRO implementation is AccECN
> capable.
> 
> Disabling HW-GRO can be a huge efficiency regression. I suspect
> many users will prioritize the efficiency over preserving the AccECN
> signal.
> 
> That said, some devices may have other ways to configure such
> finer details of their HW-GRO, even though not available through
> Ethtool.

Re: [PATCH v5 net-next 1/2] net: update comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

[Willem de Bruijn]

On Fri, Aug 14, 2026 at 3:01 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 14 Aug 2026 09:34:01 -0400 Willem de Bruijn wrote:
> > > Current SW GRO sets SKB_GSO_TCP_ACCECN when the flushed skb carries CWR in tcp_gro_complete():
> > > if (th->cwr)
> > >     shinfo->gso_type |= SKB_GSO_TCP_ACCECN;
> >
> > And I suppose it follows correct AccECN rules for coalescing.
> >
> > That is a performance regression from RFC 3168 ECN, as it allows for
> > less effective coalescing. I have no intuition how much it will
> > differ in practice.
> >
> > > For HW GRO of a legacy device that implementing RFC3168 semantics, setting SKB_GSO_TCP_ECN seems reasonable.
> > > However, such a device would not be able to preserve ACCECN signaling across the GRO/GSO.
> > > In that case, if preserving AccECN signaling is required, disabling HW GRO may indeed be necessary.
> >
> > Right.
>
> I'm still not following.. Maybe Willem can ELI5 what the problem is.
>
> _SW_ GRO follows only the AccECN rules.
> But if HW GRO follows RFC 3168 and we mark the aggregate as
> SKB_GSO_TCP_ECN - TSO will also abide, and segmented output
> will be identical to pre-GRO input.

+1

> Are we trying to ban RFC 3168 behavior in HW purely to match SW?

I think that's the intent here?

>
> > And there currently is no kernel API to disable only ECN
> > coalescing. NETIF_F_GRO_HW enables or disables HW-GRO entirely.
> > Or even to signal whether a HW-GRO implementation is AccECN
> > capable.
> >
> > Disabling HW-GRO can be a huge efficiency regression. I suspect
> > many users will prioritize the efficiency over preserving the AccECN
> > signal.
> >
> > That said, some devices may have other ways to configure such
> > finer details of their HW-GRO, even though not available through
> > Ethtool.

[PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[chia-yu.chang]

From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

Currently, the hns3 Rx path sets the SKB_GSO_TCP_ECN flag when a TCP
segment with the CWR flag set is received. This is incorrect because
the hns3 hardware GRO resets the IP ToS byte to 0 during packet
aggregation. Consequently, no valid ECN signals are carreid, meaning
that it cannot support RFC3168 ECN or Accurate ECN protocols. Setting
SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN under these conditions misleads
the upper network stack.

Fix this by removing the incorrect SKB_GSO_TCP_ECN flag assignment
in hns3_gro_complete().

Fixes: d474d88f88261 ("net: hns3: add hns3_gro_complete for HW GRO process")

Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>

---
v5:
- Remove SKB_GSO_TCP_ECN flag and update the commit message

v3:
- Rewrite the commit message for clarity
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 6ecb32e28e79..5149eca0104a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -3916,8 +3916,6 @@ static int hns3_gro_complete(struct sk_buff *skb, u32 l234info)
 	}
 
 	skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
-	if (th->cwr)
-		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
 
 	if (l234info & BIT(HNS3_RXD_GRO_FIXID_B))
 		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_FIXEDID;
-- 
2.34.1

Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Jijie Shao]

on 2026/8/5 5:35, chia-yu.chang@nokia-bell-labs.com wrote:
> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
>
> Currently, the hns3 Rx path sets the SKB_GSO_TCP_ECN flag when a TCP
> segment with the CWR flag set is received. This is incorrect because
> the hns3 hardware GRO resets the IP ToS byte to 0 during packet
> aggregation. Consequently, no valid ECN signals are carreid, meaning
> that it cannot support RFC3168 ECN or Accurate ECN protocols. Setting
> SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN under these conditions misleads
> the upper network stack.
>
> Fix this by removing the incorrect SKB_GSO_TCP_ECN flag assignment
> in hns3_gro_complete().
>
> Fixes: d474d88f88261 ("net: hns3: add hns3_gro_complete for HW GRO process")

One note:
since this patch fixes a bug (incorrect SKB_GSO_TCP_ECN flag set in hns3_gro_complete),
it should target net instead of net-next.

I'm running HW tests on hns3 to verify the fix. Will share results
based on what I observe.

Thanks,
Jijie Shao

Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Jijie Shao]

on 2026/8/5 5:35, chia-yu.chang@nokia-bell-labs.com wrote:
> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
>
> Currently, the hns3 Rx path sets the SKB_GSO_TCP_ECN flag when a TCP
> segment with the CWR flag set is received. This is incorrect because
> the hns3 hardware GRO resets the IP ToS byte to 0 during packet
> aggregation. Consequently, no valid ECN signals are carreid, meaning
> that it cannot support RFC3168 ECN or Accurate ECN protocols. Setting
> SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN under these conditions misleads
> the upper network stack.
>
> Fix this by removing the incorrect SKB_GSO_TCP_ECN flag assignment
> in hns3_gro_complete().


Tested on hns3 HW (2x 100G, direct cable, openEuler 24.03, kernel 7.2.0-rc6).

tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets carry
tos 0x0, while non-aggregated packets keep their ToS/ECN marks — so the
zeroing is done by HW GRO, not the sender. Before patch, CWR packets had
SKB_GSO_TCP_ECN set (0x5 = SKB_GSO_TCPV4 | SKB_GSO_TCP_ECN). After patch,
all GRO events gso_type=0x1 (SKB_GSO_TCPV4 only), no 0x5 observed.

Tested-by: Jijie Shao <shaojijie@huawei.com>

RE: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Chia-Yu Chang (Nokia)]

> -----Original Message-----
> From: Jijie Shao <shaojijie@huawei.com> 
> Sent: Tuesday, August 11, 2026 3:32 PM
> To: Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>; shenjian15@huawei.com; linux-rdma@vger.kernel.org; eperezma@redhat.com; jasowang@redhat.com; virtualization@lists.linux.dev; mst@redhat.com; xuanzhuo@linux.alibaba.com; pabeni@redhat.com; edumazet@google.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; ncardwell@google.com; Koen De Schepper (Nokia) <koen.de_schepper@nokia-bell-labs.com>; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com
> Cc: shaojijie@huawei.com
> Subject: Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path
> 
> 
> on 2026/8/5 5:35, chia-yu.chang@nokia-bell-labs.com wrote:
> > From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> >
> > Currently, the hns3 Rx path sets the SKB_GSO_TCP_ECN flag when a TCP 
> > segment with the CWR flag set is received. This is incorrect because 
> > the hns3 hardware GRO resets the IP ToS byte to 0 during packet 
> > aggregation. Consequently, no valid ECN signals are carreid, meaning 
> > that it cannot support RFC3168 ECN or Accurate ECN protocols. Setting 
> > SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN under these conditions misleads 
> > the upper network stack.
> >
> > Fix this by removing the incorrect SKB_GSO_TCP_ECN flag assignment in 
> > hns3_gro_complete().
> 
> 
> Tested on hns3 HW (2x 100G, direct cable, openEuler 24.03, kernel 7.2.0-rc6).
> 
> tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets carry tos 0x0, while non-aggregated packets keep their ToS/ECN marks — so the zeroing is done by HW GRO, not the sender. Before patch, CWR packets had SKB_GSO_TCP_ECN set (0x5 = SKB_GSO_TCPV4 | SKB_GSO_TCP_ECN). After patch, all GRO events gso_type=0x1 (SKB_GSO_TCPV4 only), no 0x5 observed.
> 
> Tested-by: Jijie Shao <shaojijie@huawei.com>

Thanks Jijie for confirmation, I will submit to net in the next round.

Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Jakub Kicinski]

On Wed, 12 Aug 2026 10:35:25 +0000 Chia-Yu Chang (Nokia) wrote:
> > Tested on hns3 HW (2x 100G, direct cable, openEuler 24.03, kernel 7.2.0-rc6).
> > 
> > tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets carry tos 0x0, while non-aggregated packets keep their ToS/ECN marks — so the zeroing is done by HW GRO, not the sender. Before patch, CWR packets had SKB_GSO_TCP_ECN set (0x5 = SKB_GSO_TCPV4 | SKB_GSO_TCP_ECN). After patch, all GRO events gso_type=0x1 (SKB_GSO_TCPV4 only), no 0x5 observed.
> > 
> > Tested-by: Jijie Shao <shaojijie@huawei.com>  
> 
> Thanks Jijie for confirmation, I will submit to net in the next round.

Hold on.. Sounds like hns3 zeros ToS out (?!)

This would be invalid and illegal for HW-GRO. If hns3 nukes crucial IP
header fields it should probably be advertising LRO not HW-GRO in the
first place.

Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Jijie Shao]

on 2026/8/13 6:59, Jakub Kicinski wrote:
> On Wed, 12 Aug 2026 10:35:25 +0000 Chia-Yu Chang (Nokia) wrote:
>>> Tested on hns3 HW (2x 100G, direct cable, openEuler 24.03, kernel 7.2.0-rc6).
>>>
>>> tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets carry tos 0x0, while non-aggregated packets keep their ToS/ECN marks — so the zeroing is done by HW GRO, not the sender. Before patch, CWR packets had SKB_GSO_TCP_ECN set (0x5 = SKB_GSO_TCPV4 | SKB_GSO_TCP_ECN). After patch, all GRO events gso_type=0x1 (SKB_GSO_TCPV4 only), no 0x5 observed.
>>>
>>> Tested-by: Jijie Shao <shaojijie@huawei.com>
>> Thanks Jijie for confirmation, I will submit to net in the next round.
> Hold on.. Sounds like hns3 zeros ToS out (?!)
>
> This would be invalid and illegal for HW-GRO. If hns3 nukes crucial IP
> header fields it should probably be advertising LRO not HW-GRO in the
> first place.

Your assessment is correct — HW GRO on hns3 does NOT zero IP ToS.

My earlier Tested-by claiming "HW GRO zeroes IP ToS" was wrong. The
tos=0x0 I observed was caused by a test methodology error: I used
iperf3 with --tos but did not set net.ipv4.tcp_ecn=1, so the sender's
kernel TCP stack cleared the ECN bits before the packets hit the wire.
I mistakenly attributed this TX-side clearing to HW GRO.

I have retested with tesgine (direct packet construction, bypassing
the kernel TCP stack) and confirmed:
   - HW GRO preserves IP ToS — each aggregate group retains the head
     packet's ToS value (verified with 0x00/0x02/0x03/0x10/0xB8)
   - HW GRO segments flows by ToS boundaries — any ToS change (DSCP or
     ECN bits) terminates the current aggregate group
   - ECN signals are intact — CE packets (tos=0x03) are delivered
     independently with the CE mark preserved
     
hns3 HW GRO is behaving correctly. Sorry for the confusion caused by
my earlier report.

Full test details in my correction reply to the patch 2/2 thread:
https://lore.kernel.org/all/dd817755-eb4d-4ac2-b282-ac06f847270d@huawei.com/

Regards,
Jijie Shao

Re: [PATCH v5 net-next 2/2] net: hns3: fix GSO_ECN flag setting in the RX path

[Jijie Shao]

on 2026/8/11 21:31, Jijie Shao wrote:
>
> on 2026/8/5 5:35, chia-yu.chang@nokia-bell-labs.com wrote:
>> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
>>
>> Currently, the hns3 Rx path sets the SKB_GSO_TCP_ECN flag when a TCP
>> segment with the CWR flag set is received. This is incorrect because
>> the hns3 hardware GRO resets the IP ToS byte to 0 during packet
>> aggregation. Consequently, no valid ECN signals are carreid, meaning
>> that it cannot support RFC3168 ECN or Accurate ECN protocols. Setting
>> SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN under these conditions misleads
>> the upper network stack.
>>
>> Fix this by removing the incorrect SKB_GSO_TCP_ECN flag assignment
>> in hns3_gro_complete().
>
>
> Tested on hns3 HW (2x 100G, direct cable, openEuler 24.03, kernel 
> 7.2.0-rc6).
>
> tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets carry
> tos 0x0, while non-aggregated packets keep their ToS/ECN marks — so the
> zeroing is done by HW GRO, not the sender. Before patch, CWR packets had
> SKB_GSO_TCP_ECN set (0x5 = SKB_GSO_TCPV4 | SKB_GSO_TCP_ECN). After patch,
> all GRO events gso_type=0x1 (SKB_GSO_TCPV4 only), no 0x5 observed.
>
> Tested-by: Jijie Shao <shaojijie@huawei.com>


I need to correct my earlier test report where I stated:

   "tcpdump confirms HW GRO zeroes IP ToS: aggregated (>MTU) packets
    carry tos 0x0, while non-aggregated packets keep their ToS/ECN
    marks — so the zeroing is done by HW GRO, not the sender"

This conclusion was wrong. I am withdrawing that Tested-by tag.

Root cause of the error
========================

I mixed two test tools with different sender-side paths and incorrectly
compared their results:

1. The observation that "non-aggregated packets keep their ToS/ECN
    marks" came from a Python script (scapy). Scapy constructs L2/L3
    packets directly, bypassing the kernel TCP stack, so the ToS value
    on the wire is exactly what the script sets — this says nothing
    about whether a kernel-stack sender would clear ToS or not.

2. The observation that "aggregated packets carry tos 0x0" came from
    iperf3 with --tos. iperf3 goes through the kernel TCP stack, and I
    did not set net.ipv4.tcp_ecn=1, so the kernel cleared the ECN bits
    (low 2 bits of ToS) before the packets hit the wire. The tos=0x0 on
    aggregated packets was already tos=0x0 on the wire at the sender
    side — it was a TX-side kernel behavior, not HW GRO zeroing.

I mistakenly used the scapy observation ("non-aggregated packets keep
ToS") as evidence that "the sender didn't clear ToS", then concluded
that iperf3's tos=0x0 must be caused by HW GRO. But these two
observations came from different sender paths and cannot be compared
this way. I only captured at the receiver side with no sender-side
wire capture to compare against, so I wrongly attributed the TX-side
clearing to HW GRO.

New test with tesgine (direct packet construction)
===================================================

To eliminate the kernel TCP stack interference, I retested using
tesgine, a packet generator that constructs TCP segments directly at
L2/L3 with full control over the IP ToS byte. This bypasses the kernel
TCP ECN state machine entirely, so the ToS value on the wire is exactly
what was configured.

Test setup:
   - hns3 NIC, HW GRO on, software GRO off (ethtool -K <dev> gro off)
   - tesgine sends 1460-byte TCP segments with controlled ToS values
   - Capture at receiver post-GRO

Four test cases were run, each verifying a different ToS boundary:

Test 1: Non-ECT <-> ECT(0)
   Sent:  4x tos=0x00 + 1x tos=0x02 + 4x tos=0x00 + 2x tos=0x02 + 1x tos=0x00
   Got:   4 aggregated (tos=0x00) | 1 single (tos=0x02)
        | 4 aggregated (tos=0x00) | 2 aggregated (tos=0x02)
        | 1 single (tos=0x00)
   => GRO segments at every ToS change, each group keeps its ToS.

Test 2: ECT(0) <-> CE
   Sent:  4x tos=0x02 + 1x tos=0x03 (CE) + 4x tos=0x02
   Got:   4 aggregated (tos=0x02) | 1 single (tos=0x03 CE)
        | 4 aggregated (tos=0x02)
   => CE packet is delivered independently, CE mark preserved.

Test 3: Non-ECT <-> CE
   Sent:  4x tos=0x00 + 1x tos=0x03 (CE)
   Got:   4 aggregated (tos=0x00) | 1 single (tos=0x03 CE)
   => Same behavior: CE is segmented out, ToS preserved.

Test 4: Pure DSCP change (ECN bits identical)
   Sent:  4x tos=0x10 (DSCP4) + 4x tos=0xB8 (DSCP46)
   Got:   4 aggregated (tos=0x10) | 4 aggregated (tos=0xB8)
   => Even pure DSCP change (no ECN bit change) triggers segmentation.

Conclusions from tesgine testing
================================

   1. HW GRO does NOT zero IP ToS. Each aggregate group retains the
      head packet's ToS value (verified with 0x00/0x02/0x03/0x10/0xB8).

   2. HW GRO segments flows by ToS boundaries. Any ToS change — whether
      DSCP (bits 0-5) or ECN (bits 6-7) — terminates the current
      aggregate group and starts a new one.

   3. ECN signals are preserved: CE packets (tos=0x03) are delivered
      independently with the CE mark intact, not merged into non-CE
      aggregates.

Impact on this patch
====================

The commit message states:

   "the hns3 hardware GRO resets the IP ToS byte to 0 during packet
    aggregation. Consequently, no valid ECN signals are carried"

This factual claim is incorrect per the tesgine test results above.
HW GRO preserves ToS and segments by ToS boundaries, so ECN signals
are not lost during aggregation.

The code change itself (removing the SKB_GSO_TCP_ECN assignment in
hns3_gro_complete) may still be correct, but the justification should
be based on the RX-path semantic argument from patch 1/2 (SKB_GSO_TCP_ECN
is a TX-path flag; using it in RX can corrupt the ACE signal in AccECN
forwarding scenarios), not on the ToS-zeroing claim. Or change to AccECN.

I will re-verify and provide a new Tested-by once the commit message is
updated. Sorry for the confusion caused by my earlier report.

Test captures and methodology details available on request.

Regards,
Jijie Shao