[spp] [PATCH 0/6] Replace deprecated APIs

[spp] [PATCH 0/6] Replace deprecated APIs

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

>From DPDK-18.08, the follwing APIs become deprecated and 
will be deleted in DPDK18.08.
- rte_eth_dev_attach()
- rte_eth_dev_detach()

For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.

To follow the above changes, this patch set provides replacement of 
those APIs. 

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>

Hideyuki Yamashita (6):
  shared: addition of attach()
  spp_nfv: replacement of rte_eth_dev_attach()
  spp_vf:replacement of rte_eth_dev_attach()
  shared: addition of detach()
  spp_nfv: replacement of rte_eth_dev_detach()
  spp_vm: replacement of rte_eth_dev_detach().

 src/nfv/nfv.c       | 12 ++++-----
 src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 src/shared/common.h | 32 +++++++++++++++++++++++
 src/vf/spp_vf.c     |  4 +--
 src/vm/main.c       |  2 +-
 5 files changed, 105 insertions(+), 9 deletions(-)

-- 
2.18.0

Re: [spp] [PATCH 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> 
>>From DPDK-18.08, the follwing APIs become deprecated and 
> will be deleted in DPDK18.08.
> - rte_eth_dev_attach()
> - rte_eth_dev_detach()
> 
> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
> 
> To follow the above changes, this patch set provides replacement of
> those APIs.
Hideyuki,

Thank you for suggesting to update to v18.08!

Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just 
`attach` or `detach`. It is preferable to be self explanatory for how your function works.

Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for 
documentation guidelines.

Thanks
> 
> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> 
> Hideyuki Yamashita (6):
>    shared: addition of attach()
>    spp_nfv: replacement of rte_eth_dev_attach()
>    spp_vf:replacement of rte_eth_dev_attach()
>    shared: addition of detach()
>    spp_nfv: replacement of rte_eth_dev_detach()
>    spp_vm: replacement of rte_eth_dev_detach().
> 
>   src/nfv/nfv.c       | 12 ++++-----
>   src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>   src/shared/common.h | 32 +++++++++++++++++++++++
>   src/vf/spp_vf.c     |  4 +--
>   src/vm/main.c       |  2 +-
>   5 files changed, 105 insertions(+), 9 deletions(-)
> 


-- 
Yasufumi Ogawa
NTT Network Service Systems Labs

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Hideyuki Yamashita]

Hello Yasufumi-san,

Thanks so much for your comments.

About your first point, I think the follwing naming may be fit with DPDK
and SPP naming conventions, what do you think?

attach -> spp_eth_dev_attach
detach -> spp_eth_dev_detach

About your second point, I appologize about my mistake.

Once I get ack for above new naming of two interfaces from you,
I will revise my patch set including cover-letter and will send
those to mailing list.

Thanks for your co-opearation!

BR,
Hideyuki Yamashita
NTT TechnoCorss

> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
> > From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> >
> >>From DPDK-18.08, the follwing APIs become deprecated and
> > will be deleted in DPDK18.08.
> > - rte_eth_dev_attach()
> > - rte_eth_dev_detach()
> >
> > For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
> > For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
> >
> > To follow the above changes, this patch set provides replacement of
> > those APIs.
> Hideyuki,
> 
> Thank you for suggesting to update to v18.08!
> 
> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works.
> 
> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines.
> 
> Thanks
> >
> > Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> > Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> >
> > Hideyuki Yamashita (6):
> >    shared: addition of attach()
> >    spp_nfv: replacement of rte_eth_dev_attach()
> >    spp_vf:replacement of rte_eth_dev_attach()
> >    shared: addition of detach()
> >    spp_nfv: replacement of rte_eth_dev_detach()
> >    spp_vm: replacement of rte_eth_dev_detach().
> >
> >   src/nfv/nfv.c       | 12 ++++-----
> >   src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
> >   src/shared/common.h | 32 +++++++++++++++++++++++
> >   src/vf/spp_vf.c     |  4 +--
> >   src/vm/main.c       |  2 +-
> >   5 files changed, 105 insertions(+), 9 deletions(-)
> > 
> 
> -- Yasufumi Ogawa
> NTT Network Service Systems Labs
> 

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

> Hello Yasufumi-san,
>
> Thanks so much for your comments.
>
> About your first point, I think the follwing naming may be fit with DPDK
> and SPP naming conventions, what do you think?
>
> attach -> spp_eth_dev_attach
> detach -> spp_eth_dev_detach
I think you should not use prefix `spp` without if it defines a 
behaviour of SPP itself. This function is more essential to attach device.

For `spp_eth_dev_attach`, You might think similar name from original 
`rte_eth_dev_attach`, but I think we do not keep the name of deprecated 
APIs. More simply, how about `dev_attach_by_devargs` instead of?

For `spp_eth_dev_detach`, I think `dev_detach_by_port_id` is more 
prefer. Other than the name of the function, I am curious why it takes 
second argument `name` because it is not used in the function.

By the way, I am not clear what is the difference between your functions 
and `rte_eth_dev_attach` defined in 
`dpdk/lib/librte_ethdev/rte_ethdev.c` exactly. Could you explain about 
the difference shortly for helping my understanding?

Thanks
> About your second point, I appologize about my mistake.
>
> Once I get ack for above new naming of two interfaces from you,
> I will revise my patch set including cover-letter and will send
> those to mailing list.
>
> Thanks for your co-opearation!
>
> BR,
> Hideyuki Yamashita
> NTT TechnoCorss
>
>> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
>>> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
>>>
>>> >From DPDK-18.08, the follwing APIs become deprecated and
>>> will be deleted in DPDK18.08.
>>> - rte_eth_dev_attach()
>>> - rte_eth_dev_detach()
>>>
>>> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
>>> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
>>>
>>> To follow the above changes, this patch set provides replacement of
>>> those APIs.
>> Hideyuki,
>>
>> Thank you for suggesting to update to v18.08!
>>
>> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works.
>>
>> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines.
>>
>> Thanks
>>> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
>>> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
>>>
>>> Hideyuki Yamashita (6):
>>>     shared: addition of attach()
>>>     spp_nfv: replacement of rte_eth_dev_attach()
>>>     spp_vf:replacement of rte_eth_dev_attach()
>>>     shared: addition of detach()
>>>     spp_nfv: replacement of rte_eth_dev_detach()
>>>     spp_vm: replacement of rte_eth_dev_detach().
>>>
>>>    src/nfv/nfv.c       | 12 ++++-----
>>>    src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>>>    src/shared/common.h | 32 +++++++++++++++++++++++
>>>    src/vf/spp_vf.c     |  4 +--
>>>    src/vm/main.c       |  2 +-
>>>    5 files changed, 105 insertions(+), 9 deletions(-)
>>>
>>
>>

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Hideyuki Yamashita]

Hello Yasufumi-san,

Thanks for your comments.
Please see inline.

> > Hello Yasufumi-san,
> >
> > Thanks so much for your comments.
> >
> > About your first point, I think the follwing naming may be fit with DPDK
> > and SPP naming conventions, what do you think?
> >
> > attach -> spp_eth_dev_attach
> > detach -> spp_eth_dev_detach
> I think you should not use prefix `spp` without if it defines a behaviour of SPP itself. This function is more essential to attach device.
> 
> For `spp_eth_dev_attach`, You might think similar name from original `rte_eth_dev_attach`, but I think we do not keep the name of deprecated APIs. More simply, how about `dev_attach_by_devargs` instead of?
I have no problem about naming `dev_attach_by_devargs`.

> For `spp_eth_dev_detach`, I think `dev_detach_by_port_id` is more prefer. Other than the name of the function, I am curious why it takes second argument `name` because it is not used in the function.
I have no problem about namign  `dev_detach_by_port_id`.
As you say, second argument is NOT used within the function and I will
remove it from revised version.

> By the way, I am not clear what is the difference between your functions and `rte_eth_dev_attach` defined in `dpdk/lib/librte_ethdev/rte_ethdev.c` exactly. Could you explain about the difference shortly for helping my understanding?
Basically both of those functions uses `rte_eal_hotplug_add` to hotplug
device.

Difference is described as following:
`rte_eth_dev_attach` uses global variable named
eth_dev_last_created_port when retrieving newly created dpdk port while
my new function uses `rte_eth_dev_get_port_by_name` becauase this function is defined outside
of dpdk library and can not retrieve global variable within dpdk library.

BR,
Hideyuki Yamashita
NTT TechnoCross
 
> Thanks
> > About your second point, I appologize about my mistake.
> >
> > Once I get ack for above new naming of two interfaces from you,
> > I will revise my patch set including cover-letter and will send
> > those to mailing list.
> >
> > Thanks for your co-opearation!
> >
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCorss
> >
> >> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
> >>> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> >>>
> >>> >From DPDK-18.08, the follwing APIs become deprecated and
> >>> will be deleted in DPDK18.08.
> >>> - rte_eth_dev_attach()
> >>> - rte_eth_dev_detach()
> >>>
> >>> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
> >>> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
> >>>
> >>> To follow the above changes, this patch set provides replacement of
> >>> those APIs.
> >> Hideyuki,
> >>
> >> Thank you for suggesting to update to v18.08!
> >>
> >> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works.
> >>
> >> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines.
> >>
> >> Thanks
> >>> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> >>> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> >>>
> >>> Hideyuki Yamashita (6):
> >>>     shared: addition of attach()
> >>>     spp_nfv: replacement of rte_eth_dev_attach()
> >>>     spp_vf:replacement of rte_eth_dev_attach()
> >>>     shared: addition of detach()
> >>>     spp_nfv: replacement of rte_eth_dev_detach()
> >>>     spp_vm: replacement of rte_eth_dev_detach().
> >>>
> >>>    src/nfv/nfv.c       | 12 ++++-----
> >>>    src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
> >>>    src/shared/common.h | 32 +++++++++++++++++++++++
> >>>    src/vf/spp_vf.c     |  4 +--
> >>>    src/vm/main.c       |  2 +-
> >>>    5 files changed, 105 insertions(+), 9 deletions(-)
> >>>
> >>
> >>

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

> Hello Yasufumi-san,
> 
> Thanks for your comments.
> Please see inline.
> 
>>> Hello Yasufumi-san,
>>>
>>> Thanks so much for your comments.
>>>
>>> About your first point, I think the follwing naming may be fit with DPDK
>>> and SPP naming conventions, what do you think?
>>>
>>> attach -> spp_eth_dev_attach
>>> detach -> spp_eth_dev_detach
>> I think you should not use prefix `spp` without if it defines a behaviour of SPP itself. This function is more essential to attach device.
>>
>> For `spp_eth_dev_attach`, You might think similar name from original `rte_eth_dev_attach`, but I think we do not keep the name of deprecated APIs. More simply, how about `dev_attach_by_devargs` instead of?
> I have no problem about naming `dev_attach_by_devargs`.
> 
>> For `spp_eth_dev_detach`, I think `dev_detach_by_port_id` is more prefer. Other than the name of the function, I am curious why it takes second argument `name` because it is not used in the function.
> I have no problem about namign  `dev_detach_by_port_id`.
> As you say, second argument is NOT used within the function and I will
> remove it from revised version.
> 
>> By the way, I am not clear what is the difference between your functions and `rte_eth_dev_attach` defined in `dpdk/lib/librte_ethdev/rte_ethdev.c` exactly. Could you explain about the difference shortly for helping my understanding?
> Basically both of those functions uses `rte_eal_hotplug_add` to hotplug
> device.
Thank you for considering updates.
> 
> Difference is described as following:
> `rte_eth_dev_attach` uses global variable named
> eth_dev_last_created_port when retrieving newly created dpdk port while
> my new function uses `rte_eth_dev_get_port_by_name` becauase this function is defined outside
> of dpdk library and can not retrieve global variable within dpdk library.
I've got understand your point. Could you confirm why you avoid to use global variable?

Thanks
> BR,
> Hideyuki Yamashita
> NTT TechnoCross
>   
>> Thanks
>>> About your second point, I appologize about my mistake.
>>>
>>> Once I get ack for above new naming of two interfaces from you,
>>> I will revise my patch set including cover-letter and will send
>>> those to mailing list.
>>>
>>> Thanks for your co-opearation!
>>>
>>> BR,
>>> Hideyuki Yamashita
>>> NTT TechnoCorss
>>>
>>>> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
>>>>> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
>>>>>
>>>>> >From DPDK-18.08, the follwing APIs become deprecated and
>>>>> will be deleted in DPDK18.08.
>>>>> - rte_eth_dev_attach()
>>>>> - rte_eth_dev_detach()
>>>>>
>>>>> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
>>>>> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
>>>>>
>>>>> To follow the above changes, this patch set provides replacement of
>>>>> those APIs.
>>>> Hideyuki,
>>>>
>>>> Thank you for suggesting to update to v18.08!
>>>>
>>>> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works.
>>>>
>>>> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines.
>>>>
>>>> Thanks
>>>>> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
>>>>> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
>>>>>
>>>>> Hideyuki Yamashita (6):
>>>>>      shared: addition of attach()
>>>>>      spp_nfv: replacement of rte_eth_dev_attach()
>>>>>      spp_vf:replacement of rte_eth_dev_attach()
>>>>>      shared: addition of detach()
>>>>>      spp_nfv: replacement of rte_eth_dev_detach()
>>>>>      spp_vm: replacement of rte_eth_dev_detach().
>>>>>
>>>>>     src/nfv/nfv.c       | 12 ++++-----
>>>>>     src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>>>>>     src/shared/common.h | 32 +++++++++++++++++++++++
>>>>>     src/vf/spp_vf.c     |  4 +--
>>>>>     src/vm/main.c       |  2 +-
>>>>>     5 files changed, 105 insertions(+), 9 deletions(-)
>>>>>
>>>>
>>>>
> 
> 
> 
> 


-- 
Yasufumi Ogawa
NTT Network Service Systems Labs

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Hideyuki Yamashita]

Hello Yasufumi-san,

Please see inline.

> > Hello Yasufumi-san,
> >
> > Thanks for your comments.
> > Please see inline.
> >
> >>> Hello Yasufumi-san,
> >>>
> >>> Thanks so much for your comments.
> >>>
> >>> About your first point, I think the follwing naming may be fit with DPDK
> >>> and SPP naming conventions, what do you think?
> >>>
> >>> attach -> spp_eth_dev_attach
> >>> detach -> spp_eth_dev_detach
> >> I think you should not use prefix `spp` without if it defines a behaviour of SPP itself. This function is more essential to attach device.
> >>
> >> For `spp_eth_dev_attach`, You might think similar name from original `rte_eth_dev_attach`, but I think we do not keep the name of deprecated APIs. More simply, how about `dev_attach_by_devargs` instead of?
> > I have no problem about naming `dev_attach_by_devargs`.
> >
> >> For `spp_eth_dev_detach`, I think `dev_detach_by_port_id` is more prefer. Other than the name of the function, I am curious why it takes second argument `name` because it is not used in the function.
> > I have no problem about namign  `dev_detach_by_port_id`.
> > As you say, second argument is NOT used within the function and I will
> > remove it from revised version.
> >
> >> By the way, I am not clear what is the difference between your functions and `rte_eth_dev_attach` defined in `dpdk/lib/librte_ethdev/rte_ethdev.c` exactly. Could you explain about the difference shortly for helping my understanding?
> > Basically both of those functions uses `rte_eal_hotplug_add` to hotplug
> > device.
> Thank you for considering updates.
> >
> > Difference is described as following:
> > `rte_eth_dev_attach` uses global variable named
> > eth_dev_last_created_port when retrieving newly created dpdk port while
> > my new function uses `rte_eth_dev_get_port_by_name` becauase this function is defined outside
> > of dpdk library and can not retrieve global variable within dpdk library.
> I've got understand your point. Could you confirm why you avoid to use global variable?
rte_eth_dev_attach is function inside dpdk library so it is somehow
natural to use global variable when retrieve newly created dpdk port.
As I said earlier, this time new function is located in application side.
It is unnatural to use global variable of library from application
rather I prefer to use existing function for retrieving allocated dpdk
from appliction(rte_eth_dev_get_port_by_name).

I hope above explanation answers to your question.
BTW, I am preparing revising patch set including cover letter.

BR,
Hideyuki Yamashita
NTT TechnoCross

> Thanks
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> >  >> Thanks
> >>> About your second point, I appologize about my mistake.
> >>>
> >>> Once I get ack for above new naming of two interfaces from you,
> >>> I will revise my patch set including cover-letter and will send
> >>> those to mailing list.
> >>>
> >>> Thanks for your co-opearation!
> >>>
> >>> BR,
> >>> Hideyuki Yamashita
> >>> NTT TechnoCorss
> >>>
> >>>> On 2018/11/07 14:07, x-fn-spp@sl.ntt-tx.co.jp wrote:
> >>>>> From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> >>>>>
> >>>>> >From DPDK-18.08, the follwing APIs become deprecated and
> >>>>> will be deleted in DPDK18.08.
> >>>>> - rte_eth_dev_attach()
> >>>>> - rte_eth_dev_detach()
> >>>>>
> >>>>> For rte_eth_dev_attach(), use of rte_eal_hotplug_add() is recommended.
> >>>>> For rte_eth_dev_detach(), use of rte_eal_hotplug_remove() is recommended.
> >>>>>
> >>>>> To follow the above changes, this patch set provides replacement of
> >>>>> those APIs.
> >>>> Hideyuki,
> >>>>
> >>>> Thank you for suggesting to update to v18.08!
> >>>>
> >>>> Could you re-consider the name of function you added by referring conventions of DPDK and SPP? It has almost no means if just `attach` or `detach`. It is preferable to be self explanatory for how your function works.
> >>>>
> >>>> Commit messages are also required to be revised. Update for the change of function name and modify invalid descriptions for documentation guidelines.
> >>>>
> >>>> Thanks
> >>>>> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> >>>>> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> >>>>>
> >>>>> Hideyuki Yamashita (6):
> >>>>>      shared: addition of attach()
> >>>>>      spp_nfv: replacement of rte_eth_dev_attach()
> >>>>>      spp_vf:replacement of rte_eth_dev_attach()
> >>>>>      shared: addition of detach()
> >>>>>      spp_nfv: replacement of rte_eth_dev_detach()
> >>>>>      spp_vm: replacement of rte_eth_dev_detach().
> >>>>>
> >>>>>     src/nfv/nfv.c       | 12 ++++-----
> >>>>>     src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
> >>>>>     src/shared/common.h | 32 +++++++++++++++++++++++
> >>>>>     src/vf/spp_vf.c     |  4 +--
> >>>>>     src/vm/main.c       |  2 +-
> >>>>>     5 files changed, 105 insertions(+), 9 deletions(-)
> >>>>>
> >>>>
> >>>>
> >
> >
> >
> > 
> 
> -- Yasufumi Ogawa
> NTT Network Service Systems Labs

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Nakamura Hioryuki]

Hi, Yasufumi and all,

> > I've got understand your point. Could you confirm why you avoid to use global variable?
> rte_eth_dev_attach is function inside dpdk library so it is somehow
> natural to use global variable when retrieve newly created dpdk port.
> As I said earlier, this time new function is located in application side.
> It is unnatural to use global variable of library from application
> rather I prefer to use existing function for retrieving allocated dpdk
> from appliction(rte_eth_dev_get_port_by_name).

I would like to add background to above discussion.
First, we asked simple question “How to replace rte_eth_dev_attach with
rte_eal_hotplug_add" to dpdk-dev ML. At that time, we thought that this
replacement can be realized by migrating the codes in
rte_eth_dev_attatch() in "lib/librte_ethdev/rte_ethdev.c" to spp code.
But thanks to advice from Thomas, we found that this idea is too
simple (or idiot?) to handle the "internal data (global variable)" in
rte_eth_dev_attach().

See
https://mails.dpdk.org/archives/dev/2018-September/112412.html

So, we decided to use rte_eth_dev_get_port_by_name().

-- 
Nakamura Hioryuki <nakamua.hiroyuki@po.ntt-tx.co.jp>

Re: [spp] [spp 03539] Re: [PATCH 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

On 2018/11/15 9:16, Nakamura Hioryuki wrote:
> Hi, Yasufumi and all,
> 
>>> I've got understand your point. Could you confirm why you avoid to use global variable?
>> rte_eth_dev_attach is function inside dpdk library so it is somehow
>> natural to use global variable when retrieve newly created dpdk port.
>> As I said earlier, this time new function is located in application side.
>> It is unnatural to use global variable of library from application
>> rather I prefer to use existing function for retrieving allocated dpdk
>> from appliction(rte_eth_dev_get_port_by_name).
> 
> I would like to add background to above discussion.
> First, we asked simple question “How to replace rte_eth_dev_attach with
> rte_eal_hotplug_add" to dpdk-dev ML. At that time, we thought that this
> replacement can be realized by migrating the codes in
> rte_eth_dev_attatch() in "lib/librte_ethdev/rte_ethdev.c" to spp code.
> But thanks to advice from Thomas, we found that this idea is too
> simple (or idiot?) to handle the "internal data (global variable)" in
> rte_eth_dev_attach().
> 
> See
> https://mails.dpdk.org/archives/dev/2018-September/112412.html
> 
> So, we decided to use rte_eth_dev_get_port_by_name().
Thanks Hideyuki and Hiroyuki for suggesting the update and having  
discussion with DPDK cores! It is clear what is the problem and that we  
should update. I think updating code is OK, but commit messages are  
still needed to be revised.

Thanks,
Yasufumi
> 

[spp] [PATCH v2 0/6] Replace deprecated APIs

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

>From DPDK 18.08, following APIs become deprecated and will be deleted in 18.11.
- rte_eth_dev_attach()
- rte_eth_dev_detach()

It is recommended to use followings.
- rte_eth_dev_attach() should be replaced by rte_eal_hotplug_add().
- rte_eth_dev_detach() should be replaced by rte_eal_hotplug_remove().

This series of patches is to update for the changes.
- Add dev_attach_by_devargs() as replacement of rte_eth_dev_attach(). This function
  calls rte_eal_hotplug_add() to hotplug device for given name "devargs",and returns
  newly allocated dpdk port_id.
- Add dev_detach_by_port_id() as replacement of rte_eth_dev_detach(). This function
  calls rte_eal_hotplug_remove() to detach device for given port_id.
- Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_attach_by_devargs().
- Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_detach_by_port_id().

Hideyuki Yamashita (6):
  shared: add dev_attach_by_devargs
  spp_nfv: replace deprecated rte_eth_dev_attach
  spp_vf: replace deprecated rte_eth_dev_attach
  shared: add dev_detach_by_port_id
  spp_nfv: replace deprecated rte_eth_dev_detach
  spp_vm: replace deprecated rte_eth_dev_detach

 src/nfv/nfv.c       | 18 +++++--------
 src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 src/shared/common.h | 29 ++++++++++++++++++++
 src/vf/spp_vf.c     |  4 +--
 src/vm/main.c       |  4 +--
 5 files changed, 102 insertions(+), 17 deletions(-)

-- 
2.18.0

[spp] [PATCH v2 1/6] shared: add dev_attach_by_devargs

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

SPP uses deprecated APIs removed in DPDK v18.11. Using rte_eth_hotplug_add()
is recommended instead of rte_eth_dev_attach()[1]. This patch is to add
dev_attach_by_devargs() to shared directory so that spp_primary, spp_nfv,
spp_vm and spp_vf can refer this new function.

[1]https://mails.dpdk.org/archives/dev/2018-October/117115.html

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/shared/common.c | 26 ++++++++++++++++++++++++++
 src/shared/common.h | 17 +++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/shared/common.c b/src/shared/common.c
index f1754db..c88ce14 100644
--- a/src/shared/common.c
+++ b/src/shared/common.c
@@ -500,3 +500,29 @@ append_patch_info_json(char *str,
 
 	return 0;
 }
+
+/* attach the new device, then store port_id of the device */
+int
+dev_attach_by_devargs(const char *devargs, uint16_t *port_id)
+{
+	int ret = -1;
+	struct rte_devargs da;
+
+	memset(&da, 0, sizeof(da));
+
+	/* parse devargs */
+	if (rte_devargs_parse(&da, devargs))
+		return -1;
+
+	ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
+	if (ret < 0) {
+		free(da.args);
+		return ret;
+	}
+
+	ret = rte_eth_dev_get_port_by_name(da.name, port_id);
+
+	free(da.args);
+
+	return ret;
+}
diff --git a/src/shared/common.h b/src/shared/common.h
index a5395aa..60514e5 100644
--- a/src/shared/common.h
+++ b/src/shared/common.h
@@ -18,7 +18,9 @@
 #include <rte_common.h>
 #include <rte_config.h>
 #include <rte_eal.h>
+#include <rte_devargs.h>
 #include <rte_ethdev.h>
+#include <rte_ethdev_driver.h>
 #include <rte_launch.h>
 #include <rte_lcore.h>
 #include <rte_log.h>
@@ -217,4 +219,19 @@ int spp_atoi(const char *str, int *val);
 
 #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
 
+/**
+ * Attach a new Ethernet device specified by arguments.
+ *
+ * @param devargs
+ *  A pointer to a strings array describing the new device
+ *  to be attached. The strings should be a pci address like
+ *  '0000:01:00.0' or virtual device name like 'net_pcap0'.
+ * @param port_id
+ *  A pointer to a port identifier actually attached.
+ * @return
+ *  0 on success and port_id is filled, negative on error
+ */
+int
+dev_attach_by_devargs(const char *devargs, uint16_t *port_id);
+
 #endif
-- 
2.18.0

[spp] [PATCH v2 2/6] spp_nfv: replace deprecated rte_eth_dev_attach

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

Replace rte_eth_dev_attach() with dev_attach_by_devargs() for spp_nfv.

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/nfv/nfv.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nfv/nfv.c b/src/nfv/nfv.c
index 05290ed..bf16d81 100644
--- a/src/nfv/nfv.c
+++ b/src/nfv/nfv.c
@@ -442,7 +442,7 @@ add_vhost_pmd(int index)
 	iface = get_vhost_iface_name(index);
 
 	sprintf(devargs, "%s,iface=%s,queues=%d", name, iface, nr_queues);
-	ret = rte_eth_dev_attach(devargs, &vhost_port_id);
+	ret = dev_attach_by_devargs(devargs, &vhost_port_id);
 	if (ret < 0)
 		return ret;
 
@@ -560,7 +560,7 @@ add_pcap_pmd(int index)
 	sprintf(devargs,
 			"%s,rx_pcap=%s,tx_pcap=%s",
 			name, rx_fpath, tx_fpath);
-	ret = rte_eth_dev_attach(devargs, &pcap_pmd_port_id);
+	ret = dev_attach_by_devargs(devargs, &pcap_pmd_port_id);
 
 	if (ret < 0)
 		return ret;
@@ -623,7 +623,7 @@ add_null_pmd(int index)
 
 	name = get_null_pmd_name(index);
 	sprintf(devargs, "%s", name);
-	ret = rte_eth_dev_attach(devargs, &null_pmd_port_id);
+	ret = dev_attach_by_devargs(devargs, &null_pmd_port_id);
 	if (ret < 0)
 		return ret;
 
-- 
2.18.0

[spp] [PATCH v2 3/6] spp_vf: replace deprecated rte_eth_dev_attach

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

Replace rte_eth_dev_attach() with dev_attach_by_devargs() for spp_vf.

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/vf/spp_vf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/vf/spp_vf.c b/src/vf/spp_vf.c
index 2a77ec6..e493980 100644
--- a/src/vf/spp_vf.c
+++ b/src/vf/spp_vf.c
@@ -222,9 +222,9 @@ add_vhost_pmd(int index, int client)
 
 	sprintf(devargs, "%s,iface=%s,queues=%d,client=%d",
 			name, iface, nr_queues, client);
-	ret = rte_eth_dev_attach(devargs, &vhost_port_id);
+	ret = dev_attach_by_devargs(devargs, &vhost_port_id);
 	if (unlikely(ret < 0)) {
-		RTE_LOG(ERR, APP, "rte_eth_dev_attach error. (ret = %d)\n",
+		RTE_LOG(ERR, APP, "dev_attach_by_devargs error. (ret = %d)\n",
 				ret);
 		return ret;
 	}
-- 
2.18.0

[spp] [PATCH v2 4/6] shared: add dev_detach_by_port_id

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

SPP uses deprecated APIs removed in DPDK v18.11. Using rte_eth_hotplug_remove()
is recommended instead of rte_eth_dev_detach()[1].  This patch is to add
dev_detach_by_port_id() to shared directory so that spp_primary, spp_nfv,
spp_vm and spp_vf can refer this new function.

[1]https://mails.dpdk.org/archives/dev/2018-October/117115.html

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/shared/common.c | 38 ++++++++++++++++++++++++++++++++++++++
 src/shared/common.h | 12 ++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/src/shared/common.c b/src/shared/common.c
index c88ce14..0e32fa6 100644
--- a/src/shared/common.c
+++ b/src/shared/common.c
@@ -526,3 +526,41 @@ dev_attach_by_devargs(const char *devargs, uint16_t *port_id)
 
 	return ret;
 }
+
+/* detach the device, then store the name of the device */
+int
+dev_detach_by_port_id(uint16_t port_id)
+{
+	struct rte_device *dev;
+	struct rte_bus *bus;
+	uint32_t dev_flags;
+	int ret = -1;
+
+	if (rte_eth_devices[port_id].data == NULL) {
+		RTE_LOG(INFO, APP,
+			"rte_eth_devices[%d].data is  NULL\n", port_id);
+		return 0;
+	}
+	dev_flags = rte_eth_devices[port_id].data->dev_flags;
+	if (dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
+		RTE_LOG(ERR, APP,
+			"Port %"PRIu16" is bonded, cannot detach\n", port_id);
+		return -ENOTSUP;
+	}
+
+	dev = rte_eth_devices[port_id].device;
+	if (dev == NULL)
+		return -EINVAL;
+
+	bus = rte_bus_find_by_device(dev);
+	if (bus == NULL)
+		return -ENOENT;
+
+	ret = rte_eal_hotplug_remove(bus->name, dev->name);
+	if (ret < 0)
+		return ret;
+
+	rte_eth_dev_release_port(&rte_eth_devices[port_id]);
+
+	return 0;
+}
diff --git a/src/shared/common.h b/src/shared/common.h
index 60514e5..09dbf8a 100644
--- a/src/shared/common.h
+++ b/src/shared/common.h
@@ -234,4 +234,16 @@ int spp_atoi(const char *str, int *val);
 int
 dev_attach_by_devargs(const char *devargs, uint16_t *port_id);
 
+/**
+ * Detach a Ethernet device specified by port identifier.
+ * This function must be called when the device is in the
+ * closed state.
+ *
+ * @param port_id
+ *   The port identifier of the device to detach.
+ * @return
+ *  0 on success and devname is filled, negative on error
+ */
+int dev_detach_by_port_id(uint16_t port_id);
+
 #endif
-- 
2.18.0

[spp] [PATCH v2 5/6] spp_nfv: replace deprecated rte_eth_dev_detach

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

Replace rte_eth_dev_detach() with dev_detach_by_port_id() for spp_nfv.

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/nfv/nfv.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/src/nfv/nfv.c b/src/nfv/nfv.c
index bf16d81..c47691d 100644
--- a/src/nfv/nfv.c
+++ b/src/nfv/nfv.c
@@ -309,32 +309,26 @@ do_del(char *res_uid)
 			return -1;
 
 	} else if (!strcmp(p_type, "ring")) {
-		char name[RTE_ETH_NAME_MAX_LEN];
-
 		RTE_LOG(DEBUG, APP, "Del ring id %d\n", p_id);
 		port_id = find_port_id(p_id, RING);
 		if (port_id == PORT_RESET)
 			return -1;
 
-		rte_eth_dev_detach(port_id, name);
+		dev_detach_by_port_id(port_id);
 
 	} else if (!strcmp(p_type, "pcap")) {
-		char name[RTE_ETH_NAME_MAX_LEN];
-
 		port_id = find_port_id(p_id, PCAP);
 		if (port_id == PORT_RESET)
 			return -1;
 
-		rte_eth_dev_detach(port_id, name);
+		dev_detach_by_port_id(port_id);
 
 	} else if (!strcmp(p_type, "nullpmd")) {
-		char name[RTE_ETH_NAME_MAX_LEN];
-
 		port_id = find_port_id(p_id, NULLPMD);
 		if (port_id == PORT_RESET)
 			return -1;
 
-		rte_eth_dev_detach(port_id, name);
+		dev_detach_by_port_id(port_id);
 
 	}
 
-- 
2.18.0

[spp] [PATCH v2 6/6] spp_vm: replace deprecated rte_eth_dev_detach

[x-fn-spp]

From: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>

Replace rte_eth_dev_detach() with dev_detach_by_port_id() for spp_vm.

Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
---
 src/vm/main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/vm/main.c b/src/vm/main.c
index 1d7d83a..f69ad4e 100644
--- a/src/vm/main.c
+++ b/src/vm/main.c
@@ -244,14 +244,12 @@ do_del(char *res_uid)
 	}
 
 	if (!strcmp(p_type, "ring")) {
-		char name[RTE_ETH_NAME_MAX_LEN];
-
 		RTE_LOG(DEBUG, APP, "Del ring id %d\n", p_id);
 		port_id = find_port_id(p_id, RING);
 		if (port_id == PORT_RESET)
 			return -1;
 
-		rte_eth_dev_detach(port_id, name);
+		dev_detach_by_port_id(port_id);
 	}
 
 	forward_array_remove(port_id);
-- 
2.18.0

Re: [spp] [PATCH v2 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

>>From DPDK 18.08, following APIs become deprecated and will be deleted in 18.11.
> - rte_eth_dev_attach()
> - rte_eth_dev_detach()
> 
> It is recommended to use followings.
> - rte_eth_dev_attach() should be replaced by rte_eal_hotplug_add().
> - rte_eth_dev_detach() should be replaced by rte_eal_hotplug_remove().
> 
> This series of patches is to update for the changes.
> - Add dev_attach_by_devargs() as replacement of rte_eth_dev_attach(). This function
>    calls rte_eal_hotplug_add() to hotplug device for given name "devargs",and returns
>    newly allocated dpdk port_id.
> - Add dev_detach_by_port_id() as replacement of rte_eth_dev_detach(). This function
>    calls rte_eal_hotplug_remove() to detach device for given port_id.
> - Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_attach_by_devargs().
> - Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_detach_by_port_id().
Hideyuki,

Your patches cause compile errors because DPDK APIs you use instead of 
depricated APIs are still not stable. You should add 
`-DALLOW_EXPERIMENTAL_API` to CFLAG option in each of Makefiles of SPP 
processes.

I already fixed this bug while I checked your patches. I would like to 
send patches to fix this bug.

Thanks,
Yasufumi
> 
> Hideyuki Yamashita (6):
>    shared: add dev_attach_by_devargs
>    spp_nfv: replace deprecated rte_eth_dev_attach
>    spp_vf: replace deprecated rte_eth_dev_attach
>    shared: add dev_detach_by_port_id
>    spp_nfv: replace deprecated rte_eth_dev_detach
>    spp_vm: replace deprecated rte_eth_dev_detach
> 
>   src/nfv/nfv.c       | 18 +++++--------
>   src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>   src/shared/common.h | 29 ++++++++++++++++++++
>   src/vf/spp_vf.c     |  4 +--
>   src/vm/main.c       |  4 +--
>   5 files changed, 102 insertions(+), 17 deletions(-)
> 

Re: [spp] [PATCH v2 1/6] shared: add dev_attach_by_devargs

[Yasufumi Ogawa]

> SPP uses deprecated APIs removed in DPDK v18.11. Using rte_eth_hotplug_add()
> is recommended instead of rte_eth_dev_attach()[1]. This patch is to add
> dev_attach_by_devargs() to shared directory so that spp_primary, spp_nfv,
> spp_vm and spp_vf can refer this new function.
Hideyuki,

The number of characters per line should be less than 72. It exceeds the 
limitation in 1st and 3rd lines.

Thanks
> 
> [1]https://mails.dpdk.org/archives/dev/2018-October/117115.html
> 
> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> ---
>   src/shared/common.c | 26 ++++++++++++++++++++++++++
>   src/shared/common.h | 17 +++++++++++++++++
>   2 files changed, 43 insertions(+)
> 
> diff --git a/src/shared/common.c b/src/shared/common.c
> index f1754db..c88ce14 100644
> --- a/src/shared/common.c
> +++ b/src/shared/common.c
> @@ -500,3 +500,29 @@ append_patch_info_json(char *str,
>   
>   	return 0;
>   }
> +
> +/* attach the new device, then store port_id of the device */
> +int
> +dev_attach_by_devargs(const char *devargs, uint16_t *port_id)
> +{
> +	int ret = -1;
> +	struct rte_devargs da;
> +
> +	memset(&da, 0, sizeof(da));
> +
> +	/* parse devargs */
> +	if (rte_devargs_parse(&da, devargs))
> +		return -1;
> +
> +	ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
> +	if (ret < 0) {
> +		free(da.args);
> +		return ret;
> +	}
> +
> +	ret = rte_eth_dev_get_port_by_name(da.name, port_id);
> +
> +	free(da.args);
> +
> +	return ret;
> +}
> diff --git a/src/shared/common.h b/src/shared/common.h
> index a5395aa..60514e5 100644
> --- a/src/shared/common.h
> +++ b/src/shared/common.h
> @@ -18,7 +18,9 @@
>   #include <rte_common.h>
>   #include <rte_config.h>
>   #include <rte_eal.h>
> +#include <rte_devargs.h>
>   #include <rte_ethdev.h>
> +#include <rte_ethdev_driver.h>
>   #include <rte_launch.h>
>   #include <rte_lcore.h>
>   #include <rte_log.h>
> @@ -217,4 +219,19 @@ int spp_atoi(const char *str, int *val);
>   
>   #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
>   
> +/**
> + * Attach a new Ethernet device specified by arguments.
> + *
> + * @param devargs
> + *  A pointer to a strings array describing the new device
> + *  to be attached. The strings should be a pci address like
> + *  '0000:01:00.0' or virtual device name like 'net_pcap0'.
> + * @param port_id
> + *  A pointer to a port identifier actually attached.
> + * @return
> + *  0 on success and port_id is filled, negative on error
> + */
> +int
> +dev_attach_by_devargs(const char *devargs, uint16_t *port_id);
> +
>   #endif
> 

Re: [spp] [PATCH v2 4/6] shared: add dev_detach_by_port_id

[Yasufumi Ogawa]

> SPP uses deprecated APIs removed in DPDK v18.11. Using rte_eth_hotplug_remove()
> is recommended instead of rte_eth_dev_detach()[1].  This patch is to add
> dev_detach_by_port_id() to shared directory so that spp_primary, spp_nfv,
> spp_vm and spp_vf can refer this new function.
Hideyuki,

Commit message should be wrapped at 72 characters, but 1st and 3rd lines 
exceeds the limitation.

Thanks
> 
> [1]https://mails.dpdk.org/archives/dev/2018-October/117115.html
> 
> Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> ---
>   src/shared/common.c | 38 ++++++++++++++++++++++++++++++++++++++
>   src/shared/common.h | 12 ++++++++++++
>   2 files changed, 50 insertions(+)
> 
> diff --git a/src/shared/common.c b/src/shared/common.c
> index c88ce14..0e32fa6 100644
> --- a/src/shared/common.c
> +++ b/src/shared/common.c
> @@ -526,3 +526,41 @@ dev_attach_by_devargs(const char *devargs, uint16_t *port_id)
>   
>   	return ret;
>   }
> +
> +/* detach the device, then store the name of the device */
> +int
> +dev_detach_by_port_id(uint16_t port_id)
> +{
> +	struct rte_device *dev;
> +	struct rte_bus *bus;
> +	uint32_t dev_flags;
> +	int ret = -1;
> +
> +	if (rte_eth_devices[port_id].data == NULL) {
> +		RTE_LOG(INFO, APP,
> +			"rte_eth_devices[%d].data is  NULL\n", port_id);
> +		return 0;
> +	}
> +	dev_flags = rte_eth_devices[port_id].data->dev_flags;
> +	if (dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
> +		RTE_LOG(ERR, APP,
> +			"Port %"PRIu16" is bonded, cannot detach\n", port_id);
> +		return -ENOTSUP;
> +	}
> +
> +	dev = rte_eth_devices[port_id].device;
> +	if (dev == NULL)
> +		return -EINVAL;
> +
> +	bus = rte_bus_find_by_device(dev);
> +	if (bus == NULL)
> +		return -ENOENT;
> +
> +	ret = rte_eal_hotplug_remove(bus->name, dev->name);
> +	if (ret < 0)
> +		return ret;
> +
> +	rte_eth_dev_release_port(&rte_eth_devices[port_id]);
> +
> +	return 0;
> +}
> diff --git a/src/shared/common.h b/src/shared/common.h
> index 60514e5..09dbf8a 100644
> --- a/src/shared/common.h
> +++ b/src/shared/common.h
> @@ -234,4 +234,16 @@ int spp_atoi(const char *str, int *val);
>   int
>   dev_attach_by_devargs(const char *devargs, uint16_t *port_id);
>   
> +/**
> + * Detach a Ethernet device specified by port identifier.
> + * This function must be called when the device is in the
> + * closed state.
> + *
> + * @param port_id
> + *   The port identifier of the device to detach.
> + * @return
> + *  0 on success and devname is filled, negative on error
> + */
> +int dev_detach_by_port_id(uint16_t port_id);
> +
>   #endif
> 

Re: [spp] [spp 03664] Re: [PATCH v2 1/6] shared: add dev_attach_by_devargs

[Hideyuki Yamashita]

Hello Yasufumi-san,

Thanks for your following comments on 4 patches in my patch set.
https://mails.dpdk.org/archives/spp/2018-November/000959.html
https://mails.dpdk.org/archives/spp/2018-November/000960.html
https://mails.dpdk.org/archives/spp/2018-November/000961.html
https://mails.dpdk.org/archives/spp/2018-November/000962.html
(It is odd that only one mail received to me.)

Anyway, I applogize my mistakes on those.
Of cource solution you indicated in above comments are agreeable 
from my side.

It is welcome that you kindly provide updated patch set.
I think it is faster and more reliable.

Thanks and BR,
Hideyuki Yamashita
NTT TechnoCross

> > SPP uses deprecated APIs removed in DPDK v18.11. Using rte_eth_hotplug_add()
> > is recommended instead of rte_eth_dev_attach()[1]. This patch is to add
> > dev_attach_by_devargs() to shared directory so that spp_primary, spp_nfv,
> > spp_vm and spp_vf can refer this new function.
> Hideyuki,
> 
> The number of characters per line should be less than 72. It exceeds the limitation in 1st and 3rd lines.
> 
> Thanks
> >
> > [1]https://mails.dpdk.org/archives/dev/2018-October/117115.html
> >
> > Signed-off-by: Hideyuki Yamashita <yamashita.hideyuki@po.ntt-tx.co.jp>
> > Signed-off-by: Naoki Takada <takada.naoki@lab.ntt.co.jp>
> > ---
> >   src/shared/common.c | 26 ++++++++++++++++++++++++++
> >   src/shared/common.h | 17 +++++++++++++++++
> >   2 files changed, 43 insertions(+)
> >
> > diff --git a/src/shared/common.c b/src/shared/common.c
> > index f1754db..c88ce14 100644
> > --- a/src/shared/common.c
> > +++ b/src/shared/common.c
> > @@ -500,3 +500,29 @@ append_patch_info_json(char *str,
> >  >   	return 0;
> >   }
> > +
> > +/* attach the new device, then store port_id of the device */
> > +int
> > +dev_attach_by_devargs(const char *devargs, uint16_t *port_id)
> > +{
> > +	int ret = -1;
> > +	struct rte_devargs da;
> > +
> > +	memset(&da, 0, sizeof(da));
> > +
> > +	/* parse devargs */
> > +	if (rte_devargs_parse(&da, devargs))
> > +		return -1;
> > +
> > +	ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
> > +	if (ret < 0) {
> > +		free(da.args);
> > +		return ret;
> > +	}
> > +
> > +	ret = rte_eth_dev_get_port_by_name(da.name, port_id);
> > +
> > +	free(da.args);
> > +
> > +	return ret;
> > +}
> > diff --git a/src/shared/common.h b/src/shared/common.h
> > index a5395aa..60514e5 100644
> > --- a/src/shared/common.h
> > +++ b/src/shared/common.h
> > @@ -18,7 +18,9 @@
> >   #include <rte_common.h>
> >   #include <rte_config.h>
> >   #include <rte_eal.h>
> > +#include <rte_devargs.h>
> >   #include <rte_ethdev.h>
> > +#include <rte_ethdev_driver.h>
> >   #include <rte_launch.h>
> >   #include <rte_lcore.h>
> >   #include <rte_log.h>
> > @@ -217,4 +219,19 @@ int spp_atoi(const char *str, int *val);
> >  >   #define RTE_LOGTYPE_APP RTE_LOGTYPE_USER1
> >  > +/**
> > + * Attach a new Ethernet device specified by arguments.
> > + *
> > + * @param devargs
> > + *  A pointer to a strings array describing the new device
> > + *  to be attached. The strings should be a pci address like
> > + *  '0000:01:00.0' or virtual device name like 'net_pcap0'.
> > + * @param port_id
> > + *  A pointer to a port identifier actually attached.
> > + * @return
> > + *  0 on success and port_id is filled, negative on error
> > + */
> > +int
> > +dev_attach_by_devargs(const char *devargs, uint16_t *port_id);
> > +
> >   #endif
> > 

Re: [spp] [PATCH v2 0/6] Replace deprecated APIs

[Yasufumi Ogawa]

>>From DPDK 18.08, following APIs become deprecated and will be deleted in 18.11.
> - rte_eth_dev_attach()
> - rte_eth_dev_detach()
> 
> It is recommended to use followings.
> - rte_eth_dev_attach() should be replaced by rte_eal_hotplug_add().
> - rte_eth_dev_detach() should be replaced by rte_eal_hotplug_remove().
> 
> This series of patches is to update for the changes.
> - Add dev_attach_by_devargs() as replacement of rte_eth_dev_attach(). This function
>    calls rte_eal_hotplug_add() to hotplug device for given name "devargs",and returns
>    newly allocated dpdk port_id.
> - Add dev_detach_by_port_id() as replacement of rte_eth_dev_detach(). This function
>    calls rte_eal_hotplug_remove() to detach device for given port_id.
> - Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_attach_by_devargs().
> - Replace rte_eth_dev_attach() in spp_nfv and spp_vf with dev_detach_by_port_id().
Series of patches is applied. Thanks!

Acked-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
> 
> Hideyuki Yamashita (6):
>    shared: add dev_attach_by_devargs
>    spp_nfv: replace deprecated rte_eth_dev_attach
>    spp_vf: replace deprecated rte_eth_dev_attach
>    shared: add dev_detach_by_port_id
>    spp_nfv: replace deprecated rte_eth_dev_detach
>    spp_vm: replace deprecated rte_eth_dev_detach
> 
>   src/nfv/nfv.c       | 18 +++++--------
>   src/shared/common.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>   src/shared/common.h | 29 ++++++++++++++++++++
>   src/vf/spp_vf.c     |  4 +--
>   src/vm/main.c       |  4 +--
>   5 files changed, 102 insertions(+), 17 deletions(-)