| Message ID | 1415635166-1364-6-git-send-email-olivier.matz@6wind.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
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
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;
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(-)