Re: [PATCH v2 net-next 04/14] tcp: L4S ECT(1) identifier and NEEDS_ACCECN for CC modules
From: Paolo Abeni <pabeni@redhat.com>
Date: 2025-09-23 10:05:04
Also in:
bpf, linux-doc, linux-kselftest
On 9/18/25 6:21 PM, chia-yu.chang@nokia-bell-labs.com wrote:
From: Chia-Yu Chang <redacted> Two CA module flags are added in this patch. First, a new CA module flag (TCP_CONG_NEEDS_ACCECN) defines that the CA expects to negotiate AccECN functionality using the ECE, CWR and AE flags in the TCP header. The detailed AccECN negotiaotn during the 3WHS can be found in the AccECN spec: https://tools.ietf.org/id/draft-ietf-tcpm-accurate-ecn-28.txt Second, when ECN is negociated for a TCP flow, it defaults to use ECT(0) in the IP header. L4S service, however, needs to se ECT(1). This patch enables CA to control whether ECT(0) or ECT(1) should be used on a per-segment basis. A new flag (TCP_CONG_WANTS_ECT_1)
I find this description confusing/contradictory with the implementation where TCP_CONG_WANTS_ECT_1 is actually a mask.
quoted hunk ↗ jump to hunk
@@ -1322,6 +1328,18 @@ static inline bool tcp_ca_needs_ecn(const struct sock *sk) return icsk->icsk_ca_ops->flags & TCP_CONG_NEEDS_ECN; } +static inline bool tcp_ca_needs_accecn(const struct sock *sk) +{ + const struct inet_connection_sock *icsk = inet_csk(sk); + + return icsk->icsk_ca_ops->flags & TCP_CONG_NEEDS_ACCECN; +} + +static inline bool tcp_ca_wants_ect_1(const struct sock *sk) +{ + return inet_csk(sk)->icsk_ca_ops->flags & TCP_CONG_WANTS_ECT_1;
Should the above tests be: (inet_csk(sk)->icsk_ca_ops->flags & TCP_CONG_WANTS_ECT_1) == TCP_CONG_WANTS_ECT_1 ? Otherwise existing CC with TCP_CONG_NEEDS_ECN will unexpectedly switch to ECT_1 usage. [...]
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index df758adbb445..f9efbcf1d856 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c@@ -227,7 +227,7 @@ void tcp_assign_congestion_control(struct sock *sk) memset(icsk->icsk_ca_priv, 0, sizeof(icsk->icsk_ca_priv)); if (ca->flags & TCP_CONG_NEEDS_ECN) - INET_ECN_xmit(sk); + __INET_ECN_xmit(sk, tcp_ca_wants_ect_1(sk));
Possibly a new helper for the above statement could be useful /P