Re: [dpdk-dev] [PATCH v1] Move rte_mbuf macros to common header file

[Ravi Kerur <rkerur@gmail.com>]

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