diff mbox

[dpdk-dev,05/12] mbuf: remove too specific PKT_TX_OFFLOAD_MASK definition

Message ID 1415635166-1364-6-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Olivier Matz Nov. 10, 2014, 3:59 p.m. UTC
This definition is specific to Intel PMD drivers and its definition
"indicate what bits required for building TX context" shows that it
should not be in the generic rte_mbuf.h but in the PMD driver.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 lib/librte_mbuf/rte_mbuf.h        | 5 -----
 lib/librte_pmd_e1000/igb_rxtx.c   | 3 ++-
 lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 3 ++-
 3 files changed, 4 insertions(+), 7 deletions(-)

Comments

Bruce Richardson Nov. 10, 2014, 5:14 p.m. UTC | #1
On Mon, Nov 10, 2014 at 04:59:19PM +0100, Olivier Matz wrote:
> This definition is specific to Intel PMD drivers and its definition
> "indicate what bits required for building TX context" shows that it
> should not be in the generic rte_mbuf.h but in the PMD driver.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  lib/librte_mbuf/rte_mbuf.h        | 5 -----
>  lib/librte_pmd_e1000/igb_rxtx.c   | 3 ++-
>  lib/librte_pmd_ixgbe/ixgbe_rxtx.c | 3 ++-
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 96e322b..ff11b84 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -129,11 +129,6 @@ extern "C" {
>  /* Use final bit of flags to indicate a control mbuf */
>  #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
>  
> -/**
> - * Bit Mask to indicate what bits required for building TX context
> - */
> -#define PKT_TX_OFFLOAD_MASK (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM | PKT_TX_L4_MASK)
> -
>  /* define a set of marker types that can be used to refer to set points in the
>   * mbuf */
>  typedef void    *MARKER[0];   /**< generic marker for a point in a structure */
> diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
> index 321493e..dbf5074 100644
> --- a/lib/librte_pmd_e1000/igb_rxtx.c
> +++ b/lib/librte_pmd_e1000/igb_rxtx.c
> @@ -400,7 +400,8 @@ eth_igb_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		ol_flags = tx_pkt->ol_flags;
>  		vlan_macip_lens.f.vlan_tci = tx_pkt->vlan_tci;
>  		vlan_macip_lens.f.l2_l3_len = tx_pkt->l2_l3_len;
> -		tx_ol_req = ol_flags & PKT_TX_OFFLOAD_MASK;
> +		tx_ol_req = ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
> +			PKT_TX_L4_MASK);
>  

Rather than make the change like this, might it be clearer just to copy-paste
the macro definition into this file (perhaps as IGB_TX_OFFLOAD_MASK). Similarly
with ixgbe below?

/Bruce

>  		/* If a Context Descriptor need be built . */
>  		if (tx_ol_req) {
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> index 042ee8a..70ca254 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
> @@ -580,7 +580,8 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		ol_flags = tx_pkt->ol_flags;
>  
>  		/* If hardware offload required */
> -		tx_ol_req = ol_flags & PKT_TX_OFFLOAD_MASK;
> +		tx_ol_req = ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
> +			PKT_TX_L4_MASK);
>  		if (tx_ol_req) {
>  			vlan_macip_lens.f.vlan_tci = tx_pkt->vlan_tci;
>  			vlan_macip_lens.f.l2_l3_len = tx_pkt->l2_l3_len;
> -- 
> 2.1.0
>
Olivier Matz Nov. 10, 2014, 8:59 p.m. UTC | #2
Hi Bruce,

On 11/10/2014 06:14 PM, Bruce Richardson wrote:
>> --- a/lib/librte_pmd_e1000/igb_rxtx.c
>> +++ b/lib/librte_pmd_e1000/igb_rxtx.c
>> @@ -400,7 +400,8 @@ eth_igb_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>>  		ol_flags = tx_pkt->ol_flags;
>>  		vlan_macip_lens.f.vlan_tci = tx_pkt->vlan_tci;
>>  		vlan_macip_lens.f.l2_l3_len = tx_pkt->l2_l3_len;
>> -		tx_ol_req = ol_flags & PKT_TX_OFFLOAD_MASK;
>> +		tx_ol_req = ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
>> +			PKT_TX_L4_MASK);
>>  
> 
> Rather than make the change like this, might it be clearer just to copy-paste
> the macro definition into this file (perhaps as IGB_TX_OFFLOAD_MASK). Similarly
> with ixgbe below?

As this definition was used only once per PMD, I thought it was clearer
to remove the definition. But... someone did the same comment than
you internally, so I'll change it in next version!

Regards,
Olivier
diff mbox

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 96e322b..ff11b84 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -129,11 +129,6 @@  extern "C" {
 /* Use final bit of flags to indicate a control mbuf */
 #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
 
-/**
- * Bit Mask to indicate what bits required for building TX context
- */
-#define PKT_TX_OFFLOAD_MASK (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM | PKT_TX_L4_MASK)
-
 /* define a set of marker types that can be used to refer to set points in the
  * mbuf */
 typedef void    *MARKER[0];   /**< generic marker for a point in a structure */
diff --git a/lib/librte_pmd_e1000/igb_rxtx.c b/lib/librte_pmd_e1000/igb_rxtx.c
index 321493e..dbf5074 100644
--- a/lib/librte_pmd_e1000/igb_rxtx.c
+++ b/lib/librte_pmd_e1000/igb_rxtx.c
@@ -400,7 +400,8 @@  eth_igb_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		ol_flags = tx_pkt->ol_flags;
 		vlan_macip_lens.f.vlan_tci = tx_pkt->vlan_tci;
 		vlan_macip_lens.f.l2_l3_len = tx_pkt->l2_l3_len;
-		tx_ol_req = ol_flags & PKT_TX_OFFLOAD_MASK;
+		tx_ol_req = ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
+			PKT_TX_L4_MASK);
 
 		/* If a Context Descriptor need be built . */
 		if (tx_ol_req) {
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
index 042ee8a..70ca254 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c
@@ -580,7 +580,8 @@  ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 		ol_flags = tx_pkt->ol_flags;
 
 		/* If hardware offload required */
-		tx_ol_req = ol_flags & PKT_TX_OFFLOAD_MASK;
+		tx_ol_req = ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_IP_CKSUM |
+			PKT_TX_L4_MASK);
 		if (tx_ol_req) {
 			vlan_macip_lens.f.vlan_tci = tx_pkt->vlan_tci;
 			vlan_macip_lens.f.l2_l3_len = tx_pkt->l2_l3_len;