From: Slava Ovsiienko <viacheslavo@nvidia.com>
To: "NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
Matan Azrad <matan@nvidia.com>,
Raslan Darawsheh <rasland@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "jiangheng (G)" <jiangheng14@huawei.com>
Subject: RE: [PATCH v2] net/mlx5: fix lro update tcp header cksum error
Date: Fri, 14 Jul 2023 09:10:54 +0000 [thread overview]
Message-ID: <DM6PR12MB37531FFC4E87DD28724D4159DF34A@DM6PR12MB3753.namprd12.prod.outlook.com> (raw)
In-Reply-To: <3300841.tIAgqjz4sF@thomas>
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
>
next prev parent reply other threads:[~2023-07-14 9:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-13 0:57 jiangheng (G)
2023-07-13 12:16 ` Thomas Monjalon
2023-07-14 9:10 ` Slava Ovsiienko [this message]
2023-07-18 11:58 ` Raslan Darawsheh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR12MB37531FFC4E87DD28724D4159DF34A@DM6PR12MB3753.namprd12.prod.outlook.com \
--to=viacheslavo@nvidia.com \
--cc=dev@dpdk.org \
--cc=jiangheng14@huawei.com \
--cc=matan@nvidia.com \
--cc=rasland@nvidia.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).