From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D7895A0C4D; Thu, 25 Nov 2021 16:53:22 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5E484411DD; Thu, 25 Nov 2021 16:53:22 +0100 (CET) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mails.dpdk.org (Postfix) with ESMTP id 9212F40140 for ; Thu, 25 Nov 2021 16:53:20 +0100 (CET) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 0AA265C0165; Thu, 25 Nov 2021 10:53:20 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Thu, 25 Nov 2021 10:53:20 -0500 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=fm2; bh= FnAF/xAVfch0e845cDQBJKGHAo42uioP4SSLjHgfxHU=; b=sc5L/UA7w8DZ3SjD LRE975a1v+9Iz0TCdtwA81GB82F81OWCRtZgHXj0QzQR5+DaNIjxkRVP5MeP/vu3 WKdXcOE8NFiJX9TlqakT2R2gyMOuz+Y2PhkNLsGMzFhLQ2EYQId0/25qiFyto5H0 HrF57zUapifnFTFaVEXC0t5YBrF7fSYBGdjHeKDmN6niihdRaR8EuFhqULf7huYl bvg6Jp81+VKe8etaPSDBcZ3moBM9+5TMF4JDHZwjOriG/QMsz9/to/WqAOeL55+a pES6cqeRIq+E4kyFcV7UOOBo5ra6X2niQ3qJyaiyzKCU5/ty5VhD4Att/zyElazW vfiDnw== 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=fm1; bh=FnAF/xAVfch0e845cDQBJKGHAo42uioP4SSLjHgfx HU=; b=N6M5m3SoOkQJtDS99r5+/OcnNynjXL7x2O35k8B9bTu+H8gwyE3fCRTm+ +iNMTmGrZf2zty3rxYrhP5XIedLgi7ClY9lS1jFjeBsDwd3bXauWpK+ifui/hMUT Y7OoFY2dpXx67m7UCRDR5cVx11EMRi31zizmkniNAhQvzYAroj78pxLVbrJ7eQCC PlgqZ7kT2mzzCmYskDCEbLQ1ZFf61yPErRc9a8FFqd+Snv3Q3/TdUVdZr8Kzy4ct Hc3p7ADUW62QjuKaewMuH8pQJB3QCWmLKwSxIdfZsBLvxXoozY9C6gERh5nccsCh fR+oJuaUa6ufxWZd3EMMycINXRQhQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvuddrhedtgdekudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthhqredttddtjeenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucggtf frrghtthgvrhhnpeeuuddvjeegkeduieeuuedvvefhvdetgfeuvdevkedvheevvdehfeeh feffkeejgfenucffohhmrghinhepuddurdhithenucevlhhushhtvghrufhiiigvpedtne curfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 25 Nov 2021 10:53:18 -0500 (EST) From: Thomas Monjalon To: Slava Ovsiienko , Andrew Rybchenko , Ajit Khaparde , Somnath Kotur , Rahul Lakkireddy , Ferruh Yigit Cc: "dev@dpdk.org" Subject: Re: [PATCH v3] ethdev: deprecate header fields and metadata flow actions Date: Thu, 25 Nov 2021 16:53:17 +0100 Message-ID: <2687012.0FNQnJhuYk@thomas> In-Reply-To: References: <20211123075940.5521-1-viacheslavo@nvidia.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 25/11/2021 16:14, Ferruh Yigit: > On 11/25/2021 1:56 PM, Slava Ovsiienko wrote: > > From: Ferruh Yigit > >> On 11/24/2021 3:37 PM, Viacheslav Ovsiienko wrote: > >>> The generic RTE_FLOW_ACTION_TYPE_MODIFY_FIELD action was > >> introduced by > >>> [1]. This action provides an unified way to perform various arithmetic > >>> and transfer operations over packet network header fields and packet > >>> metadata. > >>> > >>> [1] commit 641dbe4fb053 ("net/mlx5: support modify field flow action") > >>> > >>> On other side there are a bunch of multiple legacy actions, that can > >>> be superseded by the generic modify field action: > > [.. snip ..] > >>> > >>> The VLAN set actions are interrelated to VLAN header insertion/removal > >>> and supported by multiple PMDs and supposed to be just deprecated but > >>> not be removed in 22.11. > >>> > >> > >> Why not remove them for v22.11? Do you think PMDs can't change the > >> existing implementation until 22.11? > >=20 > > Yes, this is a main concern - these actions are supported by multiple = PMDs, > > and its handling might be a little bit complicated. For example, in mlx5 > > SET_VLAN_ID can be translated into different HW/FW primitives > > depending on presence of PUSH_VLAN action. I think any PMD should do > > the check of VLAN actions order for the case of PUSH, replacing with > > MODIFY_FIELD would sophisticate the check > >=20 > > In mlx5 we are going to support MODIFY_FIELD and kept SET_VLAN_XX > > in any combinations, but, actually, there is no objection about dropping > > SET_VLAN_XXX regarding mlx5. I just would not like to force > > the commitments for that from other PMDs. > >=20 > >>> Signed-off-by: Viacheslav Ovsiienko > >>> > >>> -- > >>> v2 - deprecation.rst is updated > >>> v3 - doc comments addressed > >>> - commit message comments addressed > >>> - SET_VLAN_VID and SET_VLAN_PCP actions deprecated, but will not > >>> be removed in 22.11 > >> > >> Deprecated symbols are to prevent new code using them, but for this ca= se > >> there is no alternative, since PMDs still don't support > >> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' yet. > >> This patch is forcing users to use deprecated actions (except from mlx= ). > >> > >> What about a slight change: > >> 1- In this release, update header/document as > >> 'RTE_FLOW_ACTION_TYPE_MODIFY_FIELD' > >> is preferred way if supported. Instead of deprecating old ones. > >> > >> 2- Have an agreement with PMD maintainers to switch to new action befo= re > >> v22.11, > >> and don't accept old action implementation in PMDs anymore. > >> Based on agreement update 'deprecation.rst' in this release to no= te that > >> old actions will be removed on v22.11. > >> (It would be good to have a check to prevent old actions merged d= uring that > >> time.) > >> > >> 3- In v22.11, remove old actions, the PMDs that don't support MODIFY_F= IELD > >> action will lose the feature. > >> > >> What do you think? > >=20 > > In my opinion, deprecation warns about coming changes - entity is going= to be > > removed or changed. It does not force to take some immediate action now, > > just to be aware about. > >=20 > > There are two questions: > > - do we want to replace a bunch of legacy actions with new one? I suppo= se - yes, there > > is a lot of advantages > > - are we going to do this with MODIFY_FIELD? Yes, this is an intention. > >=20 > > So, we have answers for these questions now, and should handle legacy a= ctions accordingly - > > mark as deprecated and provide the clue about MODIFY_FIELD. > >=20 > > The question remaining - how we are going to proceed. Actions can't be = removed till > > alternative is provided. So, in my understanding the process should be: > >=20 > > A. mark actions as deprecated, to advertise an intention > > B. implement MODIFY_FIELD support (maintainers agreement/commitment), a= ssign the due date for this > > C. advertise the date of legacy actions removal (if it was not claimed = in B) > > D. remove legacy actions entirely > >=20 > > We are doing step A now. > > Also step B is imposed by advertising the removal date. It is ultimate = approach, and is not exactly what we need. > > Do we have alternative softened way to encourage PMDs to be updated? No= objections to follow =F0=9F=98=8A. > >=20 >=20 > We are aligned on the intention. Yes we are mostly aligned, but we do not put the same meaning to word "deprecate". =46irst of all, "deprecate" does not mean "removed" or "forbidden". The intention is to warn that it is not the latest or future-proof API. > For (A), those symbols interface with both drivers and applications, > for drivers no concern on deprecating the symbols but I think it is wrong > to deprecate from applications perspective. >=20 > Slightly updated version: > A. document legacy actions as not preferred way, but applications still c= an use them, Of course applications can use them, because they are not removed. If we don't agree on the word "deprecated", we can reword to something like= that: "This is a legacy API, please consider using MODIFY_FIELD action". > and no change is required by applications (unless they prefer to use = new action). > For PMDs we don't allow any more implementation of these actions. Yes we should not allow new implementation of the legacy API. > B. PMDs implement 'MODIFY_FIELD', have a deadline for it (same with your = B.) > Assuming deadline is v22.11, announce that old actions will be deprec= ated > on v22.11 in this release. > C. on v22.11 deprecate old actions, exiting applications still can use th= em > but new code should use 'MODIFY_FIELD' action. > Announce removal date for the legacy actions I think we should remove most of the actions in 22.11, except VLAN ones. > D. Remove legacy actions entirely Maybe we will keep VLAN actions forever.