* [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
@ 2019-12-09 18:23 Mike Manning
2019-12-12 22:30 ` Thomas Monjalon
0 siblings, 1 reply; 5+ messages in thread
From: Mike Manning @ 2019-12-09 18:23 UTC (permalink / raw)
To: dev; +Cc: Nélio Laranjeiro
The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
status does not correspond to link speed. If status is DOWN, the speed
is expected to be ETH_SPEED_NUM_NONE (0). But as the link speed is -1
on admin down, modify the check to account for this.
Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to complete")
Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
---
drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index d80ae458b..6ef2dfd74 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev,
ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
ETH_LINK_SPEED_FIXED);
- if (((dev_link.link_speed && !dev_link.link_status) ||
- (!dev_link.link_speed && dev_link.link_status))) {
+ if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
+ ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
rte_errno = EAGAIN;
return -rte_errno;
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
2019-12-09 18:23 [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down Mike Manning
@ 2019-12-12 22:30 ` Thomas Monjalon
2019-12-13 14:43 ` Slava Ovsiienko
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Monjalon @ 2019-12-12 22:30 UTC (permalink / raw)
To: dev
Cc: Mike Manning, Nélio Laranjeiro, matan, viacheslavo, shahafs,
rasland
+Cc maintainers
09/12/2019 19:23, Mike Manning:
> The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
> status does not correspond to link speed. If status is DOWN, the speed
> is expected to be ETH_SPEED_NUM_NONE (0). But as the link speed is -1
> on admin down, modify the check to account for this.
>
> Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to complete")
> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
>
> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> ---
> drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index d80ae458b..6ef2dfd74 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev,
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> ETH_LINK_SPEED_FIXED);
> - if (((dev_link.link_speed && !dev_link.link_status) ||
> - (!dev_link.link_speed && dev_link.link_status))) {
> + if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
> + ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
> rte_errno = EAGAIN;
> return -rte_errno;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
2019-12-12 22:30 ` Thomas Monjalon
@ 2019-12-13 14:43 ` Slava Ovsiienko
2019-12-13 15:11 ` Mike Manning
0 siblings, 1 reply; 5+ messages in thread
From: Slava Ovsiienko @ 2019-12-13 14:43 UTC (permalink / raw)
To: Thomas Monjalon, dev
Cc: Mike Manning, Nélio Laranjeiro, Matan Azrad, Shahaf Shuler,
Raslan Darawsheh
Hi, Mike
In the mlx5_link_update_unlocked_gs() the dev_link.link_speed is set like this:
dev_link.link_speed = (ecmd->speed == UINT32_MAX) ? ETH_SPEED_NUM_NONE : ecmd->speed;
So, dev_link.link_speed can't be assigned with -1. Do I misunderstand you commit message?
With best regards, Slava
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, December 13, 2019 0:31
> To: dev@dpdk.org
> Cc: Mike Manning <mmanning@vyatta.att-mail.com>; Nélio Laranjeiro
> <nelio.laranjeiro@6wind.com>; Matan Azrad <matan@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
>
> +Cc maintainers
>
> 09/12/2019 19:23, Mike Manning:
> > The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
> > status does not correspond to link speed. If status is DOWN, the speed
> > is expected to be ETH_SPEED_NUM_NONE (0). But as the link speed is -1
> > on admin down, modify the check to account for this.
> >
> > Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to
> > complete")
> > Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> >
> > Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> > ---
> > drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> > b/drivers/net/mlx5/mlx5_ethdev.c index d80ae458b..6ef2dfd74 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct
> rte_eth_dev *dev,
> > ETH_LINK_HALF_DUPLEX :
> ETH_LINK_FULL_DUPLEX);
> > dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> > ETH_LINK_SPEED_FIXED);
> > - if (((dev_link.link_speed && !dev_link.link_status) ||
> > - (!dev_link.link_speed && dev_link.link_status))) {
> > + if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
> > + ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
> > rte_errno = EAGAIN;
> > return -rte_errno;
> > }
> >
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
2019-12-13 14:43 ` Slava Ovsiienko
@ 2019-12-13 15:11 ` Mike Manning
2019-12-14 9:40 ` Slava Ovsiienko
0 siblings, 1 reply; 5+ messages in thread
From: Mike Manning @ 2019-12-13 15:11 UTC (permalink / raw)
To: Slava Ovsiienko, Thomas Monjalon, dev
Cc: Nélio Laranjeiro, Matan Azrad, Shahaf Shuler, Raslan Darawsheh
Hi Slava,
Thanks, you are right - I retract my patch, as this was fixed in v19.08
by 6fd05da9efbd ("net/mlx5: fix link speed info when link is down") as
per the line you indicate.
I should explain that we are using Debian 10 which uses v18.11, here we
had a painful issue with the combination of this -EAGAIN and use of
netlink to obtain the ifindex resulting in concurrent use of the netlink
socket causing the calling thread to block indefinitely.
Regards
Mike
On 13/12/2019 14:43, Slava Ovsiienko wrote:
> Hi, Mike
>
> In the mlx5_link_update_unlocked_gs() the dev_link.link_speed is set like this:
>
> dev_link.link_speed = (ecmd->speed == UINT32_MAX) ? ETH_SPEED_NUM_NONE : ecmd->speed;
>
> So, dev_link.link_speed can't be assigned with -1. Do I misunderstand you commit message?
>
> With best regards, Slava
>
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Friday, December 13, 2019 0:31
>> To: dev@dpdk.org
>> Cc: Mike Manning <mmanning@vyatta.att-mail.com>; Nélio Laranjeiro
>> <nelio.laranjeiro@6wind.com>; Matan Azrad <matan@mellanox.com>; Slava
>> Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
>> <shahafs@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
>>
>> +Cc maintainers
>>
>> 09/12/2019 19:23, Mike Manning:
>>> The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
>>> status does not correspond to link speed. If status is DOWN, the speed
>>> is expected to be ETH_SPEED_NUM_NONE (0). But as the link speed is -1
>>> on admin down, modify the check to account for this.
>>>
>>> Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to
>>> complete")
>>> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>
>>> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
>>> ---
>>> drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
>>> b/drivers/net/mlx5/mlx5_ethdev.c index d80ae458b..6ef2dfd74 100644
>>> --- a/drivers/net/mlx5/mlx5_ethdev.c
>>> +++ b/drivers/net/mlx5/mlx5_ethdev.c
>>> @@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct
>> rte_eth_dev *dev,
>>> ETH_LINK_HALF_DUPLEX :
>> ETH_LINK_FULL_DUPLEX);
>>> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
>>> ETH_LINK_SPEED_FIXED);
>>> - if (((dev_link.link_speed && !dev_link.link_status) ||
>>> - (!dev_link.link_speed && dev_link.link_status))) {
>>> + if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
>>> + ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
>>> rte_errno = EAGAIN;
>>> return -rte_errno;
>>> }
>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
2019-12-13 15:11 ` Mike Manning
@ 2019-12-14 9:40 ` Slava Ovsiienko
0 siblings, 0 replies; 5+ messages in thread
From: Slava Ovsiienko @ 2019-12-14 9:40 UTC (permalink / raw)
To: mmanning, Thomas Monjalon, dev
Cc: Nélio Laranjeiro, Matan Azrad, Shahaf Shuler, Raslan Darawsheh
Hi, Mike
> -----Original Message-----
> From: Mike Manning <mmanning@vyatta.att-mail.com>
> Sent: Friday, December 13, 2019 17:12
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org
> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Raslan
> Darawsheh <rasland@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
>
> Hi Slava,
> Thanks, you are right - I retract my patch, as this was fixed in v19.08 by
> 6fd05da9efbd ("net/mlx5: fix link speed info when link is down") as per the
> line you indicate.
>
> I should explain that we are using Debian 10 which uses v18.11, here we had a
> painful issue with the combination of this -EAGAIN and use of netlink to
> obtain the ifindex resulting in concurrent use of the netlink socket causing the
> calling thread to block indefinitely.
JFYI, there is the patch: http://patches.dpdk.org/patch/56813/
It provides caching for ifindex, netlink is not involved anymore.
With best regards, Slava
>
> Regards
> Mike
>
> On 13/12/2019 14:43, Slava Ovsiienko wrote:
> > Hi, Mike
> >
> > In the mlx5_link_update_unlocked_gs() the dev_link.link_speed is set like
> this:
> >
> > dev_link.link_speed = (ecmd->speed == UINT32_MAX) ?
> ETH_SPEED_NUM_NONE
> > : ecmd->speed;
> >
> > So, dev_link.link_speed can't be assigned with -1. Do I misunderstand you
> commit message?
> >
> > With best regards, Slava
> >
> >> -----Original Message-----
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >> Sent: Friday, December 13, 2019 0:31
> >> To: dev@dpdk.org
> >> Cc: Mike Manning <mmanning@vyatta.att-mail.com>; Nélio Laranjeiro
> >> <nelio.laranjeiro@6wind.com>; Matan Azrad <matan@mellanox.com>;
> Slava
> >> Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
> >> <shahafs@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
> >> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
> >>
> >> +Cc maintainers
> >>
> >> 09/12/2019 19:23, Mike Manning:
> >>> The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
> >>> status does not correspond to link speed. If status is DOWN, the
> >>> speed is expected to be ETH_SPEED_NUM_NONE (0). But as the link
> >>> speed is -1 on admin down, modify the check to account for this.
> >>>
> >>> Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to
> >>> complete")
> >>> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
> >>>
> >>> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> >>> ---
> >>> drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> >>> b/drivers/net/mlx5/mlx5_ethdev.c index d80ae458b..6ef2dfd74 100644
> >>> --- a/drivers/net/mlx5/mlx5_ethdev.c
> >>> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> >>> @@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct
> >> rte_eth_dev *dev,
> >>> ETH_LINK_HALF_DUPLEX :
> >> ETH_LINK_FULL_DUPLEX);
> >>> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> >>> ETH_LINK_SPEED_FIXED);
> >>> - if (((dev_link.link_speed && !dev_link.link_status) ||
> >>> - (!dev_link.link_speed && dev_link.link_status))) {
> >>> + if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
> >>> + ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
> >>> rte_errno = EAGAIN;
> >>> return -rte_errno;
> >>> }
> >>>
> >>
> >>
> >>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-12-14 9:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-09 18:23 [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down Mike Manning
2019-12-12 22:30 ` Thomas Monjalon
2019-12-13 14:43 ` Slava Ovsiienko
2019-12-13 15:11 ` Mike Manning
2019-12-14 9:40 ` Slava Ovsiienko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).