All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
@ 2017-01-24 11:47 Jingjing Wu
  2017-01-24 11:47 ` [PATCH 2/2] net/i40e: fix bitmask of supported Tx flags Jingjing Wu
  2017-01-26 14:58 ` [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Ananyev, Konstantin
  0 siblings, 2 replies; 9+ messages in thread
From: Jingjing Wu @ 2017-01-24 11:47 UTC (permalink / raw)
  To: dev; +Cc: jingjing.wu

Some Tx offload flags are missed in Bitmask of all supported packet
Tx offload features flags.
This patch fixes it.

Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_mbuf/rte_mbuf.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index bfce9f4..e57a4d2 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -295,8 +295,12 @@ extern "C" {
  */
 #define PKT_TX_OFFLOAD_MASK (    \
 		PKT_TX_IP_CKSUM |        \
+		PKT_TX_IPV4 |            \
+		PKT_TX_IPV6 |            \
 		PKT_TX_L4_MASK |         \
 		PKT_TX_OUTER_IP_CKSUM |  \
+		PKT_TX_OUTER_IPV4 |      \
+		PKT_TX_OUTER_IPV6 |      \
 		PKT_TX_TCP_SEG |         \
 		PKT_TX_QINQ_PKT |        \
 		PKT_TX_VLAN_PKT |        \
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] net/i40e: fix bitmask of supported Tx flags
  2017-01-24 11:47 [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Jingjing Wu
@ 2017-01-24 11:47 ` Jingjing Wu
  2017-01-26 14:58 ` [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Ananyev, Konstantin
  1 sibling, 0 replies; 9+ messages in thread
From: Jingjing Wu @ 2017-01-24 11:47 UTC (permalink / raw)
  To: dev; +Cc: jingjing.wu

Some Tx offload flags are missed in Bitmask of all supported packet
Tx flags by i40e.
This patch fixes it.

Fixes: 3f33e643e5c6 ("net/i40e: add Tx preparation")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_rxtx.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 89b9bf1..509d379 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -83,11 +83,16 @@
 
 #define I40E_TX_OFFLOAD_MASK (  \
 		PKT_TX_IP_CKSUM |       \
+		PKT_TX_IPV4 |           \
+		PKT_TX_IPV6 |           \
 		PKT_TX_L4_MASK |        \
 		PKT_TX_OUTER_IP_CKSUM | \
+		PKT_TX_OUTER_IPV4 |     \
+		PKT_TX_OUTER_IPV6 |     \
 		PKT_TX_TCP_SEG |        \
 		PKT_TX_QINQ_PKT |       \
-		PKT_TX_VLAN_PKT)
+		PKT_TX_VLAN_PKT |       \
+		PKT_TX_TUNNEL_MASK)
 
 #define I40E_TX_OFFLOAD_NOTSUP_MASK \
 		(PKT_TX_OFFLOAD_MASK ^ I40E_TX_OFFLOAD_MASK)
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-24 11:47 [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Jingjing Wu
  2017-01-24 11:47 ` [PATCH 2/2] net/i40e: fix bitmask of supported Tx flags Jingjing Wu
@ 2017-01-26 14:58 ` Ananyev, Konstantin
  2017-01-26 15:05   ` Olivier MATZ
  1 sibling, 1 reply; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-01-26 14:58 UTC (permalink / raw)
  To: Wu, Jingjing, dev; +Cc: Wu, Jingjing

Hi Jingjng,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> Sent: Tuesday, January 24, 2017 11:48 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
> 
> Some Tx offload flags are missed in Bitmask of all supported packet
> Tx offload features flags.
> This patch fixes it.

Not sure what it exactly fixes?
As I remember these flags don't specify any TX offload for HW to perform,
But just provide information to the TX function.
Again, why only i40e code is modified?
As I remember we have the same code in other PMDs too.
Konstantin

> 
> Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  lib/librte_mbuf/rte_mbuf.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index bfce9f4..e57a4d2 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -295,8 +295,12 @@ extern "C" {
>   */
>  #define PKT_TX_OFFLOAD_MASK (    \
>  		PKT_TX_IP_CKSUM |        \
> +		PKT_TX_IPV4 |            \
> +		PKT_TX_IPV6 |            \
>  		PKT_TX_L4_MASK |         \
>  		PKT_TX_OUTER_IP_CKSUM |  \
> +		PKT_TX_OUTER_IPV4 |      \
> +		PKT_TX_OUTER_IPV6 |      \
>  		PKT_TX_TCP_SEG |         \
>  		PKT_TX_QINQ_PKT |        \
>  		PKT_TX_VLAN_PKT |        \
> --
> 2.4.11

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-26 14:58 ` [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Ananyev, Konstantin
@ 2017-01-26 15:05   ` Olivier MATZ
  2017-01-26 15:35     ` Ananyev, Konstantin
  0 siblings, 1 reply; 9+ messages in thread
From: Olivier MATZ @ 2017-01-26 15:05 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: Wu, Jingjing, dev

On Thu, 26 Jan 2017 14:58:08 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> Hi Jingjng,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > Sent: Tuesday, January 24, 2017 11:48 AM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>
> > Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > flags
> > 
> > Some Tx offload flags are missed in Bitmask of all supported packet
> > Tx offload features flags.
> > This patch fixes it.  
> 
> Not sure what it exactly fixes?
> As I remember these flags don't specify any TX offload for HW to
> perform, But just provide information to the TX function.
> Again, why only i40e code is modified?
> As I remember we have the same code in other PMDs too.
> Konstantin
> 
> > 
> > Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >  lib/librte_mbuf/rte_mbuf.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index bfce9f4..e57a4d2 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -295,8 +295,12 @@ extern "C" {
> >   */
> >  #define PKT_TX_OFFLOAD_MASK (    \
> >  		PKT_TX_IP_CKSUM |        \
> > +		PKT_TX_IPV4 |            \
> > +		PKT_TX_IPV6 |            \
> >  		PKT_TX_L4_MASK |         \
> >  		PKT_TX_OUTER_IP_CKSUM |  \
> > +		PKT_TX_OUTER_IPV4 |      \
> > +		PKT_TX_OUTER_IPV6 |      \
> >  		PKT_TX_TCP_SEG |         \
> >  		PKT_TX_QINQ_PKT |        \
> >  		PKT_TX_VLAN_PKT |        \
> > --
> > 2.4.11  
> 

Also, it looks like MACSEC is missing. To avoid forgetting flags in
the future, what do you think about doing the following (not tested)?


diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index b3cccfc..aa1dc76 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -182,9 +182,11 @@ extern "C" {
  */
 #define PKT_RX_TIMESTAMP     (1ULL << 17)
 
-/* add new RX flags here */
+/* add new RX flags here, and update __PKT_RX_NEXT */
+#define __PKT_RX_NEXT        (1ULL << 18)
 
-/* add new TX flags here */
+/* add new TX flags here, and update __PKT_TX_NEXT */
+#define __PKT_TX_NEXT        (1ULL << 43)
 
 /**
  * Offload the MACsec. This flag must be set by the application to enable
@@ -295,17 +297,16 @@ extern "C" {
 #define PKT_TX_OUTER_IPV6    (1ULL << 60)
 
 /**
+ * Bitmask of all supported packet Rx offload features flags,
+ * which can be set for packet.
+ */
+#define PKT_RX_OFFLOAD_MASK (__PKT_RX_NEXT - 1)
+
+/**
  * Bitmask of all supported packet Tx offload features flags,
  * which can be set for packet.
  */
-#define PKT_TX_OFFLOAD_MASK (    \
-               PKT_TX_IP_CKSUM |        \
-               PKT_TX_L4_MASK |         \
-               PKT_TX_OUTER_IP_CKSUM |  \
-               PKT_TX_TCP_SEG |         \
-               PKT_TX_QINQ_PKT |        \
-               PKT_TX_VLAN_PKT |        \
-               PKT_TX_TUNNEL_MASK)
+#define PKT_TX_OFFLOAD_MASK ((~(__PKT_TX_NEXT - 1)) & 0x1fffffffffffffff)
 
 #define __RESERVED           (1ULL << 61) /**< reserved for future mbuf use */
 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-26 15:05   ` Olivier MATZ
@ 2017-01-26 15:35     ` Ananyev, Konstantin
  2017-01-26 15:57       ` Olivier MATZ
  0 siblings, 1 reply; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-01-26 15:35 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: Wu, Jingjing, dev

Hi Olivier,

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Thursday, January 26, 2017 3:05 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
> 
> On Thu, 26 Jan 2017 14:58:08 +0000, "Ananyev, Konstantin"
> <konstantin.ananyev@intel.com> wrote:
> > Hi Jingjng,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > > Sent: Tuesday, January 24, 2017 11:48 AM
> > > To: dev@dpdk.org
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>
> > > Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > > flags
> > >
> > > Some Tx offload flags are missed in Bitmask of all supported packet
> > > Tx offload features flags.
> > > This patch fixes it.
> >
> > Not sure what it exactly fixes?
> > As I remember these flags don't specify any TX offload for HW to
> > perform, But just provide information to the TX function.
> > Again, why only i40e code is modified?
> > As I remember we have the same code in other PMDs too.
> > Konstantin
> >
> > >
> > > Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > ---
> > >  lib/librte_mbuf/rte_mbuf.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index bfce9f4..e57a4d2 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -295,8 +295,12 @@ extern "C" {
> > >   */
> > >  #define PKT_TX_OFFLOAD_MASK (    \
> > >  		PKT_TX_IP_CKSUM |        \
> > > +		PKT_TX_IPV4 |            \
> > > +		PKT_TX_IPV6 |            \
> > >  		PKT_TX_L4_MASK |         \
> > >  		PKT_TX_OUTER_IP_CKSUM |  \
> > > +		PKT_TX_OUTER_IPV4 |      \
> > > +		PKT_TX_OUTER_IPV6 |      \
> > >  		PKT_TX_TCP_SEG |         \
> > >  		PKT_TX_QINQ_PKT |        \
> > >  		PKT_TX_VLAN_PKT |        \
> > > --
> > > 2.4.11
> >
> 
> Also, it looks like MACSEC is missing. To avoid forgetting flags in
> the future, what do you think about doing the following (not tested)?
> 
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index b3cccfc..aa1dc76 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -182,9 +182,11 @@ extern "C" {
>   */
>  #define PKT_RX_TIMESTAMP     (1ULL << 17)
> 
> -/* add new RX flags here */
> +/* add new RX flags here, and update __PKT_RX_NEXT */
> +#define __PKT_RX_NEXT        (1ULL << 18)
> 
> -/* add new TX flags here */
> +/* add new TX flags here, and update __PKT_TX_NEXT */
> +#define __PKT_TX_NEXT        (1ULL << 43)
> 
>  /**
>   * Offload the MACsec. This flag must be set by the application to enable
> @@ -295,17 +297,16 @@ extern "C" {
>  #define PKT_TX_OUTER_IPV6    (1ULL << 60)
> 
>  /**
> + * Bitmask of all supported packet Rx offload features flags,
> + * which can be set for packet.
> + */
> +#define PKT_RX_OFFLOAD_MASK (__PKT_RX_NEXT - 1)
> +
> +/**
>   * Bitmask of all supported packet Tx offload features flags,
>   * which can be set for packet.
>   */
> -#define PKT_TX_OFFLOAD_MASK (    \
> -               PKT_TX_IP_CKSUM |        \
> -               PKT_TX_L4_MASK |         \
> -               PKT_TX_OUTER_IP_CKSUM |  \
> -               PKT_TX_TCP_SEG |         \
> -               PKT_TX_QINQ_PKT |        \
> -               PKT_TX_VLAN_PKT |        \
> -               PKT_TX_TUNNEL_MASK)
> +#define PKT_TX_OFFLOAD_MASK ((~(__PKT_TX_NEXT - 1)) & 0x1fffffffffffffff)

I see your point but should, let say, PKT_TX_IPV4 be part of PKT_TX_OFFLOAD_MASK at all?
It doesn't really define any offload for PMD/HW to perform.
It just provide extra information for  PMD so it can successfully process other offload requests.
Konstantin

> 
>  #define __RESERVED           (1ULL << 61) /**< reserved for future mbuf use */
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-26 15:35     ` Ananyev, Konstantin
@ 2017-01-26 15:57       ` Olivier MATZ
  2017-01-26 16:33         ` Ananyev, Konstantin
  0 siblings, 1 reply; 9+ messages in thread
From: Olivier MATZ @ 2017-01-26 15:57 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: Olivier MATZ, Wu, Jingjing, dev

On Thu, 26 Jan 2017 15:35:29 +0000, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com> wrote:
> Hi Olivier,
> 
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Thursday, January 26, 2017 3:05 PM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > flags
> > 
> > On Thu, 26 Jan 2017 14:58:08 +0000, "Ananyev, Konstantin"
> > <konstantin.ananyev@intel.com> wrote:  
> > > Hi Jingjng,
> > >  
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > > > Sent: Tuesday, January 24, 2017 11:48 AM
> > > > To: dev@dpdk.org
> > > > Cc: Wu, Jingjing <jingjing.wu@intel.com>
> > > > Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > > > flags
> > > >
> > > > Some Tx offload flags are missed in Bitmask of all supported
> > > > packet Tx offload features flags.
> > > > This patch fixes it.  
> > >
> > > Not sure what it exactly fixes?
> > > As I remember these flags don't specify any TX offload for HW to
> > > perform, But just provide information to the TX function.
> > > Again, why only i40e code is modified?
> > > As I remember we have the same code in other PMDs too.
> > > Konstantin
> > >  
> > > >
> > > > Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > ---
> > > >  lib/librte_mbuf/rte_mbuf.h | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/lib/librte_mbuf/rte_mbuf.h
> > > > b/lib/librte_mbuf/rte_mbuf.h index bfce9f4..e57a4d2 100644
> > > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > > @@ -295,8 +295,12 @@ extern "C" {
> > > >   */
> > > >  #define PKT_TX_OFFLOAD_MASK (    \
> > > >  		PKT_TX_IP_CKSUM |        \
> > > > +		PKT_TX_IPV4 |            \
> > > > +		PKT_TX_IPV6 |            \
> > > >  		PKT_TX_L4_MASK |         \
> > > >  		PKT_TX_OUTER_IP_CKSUM |  \
> > > > +		PKT_TX_OUTER_IPV4 |      \
> > > > +		PKT_TX_OUTER_IPV6 |      \
> > > >  		PKT_TX_TCP_SEG |         \
> > > >  		PKT_TX_QINQ_PKT |        \
> > > >  		PKT_TX_VLAN_PKT |        \
> > > > --
> > > > 2.4.11  
> > >  
> > 
> > Also, it looks like MACSEC is missing. To avoid forgetting flags in
> > the future, what do you think about doing the following (not
> > tested)?
> > 
> > 
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index b3cccfc..aa1dc76 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -182,9 +182,11 @@ extern "C" {
> >   */
> >  #define PKT_RX_TIMESTAMP     (1ULL << 17)
> > 
> > -/* add new RX flags here */
> > +/* add new RX flags here, and update __PKT_RX_NEXT */
> > +#define __PKT_RX_NEXT        (1ULL << 18)
> > 
> > -/* add new TX flags here */
> > +/* add new TX flags here, and update __PKT_TX_NEXT */
> > +#define __PKT_TX_NEXT        (1ULL << 43)
> > 
> >  /**
> >   * Offload the MACsec. This flag must be set by the application to
> > enable @@ -295,17 +297,16 @@ extern "C" {
> >  #define PKT_TX_OUTER_IPV6    (1ULL << 60)
> > 
> >  /**
> > + * Bitmask of all supported packet Rx offload features flags,
> > + * which can be set for packet.
> > + */
> > +#define PKT_RX_OFFLOAD_MASK (__PKT_RX_NEXT - 1)
> > +
> > +/**
> >   * Bitmask of all supported packet Tx offload features flags,
> >   * which can be set for packet.
> >   */
> > -#define PKT_TX_OFFLOAD_MASK (    \
> > -               PKT_TX_IP_CKSUM |        \
> > -               PKT_TX_L4_MASK |         \
> > -               PKT_TX_OUTER_IP_CKSUM |  \
> > -               PKT_TX_TCP_SEG |         \
> > -               PKT_TX_QINQ_PKT |        \
> > -               PKT_TX_VLAN_PKT |        \
> > -               PKT_TX_TUNNEL_MASK)
> > +#define PKT_TX_OFFLOAD_MASK ((~(__PKT_TX_NEXT - 1)) &
> > 0x1fffffffffffffff)  
> 
> I see your point but should, let say, PKT_TX_IPV4 be part of
> PKT_TX_OFFLOAD_MASK at all? It doesn't really define any offload for
> PMD/HW to perform. It just provide extra information for  PMD so it
> can successfully process other offload requests. Konstantin

Yes, that's right. On the other hand, differentiating them may confuse
the PMD developpers (that's probably the case for this patch).

Having PKT_TX_MASK that includes all TX flags automatically would also
do the job (knowing PMDs must be updated), and would avoid to forget
flags in the future... if we decide to do it, it would be better before
17.02, because PKT_TX_OFFLOAD_MASK was added after 16.11, so it is not
yet part of the API.

In any case, we need a patch to fix the missing PKT_TX_MACSEC in
PKT_TX_OFFLOAD_MASK.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-26 15:57       ` Olivier MATZ
@ 2017-01-26 16:33         ` Ananyev, Konstantin
  0 siblings, 0 replies; 9+ messages in thread
From: Ananyev, Konstantin @ 2017-01-26 16:33 UTC (permalink / raw)
  To: Olivier MATZ; +Cc: Wu, Jingjing, dev



> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Thursday, January 26, 2017 3:58 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Olivier MATZ <olivier.matz@6wind.com>; Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
> 
> On Thu, 26 Jan 2017 15:35:29 +0000, "Ananyev, Konstantin"
> <konstantin.ananyev@intel.com> wrote:
> > Hi Olivier,
> >
> > > -----Original Message-----
> > > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > > Sent: Thursday, January 26, 2017 3:05 PM
> > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > > flags
> > >
> > > On Thu, 26 Jan 2017 14:58:08 +0000, "Ananyev, Konstantin"
> > > <konstantin.ananyev@intel.com> wrote:
> > > > Hi Jingjng,
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
> > > > > Sent: Tuesday, January 24, 2017 11:48 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: Wu, Jingjing <jingjing.wu@intel.com>
> > > > > Subject: [dpdk-dev] [PATCH 1/2] mbuf: fix bitmask of Tx offload
> > > > > flags
> > > > >
> > > > > Some Tx offload flags are missed in Bitmask of all supported
> > > > > packet Tx offload features flags.
> > > > > This patch fixes it.
> > > >
> > > > Not sure what it exactly fixes?
> > > > As I remember these flags don't specify any TX offload for HW to
> > > > perform, But just provide information to the TX function.
> > > > Again, why only i40e code is modified?
> > > > As I remember we have the same code in other PMDs too.
> > > > Konstantin
> > > >
> > > > >
> > > > > Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> > > > > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > > > > ---
> > > > >  lib/librte_mbuf/rte_mbuf.h | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h
> > > > > b/lib/librte_mbuf/rte_mbuf.h index bfce9f4..e57a4d2 100644
> > > > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > > > @@ -295,8 +295,12 @@ extern "C" {
> > > > >   */
> > > > >  #define PKT_TX_OFFLOAD_MASK (    \
> > > > >  		PKT_TX_IP_CKSUM |        \
> > > > > +		PKT_TX_IPV4 |            \
> > > > > +		PKT_TX_IPV6 |            \
> > > > >  		PKT_TX_L4_MASK |         \
> > > > >  		PKT_TX_OUTER_IP_CKSUM |  \
> > > > > +		PKT_TX_OUTER_IPV4 |      \
> > > > > +		PKT_TX_OUTER_IPV6 |      \
> > > > >  		PKT_TX_TCP_SEG |         \
> > > > >  		PKT_TX_QINQ_PKT |        \
> > > > >  		PKT_TX_VLAN_PKT |        \
> > > > > --
> > > > > 2.4.11
> > > >
> > >
> > > Also, it looks like MACSEC is missing. To avoid forgetting flags in
> > > the future, what do you think about doing the following (not
> > > tested)?
> > >
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index b3cccfc..aa1dc76 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -182,9 +182,11 @@ extern "C" {
> > >   */
> > >  #define PKT_RX_TIMESTAMP     (1ULL << 17)
> > >
> > > -/* add new RX flags here */
> > > +/* add new RX flags here, and update __PKT_RX_NEXT */
> > > +#define __PKT_RX_NEXT        (1ULL << 18)
> > >
> > > -/* add new TX flags here */
> > > +/* add new TX flags here, and update __PKT_TX_NEXT */
> > > +#define __PKT_TX_NEXT        (1ULL << 43)
> > >
> > >  /**
> > >   * Offload the MACsec. This flag must be set by the application to
> > > enable @@ -295,17 +297,16 @@ extern "C" {
> > >  #define PKT_TX_OUTER_IPV6    (1ULL << 60)
> > >
> > >  /**
> > > + * Bitmask of all supported packet Rx offload features flags,
> > > + * which can be set for packet.
> > > + */
> > > +#define PKT_RX_OFFLOAD_MASK (__PKT_RX_NEXT - 1)
> > > +
> > > +/**
> > >   * Bitmask of all supported packet Tx offload features flags,
> > >   * which can be set for packet.
> > >   */
> > > -#define PKT_TX_OFFLOAD_MASK (    \
> > > -               PKT_TX_IP_CKSUM |        \
> > > -               PKT_TX_L4_MASK |         \
> > > -               PKT_TX_OUTER_IP_CKSUM |  \
> > > -               PKT_TX_TCP_SEG |         \
> > > -               PKT_TX_QINQ_PKT |        \
> > > -               PKT_TX_VLAN_PKT |        \
> > > -               PKT_TX_TUNNEL_MASK)
> > > +#define PKT_TX_OFFLOAD_MASK ((~(__PKT_TX_NEXT - 1)) &
> > > 0x1fffffffffffffff)
> >
> > I see your point but should, let say, PKT_TX_IPV4 be part of
> > PKT_TX_OFFLOAD_MASK at all? It doesn't really define any offload for
> > PMD/HW to perform. It just provide extra information for  PMD so it
> > can successfully process other offload requests. Konstantin
> 
> Yes, that's right. On the other hand, differentiating them may confuse
> the PMD developpers (that's probably the case for this patch).
> 
> Having PKT_TX_MASK that includes all TX flags automatically would also
> do the job (knowing PMDs must be updated), 

Yes, all other PMDs that do use PKT_TX_OFFLOAD_MASK have to be updated in that case...
But ok, I agree what do you propose might help to avoid confusion. 

>and would avoid to forget
> flags in the future... if we decide to do it, it would be better before
> 17.02, because PKT_TX_OFFLOAD_MASK was added after 16.11, so it is not
> yet part of the API.

Agree.

> 
> In any case, we need a patch to fix the missing PKT_TX_MACSEC in
> PKT_TX_OFFLOAD_MASK.
> 
> 

Yes, we do.
Konstantin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
  2017-01-24 11:50 Jingjing Wu
@ 2017-01-26 14:19 ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2017-01-26 14:19 UTC (permalink / raw)
  To: Jingjing Wu, dev; +Cc: helin.zhang, tomaszx.kulasek, Olivier MATZ

On 1/24/2017 11:50 AM, Jingjing Wu wrote:
> Some Tx offload flags are missed in bitmask of all supported packet
> Tx offload features flags.
> This patch fixes it.
> 
> Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>

Cc: Olivier MATZ <olivier.matz@6wind.com>

<...>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] mbuf: fix bitmask of Tx offload flags
@ 2017-01-24 11:50 Jingjing Wu
  2017-01-26 14:19 ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Jingjing Wu @ 2017-01-24 11:50 UTC (permalink / raw)
  To: dev; +Cc: jingjing.wu, helin.zhang, tomaszx.kulasek

Some Tx offload flags are missed in bitmask of all supported packet
Tx offload features flags.
This patch fixes it.

Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_mbuf/rte_mbuf.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index bfce9f4..e57a4d2 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -295,8 +295,12 @@ extern "C" {
  */
 #define PKT_TX_OFFLOAD_MASK (    \
 		PKT_TX_IP_CKSUM |        \
+		PKT_TX_IPV4 |            \
+		PKT_TX_IPV6 |            \
 		PKT_TX_L4_MASK |         \
 		PKT_TX_OUTER_IP_CKSUM |  \
+		PKT_TX_OUTER_IPV4 |      \
+		PKT_TX_OUTER_IPV6 |      \
 		PKT_TX_TCP_SEG |         \
 		PKT_TX_QINQ_PKT |        \
 		PKT_TX_VLAN_PKT |        \
-- 
2.4.11

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-26 16:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-24 11:47 [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Jingjing Wu
2017-01-24 11:47 ` [PATCH 2/2] net/i40e: fix bitmask of supported Tx flags Jingjing Wu
2017-01-26 14:58 ` [PATCH 1/2] mbuf: fix bitmask of Tx offload flags Ananyev, Konstantin
2017-01-26 15:05   ` Olivier MATZ
2017-01-26 15:35     ` Ananyev, Konstantin
2017-01-26 15:57       ` Olivier MATZ
2017-01-26 16:33         ` Ananyev, Konstantin
2017-01-24 11:50 Jingjing Wu
2017-01-26 14:19 ` Ferruh Yigit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.