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

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.