diff mbox series

[1/2] app/testpmd: fix csumonly mode when run without outer chksum

Message ID 20210816070943.26807-1-ndabilpuram@marvell.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [1/2] app/testpmd: fix csumonly mode when run without outer chksum |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Nithin Dabilpuram Aug. 16, 2021, 7:09 a.m. UTC 
  Donot use outer metadata when neither outer ip checksum nor
outer udp checksum is enabled. PMD's will ignore the
outer_l2_len and outer_l3_len in cases where none of
the outer checksum is enabled and hence only l2_len and
l3_len will be used to calculate the offsets for L2 or L3
header.

Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
Cc: jia.guo@intel.com
Cc: stable@dpdk.org

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 app/test-pmd/csumonly.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Li, Xiaoyun Aug. 16, 2021, 8:48 a.m. UTC | #1 
Hi

> -----Original Message-----
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Sent: Monday, August 16, 2021 15:10
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
> Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without outer
> chksum
> 
> Donot use outer metadata when neither outer ip checksum nor outer udp
> checksum is enabled. PMD's will ignore the outer_l2_len and outer_l3_len in
> cases where none of the outer checksum is enabled and hence only l2_len and
> l3_len will be used to calculate the offsets for L2 or L3 header.

I don't understand.
In process_outer_chksum, only PKT_TX_OUTER_IPV6 Will be set if it's ipv6 packets.
So PKT_TX_OUTER_IPV6 means this packet is tunnel ipv6 packet. So it actually needs outer l2 len and outer l3 len put them to hw.
At least i40e needs outer l2 len and outer l3 len as far as I know.

> 
> Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
> Cc: jia.guo@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  app/test-pmd/csumonly.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> 607c889..38cc256 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -961,8 +961,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  			    (tx_offloads &
>  			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
>  			    (tx_offloads &
> -			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> -			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
> +			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
>  				m->outer_l2_len = info.outer_l2_len;
>  				m->outer_l3_len = info.outer_l3_len;
>  				m->l2_len = info.l2_len;
> --
> 2.8.4
Nithin Dabilpuram Aug. 16, 2021, 10:55 a.m. UTC | #2 
On Mon, Aug 16, 2021 at 08:48:20AM +0000, Li, Xiaoyun wrote:
> Hi
> 
> > -----Original Message-----
> > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > Sent: Monday, August 16, 2021 15:10
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
> > <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
> > Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without outer
> > chksum
> > 
> > Donot use outer metadata when neither outer ip checksum nor outer udp
> > checksum is enabled. PMD's will ignore the outer_l2_len and outer_l3_len in
> > cases where none of the outer checksum is enabled and hence only l2_len and
> > l3_len will be used to calculate the offsets for L2 or L3 header.
> 
> I don't understand.
> In process_outer_chksum, only PKT_TX_OUTER_IPV6 Will be set if it's ipv6 packets.
> So PKT_TX_OUTER_IPV6 means this packet is tunnel ipv6 packet. So it actually needs outer l2 len and outer l3 len put them to hw.
> At least i40e needs outer l2 len and outer l3 len as far as I know.

Ok, do you mean m->outer_l2_len and m->outer_l3_len will be referred in i40e driver
even when both DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM and 
DEV_TX_OFFLOAD_OUTER_UDP_CKSUM are not enabled in ethdev Tx offloads ?

I thought as per spec, those fields will only be used when the Outer offloads
are enabled.

> 
> > 
> > Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
> > Cc: jia.guo@intel.com
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> >  app/test-pmd/csumonly.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > 607c889..38cc256 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -961,8 +961,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >  			    (tx_offloads &
> >  			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
> >  			    (tx_offloads &
> > -			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> > -			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
> > +			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
> >  				m->outer_l2_len = info.outer_l2_len;
> >  				m->outer_l3_len = info.outer_l3_len;
> >  				m->l2_len = info.l2_len;
> > --
> > 2.8.4
>
Li, Xiaoyun Aug. 17, 2021, 1:43 a.m. UTC | #3 
> -----Original Message-----
> From: Nithin Dabilpuram <nithind1988@gmail.com>
> Sent: Monday, August 16, 2021 18:56
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; jia.guo@intel.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix csumonly mode when run
> without outer chksum
> 
> On Mon, Aug 16, 2021 at 08:48:20AM +0000, Li, Xiaoyun wrote:
> > Hi
> >
> > > -----Original Message-----
> > > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > Sent: Monday, August 16, 2021 15:10
> > > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > > Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
> > > <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
> > > Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without
> > > outer chksum
> > >
> > > Donot use outer metadata when neither outer ip checksum nor outer
> > > udp checksum is enabled. PMD's will ignore the outer_l2_len and
> > > outer_l3_len in cases where none of the outer checksum is enabled
> > > and hence only l2_len and l3_len will be used to calculate the offsets for L2
> or L3 header.
> >
> > I don't understand.
> > In process_outer_chksum, only PKT_TX_OUTER_IPV6 Will be set if it's ipv6
> packets.
> > So PKT_TX_OUTER_IPV6 means this packet is tunnel ipv6 packet. So it actually
> needs outer l2 len and outer l3 len put them to hw.
> > At least i40e needs outer l2 len and outer l3 len as far as I know.
> 
> Ok, do you mean m->outer_l2_len and m->outer_l3_len will be referred in i40e
> driver even when both DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM and
> DEV_TX_OFFLOAD_OUTER_UDP_CKSUM are not enabled in ethdev Tx offloads ?
> 
> I thought as per spec, those fields will only be used when the Outer offloads are
> enabled.

Checked again. You're right. Tx offloads are enough.

> 
> >
> > >
> > > Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
> > > Cc: jia.guo@intel.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > ---
> > >  app/test-pmd/csumonly.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > > 607c889..38cc256 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> > > @@ -961,8 +961,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > >  			    (tx_offloads &
> > >  			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
> > >  			    (tx_offloads &
> > > -			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> > > -			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
> > > +			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
> > >  				m->outer_l2_len = info.outer_l2_len;
> > >  				m->outer_l3_len = info.outer_l3_len;
> > >  				m->l2_len = info.l2_len;
> > > --
> > > 2.8.4
> >
Li, Xiaoyun Aug. 17, 2021, 1:44 a.m. UTC | #4 
> -----Original Message-----
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Sent: Monday, August 16, 2021 15:10
> To: Li, Xiaoyun <xiaoyun.li@intel.com>
> Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
> Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without outer
> chksum
> 
> Donot use outer metadata when neither outer ip checksum nor outer udp
> checksum is enabled. PMD's will ignore the outer_l2_len and outer_l3_len in
> cases where none of the outer checksum is enabled and hence only l2_len and
> l3_len will be used to calculate the offsets for L2 or L3 header.
> 
> Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
> Cc: jia.guo@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  app/test-pmd/csumonly.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> 607c889..38cc256 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -961,8 +961,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  			    (tx_offloads &
>  			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
>  			    (tx_offloads &
> -			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> -			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
> +			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
>  				m->outer_l2_len = info.outer_l2_len;
>  				m->outer_l3_len = info.outer_l3_len;
>  				m->l2_len = info.l2_len;
> --
> 2.8.4

Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
Nithin Dabilpuram Aug. 17, 2021, 10:58 a.m. UTC | #5 
On Tue, Aug 17, 2021 at 01:43:27AM +0000, Li, Xiaoyun wrote:
> 
> 
> > -----Original Message-----
> > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > Sent: Monday, August 16, 2021 18:56
> > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > Cc: jerinj@marvell.com; dev@dpdk.org; jia.guo@intel.com; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix csumonly mode when run
> > without outer chksum
> > 
> > On Mon, Aug 16, 2021 at 08:48:20AM +0000, Li, Xiaoyun wrote:
> > > Hi
> > >
> > > > -----Original Message-----
> > > > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > > Sent: Monday, August 16, 2021 15:10
> > > > To: Li, Xiaoyun <xiaoyun.li@intel.com>
> > > > Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
> > > > <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
> > > > Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without
> > > > outer chksum
> > > >
> > > > Donot use outer metadata when neither outer ip checksum nor outer
> > > > udp checksum is enabled. PMD's will ignore the outer_l2_len and
> > > > outer_l3_len in cases where none of the outer checksum is enabled
> > > > and hence only l2_len and l3_len will be used to calculate the offsets for L2
> > or L3 header.
> > >
> > > I don't understand.
> > > In process_outer_chksum, only PKT_TX_OUTER_IPV6 Will be set if it's ipv6
> > packets.
> > > So PKT_TX_OUTER_IPV6 means this packet is tunnel ipv6 packet. So it actually
> > needs outer l2 len and outer l3 len put them to hw.
> > > At least i40e needs outer l2 len and outer l3 len as far as I know.
> > 
> > Ok, do you mean m->outer_l2_len and m->outer_l3_len will be referred in i40e
> > driver even when both DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM and
> > DEV_TX_OFFLOAD_OUTER_UDP_CKSUM are not enabled in ethdev Tx offloads ?
> > 
> > I thought as per spec, those fields will only be used when the Outer offloads are
> > enabled.
> 
> Checked again. You're right. Tx offloads are enough.

Ack, thanks.
> 
> > 
> > >
> > > >
> > > > Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
> > > > Cc: jia.guo@intel.com
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > > ---
> > > >  app/test-pmd/csumonly.c | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> > > > 607c889..38cc256 100644
> > > > --- a/app/test-pmd/csumonly.c
> > > > +++ b/app/test-pmd/csumonly.c
> > > > @@ -961,8 +961,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > > >  			    (tx_offloads &
> > > >  			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
> > > >  			    (tx_offloads &
> > > > -			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
> > > > -			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
> > > > +			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
> > > >  				m->outer_l2_len = info.outer_l2_len;
> > > >  				m->outer_l3_len = info.outer_l3_len;
> > > >  				m->l2_len = info.l2_len;
> > > > --
> > > > 2.8.4
> > >
Ferruh Yigit Aug. 25, 2021, 12:57 p.m. UTC | #6 
On 8/17/2021 2:44 AM, Li, Xiaoyun wrote:
> 
> 
>> -----Original Message-----
>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>> Sent: Monday, August 16, 2021 15:10
>> To: Li, Xiaoyun <xiaoyun.li@intel.com>
>> Cc: jerinj@marvell.com; dev@dpdk.org; Nithin Dabilpuram
>> <ndabilpuram@marvell.com>; jia.guo@intel.com; stable@dpdk.org
>> Subject: [PATCH 1/2] app/testpmd: fix csumonly mode when run without outer
>> chksum
>>
>> Donot use outer metadata when neither outer ip checksum nor outer udp
>> checksum is enabled. PMD's will ignore the outer_l2_len and outer_l3_len in
>> cases where none of the outer checksum is enabled and hence only l2_len and
>> l3_len will be used to calculate the offsets for L2 or L3 header.
>>
>> Fixes: 3c32113a1aac ("app/testpmd: fix IPv6 tunnel checksum")
>> Cc: jia.guo@intel.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 

Series applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 607c889..38cc256 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -961,8 +961,7 @@  pkt_burst_checksum_forward(struct fwd_stream *fs)
 			    (tx_offloads &
 			     DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM) ||
 			    (tx_offloads &
-			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) ||
-			    (tx_ol_flags & PKT_TX_OUTER_IPV6)) {
+			     DEV_TX_OFFLOAD_OUTER_UDP_CKSUM)) {
 				m->outer_l2_len = info.outer_l2_len;
 				m->outer_l3_len = info.outer_l3_len;
 				m->l2_len = info.l2_len;