From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 447965B16 for ; Tue, 4 Dec 2018 16:15:50 +0100 (CET) Received: from cpe-2606-a000-111b-405a-a193-cb97-58ba-1c15.dyn6.twc.com ([2606:a000:111b:405a:a193:cb97:58ba:1c15] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.63) (envelope-from ) id 1gTwX7-0003oN-O1; Mon, 03 Dec 2018 17:17:57 -0500 Date: Tue, 4 Dec 2018 10:14:57 -0500 From: Neil Horman To: David Marchand Cc: dev@dpdk.org, Timothy Redaelli , ferruh.yigit@intel.com, adrien.mazarguil@6wind.com Message-ID: <20181204151457.GA31778@hmswarspite.think-freely.org> References: <20181203164724.GA12316@hmswarspite.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Spam-Score: 0.3 (/) X-Spam-Status: No Subject: Re: [dpdk-dev] Marking symbols as experimental in the headers only 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: , X-List-Received-Date: Tue, 04 Dec 2018 15:15:50 -0000 On Tue, Dec 04, 2018 at 09:21:43AM +0100, David Marchand wrote: > On Mon, Dec 3, 2018 at 5:48 PM Neil Horman wrote: > > > On Mon, Dec 03, 2018 at 02:01:02PM +0100, David Marchand wrote: > > > Hello Neil, > > > > > > Looking at > > > > > http://doc.dpdk.org/guides/contributing/versioning.html#experimental-apis, > > > is there a real need to mark both the definition and the declaration of a > > > symbol with the __rte_experimental marker ? > > > > > When I initially wrote the feature I marked both the function prototype > > and its > > definition with the macro to be consistent, as I like to make both > > declaration > > and definition look visually simmilar, just to ease readability, but I'm > > not > > bound to doing it that way per-se > > > > As to weather or not you can only mark the declaration as experimental, > > I'm not > > 100% sure. I think thats an artifact of the compiler. That is to say, the > > macro expands to a compiler attribute that is applied at compile time, and > > checked at link time, and I'm not sure what clang or gcc will do if there > > is a > > discrepancy between the attributes listed in the declaration and the > > definition. > > > > I would say, give it a try, and if you can demonstrate that adding the > > attribute > > only to the declaration results in a link-time warning when external code > > attempts to call an experimental function, I would have no problem > > handling it > > that way. > > > > Did not experiment yet, putting this in my todolist. > Just, I can see that lib/librte_pipeline/ at least only marks the > declarations. > I just tried it. If I turn off ALLOW_EXPERIMENTAL_APIS, the ip_pipeline example breaks with warnings about deprecated functions: /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:60:4: error: ‘rte_table_hash_crc_key8’ is deprecated: Symbol is not yet part of stable ABI [-Werror=deprecated-declarations] params->lb.f_hash = rte_table_hash_crc_key8; ^~~~~~ In file included from /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:10: /home/nhorman/git/dpdk/x86_64-native-linuxapp-gcc/include/rte_table_hash_func.h:56:1: note: declared here rte_table_hash_crc_key8(void *key, void *mask, __rte_unused uint32_t key_size, ^~~~~~~~~~~~~~~~~~~~~~~ /home/nhorman/git/dpdk/examples/ip_pipeline/action.c:64:4: error: ‘rte_table_hash_crc_key16’ is deprecated: Symbol is not yet part of stable ABI [-Werror=deprecated-declarations] params->lb.f_hash = rte_table_hash_crc_key16; ^~~~~~ ... That is sufficient for me to conclude that __rte_experimental only needs to be set on the declaration, not the definiton as well. If you would like to make this adjustment, I'm fine with it, though be aware, you will likely need to make some adjustments to the check-experimental-syms script to account for this Neil > > -- > David Marchand