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

-----Original Message-----
From: Paolo Abeni <pabeni@redhat.com> 
Sent: Tuesday, November 18, 2025 1:02 PM
To: Chia-Yu Chang (Nokia) <redacted>; 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) <redacted>; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire <redacted>; rs.ietf@gmx.at; Jason_Livingood@comcast.com; Vidhi Goel <redacted>
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:

quoted

From: Chia-Yu Chang <redacted>

No functional changes.

Some real commit message is needed.

quoted

Co-developed-by: Ilpo Järvinen <ij@kernel.org>
Signed-off-by: Ilpo Järvinen <ij@kernel.org>
Signed-off-by: Chia-Yu Chang <redacted>

---
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