DPDK patches and discussions
 help / color / mirror / Atom feed
From: Matan Azrad <matan@mellanox.com>
To: "Benoit Ganne (bganne)" <bganne@cisco.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix link state update
Date: Mon, 30 Mar 2020 10:12:45 +0000
Message-ID: <AM0PR0502MB4019095F23AC15AEEEC2088DD2CB0@AM0PR0502MB4019.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <CH2PR11MB4327F3DBB07B6C72CB7B8BD8C1CB0@CH2PR11MB4327.namprd11.prod.outlook.com>

Hi

From: Benoit Ganne (bganne) <bganne@cisco.com>
> > From: Matan Azrad <matan@mellanox.com>
> 
> >> From: Benoît Ganne
> >> mlx5 PMD refuses to update link state if link speed is defined but
> >> status is down or if link speed is undefined but status is up, even
> >> if the ioctl() succeeded.
> >> This prevents application to detect link up/down event, especially
> >> when the link speed is not correctly detected.
> 
> > Do you use the wait option? Or no wait?
> 
> We are using the no wait option.

I suggest to call again if failed for N retries time.

> >> As link speed is nice to have whereas link status is mandatory for
> >> operations, always update link state regardless of link speed.
> >> The application can then check link speed if needs be.
> 
> > Is it documented well? I didn't find doc\description says link speed
> > is best effort.
> > PMD cannot guess whether link speed is mandatory for the user or not....
> 
> I do not think it is documented and I suppose at the end of the day it
> depends from the HW and configurations...
> My commit message is misleading, my apologizes. What I meant was to let
> the app decide whether it should retry or not, based on the data it gets.
> Right now, the PMD *prevents* the app to get link state if the link speed is
> undefined even if the app does not care about link speed.

In mlx5 this is not the case, we have no one updated and second not - there are going together:

You can see that we have 2 different system calls: 1 to get up\down and second to get link speed.
If link speed doesn't appropriate to the link state it may say that something was changed between the calls and the link status we got from the first call is not correct anymore.

In this case, we should call both calls again, that’s what we are doing in "nowait" option.
If the user doesn't want "nowait" option, (means PMD is not allowed to take more time for response) he should call again when the callback failed in the time and retries manner the user prefers.

> It makes an
> assumption on behalf of the apps. Moreover, it seems to be one of the few
> PMDs to make this assumption: at least the mlx4 PMD and the i40e PMD
> does not seem to make this assumption.
> This is a problem for our app (fd.io VPP) but I was also told about other apps
> having the same issue too.
> 
> > I think, at least you should update ethdev relevant API descriptions
> > and get agreement from more PMD maintainers...
> 
> I hope to get feedback 😊 it is precisely the goal of this.
> 
> ben

  reply	other threads:[~2020-03-30 10:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27 17:24 Benoît Ganne
2020-03-29  7:00 ` Matan Azrad
2020-03-30  9:55   ` Benoit Ganne (bganne)
2020-03-30 10:12     ` Matan Azrad [this message]
2020-03-30 12:03       ` Benoit Ganne (bganne)
2020-03-30 13:44         ` Matan Azrad
2020-03-30 13:53           ` Benoit Ganne (bganne)
2020-03-30 16:13             ` Matan Azrad
2020-04-01 10:17               ` Benoit Ganne (bganne)
2020-04-01 12:46                 ` Matan Azrad
2020-04-07 12:54               ` Thomas Monjalon
2020-04-07 13:41                 ` Benoit Ganne (bganne)
2020-11-19  8:30                   ` Matan Azrad
2020-11-19 14:20 ` [dpdk-dev] [PATCH v2] " Raslan Darawsheh
2020-11-19 16:27   ` Raslan Darawsheh
2020-11-19 17:48   ` Ferruh Yigit
2020-11-19 18:42     ` Thomas Monjalon
2020-11-20 10:51       ` Ferruh Yigit
2020-11-20 13:50         ` Thomas Monjalon
2020-11-20 14:21           ` Ferruh Yigit
2020-11-22 10:03             ` Raslan Darawsheh
2020-11-22 10:04   ` [dpdk-dev] [PATCH v3] net/mlx5: allow unknown link speed Raslan Darawsheh
2020-11-22 12:58     ` Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR0502MB4019095F23AC15AEEEC2088DD2CB0@AM0PR0502MB4019.eurprd05.prod.outlook.com \
    --to=matan@mellanox.com \
    --cc=bganne@cisco.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/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 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

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


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