diff mbox

[dpdk-dev,v2,3/5] net/ixgbe: fix bitmask of supported Tx flags

Message ID 1486179375-133509-4-git-send-email-jingjing.wu@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail Compilation issues

Commit Message

Jingjing Wu Feb. 4, 2017, 3:36 a.m. UTC 
  Add missed flags to bitmask of all supported packet Tx flags.

CC: konstantin.ananyev@intel.com
Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Ananyev, Konstantin Feb. 5, 2017, 11:59 a.m. UTC | #1 
Hi Jingjing,


> -----Original Message-----
> From: Wu, Jingjing
> Sent: Saturday, February 4, 2017 3:36 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> 
> Add missed flags to bitmask of all supported packet Tx flags.
> 
> CC: konstantin.ananyev@intel.com
> Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 36f1c02..8454581 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -81,13 +81,28 @@
>  #include "ixgbe_rxtx.h"
> 
>  /* Bit Mask to indicate what bits required for building TX context */
> +#ifdef RTE_LIBRTE_IEEE1588
>  #define IXGBE_TX_OFFLOAD_MASK (			 \
>  		PKT_TX_VLAN_PKT |		 \
>  		PKT_TX_IP_CKSUM |		 \
> +		PKT_TX_IPV4 |			 \
>  		PKT_TX_L4_MASK |		 \
> +		PKT_TX_IEEE1588_TMST |		 \
>  		PKT_TX_TCP_SEG |		 \
>  		PKT_TX_MACSEC |			 \
> -		PKT_TX_OUTER_IP_CKSUM)
> +		PKT_TX_OUTER_IP_CKSUM |		 \
> +		PKT_TX_OUTER_IPV4)
> +#else
> +#define IXGBE_TX_OFFLOAD_MASK (			 \
> +		PKT_TX_VLAN_PKT |		 \
> +		PKT_TX_IP_CKSUM |		 \
> +		PKT_TX_IPV4 |			 \

Wonder why ixgbe doesn't have PKT_TX_IPV6?
Konstantin

> +		PKT_TX_L4_MASK |		 \
> +		PKT_TX_TCP_SEG |		 \
> +		PKT_TX_MACSEC |			 \
> +		PKT_TX_OUTER_IP_CKSUM |		 \
> +		PKT_TX_OUTER_IPV4)
> +#endif
> 
>  #define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
>  		(PKT_TX_OFFLOAD_MASK ^ IXGBE_TX_OFFLOAD_MASK)
> --
> 2.4.11
Ananyev, Konstantin Feb. 5, 2017, 12:10 p.m. UTC | #2 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Sunday, February 5, 2017 11:59 AM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> 
> Hi Jingjing,
> 
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Saturday, February 4, 2017 3:36 AM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> >
> > Add missed flags to bitmask of all supported packet Tx flags.
> >
> > CC: konstantin.ananyev@intel.com
> > Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> > index 36f1c02..8454581 100644
> > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > @@ -81,13 +81,28 @@
> >  #include "ixgbe_rxtx.h"
> >
> >  /* Bit Mask to indicate what bits required for building TX context */
> > +#ifdef RTE_LIBRTE_IEEE1588
> >  #define IXGBE_TX_OFFLOAD_MASK (			 \
> >  		PKT_TX_VLAN_PKT |		 \
> >  		PKT_TX_IP_CKSUM |		 \
> > +		PKT_TX_IPV4 |			 \
> >  		PKT_TX_L4_MASK |		 \
> > +		PKT_TX_IEEE1588_TMST |		 \
> >  		PKT_TX_TCP_SEG |		 \
> >  		PKT_TX_MACSEC |			 \
> > -		PKT_TX_OUTER_IP_CKSUM)
> > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > +		PKT_TX_OUTER_IPV4)
> > +#else
> > +#define IXGBE_TX_OFFLOAD_MASK (			 \
> > +		PKT_TX_VLAN_PKT |		 \
> > +		PKT_TX_IP_CKSUM |		 \
> > +		PKT_TX_IPV4 |			 \
> 
> Wonder why ixgbe doesn't have PKT_TX_IPV6?

Same question for e1000 and fm10k.
Also if you decided to go that way, you'll probably need to update TX_OFFLOAD_MASK
for enic and vmxnet3.
That's why I still think it would be much less hassle not to include
these flags (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK at all.
Konstantin


> 
> > +		PKT_TX_L4_MASK |		 \
> > +		PKT_TX_TCP_SEG |		 \
> > +		PKT_TX_MACSEC |			 \
> > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > +		PKT_TX_OUTER_IPV4)
> > +#endif
> >
> >  #define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
> >  		(PKT_TX_OFFLOAD_MASK ^ IXGBE_TX_OFFLOAD_MASK)
> > --
> > 2.4.11
Jingjing Wu Feb. 6, 2017, 8:53 a.m. UTC | #3 
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Sunday, February 5, 2017 8:11 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported
> Tx flags
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> > Konstantin
> > Sent: Sunday, February 5, 2017 11:59 AM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > supported Tx flags
> >
> > Hi Jingjing,
> >
> >
> > > -----Original Message-----
> > > From: Wu, Jingjing
> > > Sent: Saturday, February 4, 2017 3:36 AM
> > > To: dev@dpdk.org
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>
> > > Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> > >
> > > Add missed flags to bitmask of all supported packet Tx flags.
> > >
> > > CC: konstantin.ananyev@intel.com
> > > Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > ---
> > >  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
> > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 36f1c02..8454581 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > @@ -81,13 +81,28 @@
> > >  #include "ixgbe_rxtx.h"
> > >
> > >  /* Bit Mask to indicate what bits required for building TX context
> > > */
> > > +#ifdef RTE_LIBRTE_IEEE1588
> > >  #define IXGBE_TX_OFFLOAD_MASK (			 \
> > >  		PKT_TX_VLAN_PKT |		 \
> > >  		PKT_TX_IP_CKSUM |		 \
> > > +		PKT_TX_IPV4 |			 \
> > >  		PKT_TX_L4_MASK |		 \
> > > +		PKT_TX_IEEE1588_TMST |		 \
> > >  		PKT_TX_TCP_SEG |		 \
> > >  		PKT_TX_MACSEC |			 \
> > > -		PKT_TX_OUTER_IP_CKSUM)
> > > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > > +		PKT_TX_OUTER_IPV4)
> > > +#else
> > > +#define IXGBE_TX_OFFLOAD_MASK (			 \
> > > +		PKT_TX_VLAN_PKT |		 \
> > > +		PKT_TX_IP_CKSUM |		 \
> > > +		PKT_TX_IPV4 |			 \
> >
> > Wonder why ixgbe doesn't have PKT_TX_IPV6?
> 
> Same question for e1000 and fm10k.
> Also if you decided to go that way, you'll probably need to update
> TX_OFFLOAD_MASK for enic and vmxnet3.
> That's why I still think it would be much less hassle not to include these flags
> (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK at all.
> Konstantin
> 
>
Thanks for pointing that. PKT_TX_IPV6 is missed.
About whether include these flags (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK, I think they should be
Included. Think about one NIC may support IPV4 L4 checksum offload, but not support IPV6? Even I don't know who it is.

Thanks
Jingjing
Ananyev, Konstantin Feb. 6, 2017, 12:11 p.m. UTC | #4 
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Monday, February 6, 2017 8:54 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Sunday, February 5, 2017 8:11 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported
> > Tx flags
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> > > Konstantin
> > > Sent: Sunday, February 5, 2017 11:59 AM
> > > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > supported Tx flags
> > >
> > > Hi Jingjing,
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wu, Jingjing
> > > > Sent: Saturday, February 4, 2017 3:36 AM
> > > > To: dev@dpdk.org
> > > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>
> > > > Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> > > >
> > > > Add missed flags to bitmask of all supported packet Tx flags.
> > > >
> > > > CC: konstantin.ananyev@intel.com
> > > > Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > ---
> > > >  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
> > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 36f1c02..8454581 100644
> > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > @@ -81,13 +81,28 @@
> > > >  #include "ixgbe_rxtx.h"
> > > >
> > > >  /* Bit Mask to indicate what bits required for building TX context
> > > > */
> > > > +#ifdef RTE_LIBRTE_IEEE1588
> > > >  #define IXGBE_TX_OFFLOAD_MASK (			 \
> > > >  		PKT_TX_VLAN_PKT |		 \
> > > >  		PKT_TX_IP_CKSUM |		 \
> > > > +		PKT_TX_IPV4 |			 \
> > > >  		PKT_TX_L4_MASK |		 \
> > > > +		PKT_TX_IEEE1588_TMST |		 \
> > > >  		PKT_TX_TCP_SEG |		 \
> > > >  		PKT_TX_MACSEC |			 \
> > > > -		PKT_TX_OUTER_IP_CKSUM)
> > > > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > > > +		PKT_TX_OUTER_IPV4)
> > > > +#else
> > > > +#define IXGBE_TX_OFFLOAD_MASK (			 \
> > > > +		PKT_TX_VLAN_PKT |		 \
> > > > +		PKT_TX_IP_CKSUM |		 \
> > > > +		PKT_TX_IPV4 |			 \
> > >
> > > Wonder why ixgbe doesn't have PKT_TX_IPV6?
> >
> > Same question for e1000 and fm10k.
> > Also if you decided to go that way, you'll probably need to update
> > TX_OFFLOAD_MASK for enic and vmxnet3.
> > That's why I still think it would be much less hassle not to include these flags
> > (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK at all.
> > Konstantin
> >
> >
> Thanks for pointing that. PKT_TX_IPV6 is missed.
> About whether include these flags (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK, I think they should be
> Included. Think about one NIC may support IPV4 L4 checksum offload, but not support IPV6? Even I don't know who it is.
> 

I don't think such combination is possible now anyway.
But ok, if your preference is it to do more work and add (PKT_TX_IPV4 | PKT_TX_IPV6)
into all required places, I wouldn't argue.

BTW, as a side notice, what will be really good is to have a function that would take
tx_offload_capabilities as an input and return tx_offload_mask.
That would remove necessity to update/support TX_OFFLOAD_MASK for each PMD,
and hopefully will allow to avoid confusion for PMD writers. 
Though that's probably subject of another patch.

Konstantin
Jingjing Wu Feb. 7, 2017, 2:29 a.m. UTC | #5 
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Monday, February 6, 2017 8:11 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported
> Tx flags
> 
> 
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Monday, February 6, 2017 8:54 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > supported Tx flags
> >
> >
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Sunday, February 5, 2017 8:11 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; dev@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > supported Tx flags
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> > > > Konstantin
> > > > Sent: Sunday, February 5, 2017 11:59 AM
> > > > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > > supported Tx flags
> > > >
> > > > Hi Jingjing,
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Wu, Jingjing
> > > > > Sent: Saturday, February 4, 2017 3:36 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin
> > > > > <konstantin.ananyev@intel.com>
> > > > > Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx
> > > > > flags
> > > > >
> > > > > Add missed flags to bitmask of all supported packet Tx flags.
> > > > >
> > > > > CC: konstantin.ananyev@intel.com
> > > > > Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> > > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > > ---
> > > > >  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
> > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 36f1c02..8454581 100644
> > > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > @@ -81,13 +81,28 @@
> > > > >  #include "ixgbe_rxtx.h"
> > > > >
> > > > >  /* Bit Mask to indicate what bits required for building TX
> > > > > context */
> > > > > +#ifdef RTE_LIBRTE_IEEE1588
> > > > >  #define IXGBE_TX_OFFLOAD_MASK (			 \
> > > > >  		PKT_TX_VLAN_PKT |		 \
> > > > >  		PKT_TX_IP_CKSUM |		 \
> > > > > +		PKT_TX_IPV4 |			 \
> > > > >  		PKT_TX_L4_MASK |		 \
> > > > > +		PKT_TX_IEEE1588_TMST |		 \
> > > > >  		PKT_TX_TCP_SEG |		 \
> > > > >  		PKT_TX_MACSEC |			 \
> > > > > -		PKT_TX_OUTER_IP_CKSUM)
> > > > > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > > > > +		PKT_TX_OUTER_IPV4)
> > > > > +#else
> > > > > +#define IXGBE_TX_OFFLOAD_MASK (			 \
> > > > > +		PKT_TX_VLAN_PKT |		 \
> > > > > +		PKT_TX_IP_CKSUM |		 \
> > > > > +		PKT_TX_IPV4 |			 \
> > > >
> > > > Wonder why ixgbe doesn't have PKT_TX_IPV6?
> > >
> > > Same question for e1000 and fm10k.
> > > Also if you decided to go that way, you'll probably need to update
> > > TX_OFFLOAD_MASK for enic and vmxnet3.
> > > That's why I still think it would be much less hassle not to include
> > > these flags
> > > (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK at all.
> > > Konstantin
> > >
> > >
> > Thanks for pointing that. PKT_TX_IPV6 is missed.
> > About whether include these flags (PKT_TX_IPV4 and PKT_TX_IPV6)  into
> > TX_OFFLOAD_MASK, I think they should be Included. Think about one NIC
> may support IPV4 L4 checksum offload, but not support IPV6? Even I don't
> know who it is.
> >
> 
> I don't think such combination is possible now anyway.
> But ok, if your preference is it to do more work and add (PKT_TX_IPV4 |
> PKT_TX_IPV6) into all required places, I wouldn't argue.
> 
> BTW, as a side notice, what will be really good is to have a function that
> would take tx_offload_capabilities as an input and return tx_offload_mask.
> That would remove necessity to update/support TX_OFFLOAD_MASK for
> each PMD, and hopefully will allow to avoid confusion for PMD writers.
> Though that's probably subject of another patch.
> 

OK. I think what I did is more than necessary. Let me simplify the change. Thanks!

And about the querying tx offload capabilities, I think it is already been done
In rte_eth_dev_info_get. But it used another set of flags which is not TX flags
Defined in mbuf. 

Jingjing
Ananyev, Konstantin Feb. 8, 2017, 12:34 a.m. UTC | #6 
> -----Original Message-----
> From: Wu, Jingjing
> Sent: Tuesday, February 7, 2017 2:30 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx flags
> 
> 
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Monday, February 6, 2017 8:11 PM
> > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of supported
> > Tx flags
> >
> >
> >
> > > -----Original Message-----
> > > From: Wu, Jingjing
> > > Sent: Monday, February 6, 2017 8:54 AM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > supported Tx flags
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ananyev, Konstantin
> > > > Sent: Sunday, February 5, 2017 8:11 PM
> > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing
> > > > <jingjing.wu@intel.com>; dev@dpdk.org
> > > > Subject: RE: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > > supported Tx flags
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> > > > > Konstantin
> > > > > Sent: Sunday, February 5, 2017 11:59 AM
> > > > > To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > > > > Subject: Re: [dpdk-dev] [PATCH v2 3/5] net/ixgbe: fix bitmask of
> > > > > supported Tx flags
> > > > >
> > > > > Hi Jingjing,
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Wu, Jingjing
> > > > > > Sent: Saturday, February 4, 2017 3:36 AM
> > > > > > To: dev@dpdk.org
> > > > > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; Ananyev, Konstantin
> > > > > > <konstantin.ananyev@intel.com>
> > > > > > Subject: [PATCH v2 3/5] net/ixgbe: fix bitmask of supported Tx
> > > > > > flags
> > > > > >
> > > > > > Add missed flags to bitmask of all supported packet Tx flags.
> > > > > >
> > > > > > CC: konstantin.ananyev@intel.com
> > > > > > Fixes: 7829b8d52be0 ("net/ixgbe: add Tx preparation")
> > > > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > > > ---
> > > > > >  drivers/net/ixgbe/ixgbe_rxtx.c | 17 ++++++++++++++++-
> > > > > >  1 file changed, 16 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > > b/drivers/net/ixgbe/ixgbe_rxtx.c index 36f1c02..8454581 100644
> > > > > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > > > > @@ -81,13 +81,28 @@
> > > > > >  #include "ixgbe_rxtx.h"
> > > > > >
> > > > > >  /* Bit Mask to indicate what bits required for building TX
> > > > > > context */
> > > > > > +#ifdef RTE_LIBRTE_IEEE1588
> > > > > >  #define IXGBE_TX_OFFLOAD_MASK (			 \
> > > > > >  		PKT_TX_VLAN_PKT |		 \
> > > > > >  		PKT_TX_IP_CKSUM |		 \
> > > > > > +		PKT_TX_IPV4 |			 \
> > > > > >  		PKT_TX_L4_MASK |		 \
> > > > > > +		PKT_TX_IEEE1588_TMST |		 \
> > > > > >  		PKT_TX_TCP_SEG |		 \
> > > > > >  		PKT_TX_MACSEC |			 \
> > > > > > -		PKT_TX_OUTER_IP_CKSUM)
> > > > > > +		PKT_TX_OUTER_IP_CKSUM |		 \
> > > > > > +		PKT_TX_OUTER_IPV4)
> > > > > > +#else
> > > > > > +#define IXGBE_TX_OFFLOAD_MASK (			 \
> > > > > > +		PKT_TX_VLAN_PKT |		 \
> > > > > > +		PKT_TX_IP_CKSUM |		 \
> > > > > > +		PKT_TX_IPV4 |			 \
> > > > >
> > > > > Wonder why ixgbe doesn't have PKT_TX_IPV6?
> > > >
> > > > Same question for e1000 and fm10k.
> > > > Also if you decided to go that way, you'll probably need to update
> > > > TX_OFFLOAD_MASK for enic and vmxnet3.
> > > > That's why I still think it would be much less hassle not to include
> > > > these flags
> > > > (PKT_TX_IPV4 and PKT_TX_IPV6)  into TX_OFFLOAD_MASK at all.
> > > > Konstantin
> > > >
> > > >
> > > Thanks for pointing that. PKT_TX_IPV6 is missed.
> > > About whether include these flags (PKT_TX_IPV4 and PKT_TX_IPV6)  into
> > > TX_OFFLOAD_MASK, I think they should be Included. Think about one NIC
> > may support IPV4 L4 checksum offload, but not support IPV6? Even I don't
> > know who it is.
> > >
> >
> > I don't think such combination is possible now anyway.
> > But ok, if your preference is it to do more work and add (PKT_TX_IPV4 |
> > PKT_TX_IPV6) into all required places, I wouldn't argue.
> >
> > BTW, as a side notice, what will be really good is to have a function that
> > would take tx_offload_capabilities as an input and return tx_offload_mask.
> > That would remove necessity to update/support TX_OFFLOAD_MASK for
> > each PMD, and hopefully will allow to avoid confusion for PMD writers.
> > Though that's probably subject of another patch.
> >
> 
> OK. I think what I did is more than necessary. Let me simplify the change. Thanks!
> 
> And about the querying tx offload capabilities, I think it is already been done
> In rte_eth_dev_info_get. But it used another set of flags which is not TX flags
> Defined in mbuf.

I am not talking about querying device offload capabilities.
I am talking about a function that would take these capabilities reported by the
device and convert them into a set of tx offload flags, that are suitable for these capabilities. 
Something like that:
uint64_t
tx_capa_to flags(uint32_t tx_capa)
{
	uint64_t ol_flags;
	
	ol_flags = 0;

	if (tx_capa &  DEV_TX_OFFLOAD_TCP_TSO)
		ol_flags |= PKT_TX_TCP_SEG |  PKT_TX_IPV4 |  PKT_TX_IP_CKSUM  | PKT_TX_IPV6;
	if (tx_capa &  DEV_TX_OFFLOAD_TCP_CKSUM)
		ol_flags |= PKT_TX_TCP_CKSUM;
	...
	return ol_flags;
}   

Then PMD writer wouldn't need to define TX_OFFLOAD_MASK for his PMD manually.
Instead he can just do at startup/configure phase:
txq->tx_offload_mask = tx_capa_to flags(supported_and_configured_capabilties);

Though, as I said, this is probably subject of another patch/discussion.
Konstantin
diff mbox

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 36f1c02..8454581 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -81,13 +81,28 @@ 
 #include "ixgbe_rxtx.h"
 
 /* Bit Mask to indicate what bits required for building TX context */
+#ifdef RTE_LIBRTE_IEEE1588
 #define IXGBE_TX_OFFLOAD_MASK (			 \
 		PKT_TX_VLAN_PKT |		 \
 		PKT_TX_IP_CKSUM |		 \
+		PKT_TX_IPV4 |			 \
 		PKT_TX_L4_MASK |		 \
+		PKT_TX_IEEE1588_TMST |		 \
 		PKT_TX_TCP_SEG |		 \
 		PKT_TX_MACSEC |			 \
-		PKT_TX_OUTER_IP_CKSUM)
+		PKT_TX_OUTER_IP_CKSUM |		 \
+		PKT_TX_OUTER_IPV4)
+#else
+#define IXGBE_TX_OFFLOAD_MASK (			 \
+		PKT_TX_VLAN_PKT |		 \
+		PKT_TX_IP_CKSUM |		 \
+		PKT_TX_IPV4 |			 \
+		PKT_TX_L4_MASK |		 \
+		PKT_TX_TCP_SEG |		 \
+		PKT_TX_MACSEC |			 \
+		PKT_TX_OUTER_IP_CKSUM |		 \
+		PKT_TX_OUTER_IPV4)
+#endif
 
 #define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
 		(PKT_TX_OFFLOAD_MASK ^ IXGBE_TX_OFFLOAD_MASK)