DPDK patches and discussions
 help / color / mirror / Atom feed
From: Long Li <longli@microsoft.com>
To: "longli@linuxonhyperv.com" <longli@linuxonhyperv.com>,
	Ferruh Yigit <ferruh.yigit@amd.com>,
	Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	Wei Hu <weh@microsoft.com>,
	Stephen Hemminger <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [EXTERNAL] [PATCH 1/4] net/netvsc: scan all net devices under the PCI device
Date: Fri, 7 Feb 2025 22:06:03 +0000	[thread overview]
Message-ID: <SA6PR21MB42315368828B9851E80B1D0BCEF12@SA6PR21MB4231.namprd21.prod.outlook.com> (raw)
In-Reply-To: <1738028106-25239-1-git-send-email-longli@linuxonhyperv.com>

> Subject: [EXTERNAL] [PATCH 1/4] net/netvsc: scan all net devices under the PCI
> device
> 
> From: Long Li <longli@microsoft.com>
> 
> The current code has the wrong assumption that a PCI device can have only one
> Ethernet device. This is not correct as a PCI device can be multi functional and
> have multiple Ethernet devices.
> 
> Fix this by scanning all the devices under a PCI device.
> 
> Fixes: a2a23a794b3a ("net/netvsc: support VF device hot add/remove")
> Cc: stable@dpdk.org
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>  drivers/net/netvsc/hn_ethdev.c | 29 ++++++++++++-----------------
>  1 file changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
> index 1736cb5d07..f848157b49 100644
> --- a/drivers/net/netvsc/hn_ethdev.c
> +++ b/drivers/net/netvsc/hn_ethdev.c
> @@ -570,7 +570,7 @@ static void netvsc_hotplug_retry(void *args)
>  	struct rte_devargs *d = &hot_ctx->da;
>  	char buf[256];
> 
> -	DIR *di;
> +	DIR *di = NULL;
>  	struct dirent *dir;
>  	struct ifreq req;
>  	struct rte_ether_addr eth_addr;
> @@ -590,7 +590,9 @@ static void netvsc_hotplug_retry(void *args)
>  	if (!di) {
>  		PMD_DRV_LOG(DEBUG, "%s: can't open directory %s, "
>  			    "retrying in 1 second", __func__, buf);
> -		goto retry;
> +		/* The device is still being initialized, retry after 1 second */
> +		rte_eal_alarm_set(1000000, netvsc_hotplug_retry, hot_ctx);
> +		return;
>  	}
> 
>  	while ((dir = readdir(di))) {
> @@ -614,10 +616,9 @@ static void netvsc_hotplug_retry(void *args)
>  				    dir->d_name);
>  			break;
>  		}
> -		if (req.ifr_hwaddr.sa_family != ARPHRD_ETHER) {
> -			closedir(di);
> -			goto free_hotadd_ctx;
> -		}
> +		if (req.ifr_hwaddr.sa_family != ARPHRD_ETHER)
> +			continue;
> +
>  		memcpy(eth_addr.addr_bytes, req.ifr_hwaddr.sa_data,
>  		       RTE_DIM(eth_addr.addr_bytes));
> 
> @@ -636,22 +637,16 @@ static void netvsc_hotplug_retry(void *args)
>  				PMD_DRV_LOG(ERR,
>  					    "Failed to add PCI device %s",
>  					    d->name);
> -				break;
>  			}
> +
> +			break;
>  		}
> -		/* When the code reaches here, we either have already added
> -		 * the device, or its MAC address did not match.
> -		 */
> -		closedir(di);
> -		goto free_hotadd_ctx;
>  	}
> -	closedir(di);
> -retry:
> -	/* The device is still being initialized, retry after 1 second */
> -	rte_eal_alarm_set(1000000, netvsc_hotplug_retry, hot_ctx);
> -	return;
> 
>  free_hotadd_ctx:
> +	if (di)
> +		closedir(di);
> +
>  	rte_spinlock_lock(&hv->hotadd_lock);
>  	LIST_REMOVE(hot_ctx, list);
>  	rte_spinlock_unlock(&hv->hotadd_lock);
> --
> 2.34.1


Hi Stephen, Ferruh

I'm going to work an EAL patch to replace the 4th patch in the series as suggested by Stephen.

Is it okay to merge the first three patches of this series?

Thanks,
Long

      parent reply	other threads:[~2025-02-07 22:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28  1:35 longli
2025-01-28  1:35 ` [PATCH 2/4] net/netvsc: remove RTE device if all its net devices are removed longli
2025-01-28  1:35 ` [PATCH 3/4] net/netvsc: log error on failure to switch data path longli
2025-01-28  1:35 ` [PATCH 4/4] net/netvsc: cache device parameters for hot plug events longli
2025-01-28 21:00   ` Stephen Hemminger
2025-01-29  0:10     ` [EXTERNAL] " Long Li
2025-01-29  0:31       ` Stephen Hemminger
2025-01-29  0:54         ` Long Li
2025-01-30  3:59       ` Stephen Hemminger
2025-02-03 19:00         ` Long Li
2025-01-28 21:01   ` Stephen Hemminger
2025-02-07 22:06 ` Long Li [this message]

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=SA6PR21MB42315368828B9851E80B1D0BCEF12@SA6PR21MB4231.namprd21.prod.outlook.com \
    --to=longli@microsoft.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=longli@linuxonhyperv.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    --cc=weh@microsoft.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).