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.mazarg...@6wind.com>
> >>> Acked-by: Andrew Rybchenko <arybche...@solarflare.com>
> >>
> >> <...>
> >>
> >
>
--
Adrien Mazarguil
6WIND
