DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
@ 2015-09-24 22:49 Ravi Kerur
  2015-09-24 22:50 ` Ravi Kerur
  0 siblings, 1 reply; 6+ messages in thread
From: Ravi Kerur @ 2015-09-24 22:49 UTC (permalink / raw)
  To: dev

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.

Ravi Kerur (1):
  Move rte_buf macros to common header file

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

-- 
1.9.1

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

* [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
  2015-09-24 22:49 [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file Ravi Kerur
@ 2015-09-24 22:50 ` Ravi Kerur
  2015-09-24 23:25   ` Stephen Hemminger
  0 siblings, 1 reply; 6+ messages in thread
From: Ravi Kerur @ 2015-09-24 22:50 UTC (permalink / raw)
  To: dev

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

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index 867b92a..28bd83f 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -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))
diff --git a/drivers/net/cxgbe/sge.c b/drivers/net/cxgbe/sge.c
index 6eb1244..8f4c025 100644
--- a/drivers/net/cxgbe/sge.c
+++ b/drivers/net/cxgbe/sge.c
@@ -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
diff --git a/drivers/net/e1000/em_rxtx.c b/drivers/net/e1000/em_rxtx.c
index 3b8776d..c7d97c1 100644
--- a/drivers/net/e1000/em_rxtx.c
+++ b/drivers/net/e1000/em_rxtx.c
@@ -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.
  */
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index 19905fd..a217cea 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -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.
  */
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index fd656d5..5ba6d27 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -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,
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index b9eca67..dbb9f00 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -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
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 7789411..9ea9b96 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -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
diff --git a/drivers/net/vmxnet3/vmxnet3_rxtx.c b/drivers/net/vmxnet3/vmxnet3_rxtx.c
index 4de5d89..bb2648b 100644
--- a/drivers/net/vmxnet3/vmxnet3_rxtx.c
+++ b/drivers/net/vmxnet3/vmxnet3_rxtx.c
@@ -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);
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index d7c9030..90bbfa8 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -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);
 
 /**
-- 
1.9.1

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

* Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
  2015-09-24 22:50 ` Ravi Kerur
@ 2015-09-24 23:25   ` Stephen Hemminger
  2015-09-26  2:46     ` Ravi Kerur
  0 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2015-09-24 23:25 UTC (permalink / raw)
  To: Ravi Kerur; +Cc: dev

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.

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

* Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
  2015-09-24 23:25   ` Stephen Hemminger
@ 2015-09-26  2:46     ` Ravi Kerur
  2015-09-29  9:55       ` Ananyev, Konstantin
  0 siblings, 1 reply; 6+ messages in thread
From: Ravi Kerur @ 2015-09-26  2:46 UTC (permalink / raw)
  To: Stephen Hemminger, Olivier Matz; +Cc: dev

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?

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

* Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
  2015-09-26  2:46     ` Ravi Kerur
@ 2015-09-29  9:55       ` Ananyev, Konstantin
  2015-09-30 19:11         ` Ravi Kerur
  0 siblings, 1 reply; 6+ messages in thread
From: Ananyev, Konstantin @ 2015-09-29  9:55 UTC (permalink / raw)
  To: Ravi Kerur, Stephen Hemminger, Olivier Matz; +Cc: dev


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?

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

* Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file
  2015-09-29  9:55       ` Ananyev, Konstantin
@ 2015-09-30 19:11         ` Ravi Kerur
  0 siblings, 0 replies; 6+ messages in thread
From: Ravi Kerur @ 2015-09-30 19:11 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev

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?
>

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

end of thread, other threads:[~2015-09-30 19:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-24 22:49 [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file Ravi Kerur
2015-09-24 22:50 ` Ravi Kerur
2015-09-24 23:25   ` Stephen Hemminger
2015-09-26  2:46     ` Ravi Kerur
2015-09-29  9:55       ` Ananyev, Konstantin
2015-09-30 19:11         ` Ravi Kerur

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).