diff mbox series

[v2,2/2] app/testpmd: fix SW L4 checksum in multi-segments

Message ID 20211229093702.1930214-3-kevinx.liu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series fix udp checksum error |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Kevin Liu Dec. 29, 2021, 9:37 a.m. UTC 
  Testpmd forwards packets in checksum mode that it needs to calculate
the checksum of each layer's protocol.

In process_inner_cksums, when parsing tunnel packets, inner L4 offset should be
outer_l2_len + outer_l3_len + l2_len + l3_len.

In process_outer_cksums, when parsing tunnel packets, outer L4 offset should be
outer_l2_len + outer_l3_len.

Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-segments")
Cc: stable@dpdk.org

Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
---
 app/test-pmd/csumonly.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Zhang, Yuying March 3, 2022, 6:29 a.m. UTC | #1 
LGTM.

> > -----Original Message-----
> > From: Liu, KevinX <kevinx.liu@intel.com>
> > Sent: Wednesday, December 29, 2021 5:37 PM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Yigit,
> > Ferruh <ferruh.yigit@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
> > stable@dpdk.org
> > Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> > multi-segments
> >
> > Testpmd forwards packets in checksum mode that it needs to calculate
> > the checksum of each layer's protocol.
> >
> > In process_inner_cksums, when parsing tunnel packets, inner L4 offset
> > should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> >
> > In process_outer_cksums, when parsing tunnel packets, outer L4 offset
> > should be outer_l2_len + outer_l3_len.
> >
> > Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> > segments")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>

Acked-by: Yuying Zhang <yuying.zhang@intel.com>

> > ---
> >  app/test-pmd/csumonly.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 02bc3929c7..c235456e58 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> 
> 
> > @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > testpmd_offload_info *info,  ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;  }
> > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > +l4_off = info->outer_l2_len +
> >   info->outer_l3_len +
> >   info->l2_len + info->l3_len;
> >  else
> > @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > testpmd_offload_info *info,  ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;  }
> > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > info->outer_l3_len +
> > +l4_off = info->outer_l2_len + info-
> > >outer_l3_len +
> >   info->l2_len + info->l3_len;
> >  else
> >  l4_off = info->l2_len + info->l3_len; @@ -
> > 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
> > testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
> > udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
> > get_udptcp_checksum(m, outer_l3_hdr,
> > -info->l2_len + info->outer_l3_len,
> > +info->outer_l2_len + info-
> > >outer_l3_len,
> >  info->outer_ethertype);
> >  }
> >
> > --
> > 2.33.1
>
Kevin Liu March 11, 2022, 7:04 a.m. UTC | #2 
Hi, Ferruh

Yuying has already reviewed it days ago.
If you can, I hope you can change the status as soon as possible and try to merge the code in RC4.
Thank you.

> -----Original Message-----
> From: Zhang, Yuying <yuying.zhang@intel.com>
> Sent: 2022年3月3日 14:30
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
> dev <dev@dpdk.org>
> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; dpdk stable
> <stable@dpdk.org>
> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> segments
> 
> LGTM.
> 
> > > -----Original Message-----
> > > From: Liu, KevinX <kevinx.liu@intel.com>
> > > Sent: Wednesday, December 29, 2021 5:37 PM
> > > To: dev@dpdk.org
> > > Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Yigit,
> > > Ferruh <ferruh.yigit@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
> > > stable@dpdk.org
> > > Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> > > multi-segments
> > >
> > > Testpmd forwards packets in checksum mode that it needs to calculate
> > > the checksum of each layer's protocol.
> > >
> > > In process_inner_cksums, when parsing tunnel packets, inner L4
> > > offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> > >
> > > In process_outer_cksums, when parsing tunnel packets, outer L4
> > > offset should be outer_l2_len + outer_l3_len.
> > >
> > > Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> > > segments")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> 
> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> 
> > > ---
> > >  app/test-pmd/csumonly.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > > 02bc3929c7..c235456e58 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> >
> >
> > > @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > > testpmd_offload_info *info,  ol_flags |=
> RTE_MBUF_F_TX_UDP_CKSUM;  }
> > > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > > +l4_off = info->outer_l2_len +
> > >   info->outer_l3_len +
> > >   info->l2_len + info->l3_len;
> > >  else
> > > @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const struct
> > > testpmd_offload_info *info,  ol_flags |=
> RTE_MBUF_F_TX_TCP_CKSUM;  }
> > > else {  if (info->is_tunnel) -l4_off = info->l2_len +
> > > info->outer_l3_len +
> > > +l4_off = info->outer_l2_len + info-
> > > >outer_l3_len +
> > >   info->l2_len + info->l3_len;
> > >  else
> > >  l4_off = info->l2_len + info->l3_len; @@ -
> > > 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
> > > testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
> > > udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
> > > get_udptcp_checksum(m, outer_l3_hdr,
> > > -info->l2_len + info->outer_l3_len,
> > > +info->outer_l2_len + info-
> > > >outer_l3_len,
> > >  info->outer_ethertype);
> > >  }
> > >
> > > --
> > > 2.33.1
> >
Singh, Aman Deep March 11, 2022, 8:02 a.m. UTC | #3 
Hi Kevin,

On 3/11/2022 12:34 PM, Liu, KevinX wrote:
> Hi, Ferruh
>
> Yuying has already reviewed it days ago.
> If you can, I hope you can change the status as soon as possible and try to merge the code in RC4.
> Thank you.
>
>> -----Original Message-----
>> From: Zhang, Yuying <yuying.zhang@intel.com>
>> Sent: 2022年3月3日 14:30
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
>> dev <dev@dpdk.org>
>> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
>> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xing, Beilei
>> <beilei.xing@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; dpdk stable
>> <stable@dpdk.org>
>> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
>> segments
>>
>> LGTM.
>>
>>>> -----Original Message-----
>>>> From: Liu, KevinX <kevinx.liu@intel.com>
>>>> Sent: Wednesday, December 29, 2021 5:37 PM
>>>> To: dev@dpdk.org
>>>> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Yigit,
>>>> Ferruh <ferruh.yigit@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
>>>> stable@dpdk.org
>>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
>>>> multi-segments
>>>>
>>>> Testpmd forwards packets in checksum mode that it needs to calculate
>>>> the checksum of each layer's protocol.
>>>>
>>>> In process_inner_cksums, when parsing tunnel packets, inner L4
>>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
>>>>
>>>> In process_outer_cksums, when parsing tunnel packets, outer L4
>>>> offset should be outer_l2_len + outer_l3_len.
>>>>
>>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
>>>> segments")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
>> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
>>
>>>> ---
>>>>   app/test-pmd/csumonly.c | 6 +++---
>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
>>>> 02bc3929c7..c235456e58 100644
>>>> --- a/app/test-pmd/csumonly.c
>>>> +++ b/app/test-pmd/csumonly.c
>>>
>>>> @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const struct
>>>> testpmd_offload_info *info,  ol_flags |=
>> RTE_MBUF_F_TX_UDP_CKSUM;  }
>>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
>>>> +l4_off = info->outer_l2_len +
>>>>    info->outer_l3_len +
>>>>    info->l2_len + info->l3_len;
This seems OK. A similar miss is present for TCP case also.
Can you please do the same for that. Line 537
>>>>   else
>>>> @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const struct
>>>> testpmd_offload_info *info,  ol_flags |=
>> RTE_MBUF_F_TX_TCP_CKSUM;  }
>>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
>>>> info->outer_l3_len +
>>>> +l4_off = info->outer_l2_len + info-
>>>>> outer_l3_len +
>>>>    info->l2_len + info->l3_len;
>>>>   else
>>>>   l4_off = info->l2_len + info->l3_len; @@ -
This change might not be required. As for normal packet (non-tunnel case)
l4_off = info->l2_len + info->l3_len;  should be valid.
Please re-check.


>>>> 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
>>>> testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
>>>> udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
>>>> get_udptcp_checksum(m, outer_l3_hdr,
>>>> -info->l2_len + info->outer_l3_len,
>>>> +info->outer_l2_len + info-
>>>>> outer_l3_len,
>>>>   info->outer_ethertype);
>>>>   }
>>>>
>>>> --
>>>> 2.33.1
Kevin Liu March 11, 2022, 8:12 a.m. UTC | #4 
> -----Original Message-----
> From: Singh, Aman Deep <aman.deep.singh@intel.com>
> Sent: 2022年3月11日 16:02
> To: Liu, KevinX <kevinx.liu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> dev <dev@dpdk.org>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>; dpdk stable <stable@dpdk.org>; Zhang, Yuying
> <yuying.zhang@intel.com>
> Subject: Re: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> segments
> 
> Hi Kevin,
> 
> On 3/11/2022 12:34 PM, Liu, KevinX wrote:
> > Hi, Ferruh
> >
> > Yuying has already reviewed it days ago.
> > If you can, I hope you can change the status as soon as possible and try to
> merge the code in RC4.
> > Thank you.
> >
> >> -----Original Message-----
> >> From: Zhang, Yuying <yuying.zhang@intel.com>
> >> Sent: 2022年3月3日 14:30
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Liu, KevinX
> >> <kevinx.liu@intel.com>; dev <dev@dpdk.org>
> >> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> >> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> >> Xing, Beilei <beilei.xing@intel.com>; Li, Xiaoyun
> >> <xiaoyun.li@intel.com>; dpdk stable <stable@dpdk.org>
> >> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> >> segments
> >>
> >> LGTM.
> >>
> >>>> -----Original Message-----
> >>>> From: Liu, KevinX <kevinx.liu@intel.com>
> >>>> Sent: Wednesday, December 29, 2021 5:37 PM
> >>>> To: dev@dpdk.org
> >>>> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> >>>> Yigit, Ferruh <ferruh.yigit@intel.com>; Liu, KevinX
> >>>> <kevinx.liu@intel.com>; stable@dpdk.org
> >>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> >>>> multi-segments
> >>>>
> >>>> Testpmd forwards packets in checksum mode that it needs to
> >>>> calculate the checksum of each layer's protocol.
> >>>>
> >>>> In process_inner_cksums, when parsing tunnel packets, inner L4
> >>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> >>>>
> >>>> In process_outer_cksums, when parsing tunnel packets, outer L4
> >>>> offset should be outer_l2_len + outer_l3_len.
> >>>>
> >>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> >>>> segments")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> >> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> >>
> >>>> ---
> >>>>   app/test-pmd/csumonly.c | 6 +++---
> >>>>   1 file changed, 3 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> >>>> index
> >>>> 02bc3929c7..c235456e58 100644
> >>>> --- a/app/test-pmd/csumonly.c
> >>>> +++ b/app/test-pmd/csumonly.c
> >>>
> >>>> @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const
> struct
> >>>> testpmd_offload_info *info,  ol_flags |=
> >> RTE_MBUF_F_TX_UDP_CKSUM;  }
> >>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
> >>>> +l4_off = info->outer_l2_len +
> >>>>    info->outer_l3_len +
> >>>>    info->l2_len + info->l3_len;
> This seems OK. A similar miss is present for TCP case also.
> Can you please do the same for that. Line 537
> >>>>   else
> >>>> @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const
> struct
> >>>> testpmd_offload_info *info,  ol_flags |=
> >> RTE_MBUF_F_TX_TCP_CKSUM;  }
> >>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
> >>>> info->outer_l3_len +
> >>>> +l4_off = info->outer_l2_len + info-
> >>>>> outer_l3_len +
> >>>>    info->l2_len + info->l3_len;
> >>>>   else
> >>>>   l4_off = info->l2_len + info->l3_len; @@ -
> This change might not be required. As for normal packet (non-tunnel case)
> l4_off = info->l2_len + info->l3_len;  should be valid.
> Please re-check.
I don't understand what you mean. I fix the code under the tunnel case, and I didn't modify the code for the non-tunnel case.
> 
> 
> >>>> 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
> >>>> testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
> >>>> udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
> >>>> get_udptcp_checksum(m, outer_l3_hdr,
> >>>> -info->l2_len + info->outer_l3_len,
> >>>> +info->outer_l2_len + info-
> >>>>> outer_l3_len,
> >>>>   info->outer_ethertype);
> >>>>   }
> >>>>
> >>>> --
> >>>> 2.33.1
Singh, Aman Deep March 11, 2022, 9:04 a.m. UTC | #5 
On 3/11/2022 1:42 PM, Liu, KevinX wrote:
>
>> -----Original Message-----
>> From: Singh, Aman Deep<aman.deep.singh@intel.com>
>> Sent: 2022年3月11日 16:02
>> To: Liu, KevinX<kevinx.liu@intel.com>; Zhang, Qi Z<qi.z.zhang@intel.com>;
>> dev<dev@dpdk.org>; Yigit, Ferruh<ferruh.yigit@intel.com>
>> Cc: Yang, Qiming<qiming.yang@intel.com>; Yang, SteveX
>> <stevex.yang@intel.com>; Xing, Beilei<beilei.xing@intel.com>; Li, Xiaoyun
>> <xiaoyun.li@intel.com>; dpdk stable<stable@dpdk.org>; Zhang, Yuying
>> <yuying.zhang@intel.com>
>> Subject: Re: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
>> segments
>>
>> Hi Kevin,
>>
>> On 3/11/2022 12:34 PM, Liu, KevinX wrote:
>>> Hi, Ferruh
>>>
>>> Yuying has already reviewed it days ago.
>>> If you can, I hope you can change the status as soon as possible and try to
>> merge the code in RC4.
>>> Thank you.
>>>
>>>> -----Original Message-----
>>>> From: Zhang, Yuying<yuying.zhang@intel.com>
>>>> Sent: 2022年3月3日 14:30
>>>> To: Zhang, Qi Z<qi.z.zhang@intel.com>; Liu, KevinX
>>>> <kevinx.liu@intel.com>; dev<dev@dpdk.org>
>>>> Cc: Yang, Qiming<qiming.yang@intel.com>; Yang, SteveX
>>>> <stevex.yang@intel.com>; Yigit, Ferruh<ferruh.yigit@intel.com>;
>>>> Xing, Beilei<beilei.xing@intel.com>; Li, Xiaoyun
>>>> <xiaoyun.li@intel.com>; dpdk stable<stable@dpdk.org>
>>>> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
>>>> segments
>>>>
>>>> LGTM.
>>>>
>>>>>> -----Original Message-----
>>>>>> From: Liu, KevinX<kevinx.liu@intel.com>
>>>>>> Sent: Wednesday, December 29, 2021 5:37 PM
>>>>>> To:dev@dpdk.org
>>>>>> Cc: Yang, Qiming<qiming.yang@intel.com>; Zhang, Qi Z
>>>>>> <qi.z.zhang@intel.com>; Yang, SteveX<stevex.yang@intel.com>;
>>>>>> Yigit, Ferruh<ferruh.yigit@intel.com>; Liu, KevinX
>>>>>> <kevinx.liu@intel.com>;stable@dpdk.org
>>>>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
>>>>>> multi-segments
>>>>>>
>>>>>> Testpmd forwards packets in checksum mode that it needs to
>>>>>> calculate the checksum of each layer's protocol.
>>>>>>
>>>>>> In process_inner_cksums, when parsing tunnel packets, inner L4
>>>>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
>>>>>>
>>>>>> In process_outer_cksums, when parsing tunnel packets, outer L4
>>>>>> offset should be outer_l2_len + outer_l3_len.
>>>>>>
>>>>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
>>>>>> segments")
>>>>>> Cc:stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Kevin Liu<kevinx.liu@intel.com>
>>>> Acked-by: Yuying Zhang<yuying.zhang@intel.com>

Acked-by: Aman Singh <aman.deep.singh@intel.com>

>>>>
>>>>>> ---
>>>>>>    app/test-pmd/csumonly.c | 6 +++---
>>>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
>>>>>> index
>>>>>> 02bc3929c7..c235456e58 100644
>>>>>> --- a/app/test-pmd/csumonly.c
>>>>>> +++ b/app/test-pmd/csumonly.c
>>>>>> @@ -513,7 +513,7 @@ process_inner_cksums(void *l3_hdr, const
>> struct
>>>>>> testpmd_offload_info *info,  ol_flags |=
>>>> RTE_MBUF_F_TX_UDP_CKSUM;  }
>>>>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
>>>>>> +l4_off = info->outer_l2_len +
>>>>>>     info->outer_l3_len +
>>>>>>     info->l2_len + info->l3_len;
>> This seems OK. A similar miss is present for TCP case also.
>> Can you please do the same for that. Line 537
>>>>>>    else
>>>>>> @@ -536,7 +536,7 @@ process_inner_cksums(void *l3_hdr, const
>> struct
>>>>>> testpmd_offload_info *info,  ol_flags |=
>>>> RTE_MBUF_F_TX_TCP_CKSUM;  }
>>>>>> else {  if (info->is_tunnel) -l4_off = info->l2_len +
>>>>>> info->outer_l3_len +
>>>>>> +l4_off = info->outer_l2_len + info-
>>>>>>> outer_l3_len +
>>>>>>     info->l2_len + info->l3_len;
>>>>>>    else
>>>>>>    l4_off = info->l2_len + info->l3_len; @@ -
>> This change might not be required. As for normal packet (non-tunnel case)
>> l4_off = info->l2_len + info->l3_len;  should be valid.
>> Please re-check.
> I don't understand what you mean. I fix the code under the tunnel case, and I didn't modify the code for the non-tunnel case.
Sorry, my bad.
>>
>>>>>> 625,7 +625,7 @@ process_outer_cksums(void *outer_l3_hdr, struct
>>>>>> testpmd_offload_info *info,  if (udp_hdr->dgram_cksum != 0) {
>>>>>> udp_hdr->dgram_cksum = 0;  udp_hdr->dgram_cksum =
>>>>>> get_udptcp_checksum(m, outer_l3_hdr,
>>>>>> -info->l2_len + info->outer_l3_len,
>>>>>> +info->outer_l2_len + info-
>>>>>>> outer_l3_len,
>>>>>>    info->outer_ethertype);
>>>>>>    }
>>>>>>
>>>>>> --
>>>>>> 2.33.1
Ferruh Yigit March 11, 2022, 10:46 a.m. UTC | #6 
On 3/11/2022 7:04 AM, Liu, KevinX wrote:

message moved down, please do not top post

>> -----Original Message-----
>> From: Zhang, Yuying <yuying.zhang@intel.com>
>> Sent: 2022年3月3日 14:30
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
>> dev <dev@dpdk.org>
>> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
>> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xing, Beilei
>> <beilei.xing@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; dpdk stable
>> <stable@dpdk.org>
>> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
>> segments
>>
>> LGTM.
>>
>>>> -----Original Message-----
>>>> From: Liu, KevinX <kevinx.liu@intel.com>
>>>> Sent: Wednesday, December 29, 2021 5:37 PM
>>>> To: dev@dpdk.org
>>>> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>; Yigit,
>>>> Ferruh <ferruh.yigit@intel.com>; Liu, KevinX <kevinx.liu@intel.com>;
>>>> stable@dpdk.org
>>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
>>>> multi-segments
>>>>
>>>> Testpmd forwards packets in checksum mode that it needs to calculate
>>>> the checksum of each layer's protocol.
>>>>
>>>> In process_inner_cksums, when parsing tunnel packets, inner L4
>>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
>>>>
>>>> In process_outer_cksums, when parsing tunnel packets, outer L4
>>>> offset should be outer_l2_len + outer_l3_len.
>>>>
>>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
>>>> segments")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
>>
>> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
>>
> Hi, Ferruh
> 
> Yuying has already reviewed it days ago.
> If you can, I hope you can change the status as soon as possible and try to merge the code in RC4.
> Thank you.
> 

+Thomas, he is getting patches for -rc4.

And just for double check Qi, are you comfortable with patch?
(I am asking because initially this is set with ice and testpmd patch)
Qi Zhang March 14, 2022, 2:33 a.m. UTC | #7 
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Friday, March 11, 2022 6:47 PM
> To: Liu, KevinX <kevinx.liu@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; dev
> <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>; dpdk stable <stable@dpdk.org>; Zhang, Yuying
> <yuying.zhang@intel.com>
> Subject: Re: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-segments
> 
> On 3/11/2022 7:04 AM, Liu, KevinX wrote:
> 
> message moved down, please do not top post
> 
> >> -----Original Message-----
> >> From: Zhang, Yuying <yuying.zhang@intel.com>
> >> Sent: 2022年3月3日 14:30
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Liu, KevinX
> >> <kevinx.liu@intel.com>; dev <dev@dpdk.org>
> >> Cc: Yang, Qiming <qiming.yang@intel.com>; Yang, SteveX
> >> <stevex.yang@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> >> Xing, Beilei <beilei.xing@intel.com>; Li, Xiaoyun
> >> <xiaoyun.li@intel.com>; dpdk stable <stable@dpdk.org>
> >> Subject: RE: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in multi-
> >> segments
> >>
> >> LGTM.
> >>
> >>>> -----Original Message-----
> >>>> From: Liu, KevinX <kevinx.liu@intel.com>
> >>>> Sent: Wednesday, December 29, 2021 5:37 PM
> >>>> To: dev@dpdk.org
> >>>> Cc: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>; Yang, SteveX <stevex.yang@intel.com>;
> >>>> Yigit, Ferruh <ferruh.yigit@intel.com>; Liu, KevinX
> >>>> <kevinx.liu@intel.com>; stable@dpdk.org
> >>>> Subject: [PATCH v2 2/2] app/testpmd: fix SW L4 checksum in
> >>>> multi-segments
> >>>>
> >>>> Testpmd forwards packets in checksum mode that it needs to
> >>>> calculate the checksum of each layer's protocol.
> >>>>
> >>>> In process_inner_cksums, when parsing tunnel packets, inner L4
> >>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> >>>>
> >>>> In process_outer_cksums, when parsing tunnel packets, outer L4
> >>>> offset should be outer_l2_len + outer_l3_len.
> >>>>
> >>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> >>>> segments")
> >>>> Cc: stable@dpdk.org
> >>>>
> >>>> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> >>
> >> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> >>
> > Hi, Ferruh
> >
> > Yuying has already reviewed it days ago.
> > If you can, I hope you can change the status as soon as possible and try to
> merge the code in RC4.
> > Thank you.
> >
> 
> +Thomas, he is getting patches for -rc4.
> 
> And just for double check Qi, are you comfortable with patch?
> (I am asking because initially this is set with ice and testpmd patch)

Yes, I'm OK with this patch. It has been verified on most of Intel PMDs.
Thomas Monjalon March 14, 2022, 8:21 p.m. UTC | #8 
14/03/2022 03:33, Zhang, Qi Z:
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > On 3/11/2022 7:04 AM, Liu, KevinX wrote:
> > > From: Zhang, Yuying <yuying.zhang@intel.com>
> > >>> From: Liu, KevinX <kevinx.liu@intel.com>
> > >>>> Testpmd forwards packets in checksum mode that it needs to
> > >>>> calculate the checksum of each layer's protocol.
> > >>>>
> > >>>> In process_inner_cksums, when parsing tunnel packets, inner L4
> > >>>> offset should be outer_l2_len + outer_l3_len + l2_len + l3_len.
> > >>>>
> > >>>> In process_outer_cksums, when parsing tunnel packets, outer L4
> > >>>> offset should be outer_l2_len + outer_l3_len.
> > >>>>
> > >>>> Fixes: e6b9d6411e91 ("app/testpmd: add SW L4 checksum in multi-
> > >>>> segments")
> > >>>> Cc: stable@dpdk.org
> > >>>>
> > >>>> Signed-off-by: Kevin Liu <kevinx.liu@intel.com>
> > >>
> > >> Acked-by: Yuying Zhang <yuying.zhang@intel.com>
> > >>
> > > Hi, Ferruh
> > >
> > > Yuying has already reviewed it days ago.
> > > If you can, I hope you can change the status as soon as possible and try to
> > merge the code in RC4.
> > > Thank you.
> > >
> > 
> > +Thomas, he is getting patches for -rc4.
> > 
> > And just for double check Qi, are you comfortable with patch?
> > (I am asking because initially this is set with ice and testpmd patch)
> 
> Yes, I'm OK with this patch. It has been verified on most of Intel PMDs.

Applied, thanks.
diff mbox series

Patch

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 02bc3929c7..c235456e58 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -513,7 +513,7 @@  process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 				ol_flags |= RTE_MBUF_F_TX_UDP_CKSUM;
 			} else {
 				if (info->is_tunnel)
-					l4_off = info->l2_len +
+					l4_off = info->outer_l2_len +
 						 info->outer_l3_len +
 						 info->l2_len + info->l3_len;
 				else
@@ -536,7 +536,7 @@  process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 			ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
 		} else {
 			if (info->is_tunnel)
-				l4_off = info->l2_len + info->outer_l3_len +
+				l4_off = info->outer_l2_len + info->outer_l3_len +
 					 info->l2_len + info->l3_len;
 			else
 				l4_off = info->l2_len + info->l3_len;
@@ -625,7 +625,7 @@  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
 		udp_hdr->dgram_cksum = get_udptcp_checksum(m, outer_l3_hdr,
-					info->l2_len + info->outer_l3_len,
+					info->outer_l2_len + info->outer_l3_len,
 					info->outer_ethertype);
 	}