[dpdk-dev,v1,01/16] ethdev: update ABI for flow API functions

Message ID 20180404150312.12304-2-adrien.mazarguil@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Adrien Mazarguil April 4, 2018, 3:56 p.m. UTC
  Subsequent patches will modify existing types and slightly alter the
behavior of the flow API. This warrants a major ABI breakage.

While it is already taken care of for 18.05 (LIBABIVER was updated to
version 9 by a prior commit), this patch explicitly adds the affected flow
API functions as a safety measure.

Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
---
 lib/librte_ether/rte_ethdev_version.map | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Thomas Monjalon April 5, 2018, 10:06 a.m. UTC | #1
04/04/2018 17:56, Adrien Mazarguil:
> Subsequent patches will modify existing types and slightly alter the
> behavior of the flow API. This warrants a major ABI breakage.
> 
> While it is already taken care of for 18.05 (LIBABIVER was updated to
> version 9 by a prior commit), this patch explicitly adds the affected flow
> API functions as a safety measure.

I don't understand this patch.

If the API is broken, you must move the function from old block to
the new one. And it must be done in the patch modifying the function.


> --- a/lib/librte_ether/rte_ethdev_version.map
> +++ b/lib/librte_ether/rte_ethdev_version.map
> +DPDK_18.05 {
> +	global:
> +
> +	rte_flow_validate;
> +	rte_flow_create;
> +	rte_flow_query;
> +	rte_flow_copy;
> +
> +} DPDK_18.02;
  
Adrien Mazarguil April 5, 2018, 12:44 p.m. UTC | #2
On Thu, Apr 05, 2018 at 12:06:10PM +0200, Thomas Monjalon wrote:
> 04/04/2018 17:56, Adrien Mazarguil:
> > Subsequent patches will modify existing types and slightly alter the
> > behavior of the flow API. This warrants a major ABI breakage.
> > 
> > While it is already taken care of for 18.05 (LIBABIVER was updated to
> > version 9 by a prior commit), this patch explicitly adds the affected flow
> > API functions as a safety measure.
> 
> I don't understand this patch.
> 
> If the API is broken, you must move the function from old block to
> the new one.

Missed that part, I'll update it.

> And it must be done in the patch modifying the function.

About that, almost each patch in this series breaks the ABI in its own
way. This left me with two options: either updating these functions once and
for all and explaining why in a dedicated patch, or updating them in the
first patch with an ABI impact, with subsequent patches piggybacking on that
change.

Unless there's a way to update the map file for each patch that breaks ABI,
I think the former is more consistent, but I don't mind if you prefer the
latter. What do you suggest?
  
Thomas Monjalon April 5, 2018, 1:36 p.m. UTC | #3
05/04/2018 14:44, Adrien Mazarguil:
> On Thu, Apr 05, 2018 at 12:06:10PM +0200, Thomas Monjalon wrote:
> > 04/04/2018 17:56, Adrien Mazarguil:
> > > Subsequent patches will modify existing types and slightly alter the
> > > behavior of the flow API. This warrants a major ABI breakage.
> > > 
> > > While it is already taken care of for 18.05 (LIBABIVER was updated to
> > > version 9 by a prior commit), this patch explicitly adds the affected flow
> > > API functions as a safety measure.
> > 
> > I don't understand this patch.
> > 
> > If the API is broken, you must move the function from old block to
> > the new one.
> 
> Missed that part, I'll update it.
> 
> > And it must be done in the patch modifying the function.
> 
> About that, almost each patch in this series breaks the ABI in its own
> way. This left me with two options: either updating these functions once and
> for all and explaining why in a dedicated patch, or updating them in the
> first patch with an ABI impact, with subsequent patches piggybacking on that
> change.
> 
> Unless there's a way to update the map file for each patch that breaks ABI,
> I think the former is more consistent, but I don't mind if you prefer the
> latter. What do you suggest?

The ABI information must be updated when breaking (2nd solution).
  

Patch

diff --git a/lib/librte_ether/rte_ethdev_version.map b/lib/librte_ether/rte_ethdev_version.map
index 34df6c8b5..78a6f5afb 100644
--- a/lib/librte_ether/rte_ethdev_version.map
+++ b/lib/librte_ether/rte_ethdev_version.map
@@ -203,6 +203,16 @@  DPDK_18.02 {
 
 } DPDK_17.11;
 
+DPDK_18.05 {
+	global:
+
+	rte_flow_validate;
+	rte_flow_create;
+	rte_flow_query;
+	rte_flow_copy;
+
+} DPDK_18.02;
+
 EXPERIMENTAL {
 	global: