Re: patch 'net/ena: fix bad checksum handling' has been queued to stable release 21.11.8

[Kevin Traynor <ktraynor@redhat.com>]

On 02/09/2024 14:01, Brandes, Shai wrote:
> Hi Kevin, 
> 
> The fix is indeed required as it fixes an issue where checksum was inspected for non tcp/udp packets.
> If you need I can prepare a dedicated patch for this on top of 21.11.8

Hi Shai,

No need, I rebased the patch with the below and pushed to:

https://git.dpdk.org/dpdk-stable/commit/?h=21.11&id=80d05a5ea82dcb5d79b90af95d8a0fa4c556651f

thanks,
Kevin.

> It should go into https://github.com/kevintraynor/dpdk-stable/blob/21.11/drivers/net/ena/ena_ethdev.c#L309:
> 
> 		packet_type |= RTE_PTYPE_L3_IPV6;
> 	}
> 
> -	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag) {
> +	if (!ena_rx_ctx->l4_csum_checked || ena_rx_ctx->frag ||
> +		!(packet_type & (RTE_PTYPE_L4_TCP | RTE_PTYPE_L4_UDP))) {
> 		ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
> 	else
> 		if (unlikely(ena_rx_ctx->l4_csum_err))
> 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
> 		else
> 			ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
> 
> All the best,
> Shai
> 
> 
>> -----Original Message-----
>> From: Kevin Traynor <ktraynor@redhat.com>
>> Sent: Monday, August 26, 2024 2:18 PM
>> To: Brandes, Shai <shaibran@amazon.com>
>> Cc: dpdk stable <stable@dpdk.org>
>> Subject: RE: [EXTERNAL] patch 'net/ena: fix bad checksum handling' has been
>> queued to stable release 21.11.8
>>
>> CAUTION: This email originated from outside of the organization. Do not click
>> links or open attachments unless you can confirm the sender and know the
>> content is safe.
>>
>>
>>
>> On 26/08/2024 11:26, Brandes, Shai wrote:
>>> Hi Kevin,
>>>
>>
>> Hi Shai,
>>
>>> I think there is one net/ena patch that was not picked up for the stable
>> release (likely since there are two patches with similar title).
>>> I would appreciate if you could verify.
>>>
>>> The missing patch: net/ena: fix wrong handling of checksum see
>>> patchwork
>>> https://patches.dpdk.org/project/dpdk/patch/20240702144626.14545-14-
>> sh
>>> aibran@amazon.com/
>>>
>>
>>
>> Thanks for checking! It wasn't listed as relevant for 21.11 LTS as the 'Fixes:'
>> tag indicates it is fixing a feature introduced after 21.11.
>>
>> commit f66055c7564050e55c7eab147d039bf01048829a
>> Author: Shai Brandes <shaibran@amazon.com>
>> Date:   Tue Jul 2 17:46:24 2024 +0300
>>
>>     net/ena: fix checksum handling
>>
>>     This change fixes an issue where a non tcp/udp packet can be indicated
>>     to have an invalid csum. If the device erroneously tries to verify the
>>     csum on a non tcp/udp packet it will result in false indication that
>>     there is a csum error. This change make the driver ignore the
>>     indication for csum error on such packets.
>>
>>     Fixes: 84daba9962b5 ("net/ena: add extra Rx checksum related xstats")
>>     Cc: stable@dpdk.org
>>
>>     Signed-off-by: Shai Brandes <shaibran@amazon.com>
>>
>>
>> $ git tag --contains 84daba9962b5
>> v22.03
>> ...
>>
>> If you believe the Fixes: tag is incorrect and it should go to 21.11 branch, let
>> me know (though I see it doesn't apply cleanly).
>>
>> thanks,
>> Kevin.
>>
>>
>>> All the best,
>>> Shai Brandes
>>> ENA drivers team
>>>
>>>> -----Original Message-----
>>>> From: Kevin Traynor <ktraynor@redhat.com>
>>>> Sent: Friday, August 23, 2024 7:19 PM
>>>> To: Brandes, Shai <shaibran@amazon.com>
>>>> Cc: dpdk stable <stable@dpdk.org>
>>>> Subject: [EXTERNAL] patch 'net/ena: fix bad checksum handling' has
>>>> been queued to stable release 21.11.8
>>>>
>>>> CAUTION: This email originated from outside of the organization. Do
>>>> not click links or open attachments unless you can confirm the sender
>>>> and know the content is safe.
>>>>
>>>>
>>>>
>>>> Hi,
>>>>
>>>> FYI, your patch has been queued to stable release 21.11.8
>>>>
>>>> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
>>>> It will be pushed if I get no objections before 08/28/24. So please
>>>> shout if anyone has objections.
>>>>
>>>> Also note that after the patch there's a diff of the upstream commit
>>>> vs the patch applied to the branch. This will indicate if there was
>>>> any rebasing needed to apply to the stable branch. If there were code
>>>> changes for rebasing
>>>> (ie: not only metadata diffs), please double check that the rebase
>>>> was correctly done.
>>>>
>>>> Queued patches are on a temporary branch at:
>>>> https://github.com/kevintraynor/dpdk-stable
>>>>
>>>> This queued commit can be viewed at:
>>>> https://github.com/kevintraynor/dpdk-
>>>> stable/commit/082bd6be7b93ae090f2bdcc55fddc48c3bf16c2f
>>>>
>>>> Thanks.
>>>>
>>>> Kevin
>>>>
>>>> ---
>>>> From 082bd6be7b93ae090f2bdcc55fddc48c3bf16c2f Mon Sep 17 00:00:00
>>>> 2001
>>>> From: Shai Brandes <shaibran@amazon.com>
>>>> Date: Tue, 2 Jul 2024 17:46:22 +0300
>>>> Subject: [PATCH] net/ena: fix bad checksum handling
>>>>
>>>> [ upstream commit fa4bb7025255036fcff9556c637efa2627ac4af4 ]
>>>>
>>>> Removed a workaround for a false L4 bad Rx csum indication from the
>>>> device. The workaround was to set it as unknown so the application
>>>> would check it instead.
>>>> The issue was fixed in the device, thus the driver bad csum handling
>>>> should be fixed in the PMD.
>>>>
>>>> Fixes: b2d2f1cf89a6 ("net/ena: fix checksum flag for L4")
>>>>
>>>> Signed-off-by: Shai Brandes <shaibran@amazon.com>
>>>> ---
>>>>  drivers/net/ena/ena_ethdev.c | 8 +-------
>>>>  1 file changed, 1 insertion(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ena/ena_ethdev.c
>>>> b/drivers/net/ena/ena_ethdev.c index 1c61f793e6..84c41f1b9d 100644
>>>> --- a/drivers/net/ena/ena_ethdev.c
>>>> +++ b/drivers/net/ena/ena_ethdev.c
>>>> @@ -311,11 +311,5 @@ static inline void ena_rx_mbuf_prepare(struct
>>>> rte_mbuf *mbuf,
>>>>         else
>>>>                 if (unlikely(ena_rx_ctx->l4_csum_err))
>>>> -                       /*
>>>> -                        * For the L4 Rx checksum offload the HW may indicate
>>>> -                        * bad checksum although it's valid. Because of that,
>>>> -                        * we're setting the UNKNOWN flag to let the app
>>>> -                        * re-verify the checksum.
>>>> -                        */
>>>> -                       ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_UNKNOWN;
>>>> +                       ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_BAD;
>>>>                 else
>>>>                         ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
>>>> --
>>>> 2.46.0
>>>>
>>>> ---
>>>>   Diff of the applied patch vs upstream commit (please double-check
>>>> if non-
>>>> empty:
>>>> ---
>>>> --- -   2024-08-23 17:18:12.913173354 +0100
>>>> +++ 0109-net-ena-fix-bad-checksum-handling.patch        2024-08-23
>>>> 17:18:09.849430556 +0100
>>>> @@ -1 +1 @@
>>>> -From fa4bb7025255036fcff9556c637efa2627ac4af4 Mon Sep 17 00:00:00
>>>> 2001
>>>> +From 082bd6be7b93ae090f2bdcc55fddc48c3bf16c2f Mon Sep 17 00:00:00
>>>> 2001
>>>> @@ -5,0 +6,2 @@
>>>> +[ upstream commit fa4bb7025255036fcff9556c637efa2627ac4af4 ]
>>>> +
>>>> @@ -13 +14,0 @@
>>>> -Cc: stable@dpdk.org
>>>> @@ -21 +22 @@
>>>> -index 4e7171e629..b43b913903 100644
>>>> +index 1c61f793e6..84c41f1b9d 100644
>>>> @@ -24,3 +25,3 @@
>>>> -@@ -675,11 +675,5 @@ static inline void ena_rx_mbuf_prepare(struct
>>>> ena_ring *rx_ring,
>>>> -               if (unlikely(ena_rx_ctx->l4_csum_err)) {
>>>> -                       ++rx_stats->l4_csum_bad;
>>>> +@@ -311,11 +311,5 @@ static inline void ena_rx_mbuf_prepare(struct
>>>> rte_mbuf *mbuf,
>>>> +       else
>>>> +               if (unlikely(ena_rx_ctx->l4_csum_err))
>>>> @@ -35,2 +36,2 @@
>>>> -               } else {
>>>> -                       ++rx_stats->l4_csum_good;
>>>> +               else
>>>> +                       ol_flags |= RTE_MBUF_F_RX_L4_CKSUM_GOOD;
>>>
>