Re: [dpdk-dev] [PATCH v1 0/7] Flow API helpers enhancements

[Adrien Mazarguil <adrien.mazarguil@6wind.com>]

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