[dpdk-dev,v2] doc: announce ABI change of struct rte_port_source_params and rte_port_sink_params

Message ID 1463667515-11766-1-git-send-email-roy.fan.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Fan Zhang May 19, 2016, 2:18 p.m. UTC
  The ABI changes are planned for rte_port_source_params and
rte_port_sink_params, which will be supported from release 16.11. Here
announces that ABI changes in detail.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 8 ++++++++
 1 file changed, 8 insertions(+)
  

Comments

Cristian Dumitrescu July 27, 2016, 10:08 a.m. UTC | #1
As Thomas mentioned, today is probably the last day to discuss ABI changes. This one is pretty small and straightforward, any issues with it?

Panu had a concern that the change from "char *" to "const char *" is too small to be regarded as ABI breakage and we should simply go ahead and do it. My conservative proposal was to put a notice anyway.

Nonetheless, what I would like to get from Thomas and Panu is a path forward for this now:
a) If we agree to consider this an ABI change, please merge the notice for 16.7;
b) If we agree this is too small for an ABI change, please let us agree now to accept our quick patch for 16.11 for this change.

Thanks,
Cristian


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Fan Zhang
> Sent: Thursday, May 19, 2016 3:19 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] doc: announce ABI change of struct
> rte_port_source_params and rte_port_sink_params
> 
> The ABI changes are planned for rte_port_source_params and
> rte_port_sink_params, which will be supported from release 16.11. Here
> announces that ABI changes in detail.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index fffe9c7..4f3fefe 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -74,3 +74,11 @@ Deprecation Notices
>    a handle, like the way kernel exposes an fd to user for locating a
>    specific file, and to keep all major structures internally, so that
>    we are likely to be free from ABI violations in future.
> +
> +* ABI will change for rte_port_source_params struct. The member
> file_name
> +  data type will be changed from char * to const char *. This change targets
> +  release 16.11
> +
> +* ABI will change for rte_port_sink_params struct. The member file_name
> +  data type will be changed from char * to const char *. This change targets
> +  release 16.11
> --
> 2.5.5
  
Thomas Monjalon July 27, 2016, 10:42 a.m. UTC | #2
2016-07-27 10:08, Dumitrescu, Cristian:
> As Thomas mentioned, today is probably the last day to discuss ABI changes. This one is pretty small and straightforward, any issues with it?
> 
> Panu had a concern that the change from "char *" to "const char *" is too small to be regarded as ABI breakage and we should simply go ahead and do it. My conservative proposal was to put a notice anyway.
> 
> Nonetheless, what I would like to get from Thomas and Panu is a path forward for this now:
> a) If we agree to consider this an ABI change, please merge the notice for 16.7;

Panu was noticing 3 things (and I agree with them):
- it is an API change
- they can be grouped in only one list item
- it is better to wait having more changes to break an API

About the third point, in this specific case, I think it is acceptable because:
- it should not break the ABI
- the impact of the API change is really small
- I'm not sure the packet framework should be considered as a DPDK API.

> b) If we agree this is too small for an ABI change, please let us agree now
> to accept our quick patch for 16.11 for this change.

For an API deprecation notice (reworded),
Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>


> > -----Original Message-----
> > The ABI changes are planned for rte_port_source_params and
> > rte_port_sink_params, which will be supported from release 16.11. Here
> > announces that ABI changes in detail.
> > 
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > ---
> > +* ABI will change for rte_port_source_params struct. The member
> > file_name
> > +  data type will be changed from char * to const char *. This change targets
> > +  release 16.11
> > +
> > +* ABI will change for rte_port_sink_params struct. The member file_name
> > +  data type will be changed from char * to const char *. This change targets
> > +  release 16.11
  
Thomas Monjalon July 28, 2016, 6:28 p.m. UTC | #3
2016-07-27 12:42, Thomas Monjalon:
> 2016-07-27 10:08, Dumitrescu, Cristian:
> > As Thomas mentioned, today is probably the last day to discuss ABI changes. This one is pretty small and straightforward, any issues with it?
> > 
> > Panu had a concern that the change from "char *" to "const char *" is too small to be regarded as ABI breakage and we should simply go ahead and do it. My conservative proposal was to put a notice anyway.
> > 
> > Nonetheless, what I would like to get from Thomas and Panu is a path forward for this now:
> > a) If we agree to consider this an ABI change, please merge the notice for 16.7;
> 
> Panu was noticing 3 things (and I agree with them):
> - it is an API change
> - they can be grouped in only one list item
> - it is better to wait having more changes to break an API
> 
> About the third point, in this specific case, I think it is acceptable because:
> - it should not break the ABI
> - the impact of the API change is really small
> - I'm not sure the packet framework should be considered as a DPDK API.
> 
> > b) If we agree this is too small for an ABI change, please let us agree now
> > to accept our quick patch for 16.11 for this change.
> 
> For an API deprecation notice (reworded),
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied, thanks
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index fffe9c7..4f3fefe 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -74,3 +74,11 @@  Deprecation Notices
   a handle, like the way kernel exposes an fd to user for locating a
   specific file, and to keep all major structures internally, so that
   we are likely to be free from ABI violations in future.
+
+* ABI will change for rte_port_source_params struct. The member file_name
+  data type will be changed from char * to const char *. This change targets
+  release 16.11
+
+* ABI will change for rte_port_sink_params struct. The member file_name
+  data type will be changed from char * to const char *. This change targets
+  release 16.11