[dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status

[dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Bruce Richardson]

When querying the link status via telemetry interface, we don't want the
client to have to wait for multiple seconds for a reply. Therefore use
"rte_eth_link_get_nowait()" rather than "rte_eth_link_get()" in the
telemetry callback.

Cc: stable@dpdk.org
Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_ethdev/rte_ethdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 17ddacc78..1f4545fe0 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -5692,7 +5692,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
-	ret = rte_eth_link_get(port_id, &link);
+	ret = rte_eth_link_get_nowait(port_id, &link);
 	if (ret < 0)
 		return -1;
 
-- 
2.27.0

Re: [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Power, Ciara]

Hi Bruce,

>-----Original Message-----
>From: Richardson, Bruce <bruce.richardson@intel.com>
>Sent: Thursday 14 January 2021 12:18
>To: dev@dpdk.org
>Cc: Richardson, Bruce <bruce.richardson@intel.com>; stable@dpdk.org;
>Thomas Monjalon <thomas@monjalon.net>; Yigit, Ferruh
><ferruh.yigit@intel.com>; Andrew Rybchenko
><andrew.rybchenko@oktetlabs.ru>; Power, Ciara <ciara.power@intel.com>;
>Wiles, Keith <keith.wiles@intel.com>
>Subject: [PATCH] ethdev: avoid blocking telemetry callback for link status
>
>When querying the link status via telemetry interface, we don't want the
>client to have to wait for multiple seconds for a reply. Therefore use
>"rte_eth_link_get_nowait()" rather than "rte_eth_link_get()" in the telemetry
>callback.
>
>Cc: stable@dpdk.org
>Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
>
>Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>---
> lib/librte_ethdev/rte_ethdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>index 17ddacc78..1f4545fe0 100644
>--- a/lib/librte_ethdev/rte_ethdev.c
>+++ b/lib/librte_ethdev/rte_ethdev.c
>@@ -5692,7 +5692,7 @@ eth_dev_handle_port_link_status(const char *cmd
>__rte_unused,
> 	if (!rte_eth_dev_is_valid_port(port_id))
> 		return -1;
>
>-	ret = rte_eth_link_get(port_id, &link);
>+	ret = rte_eth_link_get_nowait(port_id, &link);
> 	if (ret < 0)
> 		return -1;
>
>--
>2.27.0

This change looks good to me.

Acked-by: Ciara Power <ciara.power@intel.com>

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Thomas Monjalon]

> >When querying the link status via telemetry interface, we don't want the
> >client to have to wait for multiple seconds for a reply. Therefore use
> >"rte_eth_link_get_nowait()" rather than "rte_eth_link_get()" in the telemetry
> >callback.
> >
> >Cc: stable@dpdk.org
> >Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
> >
> >Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >---
> >--- a/lib/librte_ethdev/rte_ethdev.c
> >+++ b/lib/librte_ethdev/rte_ethdev.c
> >@@ -5692,7 +5692,7 @@ eth_dev_handle_port_link_status(const char *cmd
> >-	ret = rte_eth_link_get(port_id, &link);
> >+	ret = rte_eth_link_get_nowait(port_id, &link);
> 
> This change looks good to me.
> 
> Acked-by: Ciara Power <ciara.power@intel.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Ferruh Yigit]

On 1/18/2021 2:02 PM, Thomas Monjalon wrote:
>>> When querying the link status via telemetry interface, we don't want the
>>> client to have to wait for multiple seconds for a reply. Therefore use
>>> "rte_eth_link_get_nowait()" rather than "rte_eth_link_get()" in the telemetry
>>> callback.
>>>
>>> Cc: stable@dpdk.org
>>> Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>> ---
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -5692,7 +5692,7 @@ eth_dev_handle_port_link_status(const char *cmd
>>> -	ret = rte_eth_link_get(port_id, &link);
>>> +	ret = rte_eth_link_get_nowait(port_id, &link);
>>
>> This change looks good to me.
>>
>> Acked-by: Ciara Power <ciara.power@intel.com>
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 

Applied to dpdk-next-net/main, thanks.

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Min Hu (Connor)]

Hi, Bruce and all,
	Do you know the difference between "rte_eth_link_get" and
"rte_eth_link_get_nowait"? I know they call funciton "link_update"
with differenct parameter "wait_to_complete"(set 1 means wait, set 0
  means not wait). But how to define the "wait" time, and why it shoud wait?
	On the further, What are the application scenarios of the two
APIs?

	Look forward to your reply, thanks.

在 2021/1/14 20:17, Bruce Richardson 写道:
> When querying the link status via telemetry interface, we don't want the
> client to have to wait for multiple seconds for a reply. Therefore use
> "rte_eth_link_get_nowait()" rather than "rte_eth_link_get()" in the
> telemetry callback.
> 
> Cc: stable@dpdk.org
> Fixes: c190daedb9b1 ("ethdev: add telemetry callbacks")
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/librte_ethdev/rte_ethdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 17ddacc78..1f4545fe0 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -5692,7 +5692,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
>   	if (!rte_eth_dev_is_valid_port(port_id))
>   		return -1;
>   
> -	ret = rte_eth_link_get(port_id, &link);
> +	ret = rte_eth_link_get_nowait(port_id, &link);
>   	if (ret < 0)
>   		return -1;
>   
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Stephen Hemminger]

On Tue, 19 Jan 2021 09:06:48 +0800
"Min Hu (Connor)" <humin29@huawei.com> wrote:

> Hi, Bruce and all,
> 	Do you know the difference between "rte_eth_link_get" and
> "rte_eth_link_get_nowait"? I know they call funciton "link_update"
> with differenct parameter "wait_to_complete"(set 1 means wait, set 0
>   means not wait). But how to define the "wait" time, and why it shoud wait?
> 	On the further, What are the application scenarios of the two
> APIs?
> 

The default behavior of rte_eth_link_get (in some drivers) is to wait
for link to come up.  Many drivers don't do this. It seems mostly the
Intel ones that do.

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Min Hu (Connor)]

Thank Stephen,
but in which the scenarios, it should wait link to up, in which
scenarios, it should not ?
By the way, how to define the "wait" time value ?



在 2021/1/19 10:26, Stephen Hemminger 写道:
> On Tue, 19 Jan 2021 09:06:48 +0800
> "Min Hu (Connor)" <humin29@huawei.com> wrote:
> 
>> Hi, Bruce and all,
>> 	Do you know the difference between "rte_eth_link_get" and
>> "rte_eth_link_get_nowait"? I know they call funciton "link_update"
>> with differenct parameter "wait_to_complete"(set 1 means wait, set 0
>>    means not wait). But how to define the "wait" time, and why it shoud wait?
>> 	On the further, What are the application scenarios of the two
>> APIs?
>>
> 
> The default behavior of rte_eth_link_get (in some drivers) is to wait
> for link to come up.  Many drivers don't do this. It seems mostly the
> Intel ones that do.
> .
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status

[Bruce Richardson]

On Tue, Jan 19, 2021 at 10:58:42AM +0800, Min Hu (Connor) wrote:
> Thank Stephen,
> but in which the scenarios, it should wait link to up, in which
> scenarios, it should not ?
> By the way, how to define the "wait" time value ?
> 

I believe the documentation somewhere refers to a wait time of up to 9
seconds, but I don't think it's terribly well defined. Some Intel NICs will
stall for a few seconds if the link is not yet stable or if the link
is down. With the link up, I don't think any NIC fails to return
immediately.
However, if delays in the app are something you want to avoid, the _nowait
varient is the one to call to be sure.

/Bruce

> 
> 
> 在 2021/1/19 10:26, Stephen Hemminger 写道:
> > On Tue, 19 Jan 2021 09:06:48 +0800
> > "Min Hu (Connor)" <humin29@huawei.com> wrote:
> > 
> > > Hi, Bruce and all,
> > > 	Do you know the difference between "rte_eth_link_get" and
> > > "rte_eth_link_get_nowait"? I know they call funciton "link_update"
> > > with differenct parameter "wait_to_complete"(set 1 means wait, set 0
> > >    means not wait). But how to define the "wait" time, and why it shoud wait?
> > > 	On the further, What are the application scenarios of the two
> > > APIs?
> > > 
> > 
> > The default behavior of rte_eth_link_get (in some drivers) is to wait
> > for link to come up.  Many drivers don't do this. It seems mostly the
> > Intel ones that do.
> > .
> >