From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrien Mazarguil Subject: Re: [PATCH v1 0/7] Flow API helpers enhancements Date: Fri, 6 Oct 2017 10:05:50 +0200 Message-ID: <20171006080550.GB3871@6wind.com> References: <919ab2dc-1081-d139-50a3-c797fbeb7284@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Gaetan Rivet , dev@dpdk.org To: Ferruh Yigit Return-path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id B93B91B279 for ; Fri, 6 Oct 2017 10:06:02 +0200 (CEST) Received: by mail-wm0-f44.google.com with SMTP id k4so6294220wmc.1 for ; Fri, 06 Oct 2017 01:06:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: <919ab2dc-1081-d139-50a3-c797fbeb7284@intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Oct 06, 2017 at 02:13:14AM +0100, Ferruh Yigit wrote: > On 10/5/2017 10:49 AM, Adrien Mazarguil wrote: > > This series brings enhancements to various rte_flow helpers: > > > > - Allow applications to use rte_flow_error_set() by making it part of the > > public interface and documenting it as such. > > > > - Address rte_flow_copy()'s limitations by replacing it with the more > > versatile rte_flow_conv(). This new function allows retrieving other > > properties such as item/action names, enabling testpmd to finally use it > > and get rid of duplicated code. > > > > - Add a script (gen-rte_flow_conv-h.sh) to help with generating the > > resources used by rte_flow_conv(). Developers should run it when adding or > > modifying pattern items or actions (done as part of this series to add the > > missing "fuzzy" pattern item). > > > > - Future plans for rte_flow_conv() include translating error codes to > > human-readable messages, so applications do not have to make their own. > > > > All these changes address concerns raised a couple of months ago [1]. Work > > on these patches actually started at the time but I was unable to complete > > and clean them up until recently. > > > > [1] http://dpdk.org/ml/archives/dev/2017-July/070492.html > > > > Adrien Mazarguil (7): > > ethdev: expose flow API error helper > > ethdev: replace flow API object copy function > > ethdev: add flow API item/action name conversion > > app/testpmd: rely on flow API conversion function > > ethdev: enhance flow API item/action descriptions > > ethdev: generate flow API conversion header > > ethdev: update flow API conversion header > > Hi Adrien, > > This received too late for this release cycle, and changes in rte_flow > library may effect PMDs. > > I suggest deferring the set to next release, what do you think? Hi Ferruh, My opinion as the author (since you're asking :) is that it would be nice to have it in this release assuming reviewers don't find blocker issues with it. To summarize the changes from a PMD standpoint: - rte_flow_error() (previously not public) switching from positive to negative return value like the other rte_flow_*() functions. The only PMDs relying on its return value so far are mlx4 and tap. - rte_flow_copy() disappearing. This function was temporary pending a better solution, and so far is only used by fail-safe PMD (modified as part of this series). Besides fail-safe, PMDs did not a have a use case for this function. These patches were originally targeted at 17.08, and since the "fuzzy" item is missing from rte_flow_copy() (GTP/GTPU/GTPC are now also missing by the way) and there is currently a lot of redundancy between this function and testpmd's internals, I thought it would be a good time to have everything in a single place. I was also considering using rte_flow_conv() in upcoming mlx4 patches in case it was included. So here's my suggestion: I can track all rte_flow-related changes in PMDs and in rte_flow itself and update this series accordingly until things have settled (e.g. I'll re-submit to rebase and include GTP). Once applied, I will check all new code that relies on these two functions and update it if necessary until the release. How about that? -- Adrien Mazarguil 6WIND