[dpdk-dev,v2,18/18] doc: remove devargs deprecation notices

Message ID c500984327843ad210a7356686288af8f8f5c483.1507796100.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply patch file failure

Commit Message

Gaëtan Rivet Oct. 12, 2017, 8:21 a.m. UTC
  These actions have been enacted.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 doc/guides/rel_notes/deprecation.rst | 13 -------------
 1 file changed, 13 deletions(-)
  

Comments

John McNamara Oct. 12, 2017, 11:28 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaetan Rivet
> Sent: Thursday, October 12, 2017 9:21 AM
> To: dev@dpdk.org
> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
> Subject: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation
> notices
> 
> These actions have been enacted.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Acked-by: John McNamara <john.mcnamara@intel.com>
  
Shreyansh Jain Dec. 13, 2017, 10:17 a.m. UTC | #2
Hello Gaetan,

On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> These actions have been enacted.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>   doc/guides/rel_notes/deprecation.rst | 13 -------------
>   1 file changed, 13 deletions(-)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ef2264f..23faa19 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -16,19 +16,6 @@ Deprecation Notices
>     - ``rte_set_log_type``, replaced by ``rte_log_set_level``
>     - ``rte_get_log_type``, replaced by ``rte_log_get_level``
>   
> -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
> -  The format of device command line parameters will change. The bus will need
> -  to be explicitly stated in the device declaration. The enum ``rte_devtype``
> -  was used to identify a bus and will disappear.
> -  The structure ``rte_devargs`` will change.
> -  The ``rte_devargs_list`` will be made private.
> -  The following functions are deprecated starting from 17.08 and will either be
> -  modified or removed in 17.11:
> -
> -  - ``rte_eal_devargs_add``
> -  - ``rte_eal_devargs_type_count``
> -  - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> -
>   * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
>     translation scheme.
>     Reference to phys address in EAL data-structure or functions may change to
> 

Once this patch is formalized, the documentation reference for 
rte_devargs.h also needs to be changed as it still refers to RTE devargs as:

"...These devices can be PCI devices or virtual devices....".

Similarly, the rte_devargs_parse too has PCI traces.

Next step would be to remove the "pci"/"vdev" reference from 
rte_eal_dev_attach.

Former can be part of this series, but the later needs to be a separate 
patch, I think. Let me know if you want me to work on these (or later).

Other than that, I think I am OK with overall patch. If you can push the 
final series (I am not sure it would be with or without bus control), I 
can give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
  
Gaëtan Rivet Dec. 13, 2017, 10:25 a.m. UTC | #3
Hello Shreyansh,

On Wed, Dec 13, 2017 at 03:47:04PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
> 
> On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> > These actions have been enacted.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >   doc/guides/rel_notes/deprecation.rst | 13 -------------
> >   1 file changed, 13 deletions(-)
> > 
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index ef2264f..23faa19 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -16,19 +16,6 @@ Deprecation Notices
> >     - ``rte_set_log_type``, replaced by ``rte_log_set_level``
> >     - ``rte_get_log_type``, replaced by ``rte_log_get_level``
> > -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
> > -  The format of device command line parameters will change. The bus will need
> > -  to be explicitly stated in the device declaration. The enum ``rte_devtype``
> > -  was used to identify a bus and will disappear.
> > -  The structure ``rte_devargs`` will change.
> > -  The ``rte_devargs_list`` will be made private.
> > -  The following functions are deprecated starting from 17.08 and will either be
> > -  modified or removed in 17.11:
> > -
> > -  - ``rte_eal_devargs_add``
> > -  - ``rte_eal_devargs_type_count``
> > -  - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> > -
> >   * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
> >     translation scheme.
> >     Reference to phys address in EAL data-structure or functions may change to
> > 
> 
> Once this patch is formalized, the documentation reference for rte_devargs.h
> also needs to be changed as it still refers to RTE devargs as:
> 
> "...These devices can be PCI devices or virtual devices....".
> 
> Similarly, the rte_devargs_parse too has PCI traces.
> 
> Next step would be to remove the "pci"/"vdev" reference from
> rte_eal_dev_attach.
> 

Noted, thanks.

> Former can be part of this series, but the later needs to be a separate
> patch, I think. Let me know if you want me to work on these (or later).
> 
> Other than that, I think I am OK with overall patch. If you can push the
> final series (I am not sure it would be with or without bus control), I can
> give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).

Indeed, I also think everything should be settled first.
I have mostly finished working on this series yesterday,
I will integrate your above remarks which will be short.

(Well, by finished I mean I finished the first 90%. The other 90% is
still in progress...)

I removed the rte_devargs unit test and am not too happy about it. There
are parsing functions there, which are extremely error-prone and would
like to have at least the basis for some tests, that we could populate
as we go. If I have the courage I will try to write it and send it with
this series.

I would certainly appreciate if you are able to fix the pci / vdev
limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
with work (trying to finish a lot of things before the holidays).

Thanks for the review!
  
Shreyansh Jain Dec. 13, 2017, 10:54 a.m. UTC | #4
Hello Gaetan,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, December 13, 2017 3:56 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
> 
> Hello Shreyansh,
> 
> On Wed, Dec 13, 2017 at 03:47:04PM +0530, Shreyansh Jain wrote:
> > Hello Gaetan,
> >
> > On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> > > These actions have been enacted.
> > >
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > ---
> > >   doc/guides/rel_notes/deprecation.rst | 13 -------------
> > >   1 file changed, 13 deletions(-)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > > index ef2264f..23faa19 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -16,19 +16,6 @@ Deprecation Notices
> > >     - ``rte_set_log_type``, replaced by ``rte_log_set_level``
> > >     - ``rte_get_log_type``, replaced by ``rte_log_get_level``
> > > -* eal: several API and ABI changes are planned for ``rte_devargs`` in
> v17.11.
> > > -  The format of device command line parameters will change. The bus will
> need
> > > -  to be explicitly stated in the device declaration. The enum
> ``rte_devtype``
> > > -  was used to identify a bus and will disappear.
> > > -  The structure ``rte_devargs`` will change.
> > > -  The ``rte_devargs_list`` will be made private.
> > > -  The following functions are deprecated starting from 17.08 and will
> either be
> > > -  modified or removed in 17.11:
> > > -
> > > -  - ``rte_eal_devargs_add``
> > > -  - ``rte_eal_devargs_type_count``
> > > -  - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> > > -
> > >   * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA
> address
> > >     translation scheme.
> > >     Reference to phys address in EAL data-structure or functions may
> change to
> > >
> >
> > Once this patch is formalized, the documentation reference for
> rte_devargs.h
> > also needs to be changed as it still refers to RTE devargs as:
> >
> > "...These devices can be PCI devices or virtual devices....".
> >
> > Similarly, the rte_devargs_parse too has PCI traces.
> >
> > Next step would be to remove the "pci"/"vdev" reference from
> > rte_eal_dev_attach.
> >
> 
> Noted, thanks.
> 
> > Former can be part of this series, but the later needs to be a separate
> > patch, I think. Let me know if you want me to work on these (or later).
> >
> > Other than that, I think I am OK with overall patch. If you can push the
> > final series (I am not sure it would be with or without bus control), I can
> > give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
> 
> Indeed, I also think everything should be settled first.
> I have mostly finished working on this series yesterday,
> I will integrate your above remarks which will be short.
> 
> (Well, by finished I mean I finished the first 90%. The other 90% is
> still in progress...)
> 
> I removed the rte_devargs unit test and am not too happy about it. There
> are parsing functions there, which are extremely error-prone and would
> like to have at least the basis for some tests, that we could populate
> as we go. If I have the courage I will try to write it and send it with
> this series.
 
While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.

> 
> I would certainly appreciate if you are able to fix the pci / vdev
> limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
> with work (trying to finish a lot of things before the holidays).
 
OK.
Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).

And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.

> 
> Thanks for the review!
> 
> --
> Gaëtan Rivet
> 6WIND
  
Shreyansh Jain Dec. 22, 2017, 4:59 a.m. UTC | #5
Hello Gaetan,

On Wednesday 13 December 2017 04:24 PM, Shreyansh Jain wrote:
> Hello Gaetan,
> 
>> -----Original Message-----
>> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
>> Sent: Wednesday, December 13, 2017 3:56 PM
>> To: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
>>

[...]

>   
> While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
> 
>>
>> I would certainly appreciate if you are able to fix the pci / vdev
>> limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
>> with work (trying to finish a lot of things before the holidays).
>   
> OK.
> Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
> 
> And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.

Have you pushed the new version of the devargs patches?
Just wanted to check in case I have missed it.

-
Shreyansh
  
Gaëtan Rivet Dec. 22, 2017, 8:33 a.m. UTC | #6
Hi Shreyansh,

On Fri, Dec 22, 2017 at 10:29:14AM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
> 
> On Wednesday 13 December 2017 04:24 PM, Shreyansh Jain wrote:
> > Hello Gaetan,
> > 
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > > Sent: Wednesday, December 13, 2017 3:56 PM
> > > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
> > > 
> 
> [...]
> 
> > While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
> > 
> > > 
> > > I would certainly appreciate if you are able to fix the pci / vdev
> > > limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
> > > with work (trying to finish a lot of things before the holidays).
> > OK.
> > Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
> > 
> > And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
> 
> Have you pushed the new version of the devargs patches?
> Just wanted to check in case I have missed it.
> 

No sorry,

I am removing the dependency on the bus control framework from the
devargs patchset.

I finished the patchset otherwise (redid the unit test), but after some
thinking I found that the bus control was maybe not ideal.

I will send the devargs patchset soon, once it is made independent, and
will expose the issue with the bus control.

Best,
  

Patch

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ef2264f..23faa19 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -16,19 +16,6 @@  Deprecation Notices
   - ``rte_set_log_type``, replaced by ``rte_log_set_level``
   - ``rte_get_log_type``, replaced by ``rte_log_get_level``
 
-* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
-  The format of device command line parameters will change. The bus will need
-  to be explicitly stated in the device declaration. The enum ``rte_devtype``
-  was used to identify a bus and will disappear.
-  The structure ``rte_devargs`` will change.
-  The ``rte_devargs_list`` will be made private.
-  The following functions are deprecated starting from 17.08 and will either be
-  modified or removed in 17.11:
-
-  - ``rte_eal_devargs_add``
-  - ``rte_eal_devargs_type_count``
-  - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
-
 * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
   translation scheme.
   Reference to phys address in EAL data-structure or functions may change to