Message ID | 20180803132032.29038-1-adrien.mazarguil@6wind.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id CF6BE1B569; Fri, 3 Aug 2018 15:36:50 +0200 (CEST) Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 58D371B567 for <dev@dpdk.org>; Fri, 3 Aug 2018 15:36:49 +0200 (CEST) Received: by mail-wm0-f68.google.com with SMTP id c14-v6so6317192wmb.4 for <dev@dpdk.org>; Fri, 03 Aug 2018 06:36:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=45o1GspZTF6xJ0haXP6NTDoNsNN40j5hY4B1etmcHlY=; b=fa8EjQH/kWYVJqhpVllRZZ6Bg9KjDSpphkyv+EBRdJzCEeXQvMC0PiefWeN0nxScaH 3LQetvDfLLEhyCT21N69i3zH+iS+KeNHwtoWlZw3nZ0NU1CpZi+IWm1jYbaxy2+84hKZ lvKx0wArsmeE0dbWxRbddWI46J2ZGPn1iQVnEK/r1F5QBDbj++Ic8Dtx+go+/TK3X721 ShQQ5rb3JVtNwcyLgDVYYp5eckr/W1dPw29+ukMkhWgS/zdMZAFea7aqMvvqYEc55kmC NJLCj6WrYMxVbQiWf6M6O8kAgWwR5zaWyt2CHRgDqt4c9ZACJkbGI1A/SlvY10twJzSR HeIw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=45o1GspZTF6xJ0haXP6NTDoNsNN40j5hY4B1etmcHlY=; b=MUHl5Cr1nE71guU2KJ9vRDNDv/i4PqHQO+7/0Sbt/xSxbFziJcpgVXbVaBx+vWAMFj HmHdEu9puz4JrI1E3V3yt39T0N48TxRXYeSfGoVshfEXu9Aeh+SMkqGcp3COqfZ+psnC lW39D/XooeO17Ax+mNZx/QhSTIW1wVTUeGbitBH8X4KA/O7YffQppXONxSJNsLi3DJ1W IdqomudvFu61RtSZB7YCEsZnwWhLi+HyW+lRf0F2X/atkTxYgwWjJzBMK4ojHXsJqf89 8ytPJxC7z7FMJ0+j+ZsNuzPW1IWAu4X/jv5S+m7rtxPSkGyGznM0S6Pp/WcApctGevdj a+mw== X-Gm-Message-State: AOUpUlGr87P9agln3A52Gy8t9+hMFLSvtyD37FL8e52y2QdyZGeQttUB dlbPYo+8wrsSJERGk00kC3wFZQ== X-Google-Smtp-Source: AAOMgpcivMc/MgyGuOBEA0v5zslza6ddO5yabkXepUxqLhko25TdYGhatoQEyA6zRHasq+c5cwByDA== X-Received: by 2002:a1c:69cb:: with SMTP id z72-v6mr5043376wmh.10.1533303408094; Fri, 03 Aug 2018 06:36:48 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p12-v6sm3334312wrw.3.2018.08.03.06.36.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 03 Aug 2018 06:36:47 -0700 (PDT) Date: Fri, 3 Aug 2018 15:36:30 +0200 From: Adrien Mazarguil <adrien.mazarguil@6wind.com> To: Ferruh Yigit <ferruh.yigit@intel.com> Cc: dev@dpdk.org Message-ID: <20180803132032.29038-1-adrien.mazarguil@6wind.com> References: <cover.1507193185.git.adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <cover.1507193185.git.adrien.mazarguil@6wind.com> X-Mailer: git-send-email 2.11.0 Subject: [dpdk-dev] [PATCH v2 0/7] ethdev: add flow API object converter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
ethdev: add flow API object converter
|
|
Message
Adrien Mazarguil
Aug. 3, 2018, 1:36 p.m. UTC
This is a follow up to the "Flow API helpers enhancements" series submitted almost a year ago [1]. The new title is due to the reduced scope of this version. rte_flow_conv() is a flexible replacement to rte_flow_copy(), itself a temporary solution pending something better [2]. It replaces a lot of duplicated code found in testpmd and removes some of the maintenance burden that developers tend to forget (me included) when modifying pattern item or actions (updating app/test-pmd/config.c to be clear). This series was unearthed in order to complete the implementation of RTE_FLOW_ACTION_TYPE_ENCAP_(VXLAN|NVGRE) in testpmd [3] without having to duplicate existing code once again. See individual patches for specific changes in this version. v2 changes: - rte_flow_copy() is kept, albeit deprecated, no API/ABI impact. - Updated bonding PMD. - No more automatic generation of rte_flow_conv.h. [1] https://mails.dpdk.org/archives/dev/2017-October/077551.html [2] https://mails.dpdk.org/archives/dev/2017-July/070492.html [3] Currently the command-line parser (cmdline_flow.c) is aware of these actions, however config.c isn't. Flow rules with such actions cannot be created and cannot be validated with PMDs that implement them. Adrien Mazarguil (7): ethdev: add flow API object converter ethdev: add flow API item/action name conversion app/testpmd: rely on flow API conversion function net/failsafe: switch to flow API object conversion function net/bonding: switch to flow API object conversion function ethdev: deprecate rte_flow_copy function ethdev: add missing item/actions to flow object converter app/test-pmd/config.c | 407 +++------------ app/test-pmd/testpmd.h | 7 +- doc/guides/prog_guide/rte_flow.rst | 20 + drivers/net/bonding/rte_eth_bond_api.c | 6 +- drivers/net/bonding/rte_eth_bond_flow.c | 31 +- drivers/net/bonding/rte_eth_bond_private.h | 5 +- drivers/net/failsafe/failsafe_ether.c | 6 +- drivers/net/failsafe/failsafe_flow.c | 31 +- drivers/net/failsafe/failsafe_private.h | 5 +- lib/librte_ethdev/rte_ethdev_version.map | 1 + lib/librte_ethdev/rte_flow.c | 666 ++++++++++++++++++------ lib/librte_ethdev/rte_flow.h | 230 +++++++- 12 files changed, 883 insertions(+), 532 deletions(-)
Comments
03/08/2018 15:36, Adrien Mazarguil: > Adrien Mazarguil (7): > ethdev: add flow API object converter > ethdev: add flow API item/action name conversion > app/testpmd: rely on flow API conversion function > net/failsafe: switch to flow API object conversion function > net/bonding: switch to flow API object conversion function > ethdev: deprecate rte_flow_copy function > ethdev: add missing item/actions to flow object converter This series will be considered for 18.11. As you plan to deprecate rte_flow_copy function, please send a deprecation notice which could enter in 18.08 release notes. Thanks
On 8/3/2018 2:36 PM, Adrien Mazarguil wrote: > This is a follow up to the "Flow API helpers enhancements" series submitted > almost a year ago [1]. The new title is due to the reduced scope of this > version. > > rte_flow_conv() is a flexible replacement to rte_flow_copy(), itself a > temporary solution pending something better [2]. It replaces a lot of > duplicated code found in testpmd and removes some of the maintenance burden > that developers tend to forget (me included) when modifying pattern > item or actions (updating app/test-pmd/config.c to be clear). > > This series was unearthed in order to complete the implementation of > RTE_FLOW_ACTION_TYPE_ENCAP_(VXLAN|NVGRE) in testpmd [3] without having to > duplicate existing code once again. > > See individual patches for specific changes in this version. > > v2 changes: > > - rte_flow_copy() is kept, albeit deprecated, no API/ABI impact. > - Updated bonding PMD. > - No more automatic generation of rte_flow_conv.h. > > [1] https://mails.dpdk.org/archives/dev/2017-October/077551.html > [2] https://mails.dpdk.org/archives/dev/2017-July/070492.html > [3] Currently the command-line parser (cmdline_flow.c) is aware of these > actions, however config.c isn't. Flow rules with such actions cannot > be created and cannot be validated with PMDs that implement them. > > Adrien Mazarguil (7): > ethdev: add flow API object converter > ethdev: add flow API item/action name conversion > app/testpmd: rely on flow API conversion function > net/failsafe: switch to flow API object conversion function > net/bonding: switch to flow API object conversion function > ethdev: deprecate rte_flow_copy function > ethdev: add missing item/actions to flow object converter Patch needs to be rebased to target v18.11 (in map file), and indeed new APIs (rte_flow_conv) needs to be experimental. And needs to remove deprecation notice in this patchset. Also do you think does make sense to announce this change in release notes? Apart from above, any volunteer for reviewing actual implementation?
On 8/3/2018 2:36 PM, Adrien Mazarguil wrote: > This is a follow up to the "Flow API helpers enhancements" series submitted > almost a year ago [1]. The new title is due to the reduced scope of this > version. > > rte_flow_conv() is a flexible replacement to rte_flow_copy(), itself a > temporary solution pending something better [2]. It replaces a lot of > duplicated code found in testpmd and removes some of the maintenance burden > that developers tend to forget (me included) when modifying pattern > item or actions (updating app/test-pmd/config.c to be clear). > > This series was unearthed in order to complete the implementation of > RTE_FLOW_ACTION_TYPE_ENCAP_(VXLAN|NVGRE) in testpmd [3] without having to > duplicate existing code once again. > > See individual patches for specific changes in this version. > > v2 changes: > > - rte_flow_copy() is kept, albeit deprecated, no API/ABI impact. > - Updated bonding PMD. > - No more automatic generation of rte_flow_conv.h. > > [1] https://mails.dpdk.org/archives/dev/2017-October/077551.html > [2] https://mails.dpdk.org/archives/dev/2017-July/070492.html > [3] Currently the command-line parser (cmdline_flow.c) is aware of these > actions, however config.c isn't. Flow rules with such actions cannot > be created and cannot be validated with PMDs that implement them. > > Adrien Mazarguil (7): > ethdev: add flow API object converter > ethdev: add flow API item/action name conversion > app/testpmd: rely on flow API conversion function > net/failsafe: switch to flow API object conversion function > net/bonding: switch to flow API object conversion function > ethdev: deprecate rte_flow_copy function > ethdev: add missing item/actions to flow object converter Causing build error for arm, it looks like related to rte_memcpy macro: .../lib/librte_ethdev/rte_flow.c: In function ‘rte_flow_conv_item_spec’: .../lib/librte_ethdev/rte_flow.c:373:58: error: macro "rte_memcpy" passed 9 arguments, but takes just 3 (size > sizeof(*dst.raw) ? sizeof(*dst.raw) : size)); ^
On Fri, Aug 24, 2018 at 11:58:39AM +0100, Ferruh Yigit wrote: > On 8/3/2018 2:36 PM, Adrien Mazarguil wrote: > > This is a follow up to the "Flow API helpers enhancements" series submitted > > almost a year ago [1]. The new title is due to the reduced scope of this > > version. > > > > rte_flow_conv() is a flexible replacement to rte_flow_copy(), itself a > > temporary solution pending something better [2]. It replaces a lot of > > duplicated code found in testpmd and removes some of the maintenance burden > > that developers tend to forget (me included) when modifying pattern > > item or actions (updating app/test-pmd/config.c to be clear). > > > > This series was unearthed in order to complete the implementation of > > RTE_FLOW_ACTION_TYPE_ENCAP_(VXLAN|NVGRE) in testpmd [3] without having to > > duplicate existing code once again. > > > > See individual patches for specific changes in this version. > > > > v2 changes: > > > > - rte_flow_copy() is kept, albeit deprecated, no API/ABI impact. > > - Updated bonding PMD. > > - No more automatic generation of rte_flow_conv.h. > > > > [1] https://mails.dpdk.org/archives/dev/2017-October/077551.html > > [2] https://mails.dpdk.org/archives/dev/2017-July/070492.html > > [3] Currently the command-line parser (cmdline_flow.c) is aware of these > > actions, however config.c isn't. Flow rules with such actions cannot > > be created and cannot be validated with PMDs that implement them. > > > > Adrien Mazarguil (7): > > ethdev: add flow API object converter > > ethdev: add flow API item/action name conversion > > app/testpmd: rely on flow API conversion function > > net/failsafe: switch to flow API object conversion function > > net/bonding: switch to flow API object conversion function > > ethdev: deprecate rte_flow_copy function > > ethdev: add missing item/actions to flow object converter > > Causing build error for arm, it looks like related to rte_memcpy macro: > > .../lib/librte_ethdev/rte_flow.c: In function ‘rte_flow_conv_item_spec’: > .../lib/librte_ethdev/rte_flow.c:373:58: error: macro "rte_memcpy" passed 9 > arguments, but takes just 3 > (size > sizeof(*dst.raw) ? sizeof(*dst.raw) : size)); Thanks, noticed it after sending v2. I'll fix it for v3.
On Thu, Aug 23, 2018 at 02:48:37PM +0100, Ferruh Yigit wrote: > On 8/3/2018 2:36 PM, Adrien Mazarguil wrote: > > This is a follow up to the "Flow API helpers enhancements" series submitted > > almost a year ago [1]. The new title is due to the reduced scope of this > > version. > > > > rte_flow_conv() is a flexible replacement to rte_flow_copy(), itself a > > temporary solution pending something better [2]. It replaces a lot of > > duplicated code found in testpmd and removes some of the maintenance burden > > that developers tend to forget (me included) when modifying pattern > > item or actions (updating app/test-pmd/config.c to be clear). > > > > This series was unearthed in order to complete the implementation of > > RTE_FLOW_ACTION_TYPE_ENCAP_(VXLAN|NVGRE) in testpmd [3] without having to > > duplicate existing code once again. > > > > See individual patches for specific changes in this version. > > > > v2 changes: > > > > - rte_flow_copy() is kept, albeit deprecated, no API/ABI impact. > > - Updated bonding PMD. > > - No more automatic generation of rte_flow_conv.h. > > > > [1] https://mails.dpdk.org/archives/dev/2017-October/077551.html > > [2] https://mails.dpdk.org/archives/dev/2017-July/070492.html > > [3] Currently the command-line parser (cmdline_flow.c) is aware of these > > actions, however config.c isn't. Flow rules with such actions cannot > > be created and cannot be validated with PMDs that implement them. > > > > Adrien Mazarguil (7): > > ethdev: add flow API object converter > > ethdev: add flow API item/action name conversion > > app/testpmd: rely on flow API conversion function > > net/failsafe: switch to flow API object conversion function > > net/bonding: switch to flow API object conversion function > > ethdev: deprecate rte_flow_copy function > > ethdev: add missing item/actions to flow object converter > > Patch needs to be rebased to target v18.11 (in map file), Right, will do it for v3. > and indeed new APIs > (rte_flow_conv) needs to be experimental. This is what I did at first. Problem is that experimental APIs cannot be used in internal code without triggering a compilation error unless ALLOW_EXPERIMENTAL_API is defined (bonding cannot rely on an API marked as experimental). Since this series reimplements rte_flow_copy() as a wrapper to rte_flow_conv(), I thought it didn't make sense for internal code to keep using the former either. Considering this, shall I add -DDALLOW_EXPERIMENTAL_API to bonding PMD or keep things not experimental? > And needs to remove deprecation notice in this patchset. Doesn't it make sense to deprecate this function immediately after providing a replacement on top of which it is reimplemented? Users end up using the new function whether they want it or not. I don't think maintaining the old duplicated code around is the right thing to do either. > Also do you think does make sense to announce this change in release notes? I'm not sure it's worth a release note. It's a rather obscure helper function part of rte_flow. We didn't do it for rte_flow_copy() for instance. Please confirm if you think it's needed. > Apart from above, any volunteer for reviewing actual implementation? I hope Gaetan will take a look, he added rte_flow_copy() after all :)