DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Aaron Conole <aconole@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	 Adrien Mazarguil <adrien.mazarguil@6wind.com>
Cc: Gage Eads <gage.eads@intel.com>, dev <dev@dpdk.org>,
	 Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	Van Haaren Harry <harry.van.haaren@intel.com>,
	 Nikhil Rao <nikhil.rao@intel.com>,
	Erik Gabriel Carrillo <erik.g.carrillo@intel.com>,
	 Bruce Richardson <bruce.richardson@intel.com>,
	Pablo de Lara <pablo.de.lara.guarch@intel.com>
Subject: Re: [dpdk-dev] [PATCH] eal: promote some service core functions to stable
Date: Fri, 21 Jun 2019 14:45:45 +0200	[thread overview]
Message-ID: <CAJFAV8xZXjGaNgTVd4OF9P1i_HV7_eHmio6cfUHubenUVB3T5w@mail.gmail.com> (raw)
In-Reply-To: <CAJFAV8xV1dXz3=a5pzyKvnUAqKMZWxQp0f+tTtEAz=ofWYrc0A@mail.gmail.com>

On Thu, Jun 20, 2019 at 10:16 PM David Marchand <david.marchand@redhat.com>
wrote:

>
>
> On Thu, Jun 20, 2019 at 9:45 PM David Marchand <david.marchand@redhat.com>
> wrote:
>
>>
>>
>> On Thu, Jun 20, 2019 at 8:26 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>>> Gage Eads <gage.eads@intel.com> writes:
>>>
>>> > The functions rte_service_may_be_active(),
>>> rte_service_lcore_attr_get(),
>>> > and rte_service_attr_reset_all() were introduced nearly a year ago in
>>> DPDK
>>> > 18.08. They can be considered non-experimental for the 19.08 release.
>>> >
>>> > rte_service_may_be_active() is used by eventdev and the sw PMD, and
>>> this
>>> > commit allows them to not need any experimental API.
>>> >
>>> > Signed-off-by: Gage Eads <gage.eads@intel.com>
>>> > ---
>>> >  drivers/event/sw/Makefile                   |  1 -
>>> >  drivers/event/sw/meson.build                |  1 -
>>> >  lib/librte_eal/common/include/rte_service.h | 15 +++------------
>>> >  lib/librte_eal/common/rte_service.c         |  6 +++---
>>> >  lib/librte_eal/rte_eal_version.map          |  6 +++---
>>> >  lib/librte_eventdev/Makefile                |  1 -
>>> >  lib/librte_eventdev/meson.build             |  1 -
>>> >  7 files changed, 9 insertions(+), 22 deletions(-)
>>> >
>>> > diff --git a/drivers/event/sw/Makefile b/drivers/event/sw/Makefile
>>> > index 81236a392..c6600e836 100644
>>> > --- a/drivers/event/sw/Makefile
>>> > +++ b/drivers/event/sw/Makefile
>>> > @@ -7,7 +7,6 @@ include $(RTE_SDK)/mk/rte.vars.mk
>>> >  LIB = librte_pmd_sw_event.a
>>> >
>>> >  # build flags
>>> > -CFLAGS += -DALLOW_EXPERIMENTAL_API
>>> >  CFLAGS += -O3
>>> >  CFLAGS += $(WERROR_FLAGS)
>>> >  # for older GCC versions, allow us to initialize an event using
>>> > diff --git a/drivers/event/sw/meson.build
>>> b/drivers/event/sw/meson.build
>>> > index 30d221647..985012219 100644
>>> > --- a/drivers/event/sw/meson.build
>>> > +++ b/drivers/event/sw/meson.build
>>> > @@ -1,7 +1,6 @@
>>> >  # SPDX-License-Identifier: BSD-3-Clause
>>> >  # Copyright(c) 2017 Intel Corporation
>>> >
>>> > -allow_experimental_apis = true
>>>
>>> I don't think you can remove these.  There are still some experimental
>>> APIs (f.e. the rename for rte_cryptodev_sym_session_get_private_data
>>> marked that function as experimental and it will cause build breakage).
>>>
>>> Maybe I'm mis understanding it?  It would be good to get verification
>>> from Bruce whether that API should not be marked as experimental (it was
>>> just a rename, so not sure...) - maybe that's a follow up for this
>>> patch?
>>>
>>> See: https://travis-ci.com/ovsrobot/dpdk/jobs/209722145 for an example
>>>
>>> The odd thing is I only see it on the clang builds - perhaps it's a
>>> missing definition for the clang compiler.
>>>
>>
>> Erf, it looks like the __rte_experimental tag is affected by the order in
>> the declaration of the symbol.
>>
>> --- a/lib/librte_cryptodev/rte_cryptodev.h
>> +++ b/lib/librte_cryptodev/rte_cryptodev.h
>> @@ -1245,7 +1245,7 @@ struct rte_cryptodev_asym_session *
>> __rte_experimental
>>   *  - On success return pointer to user data.
>>   *  - On failure returns NULL.
>>   */
>> -void * __rte_experimental
>> +__rte_experimental void *
>>  rte_cryptodev_sym_session_get_user_data(
>>                                         struct rte_cryptodev_sym_session
>> *sess);
>>
>>
> https://hastebin.com/micomogoqi.cs
>
> Does it mean that the "void *" type had been marked as deprecated with the
> previous syntax?
> Requesting compiler experts assistance :-)
>


Ok, did a new pass on the tree.. found quite some sites where we have
issues (and other discrepancies... I started a new patchset).
Looked at gcc documentation [1], and to me the safer approach would be to
enforce that __rte_experimental is the first thing of a symbol declaration.

Comments?


1: https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html

-- 
David Marchand

  reply	other threads:[~2019-06-21 12:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-20 16:42 Gage Eads
2019-06-20 18:25 ` Aaron Conole
2019-06-20 18:39   ` Eads, Gage
2019-06-20 19:45   ` David Marchand
2019-06-20 20:16     ` David Marchand
2019-06-21 12:45       ` David Marchand [this message]
2019-06-21 16:27         ` Neil Horman
2019-06-21 16:47           ` David Marchand
2019-06-21 17:40             ` Neil Horman
2019-06-21 19:58               ` David Marchand
2019-06-22 16:17                 ` Neil Horman
2019-06-22 17:51                   ` David Marchand
2019-06-22 19:33                     ` Neil Horman
2019-06-20 19:02 ` [dpdk-dev] [PATCH v2] " Gage Eads
2019-06-27 12:48   ` David Marchand
2019-06-27 16:25     ` Eads, Gage
2019-07-08 10:23   ` Thomas Monjalon

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=CAJFAV8xZXjGaNgTVd4OF9P1i_HV7_eHmio6cfUHubenUVB3T5w@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=aconole@redhat.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=erik.g.carrillo@intel.com \
    --cc=gage.eads@intel.com \
    --cc=harry.van.haaren@intel.com \
    --cc=jerinj@marvell.com \
    --cc=nhorman@tuxdriver.com \
    --cc=nikhil.rao@intel.com \
    --cc=pablo.de.lara.guarch@intel.com \
    /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).