Re: [PATCH v4 net 2/3] net: mlx5e: fix CWR handling in drivers to preserve ACE signal

[Paolo Abeni <pabeni@redhat.com>]

On 4/17/26 5:26 PM, chia-yu.chang@nokia-bell-labs.com wrote:
> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> 
> Currently, mlx5 Rx paths use the SKB_GSO_TCP_ECN flag when a TCP segment
> with the CWR flag set. This is wrong because SKB_GSO_TCP_ECN is only
> valid for RFC3168 ECN on Tx, and using it on Rx allows RFC3168 ECN
> offload to clear the CWR flag. As a result, incoming TCP segments
> may lose their ACE signal integrity required for AccECN (RFC9768),
> especially when the packet is forwarded and later re-segmented by GSO.
> 
> Fix this by setting SKB_GSO_TCP_ACCECN for any Rx segment with the CWR
> flag set. SKB_GSO_TCP_ACCECN ensures that RFC3168 ECN offload will
> not clear the CWR flag, therefore preserving the ACE signal.
> 
> Fixes: 92552d3abd329 ("net/mlx5e: HW_GRO cqe handler implementation")
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 5b60aa47c75b..9b1c80079532 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -1180,7 +1180,7 @@ static void mlx5e_shampo_update_ipv4_tcp_hdr(struct mlx5e_rq *rq, struct iphdr *
>  	skb->csum_offset = offsetof(struct tcphdr, check);
>  
>  	if (tcp->cwr)
> -		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
> +		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ACCECN;

Here there is an open question for nVidia:

Is the above enough or will later segmentation lead to the wrong
results? I think/guess the firmware is (still) aggregating the wire
frames using the ECN schema, i.e. the first wire packet has CWR == 1,
the later CWR==0.

If so, later segmentation of this GSO packet will emit CWR == 1 on all
the packets, making the egress stream different from ingress.

@Saeed, Leon, Tariq: could you please have a look here?

I guess that with a more conservative approach drivers update should be
omitted, and the updated documentation should be less forceful (i.e.
"TCP_ECN should not be used in RX")

/P

Thanks,

Paolo