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

[Dragos Tatulea <dtatulea@nvidia.com>]

On 18.05.26 11:20, Chia-Yu Chang (Nokia) wrote:
>> -----Original Message-----
>> From: Dragos Tatulea <dtatulea@nvidia.com> 
>> Sent: Thursday, April 23, 2026 10:13 PM
>> To: Paolo Abeni <pabeni@redhat.com>; Chia-Yu Chang (Nokia) <chia-yu.chang@nokia-bell-labs.com>; linyunsheng@huawei.com; andrew+netdev@lunn.ch; parav@nvidia.com; jasowang@redhat.com; mst@redhat.com; shenjian15@huawei.com; salil.mehta@huawei.com; shaojijie@huawei.com; saeedm@nvidia.com; tariqt@nvidia.com; mbloch@nvidia.com; leonro@nvidia.com; linux-rdma@vger.kernel.org; netdev@vger.kernel.org; davem@davemloft.net; edumazet@google.com; kuba@kernel.org; horms@kernel.org; ij@kernel.org; ncardwell@google.com; Koen De Schepper (Nokia) <koen.de_schepper@nokia-bell-labs.com>; g.white@cablelabs.com; ingemar.s.johansson@ericsson.com; mirja.kuehlewind@ericsson.com; cheshire@apple.com; rs.ietf@gmx.at; Jason_Livingood@comcast.com; vidhi_goel@apple.com
>> Subject: Re: [PATCH v4 net 2/3] net: mlx5e: fix CWR handling in drivers to preserve ACE signal
>>
>>
>> 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 23.04.26 19:40, Paolo Abeni wrote:
>>> On 4/23/26 4:19 PM, Dragos Tatulea wrote:
>>>> On 23.04.26 09:30, Paolo Abeni wrote:
>>>> [...]
>>>>>> ---
>>>>>>  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:
>>>>>
>>>> Sorry for missing this question in v3.
>>>>
>>>>> 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.
>>>>>
>>>> For mlx5 HW-GRO a packet with the CWR flag will flush the previous 
>>>> GRO session and will not start a GRO session for this packet 
>>>> (napi_gro_receive() will be called on this single segment skb).
>>>>
>>>> So this change won't impact the current GRO behavior from the mlx5 driver/hw side.
>>>
>>> OK, thanks!
>>>
>>> For my education: doesn't the above also means that mlx5 will never 
>>> build GSO packets with CWR set (and so the above statement should 
>>> never be reached)?
>>>
>> It does look like it... Thanks for pointing it out! Will be addressed in a patch.
>>
>> Thanks,
>> Dragos
> 
> Hi Paolo and Dragos,
> 
> First, sorry for my late reply and thanks for clarification.
> 
> The table below is to ensure my understanding of mlx5 is correct.
> 
> +===================+==========+===============+=================+
> |     Packet id     | CWR flag |   Flushing?   |     gso_type    |
> +===================+==========+===============+=================+
> |         0         |     0    |       0       |        0        |
> |        ...        |     0    |       0       |        0        |
> |       n - 1       |     0    |       1       |        0        |
> +-------------------+----------+---------------+-----------------+
> |         n         |     1    |       1       | SKB_GSO_TCP_ECN |
> |       n + 1       |     0    |       1       |        0        |
> +-------------------+----------+---------------+-----------------+
> |       n + 2       |     1    |       1       | SKB_GSO_TCP_ECN |
> |       n + 3       |     1    |       1       | SKB_GSO_TCP_ECN |
> +===================+==========+===============+=================+
> 
> Currently, mlx5 will flush GRO session and emit a single packet when the CWR flag is set on the RX.
> 
> So, this patch (changing gso_type from SKB_GSO_TCP_ECN to SKB_GSO_TCP_ACCECN) seems more for future-proof than a bug fix.
> 
> I will update the commit message to spcify that (or please suggest other changes needed).
>
Hi Chia-Yu,

Based on Paolo's comment, we have a pending patch that completely
removes the setting of the gso_type in the HW-GRO path.

Thanks,
Dragos