[dpdk-dev,v4,01/16] ethdev: add error types to flow API

  These enable more precise reporting of objects responsible for errors.

This breaks ABI compatibility for the following public functions:

- rte_flow_create()
- rte_flow_destroy()
- rte_flow_error_set()
- rte_flow_flush()
- rte_flow_isolate()
- rte_flow_query()
- rte_flow_validate()

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 app/test-pmd/config.c                   |  4 ++++
 lib/librte_ether/rte_ethdev_version.map | 20 +++++++++++++-------
 lib/librte_ether/rte_flow.h             |  4 ++++
 3 files changed, 21 insertions(+), 7 deletions(-)
  

On 4/16/2018 5:22 PM, Adrien Mazarguil wrote:
> These enable more precise reporting of objects responsible for errors.
> 
> This breaks ABI compatibility for the following public functions:
> 
> - rte_flow_create()
> - rte_flow_destroy()
> - rte_flow_error_set()
> - rte_flow_flush()
> - rte_flow_isolate()
> - rte_flow_query()
> - rte_flow_validate()

Is there a deprecation notice for this API break?

> 
> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>
  

On Tue, Apr 17, 2018 at 08:37:31PM +0100, Ferruh Yigit wrote:
> On 4/16/2018 5:22 PM, Adrien Mazarguil wrote:
> > These enable more precise reporting of objects responsible for errors.
> > 
> > This breaks ABI compatibility for the following public functions:
> > 
> > - rte_flow_create()
> > - rte_flow_destroy()
> > - rte_flow_error_set()
> > - rte_flow_flush()
> > - rte_flow_isolate()
> > - rte_flow_query()
> > - rte_flow_validate()
> 
> Is there a deprecation notice for this API break?

A notice covering the main changes in this series (most patches have an ABI
impact) was sent but not included [1]. This particular patch rides on the
announced ABI breakage in order to add a relatively minor feature while
there.

This ABI change was implicitly needed by upcoming work for 18.05 (Xueming's
RSS stuff [2][3], Declan's TEP [4], the rest is summarized by a RFC [5]) due
to the necessary changes of behavior in flow rules.

Note that Xueming's deprecation notice [3] alone would have triggered such
an ABI change because struct rte_flow_action_rss wouldn't have been binary 
compatible if struct rte_eth_rss_conf was updated. This change would have
propagated back to rte_flow functions manipulating them.

[1] "doc: announce API changes for flow rules"
     http://dpdk.org/ml/archives/dev/2018-February/090988.html
[2] "MLX5 tunnel Rx offloading"
    http://dpdk.org/ml/archives/dev/2018-February/091461.html
[3] "doc: annouce ABI change for RSS configuraiton structure"
    http://dpdk.org/ml/archives/dev/2018-February/090127.html
[4] "tunnel endpoint hw acceleration enablement"
    http://dpdk.org/ml/archives/dev/2017-December/084676.html
[5] "Switch device offload with DPDK"
    http://dpdk.org/ml/archives/dev/2018-March/092513.html

> > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> <...>
>
  

On 4/18/2018 9:41 AM, Adrien Mazarguil wrote:
> On Tue, Apr 17, 2018 at 08:37:31PM +0100, Ferruh Yigit wrote:
>> On 4/16/2018 5:22 PM, Adrien Mazarguil wrote:
>>> These enable more precise reporting of objects responsible for errors.
>>>
>>> This breaks ABI compatibility for the following public functions:
>>>
>>> - rte_flow_create()
>>> - rte_flow_destroy()
>>> - rte_flow_error_set()
>>> - rte_flow_flush()
>>> - rte_flow_isolate()
>>> - rte_flow_query()
>>> - rte_flow_validate()
>>
>> Is there a deprecation notice for this API break?
> 
> A notice covering the main changes in this series (most patches have an ABI
> impact) was sent but not included [1]. This particular patch rides on the
> announced ABI breakage in order to add a relatively minor feature while
> there.

My take from "announced ABI breakage" is the deprecation notice get three acks
and merged into release, so it seems there is no deprecation notice and
according process first deprecation notice should go in this release.

Hi Thomas,

Any comment on issue?

> 
> This ABI change was implicitly needed by upcoming work for 18.05 (Xueming's
> RSS stuff [2][3], Declan's TEP [4], the rest is summarized by a RFC [5]) due
> to the necessary changes of behavior in flow rules.
> 
> Note that Xueming's deprecation notice [3] alone would have triggered such
> an ABI change because struct rte_flow_action_rss wouldn't have been binary 
> compatible if struct rte_eth_rss_conf was updated. This change would have
> propagated back to rte_flow functions manipulating them.

To be honest I lost track of Xueming's patches, because of split/merge of
patchset, multiple set with multiple versions out.

Is it possible to document the dependency graph including your set?

> 
> [1] "doc: announce API changes for flow rules"
>      http://dpdk.org/ml/archives/dev/2018-February/090988.html
> [2] "MLX5 tunnel Rx offloading"
>     http://dpdk.org/ml/archives/dev/2018-February/091461.html
> [3] "doc: annouce ABI change for RSS configuraiton structure"
>     http://dpdk.org/ml/archives/dev/2018-February/090127.html
> [4] "tunnel endpoint hw acceleration enablement"
>     http://dpdk.org/ml/archives/dev/2017-December/084676.html
> [5] "Switch device offload with DPDK"
>     http://dpdk.org/ml/archives/dev/2018-March/092513.html
> 
>>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>
>> <...>
>>
>
  

On Wed, Apr 18, 2018 at 10:24:10AM +0100, Ferruh Yigit wrote:
> On 4/18/2018 9:41 AM, Adrien Mazarguil wrote:
> > On Tue, Apr 17, 2018 at 08:37:31PM +0100, Ferruh Yigit wrote:
> >> On 4/16/2018 5:22 PM, Adrien Mazarguil wrote:
> >>> These enable more precise reporting of objects responsible for errors.
> >>>
> >>> This breaks ABI compatibility for the following public functions:
> >>>
> >>> - rte_flow_create()
> >>> - rte_flow_destroy()
> >>> - rte_flow_error_set()
> >>> - rte_flow_flush()
> >>> - rte_flow_isolate()
> >>> - rte_flow_query()
> >>> - rte_flow_validate()
> >>
> >> Is there a deprecation notice for this API break?
> > 
> > A notice covering the main changes in this series (most patches have an ABI
> > impact) was sent but not included [1]. This particular patch rides on the
> > announced ABI breakage in order to add a relatively minor feature while
> > there.
> 
> My take from "announced ABI breakage" is the deprecation notice get three acks
> and merged into release, so it seems there is no deprecation notice and
> according process first deprecation notice should go in this release.

True, so just describe how ABI impact is no worse than what was announced
(and included) [3] and most of them are actually opportune changes to
improve consistency and documentation since ABI would be broken for this
release regardless. Regarding individual patches:

- 01/16 "ethdev: add error types to flow API"
  => New error types are added in the middle of an existing enum.

- 02/16 "ethdev: clarify flow API pattern items and actions"
  => No impact.

- 03/16 "doc: remove flow API migration section"
  => No impact.

- 04/16 "ethdev: remove DUP action from flow API"
  => An action that no PMD supports is removed from an existing enum.

- 05/16 "ethdev: alter behavior of flow API actions"
  => A documentation change in how actions are processed logically
     breaks ABI in the case of repeated actions (currently a corner
     case). Most PMDs do not implement the original behavior correctly
     anyway (see commit log).

- 06/16 "ethdev: remove C99 flexible arrays from flow API"
  => ABI impact is primarily triggered by the RSS action change (already
     covered [3]). The RAW pattern item is also modified for consistency.
     On the API side, except when allocating these structures, there is no
     difference in usage (i.e. array[index] => pointer[index]).

- 07/16 "ethdev: flatten RSS configuration in flow API"
  => Already covered [3].

- 08/16 "ethdev: add hash function to RSS flow API action"
  => Already covered [3].

- 09/16 "ethdev: add encap level to RSS flow API action"
  => Already covered [3].

- 10/16 "ethdev: refine TPID handling in flow API"
  => No PMD supports the poorly defined TPID matching, applications couldn't
     possibly rely on it.

- 11/16 "ethdev: limit default VLAN TCI mask in flow API"
  => No ABI breakage, but a different behavior for applications that rely on
     the default mask. It doesn't look like any PMD supports PCP/DEI
     matching so again applications could not rely on it (they still can do
     it by providing a specific mask).

- 12/16 "ethdev: add transfer attribute to flow API"
  => Minor ABI impact (read: logical) due to the addition of a bit in an
     existing bit-field. No practical impact on applications.

- 13/16 "ethdev: update behavior of VF/PF in flow API"
  => Documentation (API) change. The "transfer" bit must now be set in order to
     use these actions with PMDs that support them.

- 14/16 "ethdev: rename physical port item in flow API"
  => API change for a pattern item supported by no PMD.

- 15/16 "ethdev: add physical port action to flow API"
  => New action added in the middle of an existing enum.

- 16/16 "ethdev: add port ID item and action to flow API"
  => New item/action added in the middle of existing enums.

> Hi Thomas,
> 
> Any comment on issue?
> 
> > 
> > This ABI change was implicitly needed by upcoming work for 18.05 (Xueming's
> > RSS stuff [2][3], Declan's TEP [4], the rest is summarized by a RFC [5]) due
> > to the necessary changes of behavior in flow rules.
> > 
> > Note that Xueming's deprecation notice [3] alone would have triggered such
> > an ABI change because struct rte_flow_action_rss wouldn't have been binary 
> > compatible if struct rte_eth_rss_conf was updated. This change would have
> > propagated back to rte_flow functions manipulating them.
> 
> To be honest I lost track of Xueming's patches, because of split/merge of
> patchset, multiple set with multiple versions out.
> 
> Is it possible to document the dependency graph including your set?

I hopefully didn't miss any:

 Bunch of flow API-related fixes (v5) [6]
 |
 `-- Flow API overhaul for switch offloads (v4) [7]
     |
     +-- additions to support tunnel encap/decap (v4) [8]
     |
     +-- introduce new tunnel types (v5) [9]
     |   |
     |   `-- mlx5 Rx tunnel offloading (v4) [10]
     |
     +-- rte_flow extension for vSwitch acceleration (v3.2) [11]
     |
     `-- net/sfc: RSS improvements [12]

[6] http://dpdk.org/ml/archives/dev/2018-April/097411.html
[7] http://dpdk.org/ml/archives/dev/2018-April/097423.html
[8] http://dpdk.org/ml/archives/dev/2018-April/097956.html
[9] http://dpdk.org/ml/archives/dev/2018-April/097669.html
[10] http://dpdk.org/ml/archives/dev/2018-April/097673.html
[11] http://dpdk.org/ml/archives/dev/2018-April/097266.html
[12] http://dpdk.org/ml/archives/dev/2018-April/095872.html

I gave the current version for these series but some of them are still under
review and may change. I plan to send updates for [6] and [7] shortly.

> > [1] "doc: announce API changes for flow rules"
> >      http://dpdk.org/ml/archives/dev/2018-February/090988.html
> > [2] "MLX5 tunnel Rx offloading"
> >     http://dpdk.org/ml/archives/dev/2018-February/091461.html
> > [3] "doc: annouce ABI change for RSS configuraiton structure"
> >     http://dpdk.org/ml/archives/dev/2018-February/090127.html
> > [4] "tunnel endpoint hw acceleration enablement"
> >     http://dpdk.org/ml/archives/dev/2017-December/084676.html
> > [5] "Switch device offload with DPDK"
> >     http://dpdk.org/ml/archives/dev/2018-March/092513.html
> > 
> >>> Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> >>> Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
> >>
> >> <...>
> >>
> > 
>