From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] Re: [PATCH] devtools: skip the symbol check when map file under drivers
Date: Thu, 23 May 2019 14:21:29 +0000 [thread overview]
Message-ID: <BYAPR18MB2424AAFDEEC8B6899C06853FC8010@BYAPR18MB2424.namprd18.prod.outlook.com> (raw)
> -----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
> > > >
> > > >
> >
next reply other threads:[~2019-05-23 14:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 14:21 Jerin Jacob Kollanukkaran [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BYAPR18MB2424AAFDEEC8B6899C06853FC8010@BYAPR18MB2424.namprd18.prod.outlook.com \
--to=jerinj@marvell.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=nhorman@tuxdriver.com \
--cc=stable@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).