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

[Jijie Shao <shaojijie@huawei.com>]

on 2026/4/25 22:30, Chia-Yu Chang (Nokia) wrote:
>> -----Original Message-----
>> From: Jijie Shao <shaojijie@huawei.com>
>> Sent: Saturday, April 25, 2026 11:35 AM
>> To: 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; 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; pabeni@redhat.com; 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
>> Cc: shaojijie@huawei.com
>> Subject: Re: [PATCH v4 net 3/3] net: hns3: fix CWR handling in drivers to preserve ACE signal
>>
>>
>> on 2026/4/17 23:26, chia-yu.chang@nokia-bell-labs.com wrote:
>>> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
>>>
>>> Currently, hns3 Rx paths use 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 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 ensure that RFC3168 ECN offload will not
>>> clear the CWR flag, therefore preserving the ACE signal.
>>>
>>> Fixes: d474d88f88261 ("net: hns3: add hns3_gro_complete for HW GRO
>>> process")
>>> Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
>>> ---
>>>    drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>>> b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>>> index a3206c97923e..e1b0dba56182 100644
>>> --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
>>> @@ -3904,7 +3904,7 @@ static int hns3_gro_complete(struct sk_buff
>>> *skb, u32 l234info)
>>>    
>>>    	skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
>>>    	if (th->cwr)
>>> -		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
>>> +		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ACCECN;
>>>    
>>>    	if (l234info & BIT(HNS3_RXD_GRO_FIXID_B))
>>>    		skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_FIXEDID;
>> I agree with Paolo's previous point;
>> for already released hardware, it is indeed not suitable to modify it.
>> During the hardware aggregation process, the ACE signal may have already been lost.
>>
>> Jijie Shao
> Hi Jijie,
>
> I would disagree with not fixing on released hardware. (Did Paolo explicit mention that?)
> The ACCECN protocol is based on ACE signal, and a broken ACE signal might be due to SKB_GSO_TCP_ECN at the RX path.
> You can see the explicit explanations and examples in the commit message.
> There is already a fix in patch 4e4f7cefb130af6aba6a393b2d13930b49390df9 for tcp_gro_receive() of tcp_offload.c
>
> And In this patch series, we would like to propose the similar fix on hns3 and mlx5e.
> While one main issue is to confirm is how the GRO is done in the corresponding HW-GRO.
> And if the driver can be safely changed from SKB_GSO_TCP_ECN to SKB_GSO_TCP_ACCECN, then we can ensure ECN and AccECN can be supported over existing hardware.

Sorry for the late reply.

It is confirmed that ACC_ECN is not supported.
HW-GRO will set the TOS field to 0.

Jijie Shao