patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status
@ 2021-01-14 12:17 Bruce Richardson
  2021-01-14 15:06 ` Power, Ciara
  2021-01-19  1:06 ` Min Hu (Connor)
  0 siblings, 2 replies; 8+ messages in thread
From: Bruce Richardson @ 2021-01-14 12:17 UTC (permalink / raw)
  To: dev
  Cc: Bruce Richardson, stable, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko, Ciara Power, Keith Wiles

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


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

* Re: [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-14 12:17 [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status Bruce Richardson
@ 2021-01-14 15:06 ` Power, Ciara
  2021-01-18 14:02   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
  2021-01-19  1:06 ` Min Hu (Connor)
  1 sibling, 1 reply; 8+ messages in thread
From: Power, Ciara @ 2021-01-14 15:06 UTC (permalink / raw)
  To: Richardson, Bruce, dev
  Cc: stable, Thomas Monjalon, Yigit, Ferruh, Andrew Rybchenko, Wiles, Keith

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>

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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-14 15:06 ` Power, Ciara
@ 2021-01-18 14:02   ` Thomas Monjalon
  2021-01-18 14:48     ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2021-01-18 14:02 UTC (permalink / raw)
  To: Richardson, Bruce
  Cc: dev, stable, Yigit, Ferruh, Andrew Rybchenko, Wiles, Keith, Power, Ciara

> >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>




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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-18 14:02   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
@ 2021-01-18 14:48     ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2021-01-18 14:48 UTC (permalink / raw)
  To: Thomas Monjalon, Richardson, Bruce
  Cc: dev, stable, Andrew Rybchenko, Wiles, Keith, Power, Ciara

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.

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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-14 12:17 [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status Bruce Richardson
  2021-01-14 15:06 ` Power, Ciara
@ 2021-01-19  1:06 ` Min Hu (Connor)
  2021-01-19  2:26   ` Stephen Hemminger
  1 sibling, 1 reply; 8+ messages in thread
From: Min Hu (Connor) @ 2021-01-19  1:06 UTC (permalink / raw)
  To: Bruce Richardson, dev
  Cc: stable, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko,
	Ciara Power, Keith Wiles

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;
>   
> 

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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-19  1:06 ` Min Hu (Connor)
@ 2021-01-19  2:26   ` Stephen Hemminger
  2021-01-19  2:58     ` Min Hu (Connor)
  0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2021-01-19  2:26 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: Bruce Richardson, dev, stable, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko, Ciara Power, Keith Wiles

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.

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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-19  2:26   ` Stephen Hemminger
@ 2021-01-19  2:58     ` Min Hu (Connor)
  2021-01-19 10:06       ` Bruce Richardson
  0 siblings, 1 reply; 8+ messages in thread
From: Min Hu (Connor) @ 2021-01-19  2:58 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Bruce Richardson, dev, stable, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko, Ciara Power, Keith Wiles, Beilei Xing,
	Jeff Guo, Haiyue Wang, Ajit Khaparde, somnath.kotur, qiming.yang,
	qi.z.zhang

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.
> .
> 

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

* Re: [dpdk-stable] [dpdk-dev] [PATCH] ethdev: avoid blocking telemetry callback for link status
  2021-01-19  2:58     ` Min Hu (Connor)
@ 2021-01-19 10:06       ` Bruce Richardson
  0 siblings, 0 replies; 8+ messages in thread
From: Bruce Richardson @ 2021-01-19 10:06 UTC (permalink / raw)
  To: Min Hu (Connor)
  Cc: Stephen Hemminger, dev, stable, Thomas Monjalon, Ferruh Yigit,
	Andrew Rybchenko, Ciara Power, Keith Wiles, Beilei Xing,
	Jeff Guo, Haiyue Wang, Ajit Khaparde, somnath.kotur, qiming.yang,
	qi.z.zhang

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.
> > .
> > 

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

end of thread, other threads:[~2021-01-19 10:06 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-14 12:17 [dpdk-stable] [PATCH] ethdev: avoid blocking telemetry callback for link status Bruce Richardson
2021-01-14 15:06 ` Power, Ciara
2021-01-18 14:02   ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2021-01-18 14:48     ` Ferruh Yigit
2021-01-19  1:06 ` Min Hu (Connor)
2021-01-19  2:26   ` Stephen Hemminger
2021-01-19  2:58     ` Min Hu (Connor)
2021-01-19 10:06       ` Bruce Richardson

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git