[PATCH v2] net/mlx5: fix lro update tcp header cksum error

[PATCH v2] net/mlx5: fix lro update tcp header cksum error

[jiangheng (G)]

The variable csum is the sum of three 16 bits integers, the max value
is 0x2FFFD. The corner case of sum of 3 is 0x1FFFF gives the wrong
result: 0x1 + 0xFFFF = 0x10000, the upper 16 bits are not 0.
It must be folded again to ensure that the upper 16 bits are 0.

Fixes: e4c2a16eb1de ("net/mlx5: handle LRO packets in Rx queue")
Cc: stable@dpdk.org

Signed-off-by: jiangheng <jiangheng14@huawei.com>
---
 drivers/net/mlx5/mlx5_rx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
index a2be523e9e..ae537dfffa 100644
--- a/drivers/net/mlx5/mlx5_rx.c
+++ b/drivers/net/mlx5/mlx5_rx.c
@@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *__rte_restrict tcp,
 	tcp->cksum = 0;
 	csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
 	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
+	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
 	csum = (~csum) & 0xffff;
 	if (csum == 0)
 		csum = 0xffff;
-- 
2.27.0

> Hi,  Jiangheng
>
> You are right, the corner case of sum of 3 is 0x1FFFF gives the wrong result.
> Could you,  please, format the patch according to the rules and send v2 ?
> - add Fixes: tag with reference to appropriate commit
> - add Cc: stable@dpdk.org
> - fix typos in commit message - capitalize sentences, add trailing points, etc.
>
> With best regards,
> Slava
>
> > From: jiangheng (G) <jiangheng14@huawei.com>
> > Sent: среда, 12 апреля 2023 г. 14:39
> > To: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko 
> > <viacheslavo@nvidia.com>
> > Subject: [PATCH] net/mlx5: fix lro update tcp header cksum error
> > 
> > csum is the sum of three 16 bits value it must be folded twice to 
> > ensure that the upper 16 bits are 0
> > ---
> >  drivers/net/mlx5/mlx5_rx.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c 
> > index a2be523e9e..ae537dfffa 100644
> > --- a/drivers/net/mlx5/mlx5_rx.c
> > +++ b/drivers/net/mlx5/mlx5_rx.c
> > @@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr 
> > *__rte_restrict tcp,
> >     tcp->cksum = 0;
> >     csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
> >     csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > +   csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> >     csum = (~csum) & 0xffff;
> >     if (csum == 0)
> >         csum = 0xffff;
> > --
> > 2.27.0

Re: [PATCH v2] net/mlx5: fix lro update tcp header cksum error

[Thomas Monjalon]

Any comment on this patch?

13/04/2023 02:57, jiangheng (G):
> The variable csum is the sum of three 16 bits integers, the max value
> is 0x2FFFD. The corner case of sum of 3 is 0x1FFFF gives the wrong
> result: 0x1 + 0xFFFF = 0x10000, the upper 16 bits are not 0.
> It must be folded again to ensure that the upper 16 bits are 0.
> 
> Fixes: e4c2a16eb1de ("net/mlx5: handle LRO packets in Rx queue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: jiangheng <jiangheng14@huawei.com>
> ---
>  drivers/net/mlx5/mlx5_rx.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
> index a2be523e9e..ae537dfffa 100644
> --- a/drivers/net/mlx5/mlx5_rx.c
> +++ b/drivers/net/mlx5/mlx5_rx.c
> @@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr *__rte_restrict tcp,
>  	tcp->cksum = 0;
>  	csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
>  	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> +	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
>  	csum = (~csum) & 0xffff;
>  	if (csum == 0)
>  		csum = 0xffff;
> > Hi,  Jiangheng
> >
> > You are right, the corner case of sum of 3 is 0x1FFFF gives the wrong result.
> > Could you,  please, format the patch according to the rules and send v2 ?
> > - add Fixes: tag with reference to appropriate commit
> > - add Cc: stable@dpdk.org
> > - fix typos in commit message - capitalize sentences, add trailing points, etc.
> >
> > With best regards,
> > Slava
> >
> > > From: jiangheng (G) <jiangheng14@huawei.com>
> > > Sent: среда, 12 апреля 2023 г. 14:39
> > > To: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko 
> > > <viacheslavo@nvidia.com>
> > > Subject: [PATCH] net/mlx5: fix lro update tcp header cksum error
> > > 
> > > csum is the sum of three 16 bits value it must be folded twice to 
> > > ensure that the upper 16 bits are 0
> > > ---
> > >  drivers/net/mlx5/mlx5_rx.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c 
> > > index a2be523e9e..ae537dfffa 100644
> > > --- a/drivers/net/mlx5/mlx5_rx.c
> > > +++ b/drivers/net/mlx5/mlx5_rx.c
> > > @@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr 
> > > *__rte_restrict tcp,
> > >     tcp->cksum = 0;
> > >     csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
> > >     csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > > +   csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > >     csum = (~csum) & 0xffff;
> > >     if (csum == 0)
> > >         csum = 0xffff;
> > > --
> > > 2.27.0

RE: [PATCH v2] net/mlx5: fix lro update tcp header cksum error

[Slava Ovsiienko]

Hi,

rte_raw_cksum() is doing __rte_raw_cksum_reduce(), so the returned result is always below 0x10000, upper half is zero.
We add 3 16-bit values in range 0...0xFFFF, what is possible result? Let's consider the ranges:
upper/lower halves
0          0..0xFFFF, no carry
1          0..0xFFFF, one carry  - once we add upper half - we'll get carry again, should be added
2          0..0xFFFD, two carries 

So, yes, we need this patch, sorry I've missed adjusted v2.

Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

With best regards,
Slava

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, July 13, 2023 3:16 PM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>
> Cc: dev@dpdk.org; jiangheng (G) <jiangheng14@huawei.com>
> Subject: Re: [PATCH v2] net/mlx5: fix lro update tcp header cksum error
> 
> Any comment on this patch?
> 
> 13/04/2023 02:57, jiangheng (G):
> > The variable csum is the sum of three 16 bits integers, the max value
> > is 0x2FFFD. The corner case of sum of 3 is 0x1FFFF gives the wrong
> > result: 0x1 + 0xFFFF = 0x10000, the upper 16 bits are not 0.
> > It must be folded again to ensure that the upper 16 bits are 0.
> >
> > Fixes: e4c2a16eb1de ("net/mlx5: handle LRO packets in Rx queue")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: jiangheng <jiangheng14@huawei.com>
> > ---
> >  drivers/net/mlx5/mlx5_rx.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/mlx5/mlx5_rx.c b/drivers/net/mlx5/mlx5_rx.c
> > index a2be523e9e..ae537dfffa 100644
> > --- a/drivers/net/mlx5/mlx5_rx.c
> > +++ b/drivers/net/mlx5/mlx5_rx.c
> > @@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr
> *__rte_restrict tcp,
> >  	tcp->cksum = 0;
> >  	csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
> >  	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > +	csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> >  	csum = (~csum) & 0xffff;
> >  	if (csum == 0)
> >  		csum = 0xffff;
> > > Hi,  Jiangheng
> > >
> > > You are right, the corner case of sum of 3 is 0x1FFFF gives the wrong
> result.
> > > Could you,  please, format the patch according to the rules and send v2 ?
> > > - add Fixes: tag with reference to appropriate commit
> > > - add Cc: stable@dpdk.org
> > > - fix typos in commit message - capitalize sentences, add trailing points,
> etc.
> > >
> > > With best regards,
> > > Slava
> > >
> > > > From: jiangheng (G) <jiangheng14@huawei.com>
> > > > Sent: среда, 12 апреля 2023 г. 14:39
> > > > To: dev@dpdk.org; Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> > > > <viacheslavo@nvidia.com>
> > > > Subject: [PATCH] net/mlx5: fix lro update tcp header cksum error
> > > >
> > > > csum is the sum of three 16 bits value it must be folded twice to
> > > > ensure that the upper 16 bits are 0
> > > > ---
> > > >  drivers/net/mlx5/mlx5_rx.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/net/mlx5/mlx5_rx.c
> > > > b/drivers/net/mlx5/mlx5_rx.c index a2be523e9e..ae537dfffa 100644
> > > > --- a/drivers/net/mlx5/mlx5_rx.c
> > > > +++ b/drivers/net/mlx5/mlx5_rx.c
> > > > @@ -1090,6 +1090,7 @@ mlx5_lro_update_tcp_hdr(struct rte_tcp_hdr
> > > > *__rte_restrict tcp,
> > > >     tcp->cksum = 0;
> > > >     csum += rte_raw_cksum(tcp, (tcp->data_off >> 4) * 4);
> > > >     csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > > > +   csum = ((csum & 0xffff0000) >> 16) + (csum & 0xffff);
> > > >     csum = (~csum) & 0xffff;
> > > >     if (csum == 0)
> > > >         csum = 0xffff;
> > > > --
> > > > 2.27.0
> 

RE: [PATCH v2] net/mlx5: fix lro update tcp header cksum error

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: jiangheng (G) <jiangheng14@huawei.com>
> Sent: Thursday, April 13, 2023 3:58 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org; Matan Azrad
> <matan@nvidia.com>
> Subject: [PATCH v2] net/mlx5: fix lro update tcp header cksum error
> 
> The variable csum is the sum of three 16 bits integers, the max value is
> 0x2FFFD. The corner case of sum of 3 is 0x1FFFF gives the wrong
> result: 0x1 + 0xFFFF = 0x10000, the upper 16 bits are not 0.
> It must be folded again to ensure that the upper 16 bits are 0.
> 
> Fixes: e4c2a16eb1de ("net/mlx5: handle LRO packets in Rx queue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: jiangheng <jiangheng14@huawei.com>
Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh