> -----Original Message----- > From: Paolo Abeni <[email protected]> > Sent: Tuesday, September 23, 2025 12:52 PM > To: Chia-Yu Chang (Nokia) <[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]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH v2 net-next 13/14] tcp: accecn: stop sending AccECN opt > when loss ACK w/ option > > > 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 9/18/25 6:21 PM, [email protected] wrote: > > From: Chia-Yu Chang <[email protected]> > > > > Detect spurious retransmission of a previously sent ACK carrying the > > AccECN option after the second retransmission. Since this might be > > caused by the middlebox dropping ACK with options it does not > > recognize, disable the sending of the AccECN option in all subsequent > > ACKs. This patch follows Section 3.2.3.2.2 of AccECN spec (RFC9768). > > Is this really useful/triggers in practice? > > AFAICS it will take effect only it the retransmission happens just after an > egress AccECN packet, i.e. will not trigger if the there are more later non > AccECN packets pending.
Hi Paolo, This is a simplied implementation than what is mentieond in the RFC9768: "Such a host detect loss of ACKs carrying the AccECN Option by detecting whether the acknowledged data alwaysreappears as a retransmission. In such cases, the host disable the sending of the AccECN Option for this half-connection." However, to implement the case that not that just after egressing the ACK with AccECN, I was thinking to modify struct tcp_sack_block but that maybe an over engineering. And IMO this could be useful with proper configuration on syscel tcp_accecn_option and tcp_ecn_option_beacon to see whether the middlebox is dropping all ACKs with AccECN option. An example packetdrill is at: https://github.com/minuscat/packetdrill_accecn/blob/main/gtests/net/tcp/accecn/fallback/client_accecn_options_drop.pkt What do you think? And maybe @[email protected] could help to comment from RFC perspective. Chia-Yu > > > > Signed-off-by: Chia-Yu Chang <[email protected]> > > --- > > include/linux/tcp.h | 3 ++- > > include/net/tcp_ecn.h | 1 + > > net/ipv4/tcp_input.c | 9 +++++++++ > > net/ipv4/tcp_output.c | 3 +++ > > 4 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/tcp.h b/include/linux/tcp.h index > > 53a4a0da35cc..f3b72fb6fa76 100644 > > --- a/include/linux/tcp.h > > +++ b/include/linux/tcp.h > > @@ -295,7 +295,8 @@ struct tcp_sock { > > u8 nonagle : 4,/* Disable Nagle algorithm? */ > > rate_app_limited:1; /* rate_{delivered,interval_us} limited? > > */ > > u8 received_ce_pending:4, /* Not yet transmit cnt of received_ce > > */ > > - unused2:4; > > + accecn_opt_sent:1,/* Sent AccECN option in previous ACK */ > > + unused2:3; > > u8 accecn_minlen:2,/* Minimum length of AccECN option sent */ > > est_ecnfield:2,/* ECN field for AccECN delivered estimates */ > > accecn_opt_demand:2,/* Demand AccECN option for n next > > ACKs */ diff --git a/include/net/tcp_ecn.h b/include/net/tcp_ecn.h > > index 8317c3f279c9..3eb450b4b648 100644 > > --- a/include/net/tcp_ecn.h > > +++ b/include/net/tcp_ecn.h > > @@ -401,6 +401,7 @@ static inline void tcp_accecn_init_counters(struct > > tcp_sock *tp) > > tp->received_ce_pending = 0; > > __tcp_accecn_init_bytes_counters(tp->received_ecn_bytes); > > __tcp_accecn_init_bytes_counters(tp->delivered_ecn_bytes); > > + tp->accecn_opt_sent = 0; > > tp->accecn_minlen = 0; > > tp->accecn_opt_demand = 0; > > tp->est_ecnfield = 0; > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index > > f250b4912a0b..6083260133e9 100644 > > --- a/net/ipv4/tcp_input.c > > +++ b/net/ipv4/tcp_input.c > > @@ -4804,6 +4804,7 @@ static void tcp_dsack_extend(struct sock *sk, > > u32 seq, u32 end_seq) > > > > static void tcp_rcv_spurious_retrans(struct sock *sk, const struct > > sk_buff *skb) { > > + struct tcp_sock *tp = tcp_sk(sk); > > Minor nit: please leave an empty line after variable declaration. > > /P
