Thanks Konstantin. I will send out v2 shortly. On Tue, Sep 29, 2015 at 2:55 AM, Ananyev, Konstantin < konstantin.ananyev at intel.com> wrote:
> > Hi Ravi, > > > -----Original Message----- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ravi Kerur > > Sent: Saturday, September 26, 2015 3:47 AM > > To: Stephen Hemminger; Olivier Matz > > Cc: dev at 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 at networkplumber.org> wrote: > > > > > On Thu, 24 Sep 2015 15:50:41 -0700 > > > Ravi Kerur <rkerur at 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 at 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? >
