[dpdk-dev] [EXT] [RFC PATCH 0/2] introduce __rte_internal tag

Jerin Jacob Kollanukkaran jerinj at marvell.com
Fri Jun 7 11:41:07 CEST 2019


> -----Original Message-----
> From: Neil Horman <nhorman at tuxdriver.com>
> Sent: Thursday, June 6, 2019 10:26 PM
> To: Jerin Jacob Kollanukkaran <jerinj at marvell.com>
> Cc: Bruce Richardson <bruce.richardson at intel.com>; dev at dpdk.org;
> Thomas Monjalon <thomas at monjalon.net>
> Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
> 
> On Thu, Jun 06, 2019 at 04:23:26PM +0000, Jerin Jacob Kollanukkaran wrote:
> > > -----Original Message-----
> > > From: Neil Horman <nhorman at tuxdriver.com>
> > > Sent: Thursday, June 6, 2019 8:57 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj at marvell.com>
> > > Cc: Bruce Richardson <bruce.richardson at intel.com>; dev at dpdk.org;
> > > Thomas Monjalon <thomas at monjalon.net>
> > > Subject: Re: [EXT] [RFC PATCH 0/2] introduce __rte_internal tag
> > >
> > > On Thu, Jun 06, 2019 at 03:14:42PM +0000, Jerin Jacob Kollanukkaran
> wrote:
> > > ><snip as this is getting long>
> > > >
> > > > I don't have any strong opinion on name prefix vs marking as
> > > __rte_internal.
> > > > Or combination of both. I am fine any approach.
> > > >
> > > > I have only strong option on not to  induce objdump dependency for
> > > checkpatch.
> > > > For the reason mentioned in
> > > > http://mails.dpdk.org/archives/dev/2019-
> > > June/134160.html.
> > > >
> > >
> > > Sorry, in my haste I didn't fully adress this in your previous email
> > >
> > > I'm really uncertain what you mean by introducing a checkpatch
> > > dependency on objdump here.  Theres nothing preventing you from
> > > running checkpatch before you build the library.  The only thing
> > > checkpatch does in dpdk is scan the patches for sytle violations,
> > > and for changes in the map file for movement to and from the
> EXPERIMENTAL section (i.e. no use of objdump).
> > >
> > > My patch modifies check-experimental-syms.sh (adding an objdump scan
> > > for INTERNAL symbols, and renaming the script to
> > > check-special-syms.sh to be more meaningful).  That script however,
> > > is not run by checkpatch, its run during compilation of the library
> > > to ensure that any symbol in a map file is also tagged with
> > > __rte_internal in the corresponding object).  Theres no path from
> > > checkpatches to check-experimental-syms.sh
> > >
> > > What I meant in my last comment was that any dependency on objdump
> > > in check-[experimental|special]-syms.sh already existed prior to this
> patch.
> >
> > I see. I thought your patches addressing issue related to
> > http://patches.dpdk.org/patch/53590/
> >
> It does, it just does it in a different way than you do it.
> 
> > Where checkpatch.sh complaints when we add new internal driver API
> > with out rte_experimental? That's where all the discussion started.
> > The reason why I was saying the API name prefix to mark as internal
> > API is that checkpatch can detect that case.
> >
> Ah, thats a fair point, with my approach we need to add some code to check-
> symbol-change.sh such that any symbols listed in an INTERNAL section were
> ignored.  That would provide behavioral compatibility to what you were
> doing in your patch.

OK.

> But regardless, there is no dependency on objdump that wasn't there
> before, are we in agreement on that?

Yes.

> 
> Neil
> 
> > Example:
> > ERROR: symbol otx2_mbox_alloc_msg_rsp is added in the DPDK_19.05
> > section, but is expected to be added in the EXPERIMENTAL section of
> > the version map
> >
> >
> > >
> > > So I'm unsure why you think checkpatches has a dependency.
> > >
> > > Neil
> >
> >


More information about the dev mailing list