-----Original Message-----
From: Willem de Bruijn <[email protected]>
Sent: Tuesday, August 25, 2026 6:12 PM
To: Chia-Yu Chang (Nokia) <[email protected]>; Willem de Bruijn
<[email protected]>; Jakub Kicinski <[email protected]>
Cc: Mirja Kuehlewind <[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]; [email protected]; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected]; [email protected]; Koen De
Schepper (Nokia) <[email protected]>; [email protected];
Ingemar Johansson S <[email protected]>; [email protected];
[email protected]; [email protected]; [email protected]; Parav Pandit
<[email protected]>; Willem de Bruijn <[email protected]>; [email protected]
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 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?
> > Hi Willem,
> >
> > I think we can still change SKB_GSO_TCP_ECN into SKB_GSO_TCP_ACCECN on the
> > RX path, even if HW GRO and SW GRO use different aggregation rules.
> > Currently, SW GRO flushes when the CWR state changes and sets
> > SKB_GSO_TCP_ACCECN when the resulting skb carries CWR=1.
> >
> >
> > For HW GRO, if the device flushes immediately when a CWR=1 packet arrives,
> > there is no need to set either SKB_GSO_TCP_ECN or SKB_GSO_TCP_ACCECN.
> > In that case, each CWR=1 packet is emitted separately, and there is no need
> > to preserve multiple CWR indications through GSO.
> > So, if the HW can be confirmed to always flush CWR-marked packets without
> > coalescing them, preserving multiple CWR indications through GSO is not
> > required.
> > In that case, using SKB_GSO_TCP_ACCECN would not provide any additional
> > benefit.
>
> I agree.
>
> The difference on transmit is that SKB_GSO_TCP_ACCECN will copy the ECN bits
> to every segment, whereas SKB_GSO_TCP_ECN will only set the bits on the first
> segment.
>
> From commit 023af5a72ab1 ("gso: AccECN support") that introduced
> SKB_GSO_TCP_ACCECN:
>
> With RFC 3168 ECN aware TSO (NETIF_F_TSO_ECN) CWR flag is cleared
> starting from 2nd segment which is incompatible how AccECN handles
> the CWR flag. Such super-segments are indicated by SKB_GSO_TCP_ECN.
> With AccECN, CWR flag (or more accurately, the ACE field that also
> includes ECE & AE flags) changes only when new packet(s) with CE
> mark arrives so the flag should not be changed within a super-skb.
>
> Makes me wonder what SKB_GS_TCP_ACCECN adds. Copying bits from the GSO skb to
> all segments is the default. SKB_GSO_TCP_ECN is an indication that the NIC
> knows how to diverge from this default for these specific bits.
>
> That commit confirms this:
>
> If NIC is completely unaware of RFC3168 ECN (doesn't support
> NETIF_F_TSO_ECN) or its TSO engine can be set to not touch CWR flag
> despite supporting also NETIF_F_TSO_ECN, TSO could be safely used
> with AccECN on such NIC. This should be evaluated per NIC basis
> (not done in this patch series for any NICs).`
>
> >
> > The problematic case is when a HW GRO aggregates multiple packets carrying
> > CWR=1 and only sets SKB_GSO_TCP_ECN.
>
> The driver of such a device could be updated to set SKB_GSO_TCP_ACCECN or not
> set any such ECN GSO flag.
>
> The driver of other devices that do follow RFC 3168 semantics will continue
> to have to set SKB_GSO_TCP_ECN on the GSO skb. It is quite plausible that few
> or no devices currently support ECN in their HW-GRO coalescing.
>
Yes, I agree.
In another email I was asking Jijie how their hns3 HW-GRO handles CWR-marked
packets.
If their device flushes immediately when a CWR=1 packet arrives, then the
SKB_GSO_TCP_ECN handling in hns3_enet.c may not be necessary.
In that case, each CWR-marked packet is emitted separately and there may be no
need to use either SKB_GSO_TCP_ACCECN or SKB_GSO_TCP_ECN.
If their HW-GRO can coalesce multiple CWR=1 packets, flagging the resulting skb
with SKB_GSO_TCP_ECN would lose AccECN information during segmentation.
In that case, the skb should either carry SKB_GSO_TCP_ACCECN, or no ECN GSO
flag at all if the segmentation engine preserves the CWR signaling unchanged.
> > During GSO, only the first output segment would carry CWR=1, which loses
> > the remaining CWR signaling information required by AccECN.
> > In that case, changing from SKB_GSO_TCP_ECN to SKB_GSO_TCP_ACCECN would
> > preserve the original signaling by ensuring that the segmented packets
> > carry the correct CWR information.
> > As I understand RFC3168, receiving additional CWR-marked packets is
> > harmless, because once a valid CWR has been received, the receiver already
> > stops echoing ECE.
>
> Agreed.
>
> > Additional CWR indications do not change the receiver state.
> > This would have a performance impact since it disables HW TSO for the skb
> > and falls back to software segmentation.
>
> You mean if the device advertises NETIF_F_TSO_ECN, and GSO skbs may contain
> AccECN signals, the host must downgrade from TSO to GSO to avoid corrupting
> the signal?
>
> The cost of that would be significant. Not something to do for established
> environments. But technically seemingly correct, at least for packets with
> non-zero ECN bits.
Yes, this is what I mean. And I agree the cost would not be small.
So, I am thinking of first checking the existing drivers that still use
SKB_GSO_TCP_ECN. So far I have only found hns3_enet.
I also plan to update the comments for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN
in include/linux/skbuff.h to capture the conclusions from this discussion.
Then we can at least clarify the expected semantics and review whether the
existing HW-GRO implementations and drivers are consistent with them.
Also, patch 6f74bc8b6e8d0 already added a flags_cwr GRO test, which may be
useful when reviewing the expected behavior of future HW-GRO implementations.
>
> > But IMO it preserves correctness for both RFC3168 ECN and AccECN.
>
> Side-note: this thread probably has way too many Cc: for this narrow topic.
