From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 460A0A05D3 for ; Thu, 23 May 2019 20:59:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 488131B945; Thu, 23 May 2019 20:59:15 +0200 (CEST) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 4BCDF493D; Thu, 23 May 2019 20:59:14 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 85E6125868; Thu, 23 May 2019 14:59:13 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 23 May 2019 14:59:13 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=3Fo7WTtOJdB+Iy+NSaWE1kb5DxTfItHbCWdCciKQbLk=; b=jGciBzzUe583 RjYR+L7CIu2sJ4AhL+URD3bIr4Wp1gJQ6g2R5kIngZuX4XLCSYX6CpR75eC+zrrz DB9ONJAMEv7HFamYgO8TX9tJYrDgm404ymRRRc6XO3IaZ4a++i3MDxyAV50Cb5ap 2Q/UWqrxtdeBL/lUunUSr3iu2Jr6deU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=3Fo7WTtOJdB+Iy+NSaWE1kb5DxTfItHbCWdCciKQb Lk=; b=g3rs2RoKnFFZFLiUqOq9V6HPEripmFEIUsAtlqR/7RLhc/9ULVLcyWZEy YiioLT8NJnbz5W1AvwtzLL/2QEr5FnQllVbP3gvwfRc4LY1F+mO9HuIuYJn6iz9w PyF4+AA0JE2IoUleJ8jMbRDjWwG0+7B7IUF5g+Ykky1pyhwCwTZAlxeO2UqCJetm V7NdwPkir3ILEnNrT5V3sO4iu4YyarUde9RtxQmFugjMyvlcGyeZExB2KiMRSNtH OWABBAEc9IcGbw/WknWJrOgSzNHLZTZUUPPak82s28curRU+NSNupdwRguUExCMt erkFP7VBE4zDau7VtHhsQUu6ZhzJg== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddruddugedgudefgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc fkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhepmhgrihhlfhhrohhmpeht hhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id B86A980065; Thu, 23 May 2019 14:59:10 -0400 (EDT) From: Thomas Monjalon To: Neil Horman Cc: Jerin Jacob Kollanukkaran , Bruce Richardson , "dev@dpdk.org" , "stable@dpdk.org" Date: Thu, 23 May 2019 20:59:07 +0200 Message-ID: <9743169.QmDmBymccE@xps> In-Reply-To: <20190523175752.GA18326@hmswarspite.think-freely.org> References: <20190523175752.GA18326@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] devtools: skip the symbol check when map file under drivers X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 23/05/2019 19:57, Neil Horman: > On Thu, May 23, 2019 at 02:21:29PM +0000, Jerin Jacob Kollanukkaran wrote: > > From: Neil Horman > > > > > > > > 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. > > > > I'm thinking about it a bit differently. Internal functions should never be > available to user, weather they are prototyped in DPDK header files or not. > Unfortunately, because of how library symbol exports work, there is no way to > differentiate between which exported functions are internal or external, they > are only exported or not, and as such, they are always resolveable by someone > linking against them (regardless of which hackery is used to achieve that > result). I'd like a way to prevent users who are only using the SDK (not > building it) from accessing those symbols, and the above is the best solution I > can come up with. I admit its not great, but it does place a roadblock in the > way of users who attempt to use symbols we don't want to give them access to. > And yes its circumventable by defining BUILDING_RTE_SDK, but I would think its > clear that they are not building the SDK, and so they should not be doing that. > > Just not exporting the requisite header files is an easier solution, so if thats > the consensus I can be ok with that, but I would really love to have a way to > document in the code those functions which are not meant for external > consumption. I think there are good ideas here. Please come with a patch and we'll try to apply the chosen policy to the existing code.