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

-----Original Message-----
From: Willem de Bruijn <[email protected]> 
Sent: Friday, August 14, 2026 9:15 PM
To: Jakub Kicinski <[email protected]>
Cc: Chia-Yu Chang (Nokia) <[email protected]>; 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]>
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, Aug 14, 2026 at 3:01 PM Jakub Kicinski <[email protected]> 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?
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.


The problematic case is when a HW GRO aggregates multiple packets carrying CWR=1 and only sets SKB_GSO_TCP_ECN.
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.
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.
But IMO it preserves correctness for both RFC3168 ECN and AccECN.


Best regards,
Chia-Yu
> 
> >
> > > 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.