From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id 7F4601B43D; Thu, 4 Oct 2018 16:21:53 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Oct 2018 07:21:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,340,1534834800"; d="scan'208";a="262840313" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.49]) ([10.237.221.49]) by orsmga005.jf.intel.com with ESMTP; 04 Oct 2018 07:21:50 -0700 To: Adrien Mazarguil Cc: dev@dpdk.org, Thomas Monjalon , Andrew Rybchenko , Gaetan Rivet , dpdk-techboard , Luca Boccassi , Kevin Traynor , Yongseok Koh , Christian Ehrhardt , Neil Horman References: <20180803132032.29038-1-adrien.mazarguil@6wind.com> <20180831085337.21419-1-adrien.mazarguil@6wind.com> <20180831085337.21419-8-adrien.mazarguil@6wind.com> From: Ferruh Yigit Openpgp: preference=signencrypt Message-ID: <4a967831-e02d-75d5-b729-5c8d3758d861@intel.com> Date: Thu, 4 Oct 2018 15:21:48 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180831085337.21419-8-adrien.mazarguil@6wind.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3 7/7] ethdev: deprecate rte_flow_copy function 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: Thu, 04 Oct 2018 14:21:54 -0000 On 8/31/2018 10:01 AM, Adrien Mazarguil wrote: > No users left for this function, time to deprecate it. > > Signed-off-by: Adrien Mazarguil > Cc: Thomas Monjalon > Cc: Ferruh Yigit > Cc: Andrew Rybchenko > Cc: Gaetan Rivet > -- > v3 changes: > > - Removed deprecation notice (finally got Ferruh's point), made patch last > in series. > > v2 changes: > > - Patch was not present in original series. > --- > doc/guides/rel_notes/deprecation.rst | 7 ------- > lib/librte_ethdev/rte_flow.h | 7 ++++++- > 2 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst > index e2dbee317..48cfb266b 100644 > --- a/doc/guides/rel_notes/deprecation.rst > +++ b/doc/guides/rel_notes/deprecation.rst > @@ -88,10 +88,3 @@ Deprecation Notices > - ``rte_pdump_set_socket_dir`` will be removed; > - The parameter, ``path``, of ``rte_pdump_init`` will be removed; > - The enum ``rte_pdump_socktype`` will be removed. > - > -* ethdev: flow API function ``rte_flow_copy()`` will be deprecated in v18.11 > - in favor of ``rte_flow_conv()`` (which will appear in that version) and > - subsequently removed for v19.02. > - > - This is due to a lack of flexibility and reliance on a type unusable with > - C++ programs (struct rte_flow_desc). > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > index 052ceefb6..f062ffead 100644 > --- a/lib/librte_ethdev/rte_flow.h > +++ b/lib/librte_ethdev/rte_flow.h > @@ -2332,6 +2332,7 @@ rte_flow_error_set(struct rte_flow_error *error, > const char *message); > > /** > + * @deprecated > * @see rte_flow_copy() > */ > struct rte_flow_desc { > @@ -2343,10 +2344,13 @@ struct rte_flow_desc { > }; > > /** > + * @deprecated > * Copy an rte_flow rule description. > * > * This interface is kept for compatibility with older applications but is > - * implemented as a wrapper to rte_flow_conv(). > + * implemented as a wrapper to rte_flow_conv(). It is deprecated due to its > + * lack of flexibility and reliance on a type unusable with C++ programs > + * (struct rte_flow_desc). > * > * @param[in] fd > * Flow rule description. > @@ -2365,6 +2369,7 @@ struct rte_flow_desc { > * If len is lower than the size of the flow, the number of bytes that would > * have been written to desc had it been sufficient. Nothing is written. > */ > +__rte_deprecated > size_t > rte_flow_copy(struct rte_flow_desc *fd, size_t len, > const struct rte_flow_attr *attr, > Not exactly related to this patch but I have a deprecation process question, what we are mostly doing is: 1) Release N, document in deprecation.rst the API that will be changed 2) Release N + 1, change the API and remove the note from the deprecation.rst But using __rte_deprecated makes sense, how can we include this into process? It looks like we can use __rte_deprecated only when an API replaced (add a new one & deprecate old one), but if an API changed switch needs to be atomic. Options I can think of: For replacing an API, A) a1) Release N, add note to deprecation.rst and add __rte_deprecated to old API a2) Release N + 1, add new API, remove note from deprecation.rst and remove old API B) b1) Release N, add note to deprecation.rst b2) Release N + 1, remove note from deprecation.rst add __rte_deprecated to old API and add new API (as non experimental [1]) b3) Release N + 1 + M, remove old API (perhaps cleanup on each new LTS) For changing exiting API, C) c1) Release N, add note to deprecation.rst c2) Release N + 1, remove note from deprecation.rst and switch to new API The problem with C) is, even developer sees the deprecation note, there is nothing she can do or prepare, only in "Release N + 1" she will hit to the change and will update her code. Not very useful for developer, so what about: D) d1) Release N, add note to deprecation.rst implement new API within RTE_NEXT_ABI #ifdef d2) Release N + 1, remove note from deprecation.rst and switch to new API Switching to D means one can't send deprecation notice without doing the actual implementation, so there is a big difference between C). I am for B & D, what do you think? [1] This is happening again with rte_flow_conv(), old API is deprecated, new API is experimental, from user point of view there is no stable API. I am not happy with "an API should be experimental for first release it has been introduced" policy, is it really helping, can we re-visit this again?