RE: [PATCH v6 net-next 03/14] net: update commnets for SKB_GSO_TCP_ECN and SKB_GSO_TCP_ACCECN

Chia-Yu Chang (Nokia)
2025-11-19T10:24:37Z

> -----Original Message-----
> From: Paolo Abeni <pabeni@redhat.com>
> Sent: Tuesday, November 18, 2025 1:02 PM
> To: Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>; edumazet@google.com; parav@nvidia.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 <cheshire@apple.com>; rs.ietf@gmx.at; Jason_Livingood@comcast.com; Vidhi Goel <vidhi_goel@apple.com>
> Subject: Re: [PATCH v6 net-next 03/14] net: update commnets 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.
>
>
>
> Note: typo in the subj
>
> On 11/14/25 8:13 AM, chia-yu.chang@nokia-bell-labs.com wrote:
> > From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> >
> > No functional changes.
>
> Some real commit message is needed.
>
> >
> > 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>
> >
> > ---
> > v6:
> > - Update comments.
> > ---
> > include/linux/skbuff.h | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index
> > ff90281ddf90..e09455cee8e3 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 the first TCP segment has CWR set, and any
> > + * subsequent segment in the same skb has CWR cleared. This is not
> > + * used on Rx except for virtio_net. However, because the connection
> > + * to which the segment belongs is not tracked to use RFC3168 or
> > + * Accurate ECN, and using RFC3168 ECN offload may corrupt AccECN
> > + * signal of AccECN segments. Therefore, this cannot be used on Rx.
>
> Stating both that is used by virtio_net and can not be used in the RX path is a bit confusing. Random Contributor may be tempted from removing ECN support from virtio_net
>
> Please state explicitly:
> - why it makes sense to use this in virtio_net
> - this must not be used in the RX path _outside_ the virtio net driver
>
> something alike:
>
> /* For Tx, this indicates the first TCP segment has CWR set, and any
> * subsequent segment in the same skb has CWR cleared. However, because
> * the connection to which the segment belongs is not tracked to use
> * RFC3168 or Accurate ECN, and using RFC3168 ECN offload may corrupt
> * AccECN signal of AccECN segments. Therefore, this cannot be used on
> * Rx outside the virtio_net driver. Such exception exist due to
> * <reason>
> */
>
> /P
Hi Paolo and Ilpo,

I was thinking to totally remove ECN from Rx path, and add the comments only in AccECN, like below:
Because we could use SKB_GSO_TCP_ACCECN in Rx to explicitly tell latter GSO Tx in a forwarding case that do NOT clean CWR flag.
What do you think?

/* For Tx, this indicates the first TCP segment has CWR set, and any
* subsequent segment in the same skb has CWR cleared. However, because
* the connection to which the segment belongs is not tracked to use
* RFC3168 or Accurate ECN, and using RFC3168 ECN offload may corrupt
* ACE signal of AccECN segments. Therefore, this cannot be used on Rx.
*/
SKB_GSO_TCP_ECN = 1 << 2,
[...]
/* For TX, this indicates the TCP segment uses the CWR flag as part of
* ACE signal, and the CWR flag is not modified in the skb. For RX, any
* CWR flagged segment must use SKB_GSO_TCP_ACCECN to ensure CWR flag
* is not cleared by any RFC3168 ECN offload, and thus keeping ACE
* signal of AccECN segments. This is particularly used for Rx of
* virtio_net driver in order to tell latter GSO Tx in a forwarding
* scenario that it is NOT ok to clean CWR flag from the 2nd segment.
*/
SKB_GSO_TCP_ACCECN = 1 << 19,

Chia-Yu