[dpdk-dev] [PATCH 0/2] doc: announce ABI change of struct rte_port_source_params

[dpdk-dev] [PATCH 0/2] doc: announce ABI change of struct rte_port_source_params

[Fan Zhang]

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.

Fan Zhang (2):
  doc: announce ABI change of struct rte_port_source_params
  doc: announce ABI change of struct rte_port_sink_params

 doc/guides/rel_notes/deprecation.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

-- 
2.5.5

[dpdk-dev] [PATCH 1/2] doc: announce ABI change of struct rte_port_source_params

[Fan Zhang]

The ABI changes are planned for rte_port_source_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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index fffe9c7..d228bae 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -74,3 +74,7 @@ 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.
-- 
2.5.5

[dpdk-dev] [PATCH 2/2] doc: announce ABI change of struct rte_port_sink_params

[Fan Zhang]

The ABI changes are planned for 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 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index d228bae..d2f7306 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -78,3 +78,7 @@ Deprecation Notices
 * 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

Re: [dpdk-dev] [PATCH 2/2] doc: announce ABI change of struct rte_port_sink_params

[Panu Matilainen]

On 05/16/2016 04:18 PM, Fan Zhang wrote:
> The ABI changes are planned for 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 | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index d228bae..d2f7306 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -78,3 +78,7 @@ Deprecation Notices
>  * 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.
>

Surely such a minor change doesn't require two separate announcements or 
patches for that matter. In fact I doubt it's an ABI change at all 
(although it is an API change certainly).

However to me the bigger issue is that a change like this alone doesn't 
seem like worth breaking the ABI. The ABI was just broken in 16.04 to 
introduce these struct members (among other things) and to break the ABI 
again just to fixup a const-correctness issue seems a bit much. Could it 
maybe wait until there's some actually compelling reason to break the ABI?

Note that I'm not against the change as such, const-correctness is a 
good thing.

	- Panu -

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

[Fan Zhang]

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

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

[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;
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

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

[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>


> > -----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

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

[Thomas Monjalon]

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