[dpdk-dev] [PATCH v2] mbuf: support dynamic fields and flags

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier Matz <olivier.matz at 6wind.com>
> Sent: Wednesday, October 23, 2019 12:46 PM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Cc: dev at dpdk.org; Andrew Rybchenko <arybchenko at solarflare.com>; Richardson, Bruce <bruce.richardson at intel.com>; Wang,
> Haiyue <haiyue.wang at intel.com>; Jerin Jacob Kollanukkaran <jerinj at marvell.com>; Wiles, Keith <keith.wiles at intel.com>; Morten
> Brørup <mb at smartsharesystems.com>; Stephen Hemminger <stephen at networkplumber.org>; Thomas Monjalon
> <thomas at monjalon.net>
> Subject: Re: [PATCH v2] mbuf: support dynamic fields and flags
> 
> On Wed, Oct 23, 2019 at 12:19:46PM +0200, Olivier Matz wrote:
> > On Tue, Oct 22, 2019 at 10:51:51PM +0000, Ananyev, Konstantin wrote:
> 
> (...)
> 
> > > > +/* Allocate and initialize the shared memory. Assume tailq is locked */
> > > > +static int
> > > > +init_shared_mem(void)
> > > > +{
> > > > +	const struct rte_memzone *mz;
> > > > +	uint64_t mask;
> > > > +
> > > > +	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> > > > +		mz = rte_memzone_reserve_aligned(RTE_MBUF_DYN_MZNAME,
> > > > +						sizeof(struct mbuf_dyn_shm),
> > > > +						SOCKET_ID_ANY, 0,
> > > > +						RTE_CACHE_LINE_SIZE);
> > > > +	} else {
> > > > +		mz = rte_memzone_lookup(RTE_MBUF_DYN_MZNAME);
> > > > +	}
> > > > +	if (mz == NULL)
> > > > +		return -1;
> > > > +
> > > > +	shm = mz->addr;
> > > > +
> > > > +#define mark_free(field)						\
> > > > +	memset(&shm->free_space[offsetof(struct rte_mbuf, field)],	\
> > > > +		1, sizeof(((struct rte_mbuf *)0)->field))
> > >
> > > Still think it would look nicer without multi-line macro defines/undef in the middle of the function.
> >
> > I rather think that macro helps to make the code more readable, but it's
> > probably just a matter of taste. Will someone puts a contract on me if I
> > keep it like this? If yes I'll do the change ;)
> 
> More seriously, do you prefer if I move the macro definition above the
> function?

Yes, would look better to me.