patches for DPDK stable branches
 help / color / mirror / Atom feed
* Re: [dpdk-stable] [dpdk-dev] Re: [PATCH] devtools: skip the symbol check when map file under drivers
@ 2019-05-23 14:21 Jerin Jacob Kollanukkaran
  2019-05-23 17:57 ` Neil Horman
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-05-23 14:21 UTC (permalink / raw)
  To: Neil Horman; +Cc: Bruce Richardson, dev, thomas, stable



> -----Original Message-----
> From: Neil Horman <nhorman@tuxdriver.com>
> Sent: Thursday, May 23, 2019 12:29 AM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; stable@dpdk.org
> Subject: Re: [EXT] Re: [dpdk-dev] Re: [PATCH] devtools: skip the symbol
> check when map file under drivers
> > > > > > IMO, The name prefix matters. The rte_* should denote it a
> > > > > > DPDK API and application suppose to use it.
> > > > > >
> > > > > It doesn't, its just a convention.  We have no documentation
> > > > > that indicates what the meaning of an rte_* prefix is
> > > > > specficially, above and beyond the fact thats how we name
> > > > > functions in the DPDK.  If you want to submit a patch to
> > > > > formalize the meaning of function prefixes, you're welcome too
> > > > > (though I won't support it, perhaps others will).  But even if
> > > > > you do, it doesn't address the underlying problem, which is that
> applications still have access to those symbols.
> > > > > Maintaining an ABI by assertion of prefix is really a lousy way
> > > > > to communicate what functions should be accessed by an
> > > > > application and which shouldn't.  If a function is exported, and
> > > > > included in the header file, people will try to use
> > > >
> > > > The current scheme in the driver/common is that, the header files
> > > > are NOT made It as public ie not installed make install.
> > > > The consumer driver includes that using relative path wrt DPDK
> > > > source
> > > directory.
> > > >
> > > Well, thats a step in the right direction.  I'd still like to see
> > > some enforcement to prevent the inadvertent use of those APIs though
> >
> > Yes header file  is  not exported. Not sure how a client can use those.
> > Other than doing some hacking.
> >
> Yes, self prototyping the exported functions would be a way around that.
> > >
> > > > Anyway I will add experimental section to make tool happy.
> > > >
> > > That really not the right solution.  Marking them as experimental is
> > > just papering over the problem, and suggests to users that they will
> > > one day be stable.
> >
> > That what my original concern.
> >
> > > What you want is to explicitly mark those symbols as internal only,
> > > so that any inadvertent use gets flagged.
> >
> > What is your final thought? I can assume the following for my patch
> > generation
> >
> > # No need to mark as experimental
> > # Add @internal to denote it is a internal function like followed some places
> in EAL.
> >
> These are both correct, yes.
> 
> In addition, I would like to see some mechanism that explicitly marks the
> function as exported only for the purposes of internal use.  I understand that
> yours is a case in which this is not expressly needed because you don't
> prototype those functions, but what I'd like to see is a macro in rte_compat.h
> somewhere like this:
> 
> #define INTERNAL_USE_ONLY do {static_assert(0, "Function is only available
> for internal DPDK usage");} while(0)
> 
> so that, in your exported header file (of which I'm sure you have one, even if
> it doesn't contain your private functions, you can do something like this:
> 
> #ifdef BUILDING_RTE_SDK
> void somefunc(int val);
> #else
> #define somefunc(x) INTERNAL_USE_ONLY
> #endif

I think, We have two cases
1) Internal functions are NOT available via  DPDK SDK exported header files
2) Internal functions are available via DPDK SDK exported header files

I think, you are trying to address case 2( as case 1 is not applicable in this context due lack of header file)
For case 2, IMO, the above scheme will not be enough as 
The consumer entity can simply add the exact C flags to skip that check in this case, -DBUILDING_RTE_SDK.
IMO, it would be correct remove private functions from public header files. No strong options on this.
 
> 
> This combination allows for 'internal' functions to be used (defining internal
> to mean access to functions only when building the DPDK SDK), while
> expressly breaking the build of any application which attempts to use these
> functions when not building the SDK (i.e. when building an application that
> expects to link to the DPDK after its built).  Again, I uderstand that in your
> case, it may be sufficient to just not prototype the functions you don't want
> used, but I think in the general case its important to have some mechanism
> to expressly prevent their usage outside the SDK
> 
> Best
> Neil
> 
> > >
> > > Neil
> > > >
> > > >
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] Re: [PATCH] devtools: skip the symbol check when map file under drivers
@ 2019-05-22 13:41 Jerin Jacob Kollanukkaran
  2019-05-22 14:11 ` Neil Horman
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-05-22 13:41 UTC (permalink / raw)
  To: Neil Horman; +Cc: Bruce Richardson, dev, thomas, stable

> -----Original Message-----
> From: Neil Horman <nhorman@tuxdriver.com>
> Sent: Wednesday, May 22, 2019 6:43 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> thomas@monjalon.net; stable@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] Re: [PATCH] devtools: skip the symbol check
> when map file under drivers
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, May 22, 2019 at 11:54:13AM +0000, Jerin Jacob Kollanukkaran
> wrote:
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Wednesday, May 22, 2019 4:21 PM
> > > To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > > Cc: Neil Horman <nhorman@tuxdriver.com>; dev@dpdk.org;
> > > thomas@monjalon.net; stable@dpdk.org
> > > Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] devtools: skip the symbol
> > > check when map file under drivers
> > >
> > > > > Sorry, but I'm not ok with this, because many of our DPDK PMDs
> > > > > have functions that get exported which are meant to be called by
> > > > > applications directly.  The
> > > >
> > > > OK. Just to update my knowledge, Should those API needs to go
> > > > through ABI/API depreciation process?
> > > >
> > > > Actually, I am concerned about the APIs, which is called between
> > > > drviers not the application. For example,
> > > > drivers/common/dpaax/rte_common_dpaax_version.map
> > > >
> > > > it is not interface to application rather it is for intra driver case.
> > > > I think, I can change my logic to Skip the symbols which NOT
> > > > starting with
> > > rte_.
> > > > Agree?
> > > >
> > > > Context:
> > > > I am adding a new driver/common/octeontx2 directory and it has
> > > > some API which Needs to shared between drivers not to the
> > > > application. For me, it does not make sense to go through any ABI
> process in such case.
> > > >
> > > >
> > > Maybe not, but other drivers will have APIs designed for apps to
> > > call directly - some NIC drivers have them, and I suspect that
> > > rawdev drivers will need them a lot. Therefore, it's best to have
> > > the drivers directory scanned by our tooling.
> >
> > Agreed. But all of those API  which called directly called from
> > application is starts with rte_ symbol. How about skipping the symbols
> > which is NOT start with rte_*
> > example:
> > drivers/common/octeontx/rte_common_octeontx_version.map
> > drivers/common/dpaax/rte_common_dpaax_version.map
> >
> 
> No, that won't work.  If you export a function, it doesn't matter if its named
> rte_* or not.  Its accessible from any library/application that cares to call it,

IMO, The name prefix matters. The rte_* should denote it a DPDK API and application
suppose to use it.

I don't think, giving experimental status to intra driver API helps anyone, neither driver nor
application.

If you think strongly that experimental needs to be added for intra driver APIs then I can add that.
 

> and so you have a responsibility to keep it stable for those users.
> 
> Currently the way we have around that is the use of the __rte_experimental
> tag.
> Adding that tag to an exported function marks it as being unstable, and while
> you can use it, it will generate a build time warning about its use, unless you
> define ALLOW_EXPERIMENTAL_API.  You could use that, understanding that
> in-tree drivers could use it safely, as you should always be keeping the API in
> sync with its users, but thats not quite what you want I don't think.
> 
> Another solution (allbeit a slightly risky one), would be to bifurcate your
> header files into a public and private version, with the private version
> prototyping your driver-only functions properly, and the public version
> aliasing them such that they generate a build time error indicating those
> functions aren't available for public use (you can use the gcc static_assert
> macro I believe).  Users could circumvent it by pulling the private header out
> of the build, or just prototyping the functions themselves, but at that point a
> user is asking for trouble anyway
> 
> Neil

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] Re: [PATCH] devtools: skip the symbol check when map file under drivers
@ 2019-05-22 11:54 Jerin Jacob Kollanukkaran
  2019-05-22 13:13 ` Neil Horman
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-05-22 11:54 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Neil Horman, dev, thomas, stable

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, May 22, 2019 4:21 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>; dev@dpdk.org;
> thomas@monjalon.net; stable@dpdk.org
> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] devtools: skip the symbol check
> when map file under drivers
> 
> > > Sorry, but I'm not ok with this, because many of our DPDK PMDs have
> > > functions that get exported which are meant to be called by
> > > applications directly.  The
> >
> > OK. Just to update my knowledge, Should those API needs to go through
> > ABI/API depreciation process?
> >
> > Actually, I am concerned about the APIs, which is called between
> > drviers not the application. For example,
> > drivers/common/dpaax/rte_common_dpaax_version.map
> >
> > it is not interface to application rather it is for intra driver case.
> > I think, I can change my logic to Skip the symbols which NOT starting with
> rte_.
> > Agree?
> >
> > Context:
> > I am adding a new driver/common/octeontx2 directory and it has some
> > API which Needs to shared between drivers not to the application. For
> > me, it does not make sense to go through any ABI process in such case.
> >
> >
> Maybe not, but other drivers will have APIs designed for apps to call directly -
> some NIC drivers have them, and I suspect that rawdev drivers will need
> them a lot. Therefore, it's best to have the drivers directory scanned by our
> tooling.

Agreed. But all of those API  which called directly called from application
is starts with rte_ symbol. How about skipping the symbols which is NOT start with rte_*
example:
drivers/common/octeontx/rte_common_octeontx_version.map
drivers/common/dpaax/rte_common_dpaax_version.map


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

end of thread, other threads:[~2019-05-23 20:17 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23 14:21 [dpdk-stable] [dpdk-dev] Re: [PATCH] devtools: skip the symbol check when map file under drivers Jerin Jacob Kollanukkaran
2019-05-23 17:57 ` Neil Horman
2019-05-23 18:59   ` Thomas Monjalon
2019-05-23 20:17     ` Neil Horman
  -- strict thread matches above, loose matches on Subject: below --
2019-05-22 13:41 Jerin Jacob Kollanukkaran
2019-05-22 14:11 ` Neil Horman
2019-05-22 11:54 Jerin Jacob Kollanukkaran
2019-05-22 13:13 ` Neil Horman

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