DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: David Marchand <david.marchand@redhat.com>
Cc: dev <dev@dpdk.org>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Bruce Richardson <bruce.richardson@intel.com>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH v1 0/9] dpdk: introduce __rte_internal tag
Date: Thu, 13 Jun 2019 06:30:38 -0400
Message-ID: <20190613103038.GB5381@hmswarspite.think-freely.org> (raw)
In-Reply-To: <CAJFAV8wioyE04g7PuaTbsz65Y7hEkMtSUhVkHhWM+y4SwCPBQQ@mail.gmail.com>

On Thu, Jun 13, 2019 at 09:53:46AM +0200, David Marchand wrote:
> On Wed, Jun 12, 2019 at 10:40 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > Hey-
> >         Based on our recent conversations regarding the use of symbols only
> > meant for internal dpdk consumption (between dpdk libraries), this is an
> > idea
> > that I've come up with that I'd like to get some feedback on
> >
> > Summary:
> > 1) We have symbols in the DPDK that are meant to be used between DPDK
> > libraries,
> > but not by applications linking to them
> > 2) We would like to document those symbols in the code, so as to note them
> > clearly as for being meant for internal use only
> > 3) Linker symbol visibility is a very coarse grained tool, and so there is
> > no
> > good way in a single library to mark items as being meant for use only by
> > other
> > DPDK libraries, at least not without some extensive runtime checking
> >
> >
> > Proposal:
> > I'm proposing that we introduce the __rte_internal tag.  From a coding
> > standpoint it works a great deal like the __rte_experimental tag in that it
> > expempts the tagged symbol from ABI constraints (as the only users should
> > be
> > represented in the DPDK build environment).  Additionally, the
> > __rte_internal
> > macro resolves differently based on the definition of the BUILDING_RTE_SDK
> > flag
> > (working under the assumption that said flag should only ever be set if we
> > are
> > actually building DPDK libraries which will make use of internal calls).
> > If the
> > BUILDING_RTE_SDK flag is set __rte_internal resolves to
> > __attribute__((section
> > "text.internal)), placing it in a special text section which is then used
> > to
> > validate that the the symbol appears in the INTERNAL section of the
> > corresponding library version map).  If BUILDING_RTE_SDK is not set, then
> > __rte_internal resolves to __attribute__((error("..."))), which causes any
> > caller of the tagged function to throw an error at compile time,
> > indicating that
> > the symbol is not available for external use.
> >
> > This isn't a perfect solution, as applications can still hack around it of
> > course, but I think it hits some of the high points, restricting symbol
> > access
> > for any library that prototypes its public and private symbols in the same
> > header file, excluding the internal symbols from ABI constraints, and
> > clearly
> > documenting those symbols which we wish to limit to internal usage.
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > CC: Thomas Monjalon <thomas@monjalon.net>
> >
> >
> >
> On the principle, are we not breaking the ABI for the libraries that we
> touch with this?
> 
Yes, strictly speaking on principle, we are, because we are removing symbols
from the exported symbol list, but the purpose of this patch set was to identify
those symbols which, by (convention/documentation/etc), we had intended to
mandate should never be used by applications outside of the DPDK.  Since this
patch is meant to identify those symbols, and somewhat more formally isolate and
prevent users from accessing them, I was making the argument that it was ok to
do so.  If you like we can go through the process of deprecating them, prior to
moving them to an internal space, but given that I was under the impression
anyone reporting a bug about the symbol disappearing would be responded to with
a note indicating that they never should have been accessed to begin with, I had
assumed the movement would be ok.

> Compilation is broken in patch 1 and 2 because the script rename is part of
> patch 3.
> 
Good point, I'll reshuffle those to be more correct.
Neil

> 
> -- 
> David Marchand

  reply index

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-25 18:43 [dpdk-dev] [RFC PATCH 0/2] " Neil Horman
2019-05-25 18:43 ` [dpdk-dev] [RFC PATCH 1/2] Add __rte_internal tag for functions and version target Neil Horman
2019-06-05 16:14   ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-05-25 18:43 ` [dpdk-dev] [RFC PATCH 2/2] Convert dpaa driver to tag internal-only symbols appropriately Neil Horman
2019-06-05 16:24 ` [dpdk-dev] [EXT] [RFC PATCH 0/2] introduce __rte_internal tag Jerin Jacob Kollanukkaran
2019-06-05 16:45   ` Bruce Richardson
2019-06-05 18:11     ` Neil Horman
2019-06-06  9:44       ` Jerin Jacob Kollanukkaran
2019-06-06 11:34         ` Neil Horman
2019-06-06 12:04           ` Jerin Jacob Kollanukkaran
2019-06-06 13:18             ` Wiles, Keith
2019-06-06 13:43               ` Neil Horman
2019-06-06 13:53                 ` Wiles, Keith
2019-06-06 14:46                   ` Neil Horman
2019-06-06 13:35             ` Neil Horman
2019-06-06 14:02               ` Jerin Jacob Kollanukkaran
2019-06-06 15:03                 ` Neil Horman
2019-06-06 15:14                   ` Jerin Jacob Kollanukkaran
2019-06-06 15:26                     ` Neil Horman
2019-06-06 16:23                       ` Jerin Jacob Kollanukkaran
2019-06-06 16:55                         ` Neil Horman
2019-06-07  9:41                           ` Jerin Jacob Kollanukkaran
2019-06-07 10:35                             ` Neil Horman
2019-06-07 15:42                   ` Ray Kinsella
2019-06-07 18:21                     ` Wiles, Keith
2019-06-12 20:38 ` [dpdk-dev] [PATCH v1 0/9] dpdk: " Neil Horman
2019-06-12 20:38   ` [dpdk-dev] [PATCH v1 1/9] Add __rte_internal tag for functions and version target Neil Horman
2019-06-12 20:38   ` [dpdk-dev] [PATCH v1 2/9] Exempt INTERNAL symbols from checking Neil Horman
2019-06-12 20:38   ` [dpdk-dev] [PATCH v1 3/9] mark dpaa driver internal-only symbols with __rte_internal Neil Horman
2019-06-12 20:38   ` [dpdk-dev] [PATCH v1 4/9] fslmc: identify internal only functions and tag them as __rte_internal Neil Horman
2019-06-12 20:38   ` [dpdk-dev] [PATCH v1 5/9] dpaa2: Adjust dpaa2 driver to mark internal symbols with __rte_internal Neil Horman
2019-06-12 20:39   ` [dpdk-dev] [PATCH v1 6/9] dpaax: mark internal functions " Neil Horman
2019-06-12 20:39   ` [dpdk-dev] [PATCH v1 7/9] cpt: " Neil Horman
2019-06-17  5:27     ` [dpdk-dev] [EXT] " Anoob Joseph
2019-06-12 20:39   ` [dpdk-dev] [PATCH v1 8/9] octeonx: " Neil Horman
2019-06-12 20:39   ` [dpdk-dev] [PATCH v1 9/9] dpaa2: " Neil Horman
2019-06-12 21:14     ` Aaron Conole
2019-06-13 10:24       ` Neil Horman
2019-06-13  7:53   ` [dpdk-dev] [PATCH v1 0/9] dpdk: introduce __rte_internal tag David Marchand
2019-06-13 10:30     ` Neil Horman [this message]
2019-06-13 14:23 ` [dpdk-dev] [PATCH v2 0/10] " Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 01/10] Add __rte_internal tag for functions and version target Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 02/10] meson: add BUILDING_RTE_SDK Neil Horman
2019-06-13 14:44     ` Bruce Richardson
2019-06-19 10:39       ` Neil Horman
2019-06-19 10:46         ` Bruce Richardson
2019-06-19 18:34           ` Neil Horman
2019-06-20 10:20             ` Bruce Richardson
2019-06-20 10:21     ` Bruce Richardson
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 03/10] Exempt INTERNAL symbols from checking Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 04/10] mark dpaa driver internal-only symbols with __rte_internal Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 05/10] fslmc: identify internal only functions and tag them as __rte_internal Neil Horman
2019-06-17  7:30     ` Hemant Agrawal
2019-06-19 10:45       ` Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 06/10] dpaa2: Adjust dpaa2 driver to mark internal symbols with __rte_internal Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 07/10] dpaax: mark internal functions " Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 08/10] cpt: " Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 09/10] octeonx: " Neil Horman
2019-06-13 14:23   ` [dpdk-dev] [PATCH v2 10/10] dpaa2: " Neil Horman
2019-08-06 10:03   ` [dpdk-dev] [PATCH v2 0/10] dpdk: introduce __rte_internal tag Thomas Monjalon
2019-08-06 12:21     ` Neil Horman

Reply instructions:

You may reply publically 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=20190613103038.GB5381@hmswarspite.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --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

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git