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

  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(-)
  

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
>
  

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