How to replace rte_eth_dev_attach with rte_eal_hotplug_add

How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Hello,

From dpdk 18.08 release rte_eth_dev_attach and 
rte_eth_dev_detach becom deprecated API and 
it is recommended to replace with rte_eal_hotplug_add
and rte_eal_hotplug_remove.

My program uses  above mentioned deprecated APIs
and have to replace those.
Note that my program uses attach to attach vhost, pcap pmd.

My question is whether it is correct to replace those as following:
find rte_eth_dev_attach function in rte_ethdev.c and
migrate those content into my program.

e.g. 
lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
lib/librte_ethdev/rte_ethdev.c line 690-720 for detach

Your advice/guidance are much appreciated.
Thanks!

BR,
Hideyuki Yamashita
-----------------------------------------
Hideyuki Yamashita
NTT TechnoCross
-----------------------------------------

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Gaëtan Rivet]

On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> Hello,
> 
> From dpdk 18.08 release rte_eth_dev_attach and 
> rte_eth_dev_detach becom deprecated API and 
> it is recommended to replace with rte_eal_hotplug_add
> and rte_eal_hotplug_remove.
> 
> My program uses  above mentioned deprecated APIs
> and have to replace those.
> Note that my program uses attach to attach vhost, pcap pmd.
> 
> My question is whether it is correct to replace those as following:
> find rte_eth_dev_attach function in rte_ethdev.c and
> migrate those content into my program.
> 
> e.g. 
> lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> 
> Your advice/guidance are much appreciated.
> Thanks!
> 
> BR,
> Hideyuki Yamashita
> -----------------------------------------
> Hideyuki Yamashita
> NTT TechnoCross
> -----------------------------------------
> 
> 

Hello Hideyuki,

You could use this code for guidance, while leaving the ethdev
specificities such as verifying the eth_dev_count_total(). The hotplug
function would already return an error if the PMD was not able to create
the necessary devices.

The main issue might be to find the port_id of your new port.
You won't be able to use eth_dev_last_created_port, so you would have to
iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
matching your parameters (you might for example match the rte_device
name with the name you used in hotplug_add, as there is no standard
naming scheme at the ethdev level).

An possible issue with the deprecation planned for those two functions is
that the hotplug API is also meant to evolve [1] this release (not in a big
way however, it would mostly simplify your usage of it).

[1]: https://mails.dpdk.org/archives/dev/2018-September/111142.html

Best,
-- 
Gaëtan Rivet
6WIND

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

20/09/2018 11:09, Gaëtan Rivet:
> On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > Hello,
> > 
> > From dpdk 18.08 release rte_eth_dev_attach and 
> > rte_eth_dev_detach becom deprecated API and 
> > it is recommended to replace with rte_eal_hotplug_add
> > and rte_eal_hotplug_remove.
> > 
> > My program uses  above mentioned deprecated APIs
> > and have to replace those.
> > Note that my program uses attach to attach vhost, pcap pmd.
> > 
> > My question is whether it is correct to replace those as following:
> > find rte_eth_dev_attach function in rte_ethdev.c and
> > migrate those content into my program.
> > 
> > e.g. 
> > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > 
> > Your advice/guidance are much appreciated.
> > Thanks!
> 
> Hello Hideyuki,
> 
> You could use this code for guidance, while leaving the ethdev
> specificities such as verifying the eth_dev_count_total(). The hotplug
> function would already return an error if the PMD was not able to create
> the necessary devices.
> 
> The main issue might be to find the port_id of your new port.
> You won't be able to use eth_dev_last_created_port, so you would have to
> iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> matching your parameters (you might for example match the rte_device
> name with the name you used in hotplug_add, as there is no standard
> naming scheme at the ethdev level).

It is recommended to register a callback to receive the notifications
of new ethdev ports.
So it may be a change of programming style: sync vs async.

> An possible issue with the deprecation planned for those two functions is
> that the hotplug API is also meant to evolve [1] this release (not in a big
> way however, it would mostly simplify your usage of it).
> 
> [1]: https://mails.dpdk.org/archives/dev/2018-September/111142.html

I will probably not change the existing functions.
A v2 will be sent soon, with new simple functions.

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Dear Gaetan and Thomas, 

Thanks for your answer.
Please see inline.

> 20/09/2018 11:09, Ga枓an Rivet:
> > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > Hello,
> > > 
> > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > rte_eth_dev_detach becom deprecated API and 
> > > it is recommended to replace with rte_eal_hotplug_add
> > > and rte_eal_hotplug_remove.
> > > 
> > > My program uses  above mentioned deprecated APIs
> > > and have to replace those.
> > > Note that my program uses attach to attach vhost, pcap pmd.
> > > 
> > > My question is whether it is correct to replace those as following:
> > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > migrate those content into my program.
> > > 
> > > e.g. 
> > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > 
> > > Your advice/guidance are much appreciated.
> > > Thanks!
> > 
> > Hello Hideyuki,
> > 
> > You could use this code for guidance, while leaving the ethdev
> > specificities such as verifying the eth_dev_count_total(). The hotplug
> > function would already return an error if the PMD was not able to create
> > the necessary devices.
> > 
> > The main issue might be to find the port_id of your new port.
> > You won't be able to use eth_dev_last_created_port, so you would have to
> > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > matching your parameters (you might for example match the rte_device
> > name with the name you used in hotplug_add, as there is no standard
> > naming scheme at the ethdev level).
First of all, thank for your answering to my question.
But I have questions.
(Sorry, I have poor knowledge about dpdk and have many basic questions)

Q1. 
Why eth_dev_last_created_port can not be used?
When I look into rte_eth_dev_atthach in 18.08, it calls 

*port_id = eth_dev_last_created_port;

at the end of the function.
 
Q2. 
Is it possible to use rte_eth_dev_get_port_by name 
instead of calling RTE_ETH_FOREACH_DEV or using
eth_dev_last_created_port?	

Q3. 
If answer to Q2 is no, then how can I get device name from each device?
For example, rte_eth_dev_info_get takes port_id as its
argument.But what I want to know is the port id of the specified device
name.

> It is recommended to register a callback to receive the notifications
> of new ethdev ports.
> So it may be a change of programming style: sync vs async.
Thanks for your advice.
In general, I agree.

But what is the example of notification that program can receive
after device hotplugged?
Device unplugged notification like specified in rte_eth_event_type?
If there are code in example, it is highly appreciated.

> > An possible issue with the deprecation planned for those two functions is
> > that the hotplug API is also meant to evolve [1] this release (not in a big
> > way however, it would mostly simplify your usage of it).
> > 
> > [1]: https://mails.dpdk.org/archives/dev/2018-September/111142.html
> 
> I will probably not change the existing functions.
> A v2 will be sent soon, with new simple functions.
> 
> 
> 

BR,
Hideyuki Yamashita
NTT TechnoCross

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

21/09/2018 09:19, Hideyuki Yamashita:
> Dear Gaetan and Thomas, 
> 
> Thanks for your answer.
> Please see inline.
> 
> > 20/09/2018 11:09, Ga�an Rivet:
> > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > Hello,
> > > > 
> > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > rte_eth_dev_detach becom deprecated API and 
> > > > it is recommended to replace with rte_eal_hotplug_add
> > > > and rte_eal_hotplug_remove.
> > > > 
> > > > My program uses  above mentioned deprecated APIs
> > > > and have to replace those.
> > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > 
> > > > My question is whether it is correct to replace those as following:
> > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > migrate those content into my program.
> > > > 
> > > > e.g. 
> > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > 
> > > > Your advice/guidance are much appreciated.
> > > > Thanks!
> > > 
> > > Hello Hideyuki,
> > > 
> > > You could use this code for guidance, while leaving the ethdev
> > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > function would already return an error if the PMD was not able to create
> > > the necessary devices.
> > > 
> > > The main issue might be to find the port_id of your new port.
> > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > matching your parameters (you might for example match the rte_device
> > > name with the name you used in hotplug_add, as there is no standard
> > > naming scheme at the ethdev level).
> First of all, thank for your answering to my question.
> But I have questions.
> (Sorry, I have poor knowledge about dpdk and have many basic questions)
> 
> Q1. 
> Why eth_dev_last_created_port can not be used?
> When I look into rte_eth_dev_atthach in 18.08, it calls 
> 
> *port_id = eth_dev_last_created_port;
> 
> at the end of the function.

You can have a race condition.
>
> Q2. 
> Is it possible to use rte_eth_dev_get_port_by name 
> instead of calling RTE_ETH_FOREACH_DEV or using
> eth_dev_last_created_port?	

This function works only if you know the ethdev name generated by the PMD.

> Q3. 
> If answer to Q2 is no, then how can I get device name from each device?
> For example, rte_eth_dev_info_get takes port_id as its
> argument.But what I want to know is the port id of the specified device
> name.

If you want the ethdev port ids created after probing (based on devargs),
you probably want to request it with the same devargs.
I will try to work on something for this need.
For now, the most reliable solution is to use the notifications.

> > It is recommended to register a callback to receive the notifications
> > of new ethdev ports.
> > So it may be a change of programming style: sync vs async.
> Thanks for your advice.
> In general, I agree.
> 
> But what is the example of notification that program can receive
> after device hotplugged?
> Device unplugged notification like specified in rte_eth_event_type?

Yes, look at these ones:
    RTE_ETH_EVENT_NEW,      /**< port is probed */
    RTE_ETH_EVENT_DESTROY,  /**< port is released */

> If there are code in example, it is highly appreciated.

You can register a callback with
	rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, ...)
like done in drivers/net/failsafe/failsafe.c

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Dear Thomas,

Thanks for your answer.
It took me a little time to digest answer.
Please see inline.


> 21/09/2018 09:19, Hideyuki Yamashita:
> > Dear Gaetan and Thomas, 
> > 
> > Thanks for your answer.
> > Please see inline.
> > 
> > > 20/09/2018 11:09, Ga?an Rivet:
> > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > Hello,
> > > > > 
> > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > and rte_eal_hotplug_remove.
> > > > > 
> > > > > My program uses  above mentioned deprecated APIs
> > > > > and have to replace those.
> > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > 
> > > > > My question is whether it is correct to replace those as following:
> > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > migrate those content into my program.
> > > > > 
> > > > > e.g. 
> > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > 
> > > > > Your advice/guidance are much appreciated.
> > > > > Thanks!
> > > > 
> > > > Hello Hideyuki,
> > > > 
> > > > You could use this code for guidance, while leaving the ethdev
> > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > function would already return an error if the PMD was not able to create
> > > > the necessary devices.
> > > > 
> > > > The main issue might be to find the port_id of your new port.
> > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > matching your parameters (you might for example match the rte_device
> > > > name with the name you used in hotplug_add, as there is no standard
> > > > naming scheme at the ethdev level).
> > First of all, thank for your answering to my question.
> > But I have questions.
> > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > 
> > Q1. 
> > Why eth_dev_last_created_port can not be used?
> > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > 
> > *port_id = eth_dev_last_created_port;
> > 
> > at the end of the function.
> 
> You can have a race condition.
Please elaborate me a bit more.

Is it correct understanding that race condition 
includes
- read information before port is available
- other device may be plugged (or unplugged)
and so using "eth_dev_last_created_port" is 
NOT reliable?


> > Q2. 
> > Is it possible to use rte_eth_dev_get_port_by name 
> > instead of calling RTE_ETH_FOREACH_DEV or using
> > eth_dev_last_created_port?	
> 
> This function works only if you know the ethdev name generated by the PMD.
Thanks
 
> > Q3. 
> > If answer to Q2 is no, then how can I get device name from each device?
> > For example, rte_eth_dev_info_get takes port_id as its
> > argument.But what I want to know is the port id of the specified device
> > name.
> 
> If you want the ethdev port ids created after probing (based on devargs),
> you probably want to request it with the same devargs.
> I will try to work on something for this need.
> For now, the most reliable solution is to use the notifications.

I think I have two alternatives:
Alt1:
call "rte_eal_hotplug_add"
and retrieve port_id for newly plugged device
using RTE_ETH_FOREACH_DEV.
(as Gaetan said)

Alt2:
call "rte_eth_dev_callback_register"  to subscribe "port probe" event
and "rte_eal_hotplug_add"
and finally get notification including port id.
(As you said)

My question is which is better alternative?
Are there any bad point on Alt1 like using
eth_dev_last_created_port?
(e.g. port not available when called RTE_ETH_FOREACH_DEV)

BR,
Hideyuki Yamashita
NTT TechnoCross

> > > It is recommended to register a callback to receive the notifications
> > > of new ethdev ports.
> > > So it may be a change of programming style: sync vs async.
> > Thanks for your advice.
> > In general, I agree.
> > 
> > But what is the example of notification that program can receive
> > after device hotplugged?
> > Device unplugged notification like specified in rte_eth_event_type?
> 
> Yes, look at these ones:
>     RTE_ETH_EVENT_NEW,      /**< port is probed */
>     RTE_ETH_EVENT_DESTROY,  /**< port is released */
> 
> > If there are code in example, it is highly appreciated.
> 
> You can register a callback with
> 	rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, ...)
> like done in drivers/net/failsafe/failsafe.c
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

27/09/2018 03:38, Hideyuki Yamashita:
> Dear Thomas,
> 
> Thanks for your answer.
> It took me a little time to digest answer.
> Please see inline.
> 
> 
> > 21/09/2018 09:19, Hideyuki Yamashita:
> > > Dear Gaetan and Thomas, 
> > > 
> > > Thanks for your answer.
> > > Please see inline.
> > > 
> > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > and rte_eal_hotplug_remove.
> > > > > > 
> > > > > > My program uses  above mentioned deprecated APIs
> > > > > > and have to replace those.
> > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > 
> > > > > > My question is whether it is correct to replace those as following:
> > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > migrate those content into my program.
> > > > > > 
> > > > > > e.g. 
> > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > 
> > > > > > Your advice/guidance are much appreciated.
> > > > > > Thanks!
> > > > > 
> > > > > Hello Hideyuki,
> > > > > 
> > > > > You could use this code for guidance, while leaving the ethdev
> > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > function would already return an error if the PMD was not able to create
> > > > > the necessary devices.
> > > > > 
> > > > > The main issue might be to find the port_id of your new port.
> > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > matching your parameters (you might for example match the rte_device
> > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > naming scheme at the ethdev level).
> > > First of all, thank for your answering to my question.
> > > But I have questions.
> > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > 
> > > Q1. 
> > > Why eth_dev_last_created_port can not be used?
> > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > 
> > > *port_id = eth_dev_last_created_port;
> > > 
> > > at the end of the function.
> > 
> > You can have a race condition.
> Please elaborate me a bit more.
> 
> Is it correct understanding that race condition 
> includes
> - read information before port is available
> - other device may be plugged (or unplugged)
> and so using "eth_dev_last_created_port" is 
> NOT reliable?

I am thinking about the second one only.

> > > Q2. 
> > > Is it possible to use rte_eth_dev_get_port_by name 
> > > instead of calling RTE_ETH_FOREACH_DEV or using
> > > eth_dev_last_created_port?	
> > 
> > This function works only if you know the ethdev name generated by the PMD.
> Thanks
>  
> > > Q3. 
> > > If answer to Q2 is no, then how can I get device name from each device?
> > > For example, rte_eth_dev_info_get takes port_id as its
> > > argument.But what I want to know is the port id of the specified device
> > > name.
> > 
> > If you want the ethdev port ids created after probing (based on devargs),
> > you probably want to request it with the same devargs.
> > I will try to work on something for this need.
> > For now, the most reliable solution is to use the notifications.
> 
> I think I have two alternatives:
> Alt1:
> call "rte_eal_hotplug_add"
> and retrieve port_id for newly plugged device
> using RTE_ETH_FOREACH_DEV.
> (as Gaetan said)
> 
> Alt2:
> call "rte_eth_dev_callback_register"  to subscribe "port probe" event
> and "rte_eal_hotplug_add"
> and finally get notification including port id.
> (As you said)
> 
> My question is which is better alternative?
> Are there any bad point on Alt1 like using
> eth_dev_last_created_port?
> (e.g. port not available when called RTE_ETH_FOREACH_DEV)

I think both are OK.

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Dear Thomas,

Thansk for your answer.
Please see inline.

> 27/09/2018 03:38, Hideyuki Yamashita:
> > Dear Thomas,
> > 
> > Thanks for your answer.
> > It took me a little time to digest answer.
> > Please see inline.
> > 
> > 
> > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > Dear Gaetan and Thomas, 
> > > > 
> > > > Thanks for your answer.
> > > > Please see inline.
> > > > 
> > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > Hello,
> > > > > > > 
> > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > and rte_eal_hotplug_remove.
> > > > > > > 
> > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > and have to replace those.
> > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > 
> > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > migrate those content into my program.
> > > > > > > 
> > > > > > > e.g. 
> > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > 
> > > > > > > Your advice/guidance are much appreciated.
> > > > > > > Thanks!
> > > > > > 
> > > > > > Hello Hideyuki,
> > > > > > 
> > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > function would already return an error if the PMD was not able to create
> > > > > > the necessary devices.
> > > > > > 
> > > > > > The main issue might be to find the port_id of your new port.
> > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > matching your parameters (you might for example match the rte_device
> > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > naming scheme at the ethdev level).
> > > > First of all, thank for your answering to my question.
> > > > But I have questions.
> > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > 
> > > > Q1. 
> > > > Why eth_dev_last_created_port can not be used?
> > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > 
> > > > *port_id = eth_dev_last_created_port;
> > > > 
> > > > at the end of the function.
> > > 
> > > You can have a race condition.
> > Please elaborate me a bit more.
> > 
> > Is it correct understanding that race condition 
> > includes
> > - read information before port is available
> > - other device may be plugged (or unplugged)
> > and so using "eth_dev_last_created_port" is 
> > NOT reliable?
> 
> I am thinking about the second one only.

If we assume there is only one DPDK application 
inside the host and within the application, only one thread
handles attach/detach of devices, then is it ok to use 
> > > > *port_id = eth_dev_last_created_port;
because there seems no possiblity race condition
takes place?

> > > > Q2. 
> > > > Is it possible to use rte_eth_dev_get_port_by name 
> > > > instead of calling RTE_ETH_FOREACH_DEV or using
> > > > eth_dev_last_created_port?	
> > > 
> > > This function works only if you know the ethdev name generated by the PMD.
> > Thanks
> >  
> > > > Q3. 
> > > > If answer to Q2 is no, then how can I get device name from each device?
> > > > For example, rte_eth_dev_info_get takes port_id as its
> > > > argument.But what I want to know is the port id of the specified device
> > > > name.
> > > 
> > > If you want the ethdev port ids created after probing (based on devargs),
> > > you probably want to request it with the same devargs.
> > > I will try to work on something for this need.
> > > For now, the most reliable solution is to use the notifications.
> > 
> > I think I have two alternatives:
> > Alt1:
> > call "rte_eal_hotplug_add"
> > and retrieve port_id for newly plugged device
> > using RTE_ETH_FOREACH_DEV.
> > (as Gaetan said)
> > 
> > Alt2:
> > call "rte_eth_dev_callback_register"  to subscribe "port probe" event
> > and "rte_eal_hotplug_add"
> > and finally get notification including port id.
> > (As you said)
> > 
> > My question is which is better alternative?
> > Are there any bad point on Alt1 like using
> > eth_dev_last_created_port?
> > (e.g. port not available when called RTE_ETH_FOREACH_DEV)
> 
> I think both are OK.
> 
Thanks,
Hideyuki Yamashita
NTT TechnoCross

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

27/09/2018 12:40, Hideyuki Yamashita:
> Dear Thomas,
> 
> Thansk for your answer.
> Please see inline.
> 
> > 27/09/2018 03:38, Hideyuki Yamashita:
> > > Dear Thomas,
> > > 
> > > Thanks for your answer.
> > > It took me a little time to digest answer.
> > > Please see inline.
> > > 
> > > 
> > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > Dear Gaetan and Thomas, 
> > > > > 
> > > > > Thanks for your answer.
> > > > > Please see inline.
> > > > > 
> > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > Hello,
> > > > > > > > 
> > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > 
> > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > and have to replace those.
> > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > 
> > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > migrate those content into my program.
> > > > > > > > 
> > > > > > > > e.g. 
> > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > 
> > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > Thanks!
> > > > > > > 
> > > > > > > Hello Hideyuki,
> > > > > > > 
> > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > the necessary devices.
> > > > > > > 
> > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > naming scheme at the ethdev level).
> > > > > First of all, thank for your answering to my question.
> > > > > But I have questions.
> > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > 
> > > > > Q1. 
> > > > > Why eth_dev_last_created_port can not be used?
> > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > 
> > > > > *port_id = eth_dev_last_created_port;
> > > > > 
> > > > > at the end of the function.
> > > > 
> > > > You can have a race condition.
> > > Please elaborate me a bit more.
> > > 
> > > Is it correct understanding that race condition 
> > > includes
> > > - read information before port is available
> > > - other device may be plugged (or unplugged)
> > > and so using "eth_dev_last_created_port" is 
> > > NOT reliable?
> > 
> > I am thinking about the second one only.
> 
> If we assume there is only one DPDK application 
> inside the host and within the application, only one thread
> handles attach/detach of devices, then is it ok to use 
> > > > > *port_id = eth_dev_last_created_port;
> because there seems no possiblity race condition
> takes place?

If you are never probing a new port outside of this thread,
I guess it's OK.
Take care of not attaching from the interrupt thread too!

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Dear Thomas and all,

About a month ago, I posted the topic related with 
how to replace rte_eth_dev_attach.

Following your advice,
my code would be as below:
(Old code using deprecated API is commented out)

rte_eth_dev_get_port_by_name is used to retrieve dpdk port
after rte_eal_hotplug_add.
Note that my application is just one of the dpdk applications(in the host)
and within the process, only one thread handles device attatch/detach.
(No race condition with regard to device hot_plug will
not take place)
-------------------------------------------------------------------
        //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
        //if (ret < 0)
        //      return ret;

        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;
        }
        free(da.args);
        ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
        if (ret < 0)
                return ret;
-----------------------------------------------------------------------------

If you have any concerns/additional advices, please let me know.

BR,
Hideyuki Yamashita
NTT TechnoCross


> 27/09/2018 12:40, Hideyuki Yamashita:
> > Dear Thomas,
> > 
> > Thansk for your answer.
> > Please see inline.
> > 
> > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > Dear Thomas,
> > > > 
> > > > Thanks for your answer.
> > > > It took me a little time to digest answer.
> > > > Please see inline.
> > > > 
> > > > 
> > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > Dear Gaetan and Thomas, 
> > > > > > 
> > > > > > Thanks for your answer.
> > > > > > Please see inline.
> > > > > > 
> > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > Hello,
> > > > > > > > > 
> > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > 
> > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > and have to replace those.
> > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > 
> > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > migrate those content into my program.
> > > > > > > > > 
> > > > > > > > > e.g. 
> > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > 
> > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > Thanks!
> > > > > > > > 
> > > > > > > > Hello Hideyuki,
> > > > > > > > 
> > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > the necessary devices.
> > > > > > > > 
> > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > naming scheme at the ethdev level).
> > > > > > First of all, thank for your answering to my question.
> > > > > > But I have questions.
> > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > 
> > > > > > Q1. 
> > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > 
> > > > > > *port_id = eth_dev_last_created_port;
> > > > > > 
> > > > > > at the end of the function.
> > > > > 
> > > > > You can have a race condition.
> > > > Please elaborate me a bit more.
> > > > 
> > > > Is it correct understanding that race condition 
> > > > includes
> > > > - read information before port is available
> > > > - other device may be plugged (or unplugged)
> > > > and so using "eth_dev_last_created_port" is 
> > > > NOT reliable?
> > > 
> > > I am thinking about the second one only.
> > 
> > If we assume there is only one DPDK application 
> > inside the host and within the application, only one thread
> > handles attach/detach of devices, then is it ok to use 
> > > > > > *port_id = eth_dev_last_created_port;
> > because there seems no possiblity race condition
> > takes place?
> 
> If you are never probing a new port outside of this thread,
> I guess it's OK.
> Take care of not attaching from the interrupt thread too!
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

Hi,

I am actively working on it.
Look how rte_eth_dev_attach is replaced in testpmd:
	https://patches.dpdk.org/patch/47019/
It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.


22/10/2018 06:34, Hideyuki Yamashita:
> Dear Thomas and all,
> 
> About a month ago, I posted the topic related with 
> how to replace rte_eth_dev_attach.
> 
> Following your advice,
> my code would be as below:
> (Old code using deprecated API is commented out)
> 
> rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> after rte_eal_hotplug_add.
> Note that my application is just one of the dpdk applications(in the host)
> and within the process, only one thread handles device attatch/detach.
> (No race condition with regard to device hot_plug will
> not take place)
> -------------------------------------------------------------------
>         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
>         //if (ret < 0)
>         //      return ret;
> 
>         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;
>         }
>         free(da.args);
>         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
>         if (ret < 0)
>                 return ret;
> -----------------------------------------------------------------------------
> 
> If you have any concerns/additional advices, please let me know.
> 
> BR,
> Hideyuki Yamashita
> NTT TechnoCross
> 
> 
> > 27/09/2018 12:40, Hideyuki Yamashita:
> > > Dear Thomas,
> > > 
> > > Thansk for your answer.
> > > Please see inline.
> > > 
> > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > Dear Thomas,
> > > > > 
> > > > > Thanks for your answer.
> > > > > It took me a little time to digest answer.
> > > > > Please see inline.
> > > > > 
> > > > > 
> > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > Dear Gaetan and Thomas, 
> > > > > > > 
> > > > > > > Thanks for your answer.
> > > > > > > Please see inline.
> > > > > > > 
> > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > Hello,
> > > > > > > > > > 
> > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > 
> > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > and have to replace those.
> > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > 
> > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > migrate those content into my program.
> > > > > > > > > > 
> > > > > > > > > > e.g. 
> > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > 
> > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > Thanks!
> > > > > > > > > 
> > > > > > > > > Hello Hideyuki,
> > > > > > > > > 
> > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > the necessary devices.
> > > > > > > > > 
> > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > naming scheme at the ethdev level).
> > > > > > > First of all, thank for your answering to my question.
> > > > > > > But I have questions.
> > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > 
> > > > > > > Q1. 
> > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > 
> > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > 
> > > > > > > at the end of the function.
> > > > > > 
> > > > > > You can have a race condition.
> > > > > Please elaborate me a bit more.
> > > > > 
> > > > > Is it correct understanding that race condition 
> > > > > includes
> > > > > - read information before port is available
> > > > > - other device may be plugged (or unplugged)
> > > > > and so using "eth_dev_last_created_port" is 
> > > > > NOT reliable?
> > > > 
> > > > I am thinking about the second one only.
> > > 
> > > If we assume there is only one DPDK application 
> > > inside the host and within the application, only one thread
> > > handles attach/detach of devices, then is it ok to use 
> > > > > > > *port_id = eth_dev_last_created_port;
> > > because there seems no possiblity race condition
> > > takes place?
> > 
> > If you are never probing a new port outside of this thread,
> > I guess it's OK.
> > Take care of not attaching from the interrupt thread too!
> > 
> > 
> 
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Hello Thomas,

Thanks for your info.
What is the difference between using 
rte_eth_dev_get_port_by_name and 
RTE_ETH_FOREACH_MATCHING_DEV?

I think using rte_eth_dev_get_port_by_name is 
workable.
(In fact I modified my code already and it worked with no problem)

So my question is "what is the difference" and "which is better approach".

Thanks and BR,
Hideyuki Yamashita
NTT TechnoCross

> Hi,
> 
> I am actively working on it.
> Look how rte_eth_dev_attach is replaced in testpmd:
> 	https://patches.dpdk.org/patch/47019/
> It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> 
> 
> 22/10/2018 06:34, Hideyuki Yamashita:
> > Dear Thomas and all,
> > 
> > About a month ago, I posted the topic related with 
> > how to replace rte_eth_dev_attach.
> > 
> > Following your advice,
> > my code would be as below:
> > (Old code using deprecated API is commented out)
> > 
> > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > after rte_eal_hotplug_add.
> > Note that my application is just one of the dpdk applications(in the host)
> > and within the process, only one thread handles device attatch/detach.
> > (No race condition with regard to device hot_plug will
> > not take place)
> > -------------------------------------------------------------------
> >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> >         //if (ret < 0)
> >         //      return ret;
> > 
> >         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;
> >         }
> >         free(da.args);
> >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> >         if (ret < 0)
> >                 return ret;
> > -----------------------------------------------------------------------------
> > 
> > If you have any concerns/additional advices, please let me know.
> > 
> > BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> > 
> > 
> > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > Dear Thomas,
> > > > 
> > > > Thansk for your answer.
> > > > Please see inline.
> > > > 
> > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > Dear Thomas,
> > > > > > 
> > > > > > Thanks for your answer.
> > > > > > It took me a little time to digest answer.
> > > > > > Please see inline.
> > > > > > 
> > > > > > 
> > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > 
> > > > > > > > Thanks for your answer.
> > > > > > > > Please see inline.
> > > > > > > > 
> > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > Hello,
> > > > > > > > > > > 
> > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > 
> > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > and have to replace those.
> > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > 
> > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > 
> > > > > > > > > > > e.g. 
> > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > 
> > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > Thanks!
> > > > > > > > > > 
> > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > 
> > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > the necessary devices.
> > > > > > > > > > 
> > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > But I have questions.
> > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > 
> > > > > > > > Q1. 
> > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > 
> > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > 
> > > > > > > > at the end of the function.
> > > > > > > 
> > > > > > > You can have a race condition.
> > > > > > Please elaborate me a bit more.
> > > > > > 
> > > > > > Is it correct understanding that race condition 
> > > > > > includes
> > > > > > - read information before port is available
> > > > > > - other device may be plugged (or unplugged)
> > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > NOT reliable?
> > > > > 
> > > > > I am thinking about the second one only.
> > > > 
> > > > If we assume there is only one DPDK application 
> > > > inside the host and within the application, only one thread
> > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > *port_id = eth_dev_last_created_port;
> > > > because there seems no possiblity race condition
> > > > takes place?
> > > 
> > > If you are never probing a new port outside of this thread,
> > > I guess it's OK.
> > > Take care of not attaching from the interrupt thread too!
> > > 
> > > 
> > 
> > 
> > 
> > 
> 
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

Hi,

The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
	- it is a loop, so work if multiple ports are matching
	- it uses devargs parameter, which is what the user requests

Note: your code assumes that the ethdev name is devargs.name.
It can be true by chance, but nothing forces drivers to assign port names
this way. It will be wrong, for sure, if a device has multiple ports.


22/10/2018 13:24, Hideyuki Yamashita:
> Hello Thomas,
> 
> Thanks for your info.
> What is the difference between using 
> rte_eth_dev_get_port_by_name and 
> RTE_ETH_FOREACH_MATCHING_DEV?
> 
> I think using rte_eth_dev_get_port_by_name is 
> workable.
> (In fact I modified my code already and it worked with no problem)
> 
> So my question is "what is the difference" and "which is better approach".
> 
> Thanks and BR,
> Hideyuki Yamashita
> NTT TechnoCross
> 
> > Hi,
> > 
> > I am actively working on it.
> > Look how rte_eth_dev_attach is replaced in testpmd:
> > 	https://patches.dpdk.org/patch/47019/
> > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > 
> > 
> > 22/10/2018 06:34, Hideyuki Yamashita:
> > > Dear Thomas and all,
> > > 
> > > About a month ago, I posted the topic related with 
> > > how to replace rte_eth_dev_attach.
> > > 
> > > Following your advice,
> > > my code would be as below:
> > > (Old code using deprecated API is commented out)
> > > 
> > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > after rte_eal_hotplug_add.
> > > Note that my application is just one of the dpdk applications(in the host)
> > > and within the process, only one thread handles device attatch/detach.
> > > (No race condition with regard to device hot_plug will
> > > not take place)
> > > -------------------------------------------------------------------
> > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > >         //if (ret < 0)
> > >         //      return ret;
> > > 
> > >         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;
> > >         }
> > >         free(da.args);
> > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > >         if (ret < 0)
> > >                 return ret;
> > > -----------------------------------------------------------------------------
> > > 
> > > If you have any concerns/additional advices, please let me know.
> > > 
> > > BR,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > > 
> > > 
> > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > Dear Thomas,
> > > > > 
> > > > > Thansk for your answer.
> > > > > Please see inline.
> > > > > 
> > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > Dear Thomas,
> > > > > > > 
> > > > > > > Thanks for your answer.
> > > > > > > It took me a little time to digest answer.
> > > > > > > Please see inline.
> > > > > > > 
> > > > > > > 
> > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > 
> > > > > > > > > Thanks for your answer.
> > > > > > > > > Please see inline.
> > > > > > > > > 
> > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > Hello,
> > > > > > > > > > > > 
> > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > 
> > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > 
> > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > 
> > > > > > > > > > > > e.g. 
> > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > 
> > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > Thanks!
> > > > > > > > > > > 
> > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > 
> > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > the necessary devices.
> > > > > > > > > > > 
> > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > But I have questions.
> > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > 
> > > > > > > > > Q1. 
> > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > 
> > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > 
> > > > > > > > > at the end of the function.
> > > > > > > > 
> > > > > > > > You can have a race condition.
> > > > > > > Please elaborate me a bit more.
> > > > > > > 
> > > > > > > Is it correct understanding that race condition 
> > > > > > > includes
> > > > > > > - read information before port is available
> > > > > > > - other device may be plugged (or unplugged)
> > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > NOT reliable?
> > > > > > 
> > > > > > I am thinking about the second one only.
> > > > > 
> > > > > If we assume there is only one DPDK application 
> > > > > inside the host and within the application, only one thread
> > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > because there seems no possiblity race condition
> > > > > takes place?
> > > > 
> > > > If you are never probing a new port outside of this thread,
> > > > I guess it's OK.
> > > > Take care of not attaching from the interrupt thread too!

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Hi,

Thanks for your guidance again.

Q1.
Is following my understanding correct?
If a device has multiple port, then "rte_eth_dev_get_port_by_name"
will NOT work becauase it uses strcmp and needs "exact match"
of the device name.
New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
of this issue and even if the name is "parially matched" with 
the given parameter(user provided devargs).

Q2. 
If my program only handles devices which create only one port,
then "rte_eth_dev_get_port_by_name" may work.
(Though such a program does not have extensibility and
only workable under certain limitations)

Q3.
When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
will be available?
Do I have to wait 18.11 release or can I get those in git before release?

I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
is much better than using "rte_eth_dev_get_port_by_name" with regard
to handle devices which create multiple port.
But I need to replace existing deprecated attach/detach APIs to new
codes to maintain continuity of my product.

BR,
Hideyuki Yamashita
NTT TechnoCross

> Hi,
> 
> The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> 	- it is a loop, so work if multiple ports are matching
> 	- it uses devargs parameter, which is what the user requests
> 
> Note: your code assumes that the ethdev name is devargs.name.
> It can be true by chance, but nothing forces drivers to assign port names
> this way. It will be wrong, for sure, if a device has multiple ports.
> 
> 
> 22/10/2018 13:24, Hideyuki Yamashita:
> > Hello Thomas,
> > 
> > Thanks for your info.
> > What is the difference between using 
> > rte_eth_dev_get_port_by_name and 
> > RTE_ETH_FOREACH_MATCHING_DEV?
> > 
> > I think using rte_eth_dev_get_port_by_name is 
> > workable.
> > (In fact I modified my code already and it worked with no problem)
> > 
> > So my question is "what is the difference" and "which is better approach".
> > 
> > Thanks and BR,
> > Hideyuki Yamashita
> > NTT TechnoCross
> > 
> > > Hi,
> > > 
> > > I am actively working on it.
> > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > 	https://patches.dpdk.org/patch/47019/
> > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > 
> > > 
> > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > Dear Thomas and all,
> > > > 
> > > > About a month ago, I posted the topic related with 
> > > > how to replace rte_eth_dev_attach.
> > > > 
> > > > Following your advice,
> > > > my code would be as below:
> > > > (Old code using deprecated API is commented out)
> > > > 
> > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > after rte_eal_hotplug_add.
> > > > Note that my application is just one of the dpdk applications(in the host)
> > > > and within the process, only one thread handles device attatch/detach.
> > > > (No race condition with regard to device hot_plug will
> > > > not take place)
> > > > -------------------------------------------------------------------
> > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > >         //if (ret < 0)
> > > >         //      return ret;
> > > > 
> > > >         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;
> > > >         }
> > > >         free(da.args);
> > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > >         if (ret < 0)
> > > >                 return ret;
> > > > -----------------------------------------------------------------------------
> > > > 
> > > > If you have any concerns/additional advices, please let me know.
> > > > 
> > > > BR,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > > 
> > > > 
> > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > Dear Thomas,
> > > > > > 
> > > > > > Thansk for your answer.
> > > > > > Please see inline.
> > > > > > 
> > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > Dear Thomas,
> > > > > > > > 
> > > > > > > > Thanks for your answer.
> > > > > > > > It took me a little time to digest answer.
> > > > > > > > Please see inline.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > 
> > > > > > > > > > Thanks for your answer.
> > > > > > > > > > Please see inline.
> > > > > > > > > > 
> > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > 
> > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > 
> > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > 
> > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > But I have questions.
> > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > 
> > > > > > > > > > Q1. 
> > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > 
> > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > 
> > > > > > > > > > at the end of the function.
> > > > > > > > > 
> > > > > > > > > You can have a race condition.
> > > > > > > > Please elaborate me a bit more.
> > > > > > > > 
> > > > > > > > Is it correct understanding that race condition 
> > > > > > > > includes
> > > > > > > > - read information before port is available
> > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > NOT reliable?
> > > > > > > 
> > > > > > > I am thinking about the second one only.
> > > > > > 
> > > > > > If we assume there is only one DPDK application 
> > > > > > inside the host and within the application, only one thread
> > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > because there seems no possiblity race condition
> > > > > > takes place?
> > > > > 
> > > > > If you are never probing a new port outside of this thread,
> > > > > I guess it's OK.
> > > > > Take care of not attaching from the interrupt thread too!
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

Hi,

23/10/2018 03:52, Hideyuki Yamashita:
> Hi,
> 
> Thanks for your guidance again.
> 
> Q1.
> Is following my understanding correct?
> If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> will NOT work becauase it uses strcmp and needs "exact match"
> of the device name.
> New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> of this issue and even if the name is "parially matched" with 
> the given parameter(user provided devargs).

Yes

> Q2. 
> If my program only handles devices which create only one port,
> then "rte_eth_dev_get_port_by_name" may work.
> (Though such a program does not have extensibility and
> only workable under certain limitations)

Yes it may work with most of the drivers.

> Q3.
> When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> will be available?
> Do I have to wait 18.11 release or can I get those in git before release?

Better to wait 18.11.

> I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> is much better than using "rte_eth_dev_get_port_by_name" with regard
> to handle devices which create multiple port.
> But I need to replace existing deprecated attach/detach APIs to new
> codes to maintain continuity of my product.

We do not remove a method without a replacement.
You can use rte_eth_dev_attach() until 18.08,
and switch to the new iterator with 18.11.


> > Hi,
> > 
> > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > 	- it is a loop, so work if multiple ports are matching
> > 	- it uses devargs parameter, which is what the user requests
> > 
> > Note: your code assumes that the ethdev name is devargs.name.
> > It can be true by chance, but nothing forces drivers to assign port names
> > this way. It will be wrong, for sure, if a device has multiple ports.
> > 
> > 
> > 22/10/2018 13:24, Hideyuki Yamashita:
> > > Hello Thomas,
> > > 
> > > Thanks for your info.
> > > What is the difference between using 
> > > rte_eth_dev_get_port_by_name and 
> > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > 
> > > I think using rte_eth_dev_get_port_by_name is 
> > > workable.
> > > (In fact I modified my code already and it worked with no problem)
> > > 
> > > So my question is "what is the difference" and "which is better approach".
> > > 
> > > Thanks and BR,
> > > Hideyuki Yamashita
> > > NTT TechnoCross
> > > 
> > > > Hi,
> > > > 
> > > > I am actively working on it.
> > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > 	https://patches.dpdk.org/patch/47019/
> > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > 
> > > > 
> > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > Dear Thomas and all,
> > > > > 
> > > > > About a month ago, I posted the topic related with 
> > > > > how to replace rte_eth_dev_attach.
> > > > > 
> > > > > Following your advice,
> > > > > my code would be as below:
> > > > > (Old code using deprecated API is commented out)
> > > > > 
> > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > after rte_eal_hotplug_add.
> > > > > Note that my application is just one of the dpdk applications(in the host)
> > > > > and within the process, only one thread handles device attatch/detach.
> > > > > (No race condition with regard to device hot_plug will
> > > > > not take place)
> > > > > -------------------------------------------------------------------
> > > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > >         //if (ret < 0)
> > > > >         //      return ret;
> > > > > 
> > > > >         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;
> > > > >         }
> > > > >         free(da.args);
> > > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > >         if (ret < 0)
> > > > >                 return ret;
> > > > > -----------------------------------------------------------------------------
> > > > > 
> > > > > If you have any concerns/additional advices, please let me know.
> > > > > 
> > > > > BR,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > 
> > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > Dear Thomas,
> > > > > > > 
> > > > > > > Thansk for your answer.
> > > > > > > Please see inline.
> > > > > > > 
> > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > Dear Thomas,
> > > > > > > > > 
> > > > > > > > > Thanks for your answer.
> > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > Please see inline.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > > 
> > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > Please see inline.
> > > > > > > > > > > 
> > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > > But I have questions.
> > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > > 
> > > > > > > > > > > Q1. 
> > > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > > 
> > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > 
> > > > > > > > > > > at the end of the function.
> > > > > > > > > > 
> > > > > > > > > > You can have a race condition.
> > > > > > > > > Please elaborate me a bit more.
> > > > > > > > > 
> > > > > > > > > Is it correct understanding that race condition 
> > > > > > > > > includes
> > > > > > > > > - read information before port is available
> > > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > > NOT reliable?
> > > > > > > > 
> > > > > > > > I am thinking about the second one only.
> > > > > > > 
> > > > > > > If we assume there is only one DPDK application 
> > > > > > > inside the host and within the application, only one thread
> > > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > because there seems no possiblity race condition
> > > > > > > takes place?
> > > > > > 
> > > > > > If you are never probing a new port outside of this thread,
> > > > > > I guess it's OK.
> > > > > > Take care of not attaching from the interrupt thread too!

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Hi,

> Yes it may work with most of the drivers.
Question for my understadnding.
You said that most of the drivers assign only one 
port when hotplug_add is called, right?
Then what is the exception?
What kind of device/driver assign multiple ports?

My program attach to 
 - vhost pmd
 - pcap pmd
 - null pmd
and I understand those device(?) assign only one port per attach.

BR,
Hideyuki Yamashita
NTT TechnoCross


> Hi,
> 
> 23/10/2018 03:52, Hideyuki Yamashita:
> > Hi,
> > 
> > Thanks for your guidance again.
> > 
> > Q1.
> > Is following my understanding correct?
> > If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> > will NOT work becauase it uses strcmp and needs "exact match"
> > of the device name.
> > New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> > of this issue and even if the name is "parially matched" with 
> > the given parameter(user provided devargs).
> 
> Yes
> 
> > Q2. 
> > If my program only handles devices which create only one port,
> > then "rte_eth_dev_get_port_by_name" may work.
> > (Though such a program does not have extensibility and
> > only workable under certain limitations)
> 
> Yes it may work with most of the drivers.
> 
> > Q3.
> > When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> > will be available?
> > Do I have to wait 18.11 release or can I get those in git before release?
> 
> Better to wait 18.11.
> 
> > I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> > is much better than using "rte_eth_dev_get_port_by_name" with regard
> > to handle devices which create multiple port.
> > But I need to replace existing deprecated attach/detach APIs to new
> > codes to maintain continuity of my product.
> 
> We do not remove a method without a replacement.
> You can use rte_eth_dev_attach() until 18.08,
> and switch to the new iterator with 18.11.
> 
> 
> > > Hi,
> > > 
> > > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > > 	- it is a loop, so work if multiple ports are matching
> > > 	- it uses devargs parameter, which is what the user requests
> > > 
> > > Note: your code assumes that the ethdev name is devargs.name.
> > > It can be true by chance, but nothing forces drivers to assign port names
> > > this way. It will be wrong, for sure, if a device has multiple ports.
> > > 
> > > 
> > > 22/10/2018 13:24, Hideyuki Yamashita:
> > > > Hello Thomas,
> > > > 
> > > > Thanks for your info.
> > > > What is the difference between using 
> > > > rte_eth_dev_get_port_by_name and 
> > > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > > 
> > > > I think using rte_eth_dev_get_port_by_name is 
> > > > workable.
> > > > (In fact I modified my code already and it worked with no problem)
> > > > 
> > > > So my question is "what is the difference" and "which is better approach".
> > > > 
> > > > Thanks and BR,
> > > > Hideyuki Yamashita
> > > > NTT TechnoCross
> > > > 
> > > > > Hi,
> > > > > 
> > > > > I am actively working on it.
> > > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > > 	https://patches.dpdk.org/patch/47019/
> > > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > > 
> > > > > 
> > > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > > Dear Thomas and all,
> > > > > > 
> > > > > > About a month ago, I posted the topic related with 
> > > > > > how to replace rte_eth_dev_attach.
> > > > > > 
> > > > > > Following your advice,
> > > > > > my code would be as below:
> > > > > > (Old code using deprecated API is commented out)
> > > > > > 
> > > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > > after rte_eal_hotplug_add.
> > > > > > Note that my application is just one of the dpdk applications(in the host)
> > > > > > and within the process, only one thread handles device attatch/detach.
> > > > > > (No race condition with regard to device hot_plug will
> > > > > > not take place)
> > > > > > -------------------------------------------------------------------
> > > > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > > >         //if (ret < 0)
> > > > > >         //      return ret;
> > > > > > 
> > > > > >         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;
> > > > > >         }
> > > > > >         free(da.args);
> > > > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > > >         if (ret < 0)
> > > > > >                 return ret;
> > > > > > -----------------------------------------------------------------------------
> > > > > > 
> > > > > > If you have any concerns/additional advices, please let me know.
> > > > > > 
> > > > > > BR,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > > 
> > > > > > 
> > > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > > Dear Thomas,
> > > > > > > > 
> > > > > > > > Thansk for your answer.
> > > > > > > > Please see inline.
> > > > > > > > 
> > > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > > Dear Thomas,
> > > > > > > > > > 
> > > > > > > > > > Thanks for your answer.
> > > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > > Please see inline.
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > 
> > > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > > > But I have questions.
> > > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > > > 
> > > > > > > > > > > > Q1. 
> > > > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > > > 
> > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > > 
> > > > > > > > > > > > at the end of the function.
> > > > > > > > > > > 
> > > > > > > > > > > You can have a race condition.
> > > > > > > > > > Please elaborate me a bit more.
> > > > > > > > > > 
> > > > > > > > > > Is it correct understanding that race condition 
> > > > > > > > > > includes
> > > > > > > > > > - read information before port is available
> > > > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > > > NOT reliable?
> > > > > > > > > 
> > > > > > > > > I am thinking about the second one only.
> > > > > > > > 
> > > > > > > > If we assume there is only one DPDK application 
> > > > > > > > inside the host and within the application, only one thread
> > > > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > because there seems no possiblity race condition
> > > > > > > > takes place?
> > > > > > > 
> > > > > > > If you are never probing a new port outside of this thread,
> > > > > > > I guess it's OK.
> > > > > > > Take care of not attaching from the interrupt thread too!
> 
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

25/10/2018 04:54, Hideyuki Yamashita:
> Hi,
> 
> > Yes it may work with most of the drivers.
> Question for my understadnding.
> You said that most of the drivers assign only one 
> port when hotplug_add is called, right?
> Then what is the exception?
> What kind of device/driver assign multiple ports?

At least Mellanox and Chelsio, maybe more.

> My program attach to 
>  - vhost pmd
>  - pcap pmd
>  - null pmd
> and I understand those device(?) assign only one port per attach.

No problem with vdevs.


> > Hi,
> > 
> > 23/10/2018 03:52, Hideyuki Yamashita:
> > > Hi,
> > > 
> > > Thanks for your guidance again.
> > > 
> > > Q1.
> > > Is following my understanding correct?
> > > If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> > > will NOT work becauase it uses strcmp and needs "exact match"
> > > of the device name.
> > > New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> > > of this issue and even if the name is "parially matched" with 
> > > the given parameter(user provided devargs).
> > 
> > Yes
> > 
> > > Q2. 
> > > If my program only handles devices which create only one port,
> > > then "rte_eth_dev_get_port_by_name" may work.
> > > (Though such a program does not have extensibility and
> > > only workable under certain limitations)
> > 
> > Yes it may work with most of the drivers.
> > 
> > > Q3.
> > > When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> > > will be available?
> > > Do I have to wait 18.11 release or can I get those in git before release?
> > 
> > Better to wait 18.11.
> > 
> > > I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> > > is much better than using "rte_eth_dev_get_port_by_name" with regard
> > > to handle devices which create multiple port.
> > > But I need to replace existing deprecated attach/detach APIs to new
> > > codes to maintain continuity of my product.
> > 
> > We do not remove a method without a replacement.
> > You can use rte_eth_dev_attach() until 18.08,
> > and switch to the new iterator with 18.11.
> > 
> > 
> > > > Hi,
> > > > 
> > > > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > > > 	- it is a loop, so work if multiple ports are matching
> > > > 	- it uses devargs parameter, which is what the user requests
> > > > 
> > > > Note: your code assumes that the ethdev name is devargs.name.
> > > > It can be true by chance, but nothing forces drivers to assign port names
> > > > this way. It will be wrong, for sure, if a device has multiple ports.
> > > > 
> > > > 
> > > > 22/10/2018 13:24, Hideyuki Yamashita:
> > > > > Hello Thomas,
> > > > > 
> > > > > Thanks for your info.
> > > > > What is the difference between using 
> > > > > rte_eth_dev_get_port_by_name and 
> > > > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > > > 
> > > > > I think using rte_eth_dev_get_port_by_name is 
> > > > > workable.
> > > > > (In fact I modified my code already and it worked with no problem)
> > > > > 
> > > > > So my question is "what is the difference" and "which is better approach".
> > > > > 
> > > > > Thanks and BR,
> > > > > Hideyuki Yamashita
> > > > > NTT TechnoCross
> > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > I am actively working on it.
> > > > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > > > 	https://patches.dpdk.org/patch/47019/
> > > > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > > > 
> > > > > > 
> > > > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > > > Dear Thomas and all,
> > > > > > > 
> > > > > > > About a month ago, I posted the topic related with 
> > > > > > > how to replace rte_eth_dev_attach.
> > > > > > > 
> > > > > > > Following your advice,
> > > > > > > my code would be as below:
> > > > > > > (Old code using deprecated API is commented out)
> > > > > > > 
> > > > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > > > after rte_eal_hotplug_add.
> > > > > > > Note that my application is just one of the dpdk applications(in the host)
> > > > > > > and within the process, only one thread handles device attatch/detach.
> > > > > > > (No race condition with regard to device hot_plug will
> > > > > > > not take place)
> > > > > > > -------------------------------------------------------------------
> > > > > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > > > >         //if (ret < 0)
> > > > > > >         //      return ret;
> > > > > > > 
> > > > > > >         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;
> > > > > > >         }
> > > > > > >         free(da.args);
> > > > > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > > > >         if (ret < 0)
> > > > > > >                 return ret;
> > > > > > > -----------------------------------------------------------------------------
> > > > > > > 
> > > > > > > If you have any concerns/additional advices, please let me know.
> > > > > > > 
> > > > > > > BR,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > > 
> > > > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > > > Dear Thomas,
> > > > > > > > > 
> > > > > > > > > Thansk for your answer.
> > > > > > > > > Please see inline.
> > > > > > > > > 
> > > > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > > > Dear Thomas,
> > > > > > > > > > > 
> > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > > > Please see inline.
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > > > > But I have questions.
> > > > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Q1. 
> > > > > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > > > 
> > > > > > > > > > > > > at the end of the function.
> > > > > > > > > > > > 
> > > > > > > > > > > > You can have a race condition.
> > > > > > > > > > > Please elaborate me a bit more.
> > > > > > > > > > > 
> > > > > > > > > > > Is it correct understanding that race condition 
> > > > > > > > > > > includes
> > > > > > > > > > > - read information before port is available
> > > > > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > > > > NOT reliable?
> > > > > > > > > > 
> > > > > > > > > > I am thinking about the second one only.
> > > > > > > > > 
> > > > > > > > > If we assume there is only one DPDK application 
> > > > > > > > > inside the host and within the application, only one thread
> > > > > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > because there seems no possiblity race condition
> > > > > > > > > takes place?
> > > > > > > > 
> > > > > > > > If you are never probing a new port outside of this thread,
> > > > > > > > I guess it's OK.
> > > > > > > > Take care of not attaching from the interrupt thread too!
> > 
> > 
> > 
> > 
> 
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Hideyuki Yamashita]

Hi, 

> 25/10/2018 04:54, Hideyuki Yamashita:
> > Hi,
> > 
> > > Yes it may work with most of the drivers.
> > Question for my understadnding.
> > You said that most of the drivers assign only one 
> > port when hotplug_add is called, right?
> > Then what is the exception?
> > What kind of device/driver assign multiple ports?
> 
> At least Mellanox and Chelsio, maybe more.

You are saying about physical device(e.g. NIC).
Is my understanding correct ?

> > My program attach to 
> >  - vhost pmd
> >  - pcap pmd
> >  - null pmd
> > and I understand those device(?) assign only one port per attach.
> 
> No problem with vdevs.

In my program, program never attach(hotplug) to physical devices
dynamically.
Like L2fwd example, physical NICs are bound using dpdk/usertoools/
dpdk-setup.sh.
My program only attaches to vdevs dynamically.
In the above case, "rte_eth_dev_get_port_by_name" is sufficient enough.
Is my understanding correct?

BR,
Hideyuki Yamashita
NTT TechnoCross
> 
> > > Hi,
> > > 
> > > 23/10/2018 03:52, Hideyuki Yamashita:
> > > > Hi,
> > > > 
> > > > Thanks for your guidance again.
> > > > 
> > > > Q1.
> > > > Is following my understanding correct?
> > > > If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> > > > will NOT work becauase it uses strcmp and needs "exact match"
> > > > of the device name.
> > > > New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> > > > of this issue and even if the name is "parially matched" with 
> > > > the given parameter(user provided devargs).
> > > 
> > > Yes
> > > 
> > > > Q2. 
> > > > If my program only handles devices which create only one port,
> > > > then "rte_eth_dev_get_port_by_name" may work.
> > > > (Though such a program does not have extensibility and
> > > > only workable under certain limitations)
> > > 
> > > Yes it may work with most of the drivers.
> > > 
> > > > Q3.
> > > > When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> > > > will be available?
> > > > Do I have to wait 18.11 release or can I get those in git before release?
> > > 
> > > Better to wait 18.11.
> > > 
> > > > I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> > > > is much better than using "rte_eth_dev_get_port_by_name" with regard
> > > > to handle devices which create multiple port.
> > > > But I need to replace existing deprecated attach/detach APIs to new
> > > > codes to maintain continuity of my product.
> > > 
> > > We do not remove a method without a replacement.
> > > You can use rte_eth_dev_attach() until 18.08,
> > > and switch to the new iterator with 18.11.
> > > 
> > > 
> > > > > Hi,
> > > > > 
> > > > > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > > > > 	- it is a loop, so work if multiple ports are matching
> > > > > 	- it uses devargs parameter, which is what the user requests
> > > > > 
> > > > > Note: your code assumes that the ethdev name is devargs.name.
> > > > > It can be true by chance, but nothing forces drivers to assign port names
> > > > > this way. It will be wrong, for sure, if a device has multiple ports.
> > > > > 
> > > > > 
> > > > > 22/10/2018 13:24, Hideyuki Yamashita:
> > > > > > Hello Thomas,
> > > > > > 
> > > > > > Thanks for your info.
> > > > > > What is the difference between using 
> > > > > > rte_eth_dev_get_port_by_name and 
> > > > > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > > > > 
> > > > > > I think using rte_eth_dev_get_port_by_name is 
> > > > > > workable.
> > > > > > (In fact I modified my code already and it worked with no problem)
> > > > > > 
> > > > > > So my question is "what is the difference" and "which is better approach".
> > > > > > 
> > > > > > Thanks and BR,
> > > > > > Hideyuki Yamashita
> > > > > > NTT TechnoCross
> > > > > > 
> > > > > > > Hi,
> > > > > > > 
> > > > > > > I am actively working on it.
> > > > > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > > > > 	https://patches.dpdk.org/patch/47019/
> > > > > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > > > > 
> > > > > > > 
> > > > > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > > > > Dear Thomas and all,
> > > > > > > > 
> > > > > > > > About a month ago, I posted the topic related with 
> > > > > > > > how to replace rte_eth_dev_attach.
> > > > > > > > 
> > > > > > > > Following your advice,
> > > > > > > > my code would be as below:
> > > > > > > > (Old code using deprecated API is commented out)
> > > > > > > > 
> > > > > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > > > > after rte_eal_hotplug_add.
> > > > > > > > Note that my application is just one of the dpdk applications(in the host)
> > > > > > > > and within the process, only one thread handles device attatch/detach.
> > > > > > > > (No race condition with regard to device hot_plug will
> > > > > > > > not take place)
> > > > > > > > -------------------------------------------------------------------
> > > > > > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > > > > >         //if (ret < 0)
> > > > > > > >         //      return ret;
> > > > > > > > 
> > > > > > > >         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;
> > > > > > > >         }
> > > > > > > >         free(da.args);
> > > > > > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > > > > >         if (ret < 0)
> > > > > > > >                 return ret;
> > > > > > > > -----------------------------------------------------------------------------
> > > > > > > > 
> > > > > > > > If you have any concerns/additional advices, please let me know.
> > > > > > > > 
> > > > > > > > BR,
> > > > > > > > Hideyuki Yamashita
> > > > > > > > NTT TechnoCross
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > > > > Dear Thomas,
> > > > > > > > > > 
> > > > > > > > > > Thansk for your answer.
> > > > > > > > > > Please see inline.
> > > > > > > > > > 
> > > > > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > > > > Dear Thomas,
> > > > > > > > > > > > 
> > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > > > > > But I have questions.
> > > > > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Q1. 
> > > > > > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > at the end of the function.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > You can have a race condition.
> > > > > > > > > > > > Please elaborate me a bit more.
> > > > > > > > > > > > 
> > > > > > > > > > > > Is it correct understanding that race condition 
> > > > > > > > > > > > includes
> > > > > > > > > > > > - read information before port is available
> > > > > > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > > > > > NOT reliable?
> > > > > > > > > > > 
> > > > > > > > > > > I am thinking about the second one only.
> > > > > > > > > > 
> > > > > > > > > > If we assume there is only one DPDK application 
> > > > > > > > > > inside the host and within the application, only one thread
> > > > > > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > because there seems no possiblity race condition
> > > > > > > > > > takes place?
> > > > > > > > > 
> > > > > > > > > If you are never probing a new port outside of this thread,
> > > > > > > > > I guess it's OK.
> > > > > > > > > Take care of not attaching from the interrupt thread too!
> > > 
> > > 
> > > 
> > > 
> > 
> > 
> > 
> > 
> 
> 
> 
> 

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Thomas Monjalon]

25/10/2018 11:46, Hideyuki Yamashita:
> Hi, 
> 
> > 25/10/2018 04:54, Hideyuki Yamashita:
> > > Hi,
> > > 
> > > > Yes it may work with most of the drivers.
> > > Question for my understadnding.
> > > You said that most of the drivers assign only one 
> > > port when hotplug_add is called, right?
> > > Then what is the exception?
> > > What kind of device/driver assign multiple ports?
> > 
> > At least Mellanox and Chelsio, maybe more.
> 
> You are saying about physical device(e.g. NIC).
> Is my understanding correct ?

Yes

> > > My program attach to 
> > >  - vhost pmd
> > >  - pcap pmd
> > >  - null pmd
> > > and I understand those device(?) assign only one port per attach.
> > 
> > No problem with vdevs.
> 
> In my program, program never attach(hotplug) to physical devices
> dynamically.
> Like L2fwd example, physical NICs are bound using dpdk/usertoools/
> dpdk-setup.sh.
> My program only attaches to vdevs dynamically.
> In the above case, "rte_eth_dev_get_port_by_name" is sufficient enough.
> Is my understanding correct?

Yes

 
> > > > Hi,
> > > > 
> > > > 23/10/2018 03:52, Hideyuki Yamashita:
> > > > > Hi,
> > > > > 
> > > > > Thanks for your guidance again.
> > > > > 
> > > > > Q1.
> > > > > Is following my understanding correct?
> > > > > If a device has multiple port, then "rte_eth_dev_get_port_by_name"
> > > > > will NOT work becauase it uses strcmp and needs "exact match"
> > > > > of the device name.
> > > > > New iterator RTE_ETH_FOREACH_MATCHING_DEV takes care 
> > > > > of this issue and even if the name is "parially matched" with 
> > > > > the given parameter(user provided devargs).
> > > > 
> > > > Yes
> > > > 
> > > > > Q2. 
> > > > > If my program only handles devices which create only one port,
> > > > > then "rte_eth_dev_get_port_by_name" may work.
> > > > > (Though such a program does not have extensibility and
> > > > > only workable under certain limitations)
> > > > 
> > > > Yes it may work with most of the drivers.
> > > > 
> > > > > Q3.
> > > > > When new iterator  RTE_ETH_FOREACH_MATCHING_DEV
> > > > > will be available?
> > > > > Do I have to wait 18.11 release or can I get those in git before release?
> > > > 
> > > > Better to wait 18.11.
> > > > 
> > > > > I agree with your guidance that RTE_ETH_FOREACH_MATCHING_DEV
> > > > > is much better than using "rte_eth_dev_get_port_by_name" with regard
> > > > > to handle devices which create multiple port.
> > > > > But I need to replace existing deprecated attach/detach APIs to new
> > > > > codes to maintain continuity of my product.
> > > > 
> > > > We do not remove a method without a replacement.
> > > > You can use rte_eth_dev_attach() until 18.08,
> > > > and switch to the new iterator with 18.11.
> > > > 
> > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > The better approach is using RTE_ETH_FOREACH_MATCHING_DEV for 2 reasons:
> > > > > > 	- it is a loop, so work if multiple ports are matching
> > > > > > 	- it uses devargs parameter, which is what the user requests
> > > > > > 
> > > > > > Note: your code assumes that the ethdev name is devargs.name.
> > > > > > It can be true by chance, but nothing forces drivers to assign port names
> > > > > > this way. It will be wrong, for sure, if a device has multiple ports.
> > > > > > 
> > > > > > 
> > > > > > 22/10/2018 13:24, Hideyuki Yamashita:
> > > > > > > Hello Thomas,
> > > > > > > 
> > > > > > > Thanks for your info.
> > > > > > > What is the difference between using 
> > > > > > > rte_eth_dev_get_port_by_name and 
> > > > > > > RTE_ETH_FOREACH_MATCHING_DEV?
> > > > > > > 
> > > > > > > I think using rte_eth_dev_get_port_by_name is 
> > > > > > > workable.
> > > > > > > (In fact I modified my code already and it worked with no problem)
> > > > > > > 
> > > > > > > So my question is "what is the difference" and "which is better approach".
> > > > > > > 
> > > > > > > Thanks and BR,
> > > > > > > Hideyuki Yamashita
> > > > > > > NTT TechnoCross
> > > > > > > 
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > I am actively working on it.
> > > > > > > > Look how rte_eth_dev_attach is replaced in testpmd:
> > > > > > > > 	https://patches.dpdk.org/patch/47019/
> > > > > > > > It is using a new ethdev iterator RTE_ETH_FOREACH_MATCHING_DEV.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 22/10/2018 06:34, Hideyuki Yamashita:
> > > > > > > > > Dear Thomas and all,
> > > > > > > > > 
> > > > > > > > > About a month ago, I posted the topic related with 
> > > > > > > > > how to replace rte_eth_dev_attach.
> > > > > > > > > 
> > > > > > > > > Following your advice,
> > > > > > > > > my code would be as below:
> > > > > > > > > (Old code using deprecated API is commented out)
> > > > > > > > > 
> > > > > > > > > rte_eth_dev_get_port_by_name is used to retrieve dpdk port
> > > > > > > > > after rte_eal_hotplug_add.
> > > > > > > > > Note that my application is just one of the dpdk applications(in the host)
> > > > > > > > > and within the process, only one thread handles device attatch/detach.
> > > > > > > > > (No race condition with regard to device hot_plug will
> > > > > > > > > not take place)
> > > > > > > > > -------------------------------------------------------------------
> > > > > > > > >         //ret = rte_eth_dev_attach(devargs, &vhost_port_id);
> > > > > > > > >         //if (ret < 0)
> > > > > > > > >         //      return ret;
> > > > > > > > > 
> > > > > > > > >         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;
> > > > > > > > >         }
> > > > > > > > >         free(da.args);
> > > > > > > > >         ret = rte_eth_dev_get_port_by_name(da.name, &vhost_port_id);
> > > > > > > > >         if (ret < 0)
> > > > > > > > >                 return ret;
> > > > > > > > > -----------------------------------------------------------------------------
> > > > > > > > > 
> > > > > > > > > If you have any concerns/additional advices, please let me know.
> > > > > > > > > 
> > > > > > > > > BR,
> > > > > > > > > Hideyuki Yamashita
> > > > > > > > > NTT TechnoCross
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > 27/09/2018 12:40, Hideyuki Yamashita:
> > > > > > > > > > > Dear Thomas,
> > > > > > > > > > > 
> > > > > > > > > > > Thansk for your answer.
> > > > > > > > > > > Please see inline.
> > > > > > > > > > > 
> > > > > > > > > > > > 27/09/2018 03:38, Hideyuki Yamashita:
> > > > > > > > > > > > > Dear Thomas,
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > > It took me a little time to digest answer.
> > > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > > 21/09/2018 09:19, Hideyuki Yamashita:
> > > > > > > > > > > > > > > Dear Gaetan and Thomas, 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Thanks for your answer.
> > > > > > > > > > > > > > > Please see inline.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > 20/09/2018 11:09, Ga?an Rivet:
> > > > > > > > > > > > > > > > > On Thu, Sep 20, 2018 at 05:46:37PM +0900, Hideyuki Yamashita wrote:
> > > > > > > > > > > > > > > > > > Hello,
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > From dpdk 18.08 release rte_eth_dev_attach and 
> > > > > > > > > > > > > > > > > > rte_eth_dev_detach becom deprecated API and 
> > > > > > > > > > > > > > > > > > it is recommended to replace with rte_eal_hotplug_add
> > > > > > > > > > > > > > > > > > and rte_eal_hotplug_remove.
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > My program uses  above mentioned deprecated APIs
> > > > > > > > > > > > > > > > > > and have to replace those.
> > > > > > > > > > > > > > > > > > Note that my program uses attach to attach vhost, pcap pmd.
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > My question is whether it is correct to replace those as following:
> > > > > > > > > > > > > > > > > > find rte_eth_dev_attach function in rte_ethdev.c and
> > > > > > > > > > > > > > > > > > migrate those content into my program.
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > e.g. 
> > > > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 643-686 for attach
> > > > > > > > > > > > > > > > > > lib/librte_ethdev/rte_ethdev.c line 690-720 for detach
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > Your advice/guidance are much appreciated.
> > > > > > > > > > > > > > > > > > Thanks!
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > Hello Hideyuki,
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > You could use this code for guidance, while leaving the ethdev
> > > > > > > > > > > > > > > > > specificities such as verifying the eth_dev_count_total(). The hotplug
> > > > > > > > > > > > > > > > > function would already return an error if the PMD was not able to create
> > > > > > > > > > > > > > > > > the necessary devices.
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > The main issue might be to find the port_id of your new port.
> > > > > > > > > > > > > > > > > You won't be able to use eth_dev_last_created_port, so you would have to
> > > > > > > > > > > > > > > > > iterate over the ethdev using RTE_ETH_FOREACH_DEV and find the one
> > > > > > > > > > > > > > > > > matching your parameters (you might for example match the rte_device
> > > > > > > > > > > > > > > > > name with the name you used in hotplug_add, as there is no standard
> > > > > > > > > > > > > > > > > naming scheme at the ethdev level).
> > > > > > > > > > > > > > > First of all, thank for your answering to my question.
> > > > > > > > > > > > > > > But I have questions.
> > > > > > > > > > > > > > > (Sorry, I have poor knowledge about dpdk and have many basic questions)
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Q1. 
> > > > > > > > > > > > > > > Why eth_dev_last_created_port can not be used?
> > > > > > > > > > > > > > > When I look into rte_eth_dev_atthach in 18.08, it calls 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > at the end of the function.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > You can have a race condition.
> > > > > > > > > > > > > Please elaborate me a bit more.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Is it correct understanding that race condition 
> > > > > > > > > > > > > includes
> > > > > > > > > > > > > - read information before port is available
> > > > > > > > > > > > > - other device may be plugged (or unplugged)
> > > > > > > > > > > > > and so using "eth_dev_last_created_port" is 
> > > > > > > > > > > > > NOT reliable?
> > > > > > > > > > > > 
> > > > > > > > > > > > I am thinking about the second one only.
> > > > > > > > > > > 
> > > > > > > > > > > If we assume there is only one DPDK application 
> > > > > > > > > > > inside the host and within the application, only one thread
> > > > > > > > > > > handles attach/detach of devices, then is it ok to use 
> > > > > > > > > > > > > > > *port_id = eth_dev_last_created_port;
> > > > > > > > > > > because there seems no possiblity race condition
> > > > > > > > > > > takes place?
> > > > > > > > > > 
> > > > > > > > > > If you are never probing a new port outside of this thread,
> > > > > > > > > > I guess it's OK.
> > > > > > > > > > Take care of not attaching from the interrupt thread too!

Re: How to replace rte_eth_dev_attach with rte_eal_hotplug_add

[Andrzej Ostruszka]

On 27.09.2018 12:40, Hideyuki Yamashita wrote:
[...]
>>> Is it correct understanding that race condition 
>>> includes
>>> - read information before port is available
>>> - other device may be plugged (or unplugged)
>>> and so using "eth_dev_last_created_port" is 
>>> NOT reliable?
>>
>> I am thinking about the second one only.
> 
> If we assume there is only one DPDK application 
> inside the host and within the application, only one thread
> handles attach/detach of devices, then is it ok to use

This depends also a bit on the PMD you intend to use.  Some PMDs can
create more than 1 port (e.g. for internal purposes) in which case
eth_dev_last_created_port is not reliable.

Best regards
Andrzej