[dpdk-dev] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[dpdk-dev] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[Xiaolong Ye]

Hi,

This patch series is to correct the wrong setting of
XXX_TX_OFFLOAD_MASK in multiple PMDs.

Just as the name XXX_TX_OFFLOAD_NOTSUP_MASK indicates, it should be the
mask of unsupported features (either not in PKT_TX_OFFLOAD_MASK or in
XXX_TX_OFFLOAD_MASK), however, xor will not get desired result here,
assume bit 0 of PKT_TX_OFFLOAD_MASK and XXX_TX_OFFLOAD_MAKS are 0 which
means corresponding feature is not supported in both sides, then we get
value of bit 0 of XXX_TX_OFFLOAD_NOTSUP_MASK which is 0 via xor, it
implies that it is supported which doesn't meet our expectation.

Correct it by a NOT-AND operation.

Xiaolong Ye (8):
  net/ixgbe: correct offload not supported mask
  net/avf: correct offload not supported mask
  net/ena: correct offload not supported mask
  net/atlantic: correct offload not supported mask
  net/fm10k: correct offload not supported mask
  net/qede: correct offload not supported mask
  net/vmxnet3: correct offload not supported mask
  net/e1000: correct offload not supported mask

 drivers/net/atlantic/atl_rxtx.c    | 2 +-
 drivers/net/avf/avf_rxtx.h         | 2 +-
 drivers/net/e1000/em_rxtx.c        | 2 +-
 drivers/net/e1000/igb_rxtx.c       | 2 +-
 drivers/net/ena/ena_ethdev.c       | 2 +-
 drivers/net/fm10k/fm10k_rxtx.c     | 2 +-
 drivers/net/ixgbe/ixgbe_rxtx.c     | 2 +-
 drivers/net/qede/qede_rxtx.h       | 2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
 9 files changed, 9 insertions(+), 9 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/8] net/ixgbe: correct offload not supported mask

[Xiaolong Ye]

Previously IXGBE_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would
lead to unexpected result, correct it by using a NOT-AND operation.

Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")

Cc: tomaszx.kulasek@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 2f0262ae1..a486ff4bd 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -72,7 +72,7 @@
 		IXGBE_TX_IEEE1588_TMST)
 
 #define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
-		(PKT_TX_OFFLOAD_MASK ^ IXGBE_TX_OFFLOAD_MASK)
+		~(PKT_TX_OFFLOAD_MASK & IXGBE_TX_OFFLOAD_MASK)
 
 #if 1
 #define RTE_PMD_USE_PREFETCH
-- 
2.17.1

[dpdk-dev] [PATCH 2/8] net/avf: correct offload not supported mask

[Xiaolong Ye]

Previously AVF_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: a2b29a7733ef ("net/avf: enable basic Rx Tx")

Cc: wenzhuo.lu@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/avf/avf_rxtx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/avf/avf_rxtx.h b/drivers/net/avf/avf_rxtx.h
index 898d2f387..5583b68be 100644
--- a/drivers/net/avf/avf_rxtx.h
+++ b/drivers/net/avf/avf_rxtx.h
@@ -49,7 +49,7 @@
 		PKT_TX_TCP_SEG)
 
 #define AVF_TX_OFFLOAD_NOTSUP_MASK \
-		(PKT_TX_OFFLOAD_MASK ^ AVF_TX_OFFLOAD_MASK)
+		~(PKT_TX_OFFLOAD_MASK & AVF_TX_OFFLOAD_MASK)
 
 /* HW desc structure, both 16-byte and 32-byte types are supported */
 #ifdef RTE_LIBRTE_AVF_16BYTE_RX_DESC
-- 
2.17.1

[dpdk-dev] [PATCH 3/8] net/ena: correct offload not supported mask

[Xiaolong Ye]

Previously ENA_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: b3fc5a1ae10d ("net/ena: add Tx preparation")

Cc: konstantin.ananyev@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/ena/ena_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 0c0ed9302..47250b168 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -193,7 +193,7 @@ static const struct ena_stats ena_stats_ena_com_strings[] = {
 	PKT_TX_TCP_SEG)
 
 #define	ENA_TX_OFFLOAD_NOTSUP_MASK	\
-	(PKT_TX_OFFLOAD_MASK ^ ENA_TX_OFFLOAD_MASK)
+	~(PKT_TX_OFFLOAD_MASK & ENA_TX_OFFLOAD_MASK)
 
 int ena_logtype_init;
 int ena_logtype_driver;
-- 
2.17.1

[dpdk-dev] [PATCH 4/8] net/atlantic: correct offload not supported mask

[Xiaolong Ye]

Previously ATL_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: 2b1472d7150c ("net/atlantic: implement Tx path")

Cc: pavel.belous@aquantia.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/atlantic/atl_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/atlantic/atl_rxtx.c b/drivers/net/atlantic/atl_rxtx.c
index fd909476c..7a4eaf191 100644
--- a/drivers/net/atlantic/atl_rxtx.c
+++ b/drivers/net/atlantic/atl_rxtx.c
@@ -26,7 +26,7 @@
 	PKT_TX_TCP_SEG)
 
 #define ATL_TX_OFFLOAD_NOTSUP_MASK \
-	(PKT_TX_OFFLOAD_MASK ^ ATL_TX_OFFLOAD_MASK)
+	~(PKT_TX_OFFLOAD_MASK & ATL_TX_OFFLOAD_MASK)
 
 /**
  * Structure associated with each descriptor of the RX ring of a RX queue.
-- 
2.17.1

[dpdk-dev] [PATCH 5/8] net/fm10k: correct offload not supported mask

[Xiaolong Ye]

Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: 9b134aa39716 ("net/fm10k: add Tx preparation")

Cc: tomaszx.kulasek@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/fm10k/fm10k_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/fm10k/fm10k_rxtx.c b/drivers/net/fm10k/fm10k_rxtx.c
index 4a5b46ec8..5c648eea7 100644
--- a/drivers/net/fm10k/fm10k_rxtx.c
+++ b/drivers/net/fm10k/fm10k_rxtx.c
@@ -44,7 +44,7 @@ static inline void dump_rxd(union fm10k_rx_desc *rxd)
 		PKT_TX_TCP_SEG)
 
 #define FM10K_TX_OFFLOAD_NOTSUP_MASK \
-		(PKT_TX_OFFLOAD_MASK ^ FM10K_TX_OFFLOAD_MASK)
+		~(PKT_TX_OFFLOAD_MASK & FM10K_TX_OFFLOAD_MASK)
 
 /* @note: When this function is changed, make corresponding change to
  * fm10k_dev_supported_ptypes_get()
-- 
2.17.1

[dpdk-dev] [PATCH 6/8] net/qede: correct offload not supported mask

[Xiaolong Ye]

Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: 29540be7efce ("net/qede: support LRO/TSO offloads")

Cc: harish.patil@qlogic.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/qede/qede_rxtx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h
index d3a41e92e..9da059564 100644
--- a/drivers/net/qede/qede_rxtx.h
+++ b/drivers/net/qede/qede_rxtx.h
@@ -159,7 +159,7 @@
 			      PKT_TX_TUNNEL_GRE)
 
 #define QEDE_TX_OFFLOAD_NOTSUP_MASK \
-	(PKT_TX_OFFLOAD_MASK ^ QEDE_TX_OFFLOAD_MASK)
+	~(PKT_TX_OFFLOAD_MASK & QEDE_TX_OFFLOAD_MASK)
 
 /*
  * RX BD descriptor ring
-- 
2.17.1

[dpdk-dev] [PATCH 7/8] net/vmxnet3: correct offload not supported mask

[Xiaolong Ye]

Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: baf3bbae5556 ("net/vmxnet3: add Tx preparation")

Cc: konstantin.ananyev@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index cf85f3d61..ab3505886 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -54,7 +54,7 @@
 		PKT_TX_TCP_SEG)
 
 #define	VMXNET3_TX_OFFLOAD_NOTSUP_MASK	\
-	(PKT_TX_OFFLOAD_MASK ^ VMXNET3_TX_OFFLOAD_MASK)
+	~(PKT_TX_OFFLOAD_MASK & VMXNET3_TX_OFFLOAD_MASK)
 
 static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2};
 
-- 
2.17.1

[dpdk-dev] [PATCH 8/8] net/e1000: correct offload not supported mask

[Xiaolong Ye]

Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
to unexpected result, correct it by using a NOT-AND operation.

Fixes: 2b76648872c9 ("net/e1000: add Tx preparation")

Cc: tomaszx.kulasek@intel.com
Cc: stable@dpdk.org
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 drivers/net/e1000/em_rxtx.c  | 2 +-
 drivers/net/e1000/igb_rxtx.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index a9cd76518..c3fd2cd6e 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -55,7 +55,7 @@
 		PKT_TX_VLAN_PKT)
 
 #define E1000_TX_OFFLOAD_NOTSUP_MASK \
-		(PKT_TX_OFFLOAD_MASK ^ E1000_TX_OFFLOAD_MASK)
+		~(PKT_TX_OFFLOAD_MASK & E1000_TX_OFFLOAD_MASK)
 
 /**
  * Structure associated with each descriptor of the RX ring of a RX queue.
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 25ff5f68f..055dff102 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -57,7 +57,7 @@
 		IGB_TX_IEEE1588_TMST)
 
 #define IGB_TX_OFFLOAD_NOTSUP_MASK \
-		(PKT_TX_OFFLOAD_MASK ^ IGB_TX_OFFLOAD_MASK)
+		~(PKT_TX_OFFLOAD_MASK & IGB_TX_OFFLOAD_MASK)
 
 /**
  * Structure associated with each descriptor of the RX ring of a RX queue.
-- 
2.17.1

Re: [dpdk-dev] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[Zhao1, Wei]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Xiaolong Ye
> Sent: Saturday, October 27, 2018 6:40 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Ye, Xiaolong <xiaolong.ye@intel.com>
> Subject: [dpdk-dev] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK
> 
> Hi,
> 
> This patch series is to correct the wrong setting of XXX_TX_OFFLOAD_MASK
> in multiple PMDs.
> 
> Just as the name XXX_TX_OFFLOAD_NOTSUP_MASK indicates, it should be
> the mask of unsupported features (either not in PKT_TX_OFFLOAD_MASK or
> in XXX_TX_OFFLOAD_MASK), however, xor will not get desired result here,
> assume bit 0 of PKT_TX_OFFLOAD_MASK and XXX_TX_OFFLOAD_MAKS are
> 0 which means corresponding feature is not supported in both sides, then
> we get value of bit 0 of XXX_TX_OFFLOAD_NOTSUP_MASK which is 0 via xor,
> it implies that it is supported which doesn't meet our expectation.
> 

Yes, I agree with you.
Reviewed-by: Wei Zhao <wei.zhao1@intel.com>

> Correct it by a NOT-AND operation.
> 
> Xiaolong Ye (8):
>   net/ixgbe: correct offload not supported mask
>   net/avf: correct offload not supported mask
>   net/ena: correct offload not supported mask
>   net/atlantic: correct offload not supported mask
>   net/fm10k: correct offload not supported mask
>   net/qede: correct offload not supported mask
>   net/vmxnet3: correct offload not supported mask
>   net/e1000: correct offload not supported mask
> 
>  drivers/net/atlantic/atl_rxtx.c    | 2 +-
>  drivers/net/avf/avf_rxtx.h         | 2 +-
>  drivers/net/e1000/em_rxtx.c        | 2 +-
>  drivers/net/e1000/igb_rxtx.c       | 2 +-
>  drivers/net/ena/ena_ethdev.c       | 2 +-
>  drivers/net/fm10k/fm10k_rxtx.c     | 2 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c     | 2 +-
>  drivers/net/qede/qede_rxtx.h       | 2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
>  9 files changed, 9 insertions(+), 9 deletions(-)
> 
> --
> 2.17.1

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[Ferruh Yigit]

On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
> Hi,
> 
> This patch series is to correct the wrong setting of
> XXX_TX_OFFLOAD_MASK in multiple PMDs.
> 
> Just as the name XXX_TX_OFFLOAD_NOTSUP_MASK indicates, it should be the
> mask of unsupported features (either not in PKT_TX_OFFLOAD_MASK or in
> XXX_TX_OFFLOAD_MASK), however, xor will not get desired result here,
> assume bit 0 of PKT_TX_OFFLOAD_MASK and XXX_TX_OFFLOAD_MAKS are 0 which
> means corresponding feature is not supported in both sides, then we get
> value of bit 0 of XXX_TX_OFFLOAD_NOTSUP_MASK which is 0 via xor, it
> implies that it is supported which doesn't meet our expectation.

Hi Xiaolong,

I am not sure if this is a problem.

PKT_TX_OFFLOAD_MASK should be all TX bits set, from bit 40 to 60 in latest code.
And XXX_TX_OFFLOAD_MASK can be subset of those bits set.

So your sample doesn't apply here since PKT_TX_OFFLOAD_MASK, TX related bits,
can't be 0. And (A ^ B) should be same with ~(A & B) for this case.


Am I missing something, do you have a failing usecase/testcase with real values?

> 
> Correct it by a NOT-AND operation.
> 
> Xiaolong Ye (8):
>   net/ixgbe: correct offload not supported mask
>   net/avf: correct offload not supported mask
>   net/ena: correct offload not supported mask
>   net/atlantic: correct offload not supported mask
>   net/fm10k: correct offload not supported mask
>   net/qede: correct offload not supported mask
>   net/vmxnet3: correct offload not supported mask
>   net/e1000: correct offload not supported mask
> 
>  drivers/net/atlantic/atl_rxtx.c    | 2 +-
>  drivers/net/avf/avf_rxtx.h         | 2 +-
>  drivers/net/e1000/em_rxtx.c        | 2 +-
>  drivers/net/e1000/igb_rxtx.c       | 2 +-
>  drivers/net/ena/ena_ethdev.c       | 2 +-
>  drivers/net/fm10k/fm10k_rxtx.c     | 2 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c     | 2 +-
>  drivers/net/qede/qede_rxtx.h       | 2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
>  9 files changed, 9 insertions(+), 9 deletions(-)
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH 6/8] net/qede: correct offload not supported mask

[Ferruh Yigit]

On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
> Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would lead
> to unexpected result, correct it by using a NOT-AND operation.
> 
> Fixes: 29540be7efce ("net/qede: support LRO/TSO offloads")
> 
> Cc: harish.patil@qlogic.com
> Cc: stable@dpdk.org
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
>  drivers/net/qede/qede_rxtx.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h
> index d3a41e92e..9da059564 100644
> --- a/drivers/net/qede/qede_rxtx.h
> +++ b/drivers/net/qede/qede_rxtx.h
> @@ -159,7 +159,7 @@
>  			      PKT_TX_TUNNEL_GRE)
>  
>  #define QEDE_TX_OFFLOAD_NOTSUP_MASK \
> -	(PKT_TX_OFFLOAD_MASK ^ QEDE_TX_OFFLOAD_MASK)
> +	~(PKT_TX_OFFLOAD_MASK & QEDE_TX_OFFLOAD_MASK)

Hi Harish,

The qede usage can be problematic, because of how QEDE_TX_OFFLOAD_MASK set:

#define QEDE_TX_OFFLOAD_MASK (QEDE_TX_CSUM_OFFLOAD_MASK | \
                              PKT_TX_VLAN_PKT           | \
                              PKT_TX_TUNNEL_VXLAN       | \
                              PKT_TX_TUNNEL_GENEVE      | \
                              PKT_TX_TUNNEL_MPLSINUDP   | \
                              PKT_TX_TUNNEL_GRE)

I am not sure if OFFLOAD_NOTSUP_MASK works fine with multi-bit values, I think
you should set `PKT_TX_TUNNEL_MASK` here.

But please double check in case I am missing something.

Re: [dpdk-dev] [dpdk-stable] [PATCH 6/8] net/qede: correct offload not supported mask

[Shaikh, Shahed]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: Saturday, November 3, 2018 1:32 AM
> To: Patil, Harish <Harish.Patil@cavium.com>
> Cc: Xiaolong Ye <xiaolong.ye@intel.com>; Qi Zhang <qi.z.zhang@intel.com>;
> Beilei Xing <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH 6/8] net/qede: correct offload not
> supported mask
> 
> 
> On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
> > Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would
> lead
> > to unexpected result, correct it by using a NOT-AND operation.
> >
> > Fixes: 29540be7efce ("net/qede: support LRO/TSO offloads")
> >
> > Cc: harish.patil@qlogic.com
> > Cc: stable@dpdk.org
> > Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> > ---
> >  drivers/net/qede/qede_rxtx.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h
> > index d3a41e92e..9da059564 100644
> > --- a/drivers/net/qede/qede_rxtx.h
> > +++ b/drivers/net/qede/qede_rxtx.h
> > @@ -159,7 +159,7 @@
> >                             PKT_TX_TUNNEL_GRE)
> >
> >  #define QEDE_TX_OFFLOAD_NOTSUP_MASK \
> > -     (PKT_TX_OFFLOAD_MASK ^ QEDE_TX_OFFLOAD_MASK)
> > +     ~(PKT_TX_OFFLOAD_MASK & QEDE_TX_OFFLOAD_MASK)
> 
> Hi Harish,
> 
> The qede usage can be problematic, because of how QEDE_TX_OFFLOAD_MASK
> set:
> 
> #define QEDE_TX_OFFLOAD_MASK (QEDE_TX_CSUM_OFFLOAD_MASK | \
>                               PKT_TX_VLAN_PKT           | \
>                               PKT_TX_TUNNEL_VXLAN       | \
>                               PKT_TX_TUNNEL_GENEVE      | \
>                               PKT_TX_TUNNEL_MPLSINUDP   | \
>                               PKT_TX_TUNNEL_GRE)
> 
> I am not sure if OFFLOAD_NOTSUP_MASK works fine with multi-bit values, I
> think
> you should set `PKT_TX_TUNNEL_MASK` here.
> 
> But please double check in case I am missing something.


Hi Ferruh,

You are right. Tunnel types are multi bit values. Qede PMD does not support all tunneling protocol offloads which fall under PKT_TX_TUNNEL_MASK.
Not sure how to handle unsupported tunneling protocols in OFFLOAD_NOTSUP_MASK.

One way would be to set TX_TUNNEL_MASK in QEDE_TX_OFFLOAD_MASK and then check for unsupported tunnel protocols in tx_pkt_prepare().

Thanks,
Shahed

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[Ye Xiaolong]

Hi, Ferruh

On 11/02, Ferruh Yigit wrote:
>On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
>> Hi,
>> 
>> This patch series is to correct the wrong setting of
>> XXX_TX_OFFLOAD_MASK in multiple PMDs.
>> 
>> Just as the name XXX_TX_OFFLOAD_NOTSUP_MASK indicates, it should be the
>> mask of unsupported features (either not in PKT_TX_OFFLOAD_MASK or in
>> XXX_TX_OFFLOAD_MASK), however, xor will not get desired result here,
>> assume bit 0 of PKT_TX_OFFLOAD_MASK and XXX_TX_OFFLOAD_MAKS are 0 which
>> means corresponding feature is not supported in both sides, then we get
>> value of bit 0 of XXX_TX_OFFLOAD_NOTSUP_MASK which is 0 via xor, it
>> implies that it is supported which doesn't meet our expectation.
>
>Hi Xiaolong,
>
>I am not sure if this is a problem.
>
>PKT_TX_OFFLOAD_MASK should be all TX bits set, from bit 40 to 60 in latest code.
>And XXX_TX_OFFLOAD_MASK can be subset of those bits set.
>
>So your sample doesn't apply here since PKT_TX_OFFLOAD_MASK, TX related bits,
>can't be 0. And (A ^ B) should be same with ~(A & B) for this case.
>

If this is the case, I think current code is fine. ~(A & B) is just more correct
than (A ^ B) from the logic perspective.

>
>Am I missing something, do you have a failing usecase/testcase with real values?

No real failing issue has been found, I just happened to read the code and
thought it might be a problem. :)

Thanks,
Xiaolong
>
>> 
>> Correct it by a NOT-AND operation.
>> 
>> Xiaolong Ye (8):
>>   net/ixgbe: correct offload not supported mask
>>   net/avf: correct offload not supported mask
>>   net/ena: correct offload not supported mask
>>   net/atlantic: correct offload not supported mask
>>   net/fm10k: correct offload not supported mask
>>   net/qede: correct offload not supported mask
>>   net/vmxnet3: correct offload not supported mask
>>   net/e1000: correct offload not supported mask
>> 
>>  drivers/net/atlantic/atl_rxtx.c    | 2 +-
>>  drivers/net/avf/avf_rxtx.h         | 2 +-
>>  drivers/net/e1000/em_rxtx.c        | 2 +-
>>  drivers/net/e1000/igb_rxtx.c       | 2 +-
>>  drivers/net/ena/ena_ethdev.c       | 2 +-
>>  drivers/net/fm10k/fm10k_rxtx.c     | 2 +-
>>  drivers/net/ixgbe/ixgbe_rxtx.c     | 2 +-
>>  drivers/net/qede/qede_rxtx.h       | 2 +-
>>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
>>  9 files changed, 9 insertions(+), 9 deletions(-)
>> 
>

Re: [dpdk-dev] [dpdk-stable] [PATCH 6/8] net/qede: correct offload not supported mask

[Ferruh Yigit]

On 11/5/2018 6:52 AM, Shaikh, Shahed wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
>> Sent: Saturday, November 3, 2018 1:32 AM
>> To: Patil, Harish <Harish.Patil@cavium.com>
>> Cc: Xiaolong Ye <xiaolong.ye@intel.com>; Qi Zhang <qi.z.zhang@intel.com>;
>> Beilei Xing <beilei.xing@intel.com>; dev@dpdk.org; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH 6/8] net/qede: correct offload not
>> supported mask
>>
>>
>> On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
>>> Previously XXX_TX_OFFLOAD_NOTSUP_MASK is obtained via xor which would
>> lead
>>> to unexpected result, correct it by using a NOT-AND operation.
>>>
>>> Fixes: 29540be7efce ("net/qede: support LRO/TSO offloads")
>>>
>>> Cc: harish.patil@qlogic.com
>>> Cc: stable@dpdk.org
>>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>> ---
>>>  drivers/net/qede/qede_rxtx.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/qede/qede_rxtx.h b/drivers/net/qede/qede_rxtx.h
>>> index d3a41e92e..9da059564 100644
>>> --- a/drivers/net/qede/qede_rxtx.h
>>> +++ b/drivers/net/qede/qede_rxtx.h
>>> @@ -159,7 +159,7 @@
>>>                             PKT_TX_TUNNEL_GRE)
>>>
>>>  #define QEDE_TX_OFFLOAD_NOTSUP_MASK \
>>> -     (PKT_TX_OFFLOAD_MASK ^ QEDE_TX_OFFLOAD_MASK)
>>> +     ~(PKT_TX_OFFLOAD_MASK & QEDE_TX_OFFLOAD_MASK)
>>
>> Hi Harish,
>>
>> The qede usage can be problematic, because of how QEDE_TX_OFFLOAD_MASK
>> set:
>>
>> #define QEDE_TX_OFFLOAD_MASK (QEDE_TX_CSUM_OFFLOAD_MASK | \
>>                               PKT_TX_VLAN_PKT           | \
>>                               PKT_TX_TUNNEL_VXLAN       | \
>>                               PKT_TX_TUNNEL_GENEVE      | \
>>                               PKT_TX_TUNNEL_MPLSINUDP   | \
>>                               PKT_TX_TUNNEL_GRE)
>>
>> I am not sure if OFFLOAD_NOTSUP_MASK works fine with multi-bit values, I
>> think
>> you should set `PKT_TX_TUNNEL_MASK` here.
>>
>> But please double check in case I am missing something.
> 
> 
> Hi Ferruh,
> 
> You are right. Tunnel types are multi bit values. Qede PMD does not support all tunneling protocol offloads which fall under PKT_TX_TUNNEL_MASK.
> Not sure how to handle unsupported tunneling protocols in OFFLOAD_NOTSUP_MASK.
> 
> One way would be to set TX_TUNNEL_MASK in QEDE_TX_OFFLOAD_MASK and then check for unsupported tunnel protocols in tx_pkt_prepare().

+1, I think this would work

> 
> Thanks,
> Shahed
> 

Re: [dpdk-dev] [dpdk-stable] [PATCH 0/8] Correct XXX_TX_OFFLOAD_NOTSUP_MASK

[Ferruh Yigit]

On 11/5/2018 8:56 AM, Ye Xiaolong wrote:
> Hi, Ferruh
> 
> On 11/02, Ferruh Yigit wrote:
>> On 10/27/2018 11:40 AM, Xiaolong Ye wrote:
>>> Hi,
>>>
>>> This patch series is to correct the wrong setting of
>>> XXX_TX_OFFLOAD_MASK in multiple PMDs.
>>>
>>> Just as the name XXX_TX_OFFLOAD_NOTSUP_MASK indicates, it should be the
>>> mask of unsupported features (either not in PKT_TX_OFFLOAD_MASK or in
>>> XXX_TX_OFFLOAD_MASK), however, xor will not get desired result here,
>>> assume bit 0 of PKT_TX_OFFLOAD_MASK and XXX_TX_OFFLOAD_MAKS are 0 which
>>> means corresponding feature is not supported in both sides, then we get
>>> value of bit 0 of XXX_TX_OFFLOAD_NOTSUP_MASK which is 0 via xor, it
>>> implies that it is supported which doesn't meet our expectation.
>>
>> Hi Xiaolong,
>>
>> I am not sure if this is a problem.
>>
>> PKT_TX_OFFLOAD_MASK should be all TX bits set, from bit 40 to 60 in latest code.
>> And XXX_TX_OFFLOAD_MASK can be subset of those bits set.
>>
>> So your sample doesn't apply here since PKT_TX_OFFLOAD_MASK, TX related bits,
>> can't be 0. And (A ^ B) should be same with ~(A & B) for this case.
>>
> 
> If this is the case, I think current code is fine. ~(A & B) is just more correct
> than (A ^ B) from the logic perspective.

Not sure if one is better than other, both give same result.

> 
>>
>> Am I missing something, do you have a failing usecase/testcase with real values?
> 
> No real failing issue has been found, I just happened to read the code and
> thought it might be a problem. :)

OK, if there is no real fail case, I am marking set as rejected.

Thanks,
ferruh

> 
> Thanks,
> Xiaolong
>>
>>>
>>> Correct it by a NOT-AND operation.
>>>
>>> Xiaolong Ye (8):
>>>   net/ixgbe: correct offload not supported mask
>>>   net/avf: correct offload not supported mask
>>>   net/ena: correct offload not supported mask
>>>   net/atlantic: correct offload not supported mask
>>>   net/fm10k: correct offload not supported mask
>>>   net/qede: correct offload not supported mask
>>>   net/vmxnet3: correct offload not supported mask
>>>   net/e1000: correct offload not supported mask
>>>
>>>  drivers/net/atlantic/atl_rxtx.c    | 2 +-
>>>  drivers/net/avf/avf_rxtx.h         | 2 +-
>>>  drivers/net/e1000/em_rxtx.c        | 2 +-
>>>  drivers/net/e1000/igb_rxtx.c       | 2 +-
>>>  drivers/net/ena/ena_ethdev.c       | 2 +-
>>>  drivers/net/fm10k/fm10k_rxtx.c     | 2 +-
>>>  drivers/net/ixgbe/ixgbe_rxtx.c     | 2 +-
>>>  drivers/net/qede/qede_rxtx.h       | 2 +-
>>>  drivers/net/vmxnet3/vmxnet3_rxtx.c | 2 +-
>>>  9 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>