DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
@ 2019-05-08 22:48 Erik Gabriel Carrillo
  2019-05-08 22:48 ` Erik Gabriel Carrillo
                   ` (3 more replies)
  0 siblings, 4 replies; 34+ messages in thread
From: Erik Gabriel Carrillo @ 2019-05-08 22:48 UTC (permalink / raw)
  To: thomas; +Cc: anatoly.burakov, dev

Due to an upcoming fix to allow the timer library to safely free its
allocations during the finalize() call[1], an ABI change will be
required. A new lock will be added to the rte_mem_config structure,
which will be used by the timer library to synchronize init/finalize
calls among multiple processes.

[1] http://patches.dpdk.org/patch/53334/

Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -31,6 +31,10 @@ Deprecation Notices
 
     + ``rte_eal_devargs_type_count``
 
+* eal: the ``rte_mem_config`` struct will change to include a new lock that
+  will allow the timer subsystem to safely release its allocations at cleanup
+  time. This will result in an ABI break.
+
 * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
   have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
   functions.  The due date for the removal targets DPDK 20.02.
-- 
2.6.4

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-08 22:48 [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup Erik Gabriel Carrillo
@ 2019-05-08 22:48 ` Erik Gabriel Carrillo
  2019-05-09  1:11 ` Stephen Hemminger
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 34+ messages in thread
From: Erik Gabriel Carrillo @ 2019-05-08 22:48 UTC (permalink / raw)
  To: thomas; +Cc: anatoly.burakov, dev

Due to an upcoming fix to allow the timer library to safely free its
allocations during the finalize() call[1], an ABI change will be
required. A new lock will be added to the rte_mem_config structure,
which will be used by the timer library to synchronize init/finalize
calls among multiple processes.

[1] http://patches.dpdk.org/patch/53334/

Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -31,6 +31,10 @@ Deprecation Notices
 
     + ``rte_eal_devargs_type_count``
 
+* eal: the ``rte_mem_config`` struct will change to include a new lock that
+  will allow the timer subsystem to safely release its allocations at cleanup
+  time. This will result in an ABI break.
+
 * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
   have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
   functions.  The due date for the removal targets DPDK 20.02.
-- 
2.6.4


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-08 22:48 [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup Erik Gabriel Carrillo
  2019-05-08 22:48 ` Erik Gabriel Carrillo
@ 2019-05-09  1:11 ` Stephen Hemminger
  2019-05-09  1:11   ` Stephen Hemminger
  2019-05-09  7:05   ` David Marchand
  2019-05-09 11:53 ` Burakov, Anatoly
  2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
  3 siblings, 2 replies; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-09  1:11 UTC (permalink / raw)
  To: Erik Gabriel Carrillo; +Cc: thomas, anatoly.burakov, dev

On Wed,  8 May 2019 17:48:06 -0500
Erik Gabriel Carrillo <erik.g.carrillo@intel.com> wrote:

> Due to an upcoming fix to allow the timer library to safely free its
> allocations during the finalize() call[1], an ABI change will be
> required. A new lock will be added to the rte_mem_config structure,
> which will be used by the timer library to synchronize init/finalize
> calls among multiple processes.
> 
> [1] http://patches.dpdk.org/patch/53334/
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,10 @@ Deprecation Notices
>  
>      + ``rte_eal_devargs_type_count``
>  
> +* eal: the ``rte_mem_config`` struct will change to include a new lock that
> +  will allow the timer subsystem to safely release its allocations at cleanup
> +  time. This will result in an ABI break.
> +
>  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
>    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>    functions.  The due date for the removal targets DPDK 20.02.

NAK

Please go to the effort of making rte_mem_config not part of the visible ABI.
Then change it.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  1:11 ` Stephen Hemminger
@ 2019-05-09  1:11   ` Stephen Hemminger
  2019-05-09  7:05   ` David Marchand
  1 sibling, 0 replies; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-09  1:11 UTC (permalink / raw)
  To: Erik Gabriel Carrillo; +Cc: thomas, anatoly.burakov, dev

On Wed,  8 May 2019 17:48:06 -0500
Erik Gabriel Carrillo <erik.g.carrillo@intel.com> wrote:

> Due to an upcoming fix to allow the timer library to safely free its
> allocations during the finalize() call[1], an ABI change will be
> required. A new lock will be added to the rte_mem_config structure,
> which will be used by the timer library to synchronize init/finalize
> calls among multiple processes.
> 
> [1] http://patches.dpdk.org/patch/53334/
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,10 @@ Deprecation Notices
>  
>      + ``rte_eal_devargs_type_count``
>  
> +* eal: the ``rte_mem_config`` struct will change to include a new lock that
> +  will allow the timer subsystem to safely release its allocations at cleanup
> +  time. This will result in an ABI break.
> +
>  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
>    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>    functions.  The due date for the removal targets DPDK 20.02.

NAK

Please go to the effort of making rte_mem_config not part of the visible ABI.
Then change it.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  1:11 ` Stephen Hemminger
  2019-05-09  1:11   ` Stephen Hemminger
@ 2019-05-09  7:05   ` David Marchand
  2019-05-09  7:05     ` David Marchand
  2019-05-09  8:33     ` Burakov, Anatoly
  1 sibling, 2 replies; 34+ messages in thread
From: David Marchand @ 2019-05-09  7:05 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Erik Gabriel Carrillo, Thomas Monjalon, Burakov, Anatoly, dev

On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Wed,  8 May 2019 17:48:06 -0500
> Erik Gabriel Carrillo <erik.g.carrillo@intel.com> wrote:
>
> > Due to an upcoming fix to allow the timer library to safely free its
> > allocations during the finalize() call[1], an ABI change will be
> > required. A new lock will be added to the rte_mem_config structure,
> > which will be used by the timer library to synchronize init/finalize
> > calls among multiple processes.
> >
> > [1] http://patches.dpdk.org/patch/53334/
> >
> > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -31,6 +31,10 @@ Deprecation Notices
> >
> >      + ``rte_eal_devargs_type_count``
> >
> > +* eal: the ``rte_mem_config`` struct will change to include a new lock
> that
> > +  will allow the timer subsystem to safely release its allocations at
> cleanup
> > +  time. This will result in an ABI break.
> > +
> >  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs
> which
> >    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
> >    functions.  The due date for the removal targets DPDK 20.02.
>
> NAK
>
> Please go to the effort of making rte_mem_config not part of the visible
> ABI.
> Then change it.
>

+1.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  7:05   ` David Marchand
@ 2019-05-09  7:05     ` David Marchand
  2019-05-09  8:33     ` Burakov, Anatoly
  1 sibling, 0 replies; 34+ messages in thread
From: David Marchand @ 2019-05-09  7:05 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Erik Gabriel Carrillo, Thomas Monjalon, Burakov, Anatoly, dev

On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger <stephen@networkplumber.org>
wrote:

> On Wed,  8 May 2019 17:48:06 -0500
> Erik Gabriel Carrillo <erik.g.carrillo@intel.com> wrote:
>
> > Due to an upcoming fix to allow the timer library to safely free its
> > allocations during the finalize() call[1], an ABI change will be
> > required. A new lock will be added to the rte_mem_config structure,
> > which will be used by the timer library to synchronize init/finalize
> > calls among multiple processes.
> >
> > [1] http://patches.dpdk.org/patch/53334/
> >
> > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@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 b47c8c2..7551383 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -31,6 +31,10 @@ Deprecation Notices
> >
> >      + ``rte_eal_devargs_type_count``
> >
> > +* eal: the ``rte_mem_config`` struct will change to include a new lock
> that
> > +  will allow the timer subsystem to safely release its allocations at
> cleanup
> > +  time. This will result in an ABI break.
> > +
> >  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs
> which
> >    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
> >    functions.  The due date for the removal targets DPDK 20.02.
>
> NAK
>
> Please go to the effort of making rte_mem_config not part of the visible
> ABI.
> Then change it.
>

+1.


-- 
David Marchand

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  7:05   ` David Marchand
  2019-05-09  7:05     ` David Marchand
@ 2019-05-09  8:33     ` Burakov, Anatoly
  2019-05-09  8:33       ` Burakov, Anatoly
  2019-05-09  9:06       ` Bruce Richardson
  1 sibling, 2 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09  8:33 UTC (permalink / raw)
  To: David Marchand, Stephen Hemminger
  Cc: Erik Gabriel Carrillo, Thomas Monjalon, dev

On 09-May-19 8:05 AM, David Marchand wrote:
> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger 
> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> 
>     On Wed,  8 May 2019 17:48:06 -0500
>     Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>     <mailto:erik.g.carrillo@intel.com>> wrote:
> 
>      > Due to an upcoming fix to allow the timer library to safely free its
>      > allocations during the finalize() call[1], an ABI change will be
>      > required. A new lock will be added to the rte_mem_config structure,
>      > which will be used by the timer library to synchronize init/finalize
>      > calls among multiple processes.
>      >
>      > [1] http://patches.dpdk.org/patch/53334/
>      >
>      > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>     <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>      > --- a/doc/guides/rel_notes/deprecation.rst
>      > +++ b/doc/guides/rel_notes/deprecation.rst
>      > @@ -31,6 +31,10 @@ Deprecation Notices
>      >
>      >      + ``rte_eal_devargs_type_count``
>      >
>      > +* eal: the ``rte_mem_config`` struct will change to include a
>     new lock that
>      > +  will allow the timer subsystem to safely release its
>     allocations at cleanup
>      > +  time. This will result in an ABI break.
>      > +
>      >  * vfio: removal of ``rte_vfio_dma_map`` and
>     ``rte_vfio_dma_unmap`` APIs which
>      >    have been replaced with ``rte_dev_dma_map`` and
>     ``rte_dev_dma_unmap``
>      >    functions.  The due date for the removal targets DPDK 20.02.
> 
>     NAK
> 
>     Please go to the effort of making rte_mem_config not part of the
>     visible ABI.
>     Then change it.
> 
> 
> +1.

I agree on principle, however this won't solve the issue. It doesn't 
need to be externally visible, but that's not all of its problems - it's 
also shared between processes so there's an ABI contract between primary 
and secondary too. This means that, even if the structure itself is not 
public, any changes to it will still result in an ABI break. That's the 
nature of our shared memory.

In other words, if your goal is to avoid ABI breaks on changing this 
structure, making it internal won't help in the slightest.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  8:33     ` Burakov, Anatoly
@ 2019-05-09  8:33       ` Burakov, Anatoly
  2019-05-09  9:06       ` Bruce Richardson
  1 sibling, 0 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09  8:33 UTC (permalink / raw)
  To: David Marchand, Stephen Hemminger
  Cc: Erik Gabriel Carrillo, Thomas Monjalon, dev

On 09-May-19 8:05 AM, David Marchand wrote:
> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger 
> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> 
>     On Wed,  8 May 2019 17:48:06 -0500
>     Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>     <mailto:erik.g.carrillo@intel.com>> wrote:
> 
>      > Due to an upcoming fix to allow the timer library to safely free its
>      > allocations during the finalize() call[1], an ABI change will be
>      > required. A new lock will be added to the rte_mem_config structure,
>      > which will be used by the timer library to synchronize init/finalize
>      > calls among multiple processes.
>      >
>      > [1] http://patches.dpdk.org/patch/53334/
>      >
>      > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>     <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>      > --- a/doc/guides/rel_notes/deprecation.rst
>      > +++ b/doc/guides/rel_notes/deprecation.rst
>      > @@ -31,6 +31,10 @@ Deprecation Notices
>      >
>      >      + ``rte_eal_devargs_type_count``
>      >
>      > +* eal: the ``rte_mem_config`` struct will change to include a
>     new lock that
>      > +  will allow the timer subsystem to safely release its
>     allocations at cleanup
>      > +  time. This will result in an ABI break.
>      > +
>      >  * vfio: removal of ``rte_vfio_dma_map`` and
>     ``rte_vfio_dma_unmap`` APIs which
>      >    have been replaced with ``rte_dev_dma_map`` and
>     ``rte_dev_dma_unmap``
>      >    functions.  The due date for the removal targets DPDK 20.02.
> 
>     NAK
> 
>     Please go to the effort of making rte_mem_config not part of the
>     visible ABI.
>     Then change it.
> 
> 
> +1.

I agree on principle, however this won't solve the issue. It doesn't 
need to be externally visible, but that's not all of its problems - it's 
also shared between processes so there's an ABI contract between primary 
and secondary too. This means that, even if the structure itself is not 
public, any changes to it will still result in an ABI break. That's the 
nature of our shared memory.

In other words, if your goal is to avoid ABI breaks on changing this 
structure, making it internal won't help in the slightest.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  8:33     ` Burakov, Anatoly
  2019-05-09  8:33       ` Burakov, Anatoly
@ 2019-05-09  9:06       ` Bruce Richardson
  2019-05-09  9:06         ` Bruce Richardson
  2019-05-09  9:37         ` Burakov, Anatoly
  1 sibling, 2 replies; 34+ messages in thread
From: Bruce Richardson @ 2019-05-09  9:06 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: David Marchand, Stephen Hemminger, Erik Gabriel Carrillo,
	Thomas Monjalon, dev

On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
> On 09-May-19 8:05 AM, David Marchand wrote:
> > On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> > <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> > 
> >     On Wed,  8 May 2019 17:48:06 -0500
> >     Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >     <mailto:erik.g.carrillo@intel.com>> wrote:
> > 
> >      > Due to an upcoming fix to allow the timer library to safely free its
> >      > allocations during the finalize() call[1], an ABI change will be
> >      > required. A new lock will be added to the rte_mem_config structure,
> >      > which will be used by the timer library to synchronize init/finalize
> >      > calls among multiple processes.
> >      >
> >      > [1] http://patches.dpdk.org/patch/53334/
> >      >
> >      > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >     <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >      > --- a/doc/guides/rel_notes/deprecation.rst
> >      > +++ b/doc/guides/rel_notes/deprecation.rst
> >      > @@ -31,6 +31,10 @@ Deprecation Notices
> >      >
> >      >      + ``rte_eal_devargs_type_count``
> >      >
> >      > +* eal: the ``rte_mem_config`` struct will change to include a
> >     new lock that
> >      > +  will allow the timer subsystem to safely release its
> >     allocations at cleanup
> >      > +  time. This will result in an ABI break.
> >      > +
> >      >  * vfio: removal of ``rte_vfio_dma_map`` and
> >     ``rte_vfio_dma_unmap`` APIs which
> >      >    have been replaced with ``rte_dev_dma_map`` and
> >     ``rte_dev_dma_unmap``
> >      >    functions.  The due date for the removal targets DPDK 20.02.
> > 
> >     NAK
> > 
> >     Please go to the effort of making rte_mem_config not part of the
> >     visible ABI.
> >     Then change it.
> > 
> > 
> > +1.
> 
> I agree on principle, however this won't solve the issue. It doesn't need to
> be externally visible, but that's not all of its problems - it's also shared
> between processes so there's an ABI contract between primary and secondary
> too. This means that, even if the structure itself is not public, any
> changes to it will still result in an ABI break. That's the nature of our
> shared memory.
> 
> In other words, if your goal is to avoid ABI breaks on changing this
> structure, making it internal won't help in the slightest.
>

Is there an ABI contract between primary and secondary. I always assumed
that if using secondary processes the requirement (though undocumented) was
that both had to be linked against the exact same versions of DPDK?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:06       ` Bruce Richardson
@ 2019-05-09  9:06         ` Bruce Richardson
  2019-05-09  9:37         ` Burakov, Anatoly
  1 sibling, 0 replies; 34+ messages in thread
From: Bruce Richardson @ 2019-05-09  9:06 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: David Marchand, Stephen Hemminger, Erik Gabriel Carrillo,
	Thomas Monjalon, dev

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 2901 bytes --]

On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
> On 09-May-19 8:05 AM, David Marchand wrote:
> > On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> > <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> > 
> >     On Wed,  8 May 2019 17:48:06 -0500
> >     Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >     <mailto:erik.g.carrillo@intel.com>> wrote:
> > 
> >      > Due to an upcoming fix to allow the timer library to safely free its
> >      > allocations during the finalize() call[1], an ABI change will be
> >      > required. A new lock will be added to the rte_mem_config structure,
> >      > which will be used by the timer library to synchronize init/finalize
> >      > calls among multiple processes.
> >      >
> >      > [1] http://patches.dpdk.org/patch/53334/
> >      >
> >      > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >     <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >      > --- a/doc/guides/rel_notes/deprecation.rst
> >      > +++ b/doc/guides/rel_notes/deprecation.rst
> >      > @@ -31,6 +31,10 @@ Deprecation Notices
> >      >
> >      >      + ``rte_eal_devargs_type_count``
> >      >
> >      > +* eal: the ``rte_mem_config`` struct will change to include a
> >     new lock that
> >      > +  will allow the timer subsystem to safely release its
> >     allocations at cleanup
> >      > +  time. This will result in an ABI break.
> >      > +
> >      >  * vfio: removal of ``rte_vfio_dma_map`` and
> >     ``rte_vfio_dma_unmap`` APIs which
> >      >    have been replaced with ``rte_dev_dma_map`` and
> >     ``rte_dev_dma_unmap``
> >      >    functions.  The due date for the removal targets DPDK 20.02.
> > 
> >     NAK
> > 
> >     Please go to the effort of making rte_mem_config not part of the
> >     visible ABI.
> >     Then change it.
> > 
> > 
> > +1.
> 
> I agree on principle, however this won't solve the issue. It doesn't need to
> be externally visible, but that's not all of its problems - it's also shared
> between processes so there's an ABI contract between primary and secondary
> too. This means that, even if the structure itself is not public, any
> changes to it will still result in an ABI break. That's the nature of our
> shared memory.
> 
> In other words, if your goal is to avoid ABI breaks on changing this
> structure, making it internal won't help in the slightest.
>

Is there an ABI contract between primary and secondary. I always assumed
that if using secondary processes the requirement (though undocumented) was
that both had to be linked against the exact same versions of DPDK?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:06       ` Bruce Richardson
  2019-05-09  9:06         ` Bruce Richardson
@ 2019-05-09  9:37         ` Burakov, Anatoly
  2019-05-09  9:37           ` Burakov, Anatoly
  2019-05-09  9:38           ` Thomas Monjalon
  1 sibling, 2 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09  9:37 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: David Marchand, Stephen Hemminger, Erik Gabriel Carrillo,
	Thomas Monjalon, dev

On 09-May-19 10:06 AM, Bruce Richardson wrote:
> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>> On 09-May-19 8:05 AM, David Marchand wrote:
>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>
>>>      On Wed,  8 May 2019 17:48:06 -0500
>>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>      <mailto:erik.g.carrillo@intel.com>> wrote:
>>>
>>>       > Due to an upcoming fix to allow the timer library to safely free its
>>>       > allocations during the finalize() call[1], an ABI change will be
>>>       > required. A new lock will be added to the rte_mem_config structure,
>>>       > which will be used by the timer library to synchronize init/finalize
>>>       > calls among multiple processes.
>>>       >
>>>       > [1] http://patches.dpdk.org/patch/53334/
>>>       >
>>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>       > --- a/doc/guides/rel_notes/deprecation.rst
>>>       > +++ b/doc/guides/rel_notes/deprecation.rst
>>>       > @@ -31,6 +31,10 @@ Deprecation Notices
>>>       >
>>>       >      + ``rte_eal_devargs_type_count``
>>>       >
>>>       > +* eal: the ``rte_mem_config`` struct will change to include a
>>>      new lock that
>>>       > +  will allow the timer subsystem to safely release its
>>>      allocations at cleanup
>>>       > +  time. This will result in an ABI break.
>>>       > +
>>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>      ``rte_vfio_dma_unmap`` APIs which
>>>       >    have been replaced with ``rte_dev_dma_map`` and
>>>      ``rte_dev_dma_unmap``
>>>       >    functions.  The due date for the removal targets DPDK 20.02.
>>>
>>>      NAK
>>>
>>>      Please go to the effort of making rte_mem_config not part of the
>>>      visible ABI.
>>>      Then change it.
>>>
>>>
>>> +1.
>>
>> I agree on principle, however this won't solve the issue. It doesn't need to
>> be externally visible, but that's not all of its problems - it's also shared
>> between processes so there's an ABI contract between primary and secondary
>> too. This means that, even if the structure itself is not public, any
>> changes to it will still result in an ABI break. That's the nature of our
>> shared memory.
>>
>> In other words, if your goal is to avoid ABI breaks on changing this
>> structure, making it internal won't help in the slightest.
>>
> 
> Is there an ABI contract between primary and secondary. I always assumed
> that if using secondary processes the requirement (though undocumented) was
> that both had to be linked against the exact same versions of DPDK?
> 

The fact that it's undocumented means we can't assume everyone will do 
that :)

If the community agrees that primary/secondary processes should always 
use the same DPDK version (regardless of static/dynamic builds etc.), 
then this problem would probably be solved.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:37         ` Burakov, Anatoly
@ 2019-05-09  9:37           ` Burakov, Anatoly
  2019-05-09  9:38           ` Thomas Monjalon
  1 sibling, 0 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09  9:37 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: David Marchand, Stephen Hemminger, Erik Gabriel Carrillo,
	Thomas Monjalon, dev

On 09-May-19 10:06 AM, Bruce Richardson wrote:
> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>> On 09-May-19 8:05 AM, David Marchand wrote:
>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>
>>>      On Wed,  8 May 2019 17:48:06 -0500
>>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>      <mailto:erik.g.carrillo@intel.com>> wrote:
>>>
>>>       > Due to an upcoming fix to allow the timer library to safely free its
>>>       > allocations during the finalize() call[1], an ABI change will be
>>>       > required. A new lock will be added to the rte_mem_config structure,
>>>       > which will be used by the timer library to synchronize init/finalize
>>>       > calls among multiple processes.
>>>       >
>>>       > [1] http://patches.dpdk.org/patch/53334/
>>>       >
>>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>       > --- a/doc/guides/rel_notes/deprecation.rst
>>>       > +++ b/doc/guides/rel_notes/deprecation.rst
>>>       > @@ -31,6 +31,10 @@ Deprecation Notices
>>>       >
>>>       >      + ``rte_eal_devargs_type_count``
>>>       >
>>>       > +* eal: the ``rte_mem_config`` struct will change to include a
>>>      new lock that
>>>       > +  will allow the timer subsystem to safely release its
>>>      allocations at cleanup
>>>       > +  time. This will result in an ABI break.
>>>       > +
>>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>      ``rte_vfio_dma_unmap`` APIs which
>>>       >    have been replaced with ``rte_dev_dma_map`` and
>>>      ``rte_dev_dma_unmap``
>>>       >    functions.  The due date for the removal targets DPDK 20.02.
>>>
>>>      NAK
>>>
>>>      Please go to the effort of making rte_mem_config not part of the
>>>      visible ABI.
>>>      Then change it.
>>>
>>>
>>> +1.
>>
>> I agree on principle, however this won't solve the issue. It doesn't need to
>> be externally visible, but that's not all of its problems - it's also shared
>> between processes so there's an ABI contract between primary and secondary
>> too. This means that, even if the structure itself is not public, any
>> changes to it will still result in an ABI break. That's the nature of our
>> shared memory.
>>
>> In other words, if your goal is to avoid ABI breaks on changing this
>> structure, making it internal won't help in the slightest.
>>
> 
> Is there an ABI contract between primary and secondary. I always assumed
> that if using secondary processes the requirement (though undocumented) was
> that both had to be linked against the exact same versions of DPDK?
> 

The fact that it's undocumented means we can't assume everyone will do 
that :)

If the community agrees that primary/secondary processes should always 
use the same DPDK version (regardless of static/dynamic builds etc.), 
then this problem would probably be solved.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:37         ` Burakov, Anatoly
  2019-05-09  9:37           ` Burakov, Anatoly
@ 2019-05-09  9:38           ` Thomas Monjalon
  2019-05-09  9:38             ` Thomas Monjalon
  2019-05-09  9:50             ` Ray Kinsella
  1 sibling, 2 replies; 34+ messages in thread
From: Thomas Monjalon @ 2019-05-09  9:38 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev

09/05/2019 11:37, Burakov, Anatoly:
> On 09-May-19 10:06 AM, Bruce Richardson wrote:
> > On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
> >> On 09-May-19 8:05 AM, David Marchand wrote:
> >>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> >>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> >>>
> >>>      On Wed,  8 May 2019 17:48:06 -0500
> >>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>      <mailto:erik.g.carrillo@intel.com>> wrote:
> >>>
> >>>       > Due to an upcoming fix to allow the timer library to safely free its
> >>>       > allocations during the finalize() call[1], an ABI change will be
> >>>       > required. A new lock will be added to the rte_mem_config structure,
> >>>       > which will be used by the timer library to synchronize init/finalize
> >>>       > calls among multiple processes.
> >>>       >
> >>>       > [1] http://patches.dpdk.org/patch/53334/
> >>>       >
> >>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >>>       > --- a/doc/guides/rel_notes/deprecation.rst
> >>>       > +++ b/doc/guides/rel_notes/deprecation.rst
> >>>       > @@ -31,6 +31,10 @@ Deprecation Notices
> >>>       >
> >>>       >      + ``rte_eal_devargs_type_count``
> >>>       >
> >>>       > +* eal: the ``rte_mem_config`` struct will change to include a
> >>>      new lock that
> >>>       > +  will allow the timer subsystem to safely release its
> >>>      allocations at cleanup
> >>>       > +  time. This will result in an ABI break.
> >>>       > +
> >>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
> >>>      ``rte_vfio_dma_unmap`` APIs which
> >>>       >    have been replaced with ``rte_dev_dma_map`` and
> >>>      ``rte_dev_dma_unmap``
> >>>       >    functions.  The due date for the removal targets DPDK 20.02.
> >>>
> >>>      NAK
> >>>
> >>>      Please go to the effort of making rte_mem_config not part of the
> >>>      visible ABI.
> >>>      Then change it.
> >>>
> >>>
> >>> +1.
> >>
> >> I agree on principle, however this won't solve the issue. It doesn't need to
> >> be externally visible, but that's not all of its problems - it's also shared
> >> between processes so there's an ABI contract between primary and secondary
> >> too. This means that, even if the structure itself is not public, any
> >> changes to it will still result in an ABI break. That's the nature of our
> >> shared memory.
> >>
> >> In other words, if your goal is to avoid ABI breaks on changing this
> >> structure, making it internal won't help in the slightest.
> >>
> > 
> > Is there an ABI contract between primary and secondary. I always assumed
> > that if using secondary processes the requirement (though undocumented) was
> > that both had to be linked against the exact same versions of DPDK?
> > 
> 
> The fact that it's undocumented means we can't assume everyone will do 
> that :)
> 
> If the community agrees that primary/secondary processes should always 
> use the same DPDK version (regardless of static/dynamic builds etc.), 
> then this problem would probably be solved.

+1 to document that primary/secondary with different DPDK versions
is not supported.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:38           ` Thomas Monjalon
@ 2019-05-09  9:38             ` Thomas Monjalon
  2019-05-09  9:50             ` Ray Kinsella
  1 sibling, 0 replies; 34+ messages in thread
From: Thomas Monjalon @ 2019-05-09  9:38 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev

09/05/2019 11:37, Burakov, Anatoly:
> On 09-May-19 10:06 AM, Bruce Richardson wrote:
> > On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
> >> On 09-May-19 8:05 AM, David Marchand wrote:
> >>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> >>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> >>>
> >>>      On Wed,  8 May 2019 17:48:06 -0500
> >>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>      <mailto:erik.g.carrillo@intel.com>> wrote:
> >>>
> >>>       > Due to an upcoming fix to allow the timer library to safely free its
> >>>       > allocations during the finalize() call[1], an ABI change will be
> >>>       > required. A new lock will be added to the rte_mem_config structure,
> >>>       > which will be used by the timer library to synchronize init/finalize
> >>>       > calls among multiple processes.
> >>>       >
> >>>       > [1] http://patches.dpdk.org/patch/53334/
> >>>       >
> >>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >>>       > --- a/doc/guides/rel_notes/deprecation.rst
> >>>       > +++ b/doc/guides/rel_notes/deprecation.rst
> >>>       > @@ -31,6 +31,10 @@ Deprecation Notices
> >>>       >
> >>>       >      + ``rte_eal_devargs_type_count``
> >>>       >
> >>>       > +* eal: the ``rte_mem_config`` struct will change to include a
> >>>      new lock that
> >>>       > +  will allow the timer subsystem to safely release its
> >>>      allocations at cleanup
> >>>       > +  time. This will result in an ABI break.
> >>>       > +
> >>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
> >>>      ``rte_vfio_dma_unmap`` APIs which
> >>>       >    have been replaced with ``rte_dev_dma_map`` and
> >>>      ``rte_dev_dma_unmap``
> >>>       >    functions.  The due date for the removal targets DPDK 20.02.
> >>>
> >>>      NAK
> >>>
> >>>      Please go to the effort of making rte_mem_config not part of the
> >>>      visible ABI.
> >>>      Then change it.
> >>>
> >>>
> >>> +1.
> >>
> >> I agree on principle, however this won't solve the issue. It doesn't need to
> >> be externally visible, but that's not all of its problems - it's also shared
> >> between processes so there's an ABI contract between primary and secondary
> >> too. This means that, even if the structure itself is not public, any
> >> changes to it will still result in an ABI break. That's the nature of our
> >> shared memory.
> >>
> >> In other words, if your goal is to avoid ABI breaks on changing this
> >> structure, making it internal won't help in the slightest.
> >>
> > 
> > Is there an ABI contract between primary and secondary. I always assumed
> > that if using secondary processes the requirement (though undocumented) was
> > that both had to be linked against the exact same versions of DPDK?
> > 
> 
> The fact that it's undocumented means we can't assume everyone will do 
> that :)
> 
> If the community agrees that primary/secondary processes should always 
> use the same DPDK version (regardless of static/dynamic builds etc.), 
> then this problem would probably be solved.

+1 to document that primary/secondary with different DPDK versions
is not supported.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:38           ` Thomas Monjalon
  2019-05-09  9:38             ` Thomas Monjalon
@ 2019-05-09  9:50             ` Ray Kinsella
  2019-05-09  9:50               ` Ray Kinsella
  2019-05-09 10:08               ` Burakov, Anatoly
  1 sibling, 2 replies; 34+ messages in thread
From: Ray Kinsella @ 2019-05-09  9:50 UTC (permalink / raw)
  To: Thomas Monjalon, Burakov, Anatoly
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev



On 09/05/2019 10:38, Thomas Monjalon wrote:
> 09/05/2019 11:37, Burakov, Anatoly:
>> On 09-May-19 10:06 AM, Bruce Richardson wrote:
>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>>>> On 09-May-19 8:05 AM, David Marchand wrote:
>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>>>
>>>>>      On Wed,  8 May 2019 17:48:06 -0500
>>>>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>      <mailto:erik.g.carrillo@intel.com>> wrote:
>>>>>
>>>>>       > Due to an upcoming fix to allow the timer library to safely free its
>>>>>       > allocations during the finalize() call[1], an ABI change will be
>>>>>       > required. A new lock will be added to the rte_mem_config structure,
>>>>>       > which will be used by the timer library to synchronize init/finalize
>>>>>       > calls among multiple processes.
>>>>>       >
>>>>>       > [1] http://patches.dpdk.org/patch/53334/
>>>>>       >
>>>>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>>>       > --- a/doc/guides/rel_notes/deprecation.rst
>>>>>       > +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>       > @@ -31,6 +31,10 @@ Deprecation Notices
>>>>>       >
>>>>>       >      + ``rte_eal_devargs_type_count``
>>>>>       >
>>>>>       > +* eal: the ``rte_mem_config`` struct will change to include a
>>>>>      new lock that
>>>>>       > +  will allow the timer subsystem to safely release its
>>>>>      allocations at cleanup
>>>>>       > +  time. This will result in an ABI break.
>>>>>       > +
>>>>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>>>      ``rte_vfio_dma_unmap`` APIs which
>>>>>       >    have been replaced with ``rte_dev_dma_map`` and
>>>>>      ``rte_dev_dma_unmap``
>>>>>       >    functions.  The due date for the removal targets DPDK 20.02.
>>>>>
>>>>>      NAK
>>>>>
>>>>>      Please go to the effort of making rte_mem_config not part of the
>>>>>      visible ABI.
>>>>>      Then change it.
>>>>>
>>>>>
>>>>> +1.
>>>>
>>>> I agree on principle, however this won't solve the issue. It doesn't need to
>>>> be externally visible, but that's not all of its problems - it's also shared
>>>> between processes so there's an ABI contract between primary and secondary
>>>> too. This means that, even if the structure itself is not public, any
>>>> changes to it will still result in an ABI break. That's the nature of our
>>>> shared memory.
>>>>
>>>> In other words, if your goal is to avoid ABI breaks on changing this
>>>> structure, making it internal won't help in the slightest.
>>>>
>>>
>>> Is there an ABI contract between primary and secondary. I always assumed
>>> that if using secondary processes the requirement (though undocumented) was
>>> that both had to be linked against the exact same versions of DPDK?
>>>
>>
>> The fact that it's undocumented means we can't assume everyone will do 
>> that :)
>>
>> If the community agrees that primary/secondary processes should always 
>> use the same DPDK version (regardless of static/dynamic builds etc.), 
>> then this problem would probably be solved.
> 
> +1 to document that primary/secondary with different DPDK versions
> is not supported.
> 

+1,

but I think we need to go farther - we need a secondary process to check
with the primary process.
We can't assume everyone will read the documentation.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:50             ` Ray Kinsella
@ 2019-05-09  9:50               ` Ray Kinsella
  2019-05-09 10:08               ` Burakov, Anatoly
  1 sibling, 0 replies; 34+ messages in thread
From: Ray Kinsella @ 2019-05-09  9:50 UTC (permalink / raw)
  To: Thomas Monjalon, Burakov, Anatoly
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev



On 09/05/2019 10:38, Thomas Monjalon wrote:
> 09/05/2019 11:37, Burakov, Anatoly:
>> On 09-May-19 10:06 AM, Bruce Richardson wrote:
>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>>>> On 09-May-19 8:05 AM, David Marchand wrote:
>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>>>
>>>>>      On Wed,  8 May 2019 17:48:06 -0500
>>>>>      Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>      <mailto:erik.g.carrillo@intel.com>> wrote:
>>>>>
>>>>>       > Due to an upcoming fix to allow the timer library to safely free its
>>>>>       > allocations during the finalize() call[1], an ABI change will be
>>>>>       > required. A new lock will be added to the rte_mem_config structure,
>>>>>       > which will be used by the timer library to synchronize init/finalize
>>>>>       > calls among multiple processes.
>>>>>       >
>>>>>       > [1] http://patches.dpdk.org/patch/53334/
>>>>>       >
>>>>>       > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>      <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>>>       > --- a/doc/guides/rel_notes/deprecation.rst
>>>>>       > +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>       > @@ -31,6 +31,10 @@ Deprecation Notices
>>>>>       >
>>>>>       >      + ``rte_eal_devargs_type_count``
>>>>>       >
>>>>>       > +* eal: the ``rte_mem_config`` struct will change to include a
>>>>>      new lock that
>>>>>       > +  will allow the timer subsystem to safely release its
>>>>>      allocations at cleanup
>>>>>       > +  time. This will result in an ABI break.
>>>>>       > +
>>>>>       >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>>>      ``rte_vfio_dma_unmap`` APIs which
>>>>>       >    have been replaced with ``rte_dev_dma_map`` and
>>>>>      ``rte_dev_dma_unmap``
>>>>>       >    functions.  The due date for the removal targets DPDK 20.02.
>>>>>
>>>>>      NAK
>>>>>
>>>>>      Please go to the effort of making rte_mem_config not part of the
>>>>>      visible ABI.
>>>>>      Then change it.
>>>>>
>>>>>
>>>>> +1.
>>>>
>>>> I agree on principle, however this won't solve the issue. It doesn't need to
>>>> be externally visible, but that's not all of its problems - it's also shared
>>>> between processes so there's an ABI contract between primary and secondary
>>>> too. This means that, even if the structure itself is not public, any
>>>> changes to it will still result in an ABI break. That's the nature of our
>>>> shared memory.
>>>>
>>>> In other words, if your goal is to avoid ABI breaks on changing this
>>>> structure, making it internal won't help in the slightest.
>>>>
>>>
>>> Is there an ABI contract between primary and secondary. I always assumed
>>> that if using secondary processes the requirement (though undocumented) was
>>> that both had to be linked against the exact same versions of DPDK?
>>>
>>
>> The fact that it's undocumented means we can't assume everyone will do 
>> that :)
>>
>> If the community agrees that primary/secondary processes should always 
>> use the same DPDK version (regardless of static/dynamic builds etc.), 
>> then this problem would probably be solved.
> 
> +1 to document that primary/secondary with different DPDK versions
> is not supported.
> 

+1,

but I think we need to go farther - we need a secondary process to check
with the primary process.
We can't assume everyone will read the documentation.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09  9:50             ` Ray Kinsella
  2019-05-09  9:50               ` Ray Kinsella
@ 2019-05-09 10:08               ` Burakov, Anatoly
  2019-05-09 10:08                 ` Burakov, Anatoly
                                   ` (2 more replies)
  1 sibling, 3 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09 10:08 UTC (permalink / raw)
  To: Ray Kinsella, Thomas Monjalon
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev

On 09-May-19 10:50 AM, Ray Kinsella wrote:
> 
> 
> On 09/05/2019 10:38, Thomas Monjalon wrote:
>> 09/05/2019 11:37, Burakov, Anatoly:
>>> On 09-May-19 10:06 AM, Bruce Richardson wrote:
>>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>>>>> On 09-May-19 8:05 AM, David Marchand wrote:
>>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>>>>
>>>>>>       On Wed,  8 May 2019 17:48:06 -0500
>>>>>>       Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>>       <mailto:erik.g.carrillo@intel.com>> wrote:
>>>>>>
>>>>>>        > Due to an upcoming fix to allow the timer library to safely free its
>>>>>>        > allocations during the finalize() call[1], an ABI change will be
>>>>>>        > required. A new lock will be added to the rte_mem_config structure,
>>>>>>        > which will be used by the timer library to synchronize init/finalize
>>>>>>        > calls among multiple processes.
>>>>>>        >
>>>>>>        > [1] http://patches.dpdk.org/patch/53334/
>>>>>>        >
>>>>>>        > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>>       <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>>>>        > --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>        > +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>        > @@ -31,6 +31,10 @@ Deprecation Notices
>>>>>>        >
>>>>>>        >      + ``rte_eal_devargs_type_count``
>>>>>>        >
>>>>>>        > +* eal: the ``rte_mem_config`` struct will change to include a
>>>>>>       new lock that
>>>>>>        > +  will allow the timer subsystem to safely release its
>>>>>>       allocations at cleanup
>>>>>>        > +  time. This will result in an ABI break.
>>>>>>        > +
>>>>>>        >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>>>>       ``rte_vfio_dma_unmap`` APIs which
>>>>>>        >    have been replaced with ``rte_dev_dma_map`` and
>>>>>>       ``rte_dev_dma_unmap``
>>>>>>        >    functions.  The due date for the removal targets DPDK 20.02.
>>>>>>
>>>>>>       NAK
>>>>>>
>>>>>>       Please go to the effort of making rte_mem_config not part of the
>>>>>>       visible ABI.
>>>>>>       Then change it.
>>>>>>
>>>>>>
>>>>>> +1.
>>>>>
>>>>> I agree on principle, however this won't solve the issue. It doesn't need to
>>>>> be externally visible, but that's not all of its problems - it's also shared
>>>>> between processes so there's an ABI contract between primary and secondary
>>>>> too. This means that, even if the structure itself is not public, any
>>>>> changes to it will still result in an ABI break. That's the nature of our
>>>>> shared memory.
>>>>>
>>>>> In other words, if your goal is to avoid ABI breaks on changing this
>>>>> structure, making it internal won't help in the slightest.
>>>>>
>>>>
>>>> Is there an ABI contract between primary and secondary. I always assumed
>>>> that if using secondary processes the requirement (though undocumented) was
>>>> that both had to be linked against the exact same versions of DPDK?
>>>>
>>>
>>> The fact that it's undocumented means we can't assume everyone will do
>>> that :)
>>>
>>> If the community agrees that primary/secondary processes should always
>>> use the same DPDK version (regardless of static/dynamic builds etc.),
>>> then this problem would probably be solved.
>>
>> +1 to document that primary/secondary with different DPDK versions
>> is not supported.
>>
> 
> +1,
> 
> but I think we need to go farther - we need a secondary process to check
> with the primary process.
> We can't assume everyone will read the documentation.
> 

That easily can be done, yes.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09 10:08               ` Burakov, Anatoly
@ 2019-05-09 10:08                 ` Burakov, Anatoly
  2019-05-09 19:02                 ` Stephen Hemminger
  2019-05-10 14:42                 ` Stephen Hemminger
  2 siblings, 0 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09 10:08 UTC (permalink / raw)
  To: Ray Kinsella, Thomas Monjalon
  Cc: Bruce Richardson, David Marchand, Stephen Hemminger,
	Erik Gabriel Carrillo, dev

On 09-May-19 10:50 AM, Ray Kinsella wrote:
> 
> 
> On 09/05/2019 10:38, Thomas Monjalon wrote:
>> 09/05/2019 11:37, Burakov, Anatoly:
>>> On 09-May-19 10:06 AM, Bruce Richardson wrote:
>>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:
>>>>> On 09-May-19 8:05 AM, David Marchand wrote:
>>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
>>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
>>>>>>
>>>>>>       On Wed,  8 May 2019 17:48:06 -0500
>>>>>>       Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>>       <mailto:erik.g.carrillo@intel.com>> wrote:
>>>>>>
>>>>>>        > Due to an upcoming fix to allow the timer library to safely free its
>>>>>>        > allocations during the finalize() call[1], an ABI change will be
>>>>>>        > required. A new lock will be added to the rte_mem_config structure,
>>>>>>        > which will be used by the timer library to synchronize init/finalize
>>>>>>        > calls among multiple processes.
>>>>>>        >
>>>>>>        > [1] http://patches.dpdk.org/patch/53334/
>>>>>>        >
>>>>>>        > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com
>>>>>>       <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
>>>>>>        > --- a/doc/guides/rel_notes/deprecation.rst
>>>>>>        > +++ b/doc/guides/rel_notes/deprecation.rst
>>>>>>        > @@ -31,6 +31,10 @@ Deprecation Notices
>>>>>>        >
>>>>>>        >      + ``rte_eal_devargs_type_count``
>>>>>>        >
>>>>>>        > +* eal: the ``rte_mem_config`` struct will change to include a
>>>>>>       new lock that
>>>>>>        > +  will allow the timer subsystem to safely release its
>>>>>>       allocations at cleanup
>>>>>>        > +  time. This will result in an ABI break.
>>>>>>        > +
>>>>>>        >  * vfio: removal of ``rte_vfio_dma_map`` and
>>>>>>       ``rte_vfio_dma_unmap`` APIs which
>>>>>>        >    have been replaced with ``rte_dev_dma_map`` and
>>>>>>       ``rte_dev_dma_unmap``
>>>>>>        >    functions.  The due date for the removal targets DPDK 20.02.
>>>>>>
>>>>>>       NAK
>>>>>>
>>>>>>       Please go to the effort of making rte_mem_config not part of the
>>>>>>       visible ABI.
>>>>>>       Then change it.
>>>>>>
>>>>>>
>>>>>> +1.
>>>>>
>>>>> I agree on principle, however this won't solve the issue. It doesn't need to
>>>>> be externally visible, but that's not all of its problems - it's also shared
>>>>> between processes so there's an ABI contract between primary and secondary
>>>>> too. This means that, even if the structure itself is not public, any
>>>>> changes to it will still result in an ABI break. That's the nature of our
>>>>> shared memory.
>>>>>
>>>>> In other words, if your goal is to avoid ABI breaks on changing this
>>>>> structure, making it internal won't help in the slightest.
>>>>>
>>>>
>>>> Is there an ABI contract between primary and secondary. I always assumed
>>>> that if using secondary processes the requirement (though undocumented) was
>>>> that both had to be linked against the exact same versions of DPDK?
>>>>
>>>
>>> The fact that it's undocumented means we can't assume everyone will do
>>> that :)
>>>
>>> If the community agrees that primary/secondary processes should always
>>> use the same DPDK version (regardless of static/dynamic builds etc.),
>>> then this problem would probably be solved.
>>
>> +1 to document that primary/secondary with different DPDK versions
>> is not supported.
>>
> 
> +1,
> 
> but I think we need to go farther - we need a secondary process to check
> with the primary process.
> We can't assume everyone will read the documentation.
> 

That easily can be done, yes.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-08 22:48 [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup Erik Gabriel Carrillo
  2019-05-08 22:48 ` Erik Gabriel Carrillo
  2019-05-09  1:11 ` Stephen Hemminger
@ 2019-05-09 11:53 ` Burakov, Anatoly
  2019-05-09 11:53   ` Burakov, Anatoly
  2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
  3 siblings, 1 reply; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09 11:53 UTC (permalink / raw)
  To: Erik Gabriel Carrillo, thomas; +Cc: dev

On 08-May-19 11:48 PM, Erik Gabriel Carrillo wrote:
> Due to an upcoming fix to allow the timer library to safely free its
> allocations during the finalize() call[1], an ABI change will be
> required. A new lock will be added to the rte_mem_config structure,
> which will be used by the timer library to synchronize init/finalize
> calls among multiple processes.
> 
> [1] http://patches.dpdk.org/patch/53334/
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---

As per discussion, please change the deprecation notice to instead make 
rte_eal_memconfig private.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09 11:53 ` Burakov, Anatoly
@ 2019-05-09 11:53   ` Burakov, Anatoly
  0 siblings, 0 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-09 11:53 UTC (permalink / raw)
  To: Erik Gabriel Carrillo, thomas; +Cc: dev

On 08-May-19 11:48 PM, Erik Gabriel Carrillo wrote:
> Due to an upcoming fix to allow the timer library to safely free its
> allocations during the finalize() call[1], an ABI change will be
> required. A new lock will be added to the rte_mem_config structure,
> which will be used by the timer library to synchronize init/finalize
> calls among multiple processes.
> 
> [1] http://patches.dpdk.org/patch/53334/
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---

As per discussion, please change the deprecation notice to instead make 
rte_eal_memconfig private.

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-08 22:48 [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup Erik Gabriel Carrillo
                   ` (2 preceding siblings ...)
  2019-05-09 11:53 ` Burakov, Anatoly
@ 2019-05-09 18:51 ` Erik Gabriel Carrillo
  2019-05-09 18:51   ` Erik Gabriel Carrillo
                     ` (2 more replies)
  3 siblings, 3 replies; 34+ messages in thread
From: Erik Gabriel Carrillo @ 2019-05-09 18:51 UTC (permalink / raw)
  To: thomas
  Cc: stephen, david.marchand, anatoly.burakov, bruce.richardson, mdr, dev

It is planned to make the rte_mem_config struct of the EAL private to
remove it from the visible ABI.  Add a notice to announce the intention.

Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
---
changes in v2:
 - Original deprecation notice announced a change to the rte_mem_config
   struct that would break ABI.  Update the notice to instead announce
   the struct will be made private.  (Stephen, Anatoly, David)

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

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b47c8c2..a7dff6b 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -31,6 +31,9 @@ Deprecation Notices
 
     + ``rte_eal_devargs_type_count``
 
+* eal: the ``rte_mem_config`` struct will be made private to remove it from the
+  externally visible ABI and allow it to be updated in the future.
+
 * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
   have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
   functions.  The due date for the removal targets DPDK 20.02.
-- 
2.6.4

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
@ 2019-05-09 18:51   ` Erik Gabriel Carrillo
  2019-05-10  9:31   ` Burakov, Anatoly
  2019-05-10 13:44   ` David Marchand
  2 siblings, 0 replies; 34+ messages in thread
From: Erik Gabriel Carrillo @ 2019-05-09 18:51 UTC (permalink / raw)
  To: thomas
  Cc: stephen, david.marchand, anatoly.burakov, bruce.richardson, mdr, dev

It is planned to make the rte_mem_config struct of the EAL private to
remove it from the visible ABI.  Add a notice to announce the intention.

Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
---
changes in v2:
 - Original deprecation notice announced a change to the rte_mem_config
   struct that would break ABI.  Update the notice to instead announce
   the struct will be made private.  (Stephen, Anatoly, David)

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

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index b47c8c2..a7dff6b 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -31,6 +31,9 @@ Deprecation Notices
 
     + ``rte_eal_devargs_type_count``
 
+* eal: the ``rte_mem_config`` struct will be made private to remove it from the
+  externally visible ABI and allow it to be updated in the future.
+
 * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
   have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
   functions.  The due date for the removal targets DPDK 20.02.
-- 
2.6.4


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09 10:08               ` Burakov, Anatoly
  2019-05-09 10:08                 ` Burakov, Anatoly
@ 2019-05-09 19:02                 ` Stephen Hemminger
  2019-05-09 19:02                   ` Stephen Hemminger
  2019-05-10 14:42                 ` Stephen Hemminger
  2 siblings, 1 reply; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-09 19:02 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Ray Kinsella, Thomas Monjalon, Bruce Richardson, David Marchand,
	Erik Gabriel Carrillo, dev

On Thu, 9 May 2019 11:08:30 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> >>> If the community agrees that primary/secondary processes should always
> >>> use the same DPDK version (regardless of static/dynamic builds etc.),
> >>> then this problem would probably be solved.  
> >>
> >> +1 to document that primary/secondary with different DPDK versions
> >> is not supported.
> >>  
> > 
> > +1,
> > 
> > but I think we need to go farther - we need a secondary process to check
> > with the primary process.
> > We can't assume everyone will read the documentation.
> >   
> 
> That easily can be done, yes.

Agree with the consensus that primary/secondary must be on the same version.
Think even driver internal data structures have to be compatiable.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09 19:02                 ` Stephen Hemminger
@ 2019-05-09 19:02                   ` Stephen Hemminger
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-09 19:02 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Ray Kinsella, Thomas Monjalon, Bruce Richardson, David Marchand,
	Erik Gabriel Carrillo, dev

On Thu, 9 May 2019 11:08:30 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> >>> If the community agrees that primary/secondary processes should always
> >>> use the same DPDK version (regardless of static/dynamic builds etc.),
> >>> then this problem would probably be solved.  
> >>
> >> +1 to document that primary/secondary with different DPDK versions
> >> is not supported.
> >>  
> > 
> > +1,
> > 
> > but I think we need to go farther - we need a secondary process to check
> > with the primary process.
> > We can't assume everyone will read the documentation.
> >   
> 
> That easily can be done, yes.

Agree with the consensus that primary/secondary must be on the same version.
Think even driver internal data structures have to be compatiable.


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
  2019-05-09 18:51   ` Erik Gabriel Carrillo
@ 2019-05-10  9:31   ` Burakov, Anatoly
  2019-05-10  9:31     ` Burakov, Anatoly
  2019-05-10  9:34     ` Bruce Richardson
  2019-05-10 13:44   ` David Marchand
  2 siblings, 2 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-10  9:31 UTC (permalink / raw)
  To: Erik Gabriel Carrillo, thomas
  Cc: stephen, david.marchand, bruce.richardson, mdr, dev

On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> It is planned to make the rte_mem_config struct of the EAL private to
> remove it from the visible ABI.  Add a notice to announce the intention.
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> changes in v2:
>   - Original deprecation notice announced a change to the rte_mem_config
>     struct that would break ABI.  Update the notice to instead announce
>     the struct will be made private.  (Stephen, Anatoly, David)
> 
>   doc/guides/rel_notes/deprecation.rst | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index b47c8c2..a7dff6b 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,9 @@ Deprecation Notices
>   
>       + ``rte_eal_devargs_type_count``
>   
> +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> +  externally visible ABI and allow it to be updated in the future.
> +
>   * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
>     have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>     functions.  The due date for the removal targets DPDK 20.02.
> 

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-10  9:31   ` Burakov, Anatoly
@ 2019-05-10  9:31     ` Burakov, Anatoly
  2019-05-10  9:34     ` Bruce Richardson
  1 sibling, 0 replies; 34+ messages in thread
From: Burakov, Anatoly @ 2019-05-10  9:31 UTC (permalink / raw)
  To: Erik Gabriel Carrillo, thomas
  Cc: stephen, david.marchand, bruce.richardson, mdr, dev

On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> It is planned to make the rte_mem_config struct of the EAL private to
> remove it from the visible ABI.  Add a notice to announce the intention.
> 
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> changes in v2:
>   - Original deprecation notice announced a change to the rte_mem_config
>     struct that would break ABI.  Update the notice to instead announce
>     the struct will be made private.  (Stephen, Anatoly, David)
> 
>   doc/guides/rel_notes/deprecation.rst | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index b47c8c2..a7dff6b 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,9 @@ Deprecation Notices
>   
>       + ``rte_eal_devargs_type_count``
>   
> +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> +  externally visible ABI and allow it to be updated in the future.
> +
>   * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
>     have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>     functions.  The due date for the removal targets DPDK 20.02.
> 

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-10  9:31   ` Burakov, Anatoly
  2019-05-10  9:31     ` Burakov, Anatoly
@ 2019-05-10  9:34     ` Bruce Richardson
  2019-05-10  9:34       ` Bruce Richardson
  2019-05-13 21:03       ` Thomas Monjalon
  1 sibling, 2 replies; 34+ messages in thread
From: Bruce Richardson @ 2019-05-10  9:34 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Erik Gabriel Carrillo, thomas, stephen, david.marchand, mdr, dev

On Fri, May 10, 2019 at 10:31:21AM +0100, Burakov, Anatoly wrote:
> On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> > It is planned to make the rte_mem_config struct of the EAL private to
> > remove it from the visible ABI.  Add a notice to announce the intention.
> > 
> > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> > ---
> > changes in v2:
> >   - Original deprecation notice announced a change to the rte_mem_config
> >     struct that would break ABI.  Update the notice to instead announce
> >     the struct will be made private.  (Stephen, Anatoly, David)
> > 
> >   doc/guides/rel_notes/deprecation.rst | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index b47c8c2..a7dff6b 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -31,6 +31,9 @@ Deprecation Notices
> >       + ``rte_eal_devargs_type_count``
> > +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> > +  externally visible ABI and allow it to be updated in the future.
> > +
> >   * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
> >     have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
> >     functions.  The due date for the removal targets DPDK 20.02.
> > 
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-10  9:34     ` Bruce Richardson
@ 2019-05-10  9:34       ` Bruce Richardson
  2019-05-13 21:03       ` Thomas Monjalon
  1 sibling, 0 replies; 34+ messages in thread
From: Bruce Richardson @ 2019-05-10  9:34 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Erik Gabriel Carrillo, thomas, stephen, david.marchand, mdr, dev

On Fri, May 10, 2019 at 10:31:21AM +0100, Burakov, Anatoly wrote:
> On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> > It is planned to make the rte_mem_config struct of the EAL private to
> > remove it from the visible ABI.  Add a notice to announce the intention.
> > 
> > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> > ---
> > changes in v2:
> >   - Original deprecation notice announced a change to the rte_mem_config
> >     struct that would break ABI.  Update the notice to instead announce
> >     the struct will be made private.  (Stephen, Anatoly, David)
> > 
> >   doc/guides/rel_notes/deprecation.rst | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index b47c8c2..a7dff6b 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -31,6 +31,9 @@ Deprecation Notices
> >       + ``rte_eal_devargs_type_count``
> > +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> > +  externally visible ABI and allow it to be updated in the future.
> > +
> >   * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs which
> >     have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
> >     functions.  The due date for the removal targets DPDK 20.02.
> > 
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
  2019-05-09 18:51   ` Erik Gabriel Carrillo
  2019-05-10  9:31   ` Burakov, Anatoly
@ 2019-05-10 13:44   ` David Marchand
  2019-05-10 13:44     ` David Marchand
  2 siblings, 1 reply; 34+ messages in thread
From: David Marchand @ 2019-05-10 13:44 UTC (permalink / raw)
  To: Erik Gabriel Carrillo
  Cc: Thomas Monjalon, Stephen Hemminger, Burakov, Anatoly,
	Bruce Richardson, Ray Kinsella, dev, mw, mk, gtzalik, evgenys

On Thu, May 9, 2019 at 8:53 PM Erik Gabriel Carrillo <
erik.g.carrillo@intel.com> wrote:

> It is planned to make the rte_mem_config struct of the EAL private to
> remove it from the visible ABI.  Add a notice to announce the intention.
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> changes in v2:
>  - Original deprecation notice announced a change to the rte_mem_config
>    struct that would break ABI.  Update the notice to instead announce
>    the struct will be made private.  (Stephen, Anatoly, David)
>
>  doc/guides/rel_notes/deprecation.rst | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index b47c8c2..a7dff6b 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,9 @@ Deprecation Notices
>
>      + ``rte_eal_devargs_type_count``
>
> +* eal: the ``rte_mem_config`` struct will be made private to remove it
> from the
> +  externally visible ABI and allow it to be updated in the future.
> +
>  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs
> which
>    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>    functions.  The due date for the removal targets DPDK 20.02.
> --
> 2.6.4
>
>
I had a look at the current callers, trying to see what "hiding" means.

$ git grep -Elw '(rte_mem_config|mem_config)' origin/master --
origin/master:app/test/test_memzone.c
origin/master:doc/guides/rel_notes/release_18_11.rst
origin/master:drivers/bus/fslmc/fslmc_vfio.c
origin/master:drivers/net/ena/ena_ethdev.c
origin/master:drivers/net/mlx4/mlx4_mr.c
origin/master:drivers/net/mlx5/mlx5_mr.c
origin/master:drivers/net/virtio/virtio_user/virtio_user_dev.c
origin/master:lib/librte_eal/common/eal_common_memory.c
origin/master:lib/librte_eal/common/eal_common_memzone.c
origin/master:lib/librte_eal/common/eal_common_tailqs.c
origin/master:lib/librte_eal/common/include/rte_eal.h
origin/master:lib/librte_eal/common/include/rte_eal_memconfig.h
origin/master:lib/librte_eal/common/malloc_heap.c
origin/master:lib/librte_eal/common/rte_malloc.c
origin/master:lib/librte_eal/freebsd/eal/eal.c
origin/master:lib/librte_eal/freebsd/eal/eal_memory.c
origin/master:lib/librte_eal/linux/eal/eal.c
origin/master:lib/librte_eal/linux/eal/eal_memalloc.c
origin/master:lib/librte_eal/linux/eal/eal_memory.c
origin/master:lib/librte_eal/linux/eal/eal_vfio.c

- I understand that the following drivers will need a new lock/unlock api
to protect calls to the memory api.
drivers/bus/fslmc/fslmc_vfio.c
drivers/net/mlx4/mlx4_mr.c
drivers/net/mlx5/mlx5_mr.c
drivers/net/virtio/virtio_user/virtio_user_dev.c

- I have a little trouble with this one, so there might be a new api for
this driver usecase.
A discussion will have to happen with its maintainers (added in CC:).
drivers/net/ena/ena_ethdev.c


Apart from this, I am ok with the objective, which is to hide one more eal
internal structure.

Acked-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-10 13:44   ` David Marchand
@ 2019-05-10 13:44     ` David Marchand
  0 siblings, 0 replies; 34+ messages in thread
From: David Marchand @ 2019-05-10 13:44 UTC (permalink / raw)
  To: Erik Gabriel Carrillo
  Cc: Thomas Monjalon, Stephen Hemminger, Burakov, Anatoly,
	Bruce Richardson, Ray Kinsella, dev, mw, mk, gtzalik, evgenys

On Thu, May 9, 2019 at 8:53 PM Erik Gabriel Carrillo <
erik.g.carrillo@intel.com> wrote:

> It is planned to make the rte_mem_config struct of the EAL private to
> remove it from the visible ABI.  Add a notice to announce the intention.
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> changes in v2:
>  - Original deprecation notice announced a change to the rte_mem_config
>    struct that would break ABI.  Update the notice to instead announce
>    the struct will be made private.  (Stephen, Anatoly, David)
>
>  doc/guides/rel_notes/deprecation.rst | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index b47c8c2..a7dff6b 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -31,6 +31,9 @@ Deprecation Notices
>
>      + ``rte_eal_devargs_type_count``
>
> +* eal: the ``rte_mem_config`` struct will be made private to remove it
> from the
> +  externally visible ABI and allow it to be updated in the future.
> +
>  * vfio: removal of ``rte_vfio_dma_map`` and ``rte_vfio_dma_unmap`` APIs
> which
>    have been replaced with ``rte_dev_dma_map`` and ``rte_dev_dma_unmap``
>    functions.  The due date for the removal targets DPDK 20.02.
> --
> 2.6.4
>
>
I had a look at the current callers, trying to see what "hiding" means.

$ git grep -Elw '(rte_mem_config|mem_config)' origin/master --
origin/master:app/test/test_memzone.c
origin/master:doc/guides/rel_notes/release_18_11.rst
origin/master:drivers/bus/fslmc/fslmc_vfio.c
origin/master:drivers/net/ena/ena_ethdev.c
origin/master:drivers/net/mlx4/mlx4_mr.c
origin/master:drivers/net/mlx5/mlx5_mr.c
origin/master:drivers/net/virtio/virtio_user/virtio_user_dev.c
origin/master:lib/librte_eal/common/eal_common_memory.c
origin/master:lib/librte_eal/common/eal_common_memzone.c
origin/master:lib/librte_eal/common/eal_common_tailqs.c
origin/master:lib/librte_eal/common/include/rte_eal.h
origin/master:lib/librte_eal/common/include/rte_eal_memconfig.h
origin/master:lib/librte_eal/common/malloc_heap.c
origin/master:lib/librte_eal/common/rte_malloc.c
origin/master:lib/librte_eal/freebsd/eal/eal.c
origin/master:lib/librte_eal/freebsd/eal/eal_memory.c
origin/master:lib/librte_eal/linux/eal/eal.c
origin/master:lib/librte_eal/linux/eal/eal_memalloc.c
origin/master:lib/librte_eal/linux/eal/eal_memory.c
origin/master:lib/librte_eal/linux/eal/eal_vfio.c

- I understand that the following drivers will need a new lock/unlock api
to protect calls to the memory api.
drivers/bus/fslmc/fslmc_vfio.c
drivers/net/mlx4/mlx4_mr.c
drivers/net/mlx5/mlx5_mr.c
drivers/net/virtio/virtio_user/virtio_user_dev.c

- I have a little trouble with this one, so there might be a new api for
this driver usecase.
A discussion will have to happen with its maintainers (added in CC:).
drivers/net/ena/ena_ethdev.c


Apart from this, I am ok with the objective, which is to hide one more eal
internal structure.

Acked-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-09 10:08               ` Burakov, Anatoly
  2019-05-09 10:08                 ` Burakov, Anatoly
  2019-05-09 19:02                 ` Stephen Hemminger
@ 2019-05-10 14:42                 ` Stephen Hemminger
  2019-05-10 14:42                   ` Stephen Hemminger
  2 siblings, 1 reply; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-10 14:42 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Ray Kinsella, Thomas Monjalon, Bruce Richardson, David Marchand,
	Erik Gabriel Carrillo, dev

On Thu, 9 May 2019 11:08:30 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 09-May-19 10:50 AM, Ray Kinsella wrote:
> > 
> > 
> > On 09/05/2019 10:38, Thomas Monjalon wrote:  
> >> 09/05/2019 11:37, Burakov, Anatoly:  
> >>> On 09-May-19 10:06 AM, Bruce Richardson wrote:  
> >>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:  
> >>>>> On 09-May-19 8:05 AM, David Marchand wrote:  
> >>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> >>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> >>>>>>
> >>>>>>       On Wed,  8 May 2019 17:48:06 -0500
> >>>>>>       Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>>>>       <mailto:erik.g.carrillo@intel.com>> wrote:
> >>>>>>  
> >>>>>>        > Due to an upcoming fix to allow the timer library to safely free its
> >>>>>>        > allocations during the finalize() call[1], an ABI change will be
> >>>>>>        > required. A new lock will be added to the rte_mem_config structure,
> >>>>>>        > which will be used by the timer library to synchronize init/finalize
> >>>>>>        > calls among multiple processes.
> >>>>>>        >
> >>>>>>        > [1] http://patches.dpdk.org/patch/53334/
> >>>>>>        >
> >>>>>>        > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com  
> >>>>>>       <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >>>>>>        > --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>        > +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>        > @@ -31,6 +31,10 @@ Deprecation Notices
> >>>>>>        >
> >>>>>>        >      + ``rte_eal_devargs_type_count``
> >>>>>>        >
> >>>>>>        > +* eal: the ``rte_mem_config`` struct will change to include a  
> >>>>>>       new lock that  
> >>>>>>        > +  will allow the timer subsystem to safely release its  
> >>>>>>       allocations at cleanup  
> >>>>>>        > +  time. This will result in an ABI break.
> >>>>>>        > +
> >>>>>>        >  * vfio: removal of ``rte_vfio_dma_map`` and  
> >>>>>>       ``rte_vfio_dma_unmap`` APIs which  
> >>>>>>        >    have been replaced with ``rte_dev_dma_map`` and  
> >>>>>>       ``rte_dev_dma_unmap``  
> >>>>>>        >    functions.  The due date for the removal targets DPDK 20.02.  
> >>>>>>
> >>>>>>       NAK
> >>>>>>
> >>>>>>       Please go to the effort of making rte_mem_config not part of the
> >>>>>>       visible ABI.
> >>>>>>       Then change it.
> >>>>>>
> >>>>>>
> >>>>>> +1.  
> >>>>>
> >>>>> I agree on principle, however this won't solve the issue. It doesn't need to
> >>>>> be externally visible, but that's not all of its problems - it's also shared
> >>>>> between processes so there's an ABI contract between primary and secondary
> >>>>> too. This means that, even if the structure itself is not public, any
> >>>>> changes to it will still result in an ABI break. That's the nature of our
> >>>>> shared memory.
> >>>>>
> >>>>> In other words, if your goal is to avoid ABI breaks on changing this
> >>>>> structure, making it internal won't help in the slightest.
> >>>>>  
> >>>>
> >>>> Is there an ABI contract between primary and secondary. I always assumed
> >>>> that if using secondary processes the requirement (though undocumented) was
> >>>> that both had to be linked against the exact same versions of DPDK?
> >>>>  
> >>>
> >>> The fact that it's undocumented means we can't assume everyone will do
> >>> that :)
> >>>
> >>> If the community agrees that primary/secondary processes should always
> >>> use the same DPDK version (regardless of static/dynamic builds etc.),
> >>> then this problem would probably be solved.  
> >>
> >> +1 to document that primary/secondary with different DPDK versions
> >> is not supported.
> >>  
> > 
> > +1,
> > 
> > but I think we need to go farther - we need a secondary process to check
> > with the primary process.
> > We can't assume everyone will read the documentation.
> >   
> 
> That easily can be done, yes.
> 

FYI - I submitted patches to make lcore_config private.
These patches should handle mem_config.
And I started on making eal_config private (but mem_config got in the way).

Other candidates are the internals behind ethdev and devices.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup
  2019-05-10 14:42                 ` Stephen Hemminger
@ 2019-05-10 14:42                   ` Stephen Hemminger
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Hemminger @ 2019-05-10 14:42 UTC (permalink / raw)
  To: Burakov, Anatoly
  Cc: Ray Kinsella, Thomas Monjalon, Bruce Richardson, David Marchand,
	Erik Gabriel Carrillo, dev

On Thu, 9 May 2019 11:08:30 +0100
"Burakov, Anatoly" <anatoly.burakov@intel.com> wrote:

> On 09-May-19 10:50 AM, Ray Kinsella wrote:
> > 
> > 
> > On 09/05/2019 10:38, Thomas Monjalon wrote:  
> >> 09/05/2019 11:37, Burakov, Anatoly:  
> >>> On 09-May-19 10:06 AM, Bruce Richardson wrote:  
> >>>> On Thu, May 09, 2019 at 09:33:32AM +0100, Burakov, Anatoly wrote:  
> >>>>> On 09-May-19 8:05 AM, David Marchand wrote:  
> >>>>>> On Thu, May 9, 2019 at 3:11 AM Stephen Hemminger
> >>>>>> <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> wrote:
> >>>>>>
> >>>>>>       On Wed,  8 May 2019 17:48:06 -0500
> >>>>>>       Erik Gabriel Carrillo <erik.g.carrillo@intel.com
> >>>>>>       <mailto:erik.g.carrillo@intel.com>> wrote:
> >>>>>>  
> >>>>>>        > Due to an upcoming fix to allow the timer library to safely free its
> >>>>>>        > allocations during the finalize() call[1], an ABI change will be
> >>>>>>        > required. A new lock will be added to the rte_mem_config structure,
> >>>>>>        > which will be used by the timer library to synchronize init/finalize
> >>>>>>        > calls among multiple processes.
> >>>>>>        >
> >>>>>>        > [1] http://patches.dpdk.org/patch/53334/
> >>>>>>        >
> >>>>>>        > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com  
> >>>>>>       <mailto:erik.g.carrillo@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 b47c8c2..7551383 100644
> >>>>>>        > --- a/doc/guides/rel_notes/deprecation.rst
> >>>>>>        > +++ b/doc/guides/rel_notes/deprecation.rst
> >>>>>>        > @@ -31,6 +31,10 @@ Deprecation Notices
> >>>>>>        >
> >>>>>>        >      + ``rte_eal_devargs_type_count``
> >>>>>>        >
> >>>>>>        > +* eal: the ``rte_mem_config`` struct will change to include a  
> >>>>>>       new lock that  
> >>>>>>        > +  will allow the timer subsystem to safely release its  
> >>>>>>       allocations at cleanup  
> >>>>>>        > +  time. This will result in an ABI break.
> >>>>>>        > +
> >>>>>>        >  * vfio: removal of ``rte_vfio_dma_map`` and  
> >>>>>>       ``rte_vfio_dma_unmap`` APIs which  
> >>>>>>        >    have been replaced with ``rte_dev_dma_map`` and  
> >>>>>>       ``rte_dev_dma_unmap``  
> >>>>>>        >    functions.  The due date for the removal targets DPDK 20.02.  
> >>>>>>
> >>>>>>       NAK
> >>>>>>
> >>>>>>       Please go to the effort of making rte_mem_config not part of the
> >>>>>>       visible ABI.
> >>>>>>       Then change it.
> >>>>>>
> >>>>>>
> >>>>>> +1.  
> >>>>>
> >>>>> I agree on principle, however this won't solve the issue. It doesn't need to
> >>>>> be externally visible, but that's not all of its problems - it's also shared
> >>>>> between processes so there's an ABI contract between primary and secondary
> >>>>> too. This means that, even if the structure itself is not public, any
> >>>>> changes to it will still result in an ABI break. That's the nature of our
> >>>>> shared memory.
> >>>>>
> >>>>> In other words, if your goal is to avoid ABI breaks on changing this
> >>>>> structure, making it internal won't help in the slightest.
> >>>>>  
> >>>>
> >>>> Is there an ABI contract between primary and secondary. I always assumed
> >>>> that if using secondary processes the requirement (though undocumented) was
> >>>> that both had to be linked against the exact same versions of DPDK?
> >>>>  
> >>>
> >>> The fact that it's undocumented means we can't assume everyone will do
> >>> that :)
> >>>
> >>> If the community agrees that primary/secondary processes should always
> >>> use the same DPDK version (regardless of static/dynamic builds etc.),
> >>> then this problem would probably be solved.  
> >>
> >> +1 to document that primary/secondary with different DPDK versions
> >> is not supported.
> >>  
> > 
> > +1,
> > 
> > but I think we need to go farther - we need a secondary process to check
> > with the primary process.
> > We can't assume everyone will read the documentation.
> >   
> 
> That easily can be done, yes.
> 

FYI - I submitted patches to make lcore_config private.
These patches should handle mem_config.
And I started on making eal_config private (but mem_config got in the way).

Other candidates are the internals behind ethdev and devices.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-10  9:34     ` Bruce Richardson
  2019-05-10  9:34       ` Bruce Richardson
@ 2019-05-13 21:03       ` Thomas Monjalon
  2019-05-13 21:03         ` Thomas Monjalon
  1 sibling, 1 reply; 34+ messages in thread
From: Thomas Monjalon @ 2019-05-13 21:03 UTC (permalink / raw)
  To: Erik Gabriel Carrillo
  Cc: dev, Bruce Richardson, Burakov, Anatoly, stephen, david.marchand, mdr

10/05/2019 11:34, Bruce Richardson:
> On Fri, May 10, 2019 at 10:31:21AM +0100, Burakov, Anatoly wrote:
> > On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> > > It is planned to make the rte_mem_config struct of the EAL private to
> > > remove it from the visible ABI.  Add a notice to announce the intention.
> > > 
> > > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> > > ---
> > > changes in v2:
> > >   - Original deprecation notice announced a change to the rte_mem_config
> > >     struct that would break ABI.  Update the notice to instead announce
> > >     the struct will be made private.  (Stephen, Anatoly, David)
> > > 
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> > > +  externally visible ABI and allow it to be updated in the future.
> > 
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config
  2019-05-13 21:03       ` Thomas Monjalon
@ 2019-05-13 21:03         ` Thomas Monjalon
  0 siblings, 0 replies; 34+ messages in thread
From: Thomas Monjalon @ 2019-05-13 21:03 UTC (permalink / raw)
  To: Erik Gabriel Carrillo
  Cc: dev, Bruce Richardson, Burakov, Anatoly, stephen, david.marchand, mdr

10/05/2019 11:34, Bruce Richardson:
> On Fri, May 10, 2019 at 10:31:21AM +0100, Burakov, Anatoly wrote:
> > On 09-May-19 7:51 PM, Erik Gabriel Carrillo wrote:
> > > It is planned to make the rte_mem_config struct of the EAL private to
> > > remove it from the visible ABI.  Add a notice to announce the intention.
> > > 
> > > Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> > > ---
> > > changes in v2:
> > >   - Original deprecation notice announced a change to the rte_mem_config
> > >     struct that would break ABI.  Update the notice to instead announce
> > >     the struct will be made private.  (Stephen, Anatoly, David)
> > > 
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > +* eal: the ``rte_mem_config`` struct will be made private to remove it from the
> > > +  externally visible ABI and allow it to be updated in the future.
> > 
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: David Marchand <david.marchand@redhat.com>

Applied, thanks



^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2019-05-13 21:03 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-08 22:48 [dpdk-dev] [PATCH] doc: add deprecation notice on timer lib cleanup Erik Gabriel Carrillo
2019-05-08 22:48 ` Erik Gabriel Carrillo
2019-05-09  1:11 ` Stephen Hemminger
2019-05-09  1:11   ` Stephen Hemminger
2019-05-09  7:05   ` David Marchand
2019-05-09  7:05     ` David Marchand
2019-05-09  8:33     ` Burakov, Anatoly
2019-05-09  8:33       ` Burakov, Anatoly
2019-05-09  9:06       ` Bruce Richardson
2019-05-09  9:06         ` Bruce Richardson
2019-05-09  9:37         ` Burakov, Anatoly
2019-05-09  9:37           ` Burakov, Anatoly
2019-05-09  9:38           ` Thomas Monjalon
2019-05-09  9:38             ` Thomas Monjalon
2019-05-09  9:50             ` Ray Kinsella
2019-05-09  9:50               ` Ray Kinsella
2019-05-09 10:08               ` Burakov, Anatoly
2019-05-09 10:08                 ` Burakov, Anatoly
2019-05-09 19:02                 ` Stephen Hemminger
2019-05-09 19:02                   ` Stephen Hemminger
2019-05-10 14:42                 ` Stephen Hemminger
2019-05-10 14:42                   ` Stephen Hemminger
2019-05-09 11:53 ` Burakov, Anatoly
2019-05-09 11:53   ` Burakov, Anatoly
2019-05-09 18:51 ` [dpdk-dev] [PATCH v2] doc: add deprecation notice on EAL mem config Erik Gabriel Carrillo
2019-05-09 18:51   ` Erik Gabriel Carrillo
2019-05-10  9:31   ` Burakov, Anatoly
2019-05-10  9:31     ` Burakov, Anatoly
2019-05-10  9:34     ` Bruce Richardson
2019-05-10  9:34       ` Bruce Richardson
2019-05-13 21:03       ` Thomas Monjalon
2019-05-13 21:03         ` Thomas Monjalon
2019-05-10 13:44   ` David Marchand
2019-05-10 13:44     ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).