[dpdk-dev,v1] Move rte_mbuf macros to common header file
Commit Message
Macros RTE_MBUF_DATA_DMA_ADDR and RTE_MBUF_DATA_DMA_ADDR_DEFAULT
are defined in each PMD driver file. Move those macros into common
lib/librte_mbuf/rte_mbuf.h file. All PMD drivers include rte_mbuf.h
file directly/indirectly hence no additionl header file inclusion
is necessary.
Compiled for:
> x86_64-native-linuxapp-clang
> x86_64-native-linuxapp-gcc
> i686-native-linuxapp-gcc
> x86_64-native-bsdapp-gcc
> x86_64-native-bsdapp-clang
Tested on:
> x86_64 Ubuntu 14.04, testpmd and 'make test'
> FreeBSD 10.1, testpmd
Signed-off-by: Ravi Kerur <rkerur@gmail.com>
---
drivers/net/bnx2x/bnx2x.h | 3 ---
drivers/net/cxgbe/sge.c | 3 ---
drivers/net/e1000/em_rxtx.c | 6 ------
drivers/net/e1000/igb_rxtx.c | 6 ------
drivers/net/i40e/i40e_rxtx.c | 6 ------
drivers/net/ixgbe/ixgbe_rxtx.h | 6 ------
drivers/net/virtio/virtqueue.h | 3 ---
drivers/net/vmxnet3/vmxnet3_rxtx.c | 6 ------
lib/librte_mbuf/rte_mbuf.h | 6 ++++++
9 files changed, 6 insertions(+), 39 deletions(-)
Comments
On Thu, 24 Sep 2015 15:50:41 -0700
Ravi Kerur <rkerur@gmail.com> wrote:
> Macros RTE_MBUF_DATA_DMA_ADDR and RTE_MBUF_DATA_DMA_ADDR_DEFAULT
> are defined in each PMD driver file. Move those macros into common
> lib/librte_mbuf/rte_mbuf.h file. All PMD drivers include rte_mbuf.h
> file directly/indirectly hence no additionl header file inclusion
> is necessary.
>
> Compiled for:
> > x86_64-native-linuxapp-clang
> > x86_64-native-linuxapp-gcc
> > i686-native-linuxapp-gcc
> > x86_64-native-bsdapp-gcc
> > x86_64-native-bsdapp-clang
>
> Tested on:
> > x86_64 Ubuntu 14.04, testpmd and 'make test'
> > FreeBSD 10.1, testpmd
>
> Signed-off-by: Ravi Kerur <rkerur@gmail.com>
I like the idea, should have been done long ago.
My only gripe is that you should do this as inline functions
rather than macros. Inline functions are type safe, macros are not.
On Thu, Sep 24, 2015 at 4:25 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:
> On Thu, 24 Sep 2015 15:50:41 -0700
> Ravi Kerur <rkerur@gmail.com> wrote:
>
> > Macros RTE_MBUF_DATA_DMA_ADDR and RTE_MBUF_DATA_DMA_ADDR_DEFAULT
> > are defined in each PMD driver file. Move those macros into common
> > lib/librte_mbuf/rte_mbuf.h file. All PMD drivers include rte_mbuf.h
> > file directly/indirectly hence no additionl header file inclusion
> > is necessary.
> >
> > Compiled for:
> > > x86_64-native-linuxapp-clang
> > > x86_64-native-linuxapp-gcc
> > > i686-native-linuxapp-gcc
> > > x86_64-native-bsdapp-gcc
> > > x86_64-native-bsdapp-clang
> >
> > Tested on:
> > > x86_64 Ubuntu 14.04, testpmd and 'make test'
> > > FreeBSD 10.1, testpmd
> >
> > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
>
> I like the idea, should have been done long ago.
>
> My only gripe is that you should do this as inline functions
> rather than macros. Inline functions are type safe, macros are not.
>
Agreed. However, I see another variation of the macro, users are primarily
from "app" directory and lone user from drivers/net/xenvirt/virtqueue.h
#define RTE_MBUF_DATA_DMA_ADDR(mb) \
rte_pktmbuf_mtod(mb, uint64_t)
#define rte_pktmbuf_mtod(m, t) rte_pktmbuf_mtod_offset(m, t, 0)
#define rte_pktmbuf_mtod_offset(m, t, o) \
((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
Let me know should I still go ahead and do inline variation for drivers or
use above macro?
Hi Ravi,
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ravi Kerur
> Sent: Saturday, September 26, 2015 3:47 AM
> To: Stephen Hemminger; Olivier Matz
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
>
> On Thu, Sep 24, 2015 at 4:25 PM, Stephen Hemminger <
> stephen@networkplumber.org> wrote:
>
> > On Thu, 24 Sep 2015 15:50:41 -0700
> > Ravi Kerur <rkerur@gmail.com> wrote:
> >
> > > Macros RTE_MBUF_DATA_DMA_ADDR and RTE_MBUF_DATA_DMA_ADDR_DEFAULT
> > > are defined in each PMD driver file. Move those macros into common
> > > lib/librte_mbuf/rte_mbuf.h file. All PMD drivers include rte_mbuf.h
> > > file directly/indirectly hence no additionl header file inclusion
> > > is necessary.
> > >
> > > Compiled for:
> > > > x86_64-native-linuxapp-clang
> > > > x86_64-native-linuxapp-gcc
> > > > i686-native-linuxapp-gcc
> > > > x86_64-native-bsdapp-gcc
> > > > x86_64-native-bsdapp-clang
> > >
> > > Tested on:
> > > > x86_64 Ubuntu 14.04, testpmd and 'make test'
> > > > FreeBSD 10.1, testpmd
> > >
> > > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> >
> > I like the idea, should have been done long ago.
> >
> > My only gripe is that you should do this as inline functions
> > rather than macros. Inline functions are type safe, macros are not.
> >
>
> Agreed. However, I see another variation of the macro, users are primarily
> from "app" directory and lone user from drivers/net/xenvirt/virtqueue.h
>
> #define RTE_MBUF_DATA_DMA_ADDR(mb) \
> rte_pktmbuf_mtod(mb, uint64_t)
As I can see, it is used only in one place inside xenvirt:
drivers/net/xenvirt/virtqueue.h: start_dp[idx].addr = RTE_MBUF_DATA_DMA_ADDR(cookie);
So we probably can remove that macro definition here and use
rte_pktmbuf_mtod(mb, uint64_t) directly.
Konstantin
>
> #define rte_pktmbuf_mtod(m, t) rte_pktmbuf_mtod_offset(m, t, 0)
>
> #define rte_pktmbuf_mtod_offset(m, t, o) \
> ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
>
> Let me know should I still go ahead and do inline variation for drivers or
> use above macro?
Thanks Konstantin. I will send out v2 shortly.
On Tue, Sep 29, 2015 at 2:55 AM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:
>
> Hi Ravi,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ravi Kerur
> > Sent: Saturday, September 26, 2015 3:47 AM
> > To: Stephen Hemminger; Olivier Matz
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header
> file
> >
> > On Thu, Sep 24, 2015 at 4:25 PM, Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> >
> > > On Thu, 24 Sep 2015 15:50:41 -0700
> > > Ravi Kerur <rkerur@gmail.com> wrote:
> > >
> > > > Macros RTE_MBUF_DATA_DMA_ADDR and RTE_MBUF_DATA_DMA_ADDR_DEFAULT
> > > > are defined in each PMD driver file. Move those macros into common
> > > > lib/librte_mbuf/rte_mbuf.h file. All PMD drivers include rte_mbuf.h
> > > > file directly/indirectly hence no additionl header file inclusion
> > > > is necessary.
> > > >
> > > > Compiled for:
> > > > > x86_64-native-linuxapp-clang
> > > > > x86_64-native-linuxapp-gcc
> > > > > i686-native-linuxapp-gcc
> > > > > x86_64-native-bsdapp-gcc
> > > > > x86_64-native-bsdapp-clang
> > > >
> > > > Tested on:
> > > > > x86_64 Ubuntu 14.04, testpmd and 'make test'
> > > > > FreeBSD 10.1, testpmd
> > > >
> > > > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> > >
> > > I like the idea, should have been done long ago.
> > >
> > > My only gripe is that you should do this as inline functions
> > > rather than macros. Inline functions are type safe, macros are not.
> > >
> >
> > Agreed. However, I see another variation of the macro, users are
> primarily
> > from "app" directory and lone user from drivers/net/xenvirt/virtqueue.h
> >
> > #define RTE_MBUF_DATA_DMA_ADDR(mb) \
> > rte_pktmbuf_mtod(mb, uint64_t)
>
>
> As I can see, it is used only in one place inside xenvirt:
>
> drivers/net/xenvirt/virtqueue.h: start_dp[idx].addr =
> RTE_MBUF_DATA_DMA_ADDR(cookie);
>
> So we probably can remove that macro definition here and use
> rte_pktmbuf_mtod(mb, uint64_t) directly.
>
> Konstantin
>
> >
> > #define rte_pktmbuf_mtod(m, t) rte_pktmbuf_mtod_offset(m, t, 0)
> >
> > #define rte_pktmbuf_mtod_offset(m, t, o) \
> > ((t)((char *)(m)->buf_addr + (m)->data_off + (o)))
> >
> > Let me know should I still go ahead and do inline variation for drivers
> or
> > use above macro?
>
@@ -141,9 +141,6 @@ struct bnx2x_device_type {
char *bnx2x_name;
};
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- ((uint64_t)((mb)->buf_physaddr + (mb)->data_off))
-
#define BNX2X_PAGE_SHIFT 12
#define BNX2X_PAGE_SIZE (1 << BNX2X_PAGE_SHIFT)
#define BNX2X_PAGE_MASK (~(BNX2X_PAGE_SIZE - 1))
@@ -1267,9 +1267,6 @@ static struct rte_mbuf *t4_pktgl_to_mbuf(const struct pkt_gl *gl)
return t4_pktgl_to_mbuf_usembufs(gl);
}
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- ((dma_addr_t) ((mb)->buf_physaddr + (mb)->data_off))
-
/**
* t4_ethrx_handler - process an ingress ethernet packet
* @q: the response queue that received the packet
@@ -88,12 +88,6 @@ rte_rxmbuf_alloc(struct rte_mempool *mp)
return (m);
}
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- (uint64_t) ((mb)->buf_physaddr + (mb)->data_off)
-
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
-
/**
* Structure associated with each descriptor of the RX ring of a RX queue.
*/
@@ -88,12 +88,6 @@ rte_rxmbuf_alloc(struct rte_mempool *mp)
return (m);
}
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- (uint64_t) ((mb)->buf_physaddr + (mb)->data_off)
-
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
-
/**
* Structure associated with each descriptor of the RX ring of a RX queue.
*/
@@ -78,12 +78,6 @@
PKT_TX_L4_MASK | \
PKT_TX_OUTER_IP_CKSUM)
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
-
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- ((uint64_t)((mb)->buf_physaddr + (mb)->data_off))
-
static const struct rte_memzone *
i40e_ring_dma_zone_reserve(struct rte_eth_dev *dev,
const char *ring_name,
@@ -40,12 +40,6 @@
#define RTE_IXGBE_DESCS_PER_LOOP 4
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- (uint64_t) ((mb)->buf_physaddr + (mb)->data_off)
-
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
-
#ifdef RTE_IXGBE_INC_VECTOR
#define RTE_IXGBE_RXQ_REARM_THRESH 32
#define RTE_IXGBE_MAX_RX_BURST RTE_IXGBE_RXQ_REARM_THRESH
@@ -68,9 +68,6 @@ struct rte_mbuf;
#define VIRTQUEUE_MAX_NAME_SZ 32
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- (uint64_t) ((mb)->buf_physaddr + (mb)->data_off)
-
#define VTNET_SQ_RQ_QUEUE_IDX 0
#define VTNET_SQ_TQ_QUEUE_IDX 1
#define VTNET_SQ_CQ_QUEUE_IDX 2
@@ -77,12 +77,6 @@
#include "vmxnet3_logs.h"
#include "vmxnet3_ethdev.h"
-#define RTE_MBUF_DATA_DMA_ADDR(mb) \
- (uint64_t) ((mb)->buf_physaddr + (mb)->data_off)
-
-#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
- (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
-
static const uint32_t rxprod_reg[2] = {VMXNET3_REG_RXPROD, VMXNET3_REG_RXPROD2};
static int vmxnet3_post_rx_bufs(vmxnet3_rx_queue_t*, uint8_t);
@@ -843,6 +843,12 @@ struct rte_mbuf {
uint16_t timesync;
} __rte_cache_aligned;
+#define RTE_MBUF_DATA_DMA_ADDR(mb) \
+ ((uint64_t)((mb)->buf_physaddr + (mb)->data_off))
+
+#define RTE_MBUF_DATA_DMA_ADDR_DEFAULT(mb) \
+ (uint64_t) ((mb)->buf_physaddr + RTE_PKTMBUF_HEADROOM)
+
static inline uint16_t rte_pktmbuf_priv_size(struct rte_mempool *mp);
/**