On 6/10/25 2:53 PM, chia-yu.ch...@nokia-bell-labs.com wrote:
> From: Ilpo Järvinen <i...@kernel.org>
>
> This change implements Accurate ECN without negotiation and
> AccECN Option (that will be added by later changes). Based on
> AccECN specifications:
> https://tools.ietf.org/id/draft-ietf-tcpm-accurate-ecn-28.txt
>
> Accurate ECN allows feeding back the number of CE (congestion
> experienced) marks accurately to the sender in contrast to
> RFC3168 ECN that can only signal one marks-seen-yes/no per RTT.
> Congestion control algorithms can take advantage of the accurate
> ECN information to fine-tune their congestion response to avoid
> drastic rate reduction when only mild congestion is encountered.
>
> With Accurate ECN, tp->received_ce (r.cep in AccECN spec) keeps
> track of how many segments have arrived with a CE mark. Accurate
> ECN uses ACE field (ECE, CWR, AE) to communicate the value back
> to the sender which updates tp->delivered_ce (s.cep) based on the
> feedback. This signalling channel is lossy when ACE field overflow
> occurs.
>
> Conservative strategy is selected here to deal with the ACE
> overflow, however, some strategies using the AccECN option later
> in the overall patchset mitigate against false overflows detected.
>
> The ACE field values on the wire are offset by
> TCP_ACCECN_CEP_INIT_OFFSET. Delivered_ce/received_ce count the
> real CE marks rather than forcing all downstream users to adapt
> to the wire offset.
>
> This patch uses the first 1-byte hole and the last 4-byte hole of
> the tcp_sock_write_txrx for 'received_ce_pending' and 'received_ce'.
> Also, the group size of tcp_sock_write_txrx is increased from
> 91 + 4 to 95 + 4 due to the new u32 received_ce member. Below are
> the trimmed pahole outcomes before and after this patch.
AFAICS 'received_ce' fills the existing 4 bytes hole, so
tcp_sock_write_txrx size should be now (95 + 0), am I missreading something?
> @@ -384,17 +387,16 @@ static void tcp_data_ecn_check(struct sock *sk, const
> struct sk_buff *skb)
> if (tcp_ca_needs_ecn(sk))
> tcp_ca_event(sk, CA_EVENT_ECN_IS_CE);
>
> - if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR)) {
> + if (!(tp->ecn_flags & TCP_ECN_DEMAND_CWR) &&
> + tcp_ecn_mode_rfc3168(tp)) {
> /* Better not delay acks, sender can have a very low
> cwnd */
> tcp_enter_quickack_mode(sk, 2);
> tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
> }
> - tp->ecn_flags |= TCP_ECN_SEEN;
It's not clear why you need to move this statement earlier in the code
path even for ecn_mode_rfc3168(). Either a comment or
if (!tcp_ecn_mode_rfc3168(tp))
break;
a few lines aboved could help.
> break;
> default:
> if (tcp_ca_needs_ecn(sk))
> tcp_ca_event(sk, CA_EVENT_ECN_NO_CE);
> - tp->ecn_flags |= TCP_ECN_SEEN;
Same here.
Thanks,
Paolo
