Linux-HyperV Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
@ 2019-07-23 19:02 Haiyang Zhang
  2019-07-25 18:46 ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Haiyang Zhang @ 2019-07-23 19:02 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	davem, linux-kernel

Previously the async probing caused NIC naming in random order.

The patch adds a dev_num field in vmbus channel structure. It’s assigned
to the first available number when the channel is offered. So netvsc can
use it for NIC naming based on channel offer sequence. Now we re-enable
the async probing mode by default for faster probing.

Also added a modules parameter, probe_type, to set sync probing mode if
a user wants to.

Fixes: af0a5646cb8d ("use the new async probing feature for the hyperv drivers")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/channel_mgmt.c       | 46 +++++++++++++++++++++++++++++++++++++++--
 drivers/net/hyperv/netvsc_drv.c | 33 ++++++++++++++++++++++++++---
 include/linux/hyperv.h          |  4 ++++
 3 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index addcef5..ab7c05b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -304,6 +304,8 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
 
 EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
 
+#define HV_DEV_NUM_INVALID (-1)
+
 /*
  * alloc_channel - Allocate and initialize a vmbus channel object
  */
@@ -315,6 +317,8 @@ static struct vmbus_channel *alloc_channel(void)
 	if (!channel)
 		return NULL;
 
+	channel->dev_num = HV_DEV_NUM_INVALID;
+
 	spin_lock_init(&channel->lock);
 	init_completion(&channel->rescind_event);
 
@@ -533,6 +537,42 @@ static void vmbus_add_channel_work(struct work_struct *work)
 }
 
 /*
+ * Get the first available device number of its type, then
+ * record it in the channel structure.
+ */
+static void hv_set_devnum(struct vmbus_channel *newchannel)
+{
+	struct vmbus_channel *channel;
+	unsigned int i = 0;
+	bool found;
+
+	BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
+
+	/* Only HV_NIC uses this number for now */
+	if (hv_get_dev_type(newchannel) != HV_NIC)
+		return;
+
+next:
+	found = false;
+
+	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+		if (i == channel->dev_num &&
+		    guid_equal(&channel->offermsg.offer.if_type,
+			       &newchannel->offermsg.offer.if_type)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (found) {
+		i++;
+		goto next;
+	}
+
+	newchannel->dev_num = i;
+}
+
+/*
  * vmbus_process_offer - Process the offer by creating a channel/device
  * associated with this offer
  */
@@ -561,10 +601,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 		}
 	}
 
-	if (fnew)
+	if (fnew) {
+		hv_set_devnum(newchannel);
+
 		list_add_tail(&newchannel->listentry,
 			      &vmbus_connection.chn_list);
-	else {
+	} else {
 		/*
 		 * Check to see if this is a valid sub-channel.
 		 */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index afdcc56..af53690 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -57,6 +57,10 @@
 module_param(debug, int, 0444);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+static unsigned int probe_type __ro_after_init = PROBE_PREFER_ASYNCHRONOUS;
+module_param(probe_type, uint, 0444);
+MODULE_PARM_DESC(probe_type, "Probe type: 1=async(default), 2=sync");
+
 static LIST_HEAD(netvsc_dev_list);
 
 static void netvsc_change_rx_flags(struct net_device *net, int change)
@@ -2233,10 +2237,19 @@ static int netvsc_probe(struct hv_device *dev,
 	struct net_device_context *net_device_ctx;
 	struct netvsc_device_info *device_info = NULL;
 	struct netvsc_device *nvdev;
+	char name[IFNAMSIZ];
 	int ret = -ENOMEM;
 
-	net = alloc_etherdev_mq(sizeof(struct net_device_context),
-				VRSS_CHANNEL_MAX);
+	if (probe_type == PROBE_PREFER_ASYNCHRONOUS) {
+		snprintf(name, IFNAMSIZ, "eth%d", dev->channel->dev_num);
+		net = alloc_netdev_mqs(sizeof(struct net_device_context), name,
+				       NET_NAME_ENUM, ether_setup,
+				       VRSS_CHANNEL_MAX, VRSS_CHANNEL_MAX);
+	} else {
+		net = alloc_etherdev_mq(sizeof(struct net_device_context),
+					VRSS_CHANNEL_MAX);
+	}
+
 	if (!net)
 		goto no_net;
 
@@ -2323,6 +2336,14 @@ static int netvsc_probe(struct hv_device *dev,
 		net->max_mtu = ETH_DATA_LEN;
 
 	ret = register_netdevice(net);
+
+	if (ret == -EEXIST) {
+		pr_info("NIC name %s exists, request another name.\n",
+			net->name);
+		strlcpy(net->name, "eth%d", IFNAMSIZ);
+		ret = register_netdevice(net);
+	}
+
 	if (ret != 0) {
 		pr_err("Unable to register netdev.\n");
 		goto register_failed;
@@ -2407,7 +2428,7 @@ static int netvsc_remove(struct hv_device *dev)
 	.probe = netvsc_probe,
 	.remove = netvsc_remove,
 	.driver = {
-		.probe_type = PROBE_FORCE_SYNCHRONOUS,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 };
 
@@ -2473,6 +2494,12 @@ static int __init netvsc_drv_init(void)
 	}
 	netvsc_ring_bytes = ring_size * PAGE_SIZE;
 
+	if (probe_type != PROBE_PREFER_ASYNCHRONOUS)
+		probe_type = PROBE_FORCE_SYNCHRONOUS;
+
+	netvsc_drv.driver.probe_type = probe_type;
+	pr_info("probe_type: %u\n", probe_type);
+
 	ret = vmbus_driver_register(&netvsc_drv);
 	if (ret)
 		return ret;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc3..12fc5ea 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -841,6 +841,10 @@ struct vmbus_channel {
 	 */
 	struct vmbus_channel *primary_channel;
 	/*
+	 * Used for device naming based on channel offer sequence.
+	 */
+	int dev_num;
+	/*
 	 * Support per-channel state for use by vmbus drivers.
 	 */
 	void *per_channel_state;
-- 
1.8.3.1


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

* Re: [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
  2019-07-23 19:02 [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default Haiyang Zhang
@ 2019-07-25 18:46 ` David Miller
  0 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2019-07-25 18:46 UTC (permalink / raw)
  To: haiyangz
  Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
	linux-kernel


1) Subject: line lacks proper subsystem prefix

2) No module parameters in networking drivers, sorry.  Find some generic way to do
   this via devlink or similar.


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

* RE: [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
  2019-07-10  0:29 ` David Miller
@ 2019-07-10  1:04   ` Haiyang Zhang
  0 siblings, 0 replies; 7+ messages in thread
From: Haiyang Zhang @ 2019-07-10  1:04 UTC (permalink / raw)
  To: David Miller
  Cc: sashal, linux-hyperv, netdev, KY Srinivasan, Stephen Hemminger,
	olaf, vkuznets, linux-kernel



> -----Original Message-----
> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of David Miller
> Sent: Tuesday, July 9, 2019 8:30 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net-next] Name NICs based on vmbus offer and enable
> async probe by default
> 
> 
> The net-next tree, if you are reading netdev today, has been closed.
I will re-submit when the tree re-opened. 
Thanks,
- Haiyang

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

* RE: [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
  2019-07-09 23:47 ` Stephen Hemminger
@ 2019-07-10  1:03   ` Haiyang Zhang
  0 siblings, 0 replies; 7+ messages in thread
From: Haiyang Zhang @ 2019-07-10  1:03 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: sashal, linux-hyperv, netdev, KY Srinivasan, Stephen Hemminger,
	olaf, vkuznets, davem, linux-kernel



> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Tuesday, July 9, 2019 7:47 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: sashal@kernel.org; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; olaf@aepfle.de; vkuznets
> <vkuznets@redhat.com>; davem@davemloft.net; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH net-next] Name NICs based on vmbus offer and enable
> async probe by default
> 
> On Tue, 9 Jul 2019 22:56:30 +0000
> Haiyang Zhang <haiyangz@microsoft.com> wrote:
> 
> > -				VRSS_CHANNEL_MAX);
> > +	if (probe_type == PROBE_PREFER_ASYNCHRONOUS) {
> > +		snprintf(name, IFNAMSIZ, "eth%d", dev->channel->dev_num);
> 
> What about PCI passthrough or VF devices that are also being probed and
> consuming the ethN names.  Won't there be a collision?

VF usually shows up a few seconds later than the synthetic NIC. Faster probing
will reduce the probability of collision.
Even if a collision happens, the code below will re-register the NIC with "eth%d":
+	if (ret == -EEXIST) {
+		pr_info("NIC name %s exists, request another name.\n",
+			net->name);
+		strlcpy(net->name, "eth%d", IFNAMSIZ);
+		ret = register_netdevice(net);
+	}

Thanks,
- Haiyang

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

* Re: [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
  2019-07-09 22:56 Haiyang Zhang
  2019-07-09 23:47 ` Stephen Hemminger
@ 2019-07-10  0:29 ` David Miller
  2019-07-10  1:04   ` Haiyang Zhang
  1 sibling, 1 reply; 7+ messages in thread
From: David Miller @ 2019-07-10  0:29 UTC (permalink / raw)
  To: haiyangz
  Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
	linux-kernel


The net-next tree, if you are reading netdev today, has been closed.

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

* Re: [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
  2019-07-09 22:56 Haiyang Zhang
@ 2019-07-09 23:47 ` Stephen Hemminger
  2019-07-10  1:03   ` Haiyang Zhang
  2019-07-10  0:29 ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2019-07-09 23:47 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: sashal, linux-hyperv, netdev, KY Srinivasan, Stephen Hemminger,
	olaf, vkuznets, davem, linux-kernel

On Tue, 9 Jul 2019 22:56:30 +0000
Haiyang Zhang <haiyangz@microsoft.com> wrote:

> -				VRSS_CHANNEL_MAX);
> +	if (probe_type == PROBE_PREFER_ASYNCHRONOUS) {
> +		snprintf(name, IFNAMSIZ, "eth%d", dev->channel->dev_num);

What about PCI passthrough or VF devices that are also being probed and
consuming the ethN names.  Won't there be a collision?

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

* [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default
@ 2019-07-09 22:56 Haiyang Zhang
  2019-07-09 23:47 ` Stephen Hemminger
  2019-07-10  0:29 ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Haiyang Zhang @ 2019-07-09 22:56 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf, vkuznets,
	davem, linux-kernel

Previously the async probing caused NIC naming in random order.

The patch adds a dev_num field in vmbus channel structure. It’s assigned
to the first available number when the channel is offered. So netvsc can
use it for NIC naming based on channel offer sequence. Now we re-enable
the async probing mode by default for faster probing.

Also added a modules parameter, probe_type, to set sync probing mode if
a user wants to.

Fixes: af0a5646cb8d ("use the new async probing feature for the hyperv drivers")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/hv/channel_mgmt.c       | 46 +++++++++++++++++++++++++++++++++++++++--
 drivers/net/hyperv/netvsc_drv.c | 33 ++++++++++++++++++++++++++---
 include/linux/hyperv.h          |  4 ++++
 3 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index addcef5..ab7c05b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -304,6 +304,8 @@ bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp,
 
 EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
 
+#define HV_DEV_NUM_INVALID (-1)
+
 /*
  * alloc_channel - Allocate and initialize a vmbus channel object
  */
@@ -315,6 +317,8 @@ static struct vmbus_channel *alloc_channel(void)
 	if (!channel)
 		return NULL;
 
+	channel->dev_num = HV_DEV_NUM_INVALID;
+
 	spin_lock_init(&channel->lock);
 	init_completion(&channel->rescind_event);
 
@@ -533,6 +537,42 @@ static void vmbus_add_channel_work(struct work_struct *work)
 }
 
 /*
+ * Get the first available device number of its type, then
+ * record it in the channel structure.
+ */
+static void hv_set_devnum(struct vmbus_channel *newchannel)
+{
+	struct vmbus_channel *channel;
+	unsigned int i = 0;
+	bool found;
+
+	BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
+
+	/* Only HV_NIC uses this number for now */
+	if (hv_get_dev_type(newchannel) != HV_NIC)
+		return;
+
+next:
+	found = false;
+
+	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+		if (i == channel->dev_num &&
+		    guid_equal(&channel->offermsg.offer.if_type,
+			       &newchannel->offermsg.offer.if_type)) {
+			found = true;
+			break;
+		}
+	}
+
+	if (found) {
+		i++;
+		goto next;
+	}
+
+	newchannel->dev_num = i;
+}
+
+/*
  * vmbus_process_offer - Process the offer by creating a channel/device
  * associated with this offer
  */
@@ -561,10 +601,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 		}
 	}
 
-	if (fnew)
+	if (fnew) {
+		hv_set_devnum(newchannel);
+
 		list_add_tail(&newchannel->listentry,
 			      &vmbus_connection.chn_list);
-	else {
+	} else {
 		/*
 		 * Check to see if this is a valid sub-channel.
 		 */
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index afdcc56..af53690 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -57,6 +57,10 @@
 module_param(debug, int, 0444);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+static unsigned int probe_type __ro_after_init = PROBE_PREFER_ASYNCHRONOUS;
+module_param(probe_type, uint, 0444);
+MODULE_PARM_DESC(probe_type, "Probe type: 1=async(default), 2=sync");
+
 static LIST_HEAD(netvsc_dev_list);
 
 static void netvsc_change_rx_flags(struct net_device *net, int change)
@@ -2233,10 +2237,19 @@ static int netvsc_probe(struct hv_device *dev,
 	struct net_device_context *net_device_ctx;
 	struct netvsc_device_info *device_info = NULL;
 	struct netvsc_device *nvdev;
+	char name[IFNAMSIZ];
 	int ret = -ENOMEM;
 
-	net = alloc_etherdev_mq(sizeof(struct net_device_context),
-				VRSS_CHANNEL_MAX);
+	if (probe_type == PROBE_PREFER_ASYNCHRONOUS) {
+		snprintf(name, IFNAMSIZ, "eth%d", dev->channel->dev_num);
+		net = alloc_netdev_mqs(sizeof(struct net_device_context), name,
+				       NET_NAME_ENUM, ether_setup,
+				       VRSS_CHANNEL_MAX, VRSS_CHANNEL_MAX);
+	} else {
+		net = alloc_etherdev_mq(sizeof(struct net_device_context),
+					VRSS_CHANNEL_MAX);
+	}
+
 	if (!net)
 		goto no_net;
 
@@ -2323,6 +2336,14 @@ static int netvsc_probe(struct hv_device *dev,
 		net->max_mtu = ETH_DATA_LEN;
 
 	ret = register_netdevice(net);
+
+	if (ret == -EEXIST) {
+		pr_info("NIC name %s exists, request another name.\n",
+			net->name);
+		strlcpy(net->name, "eth%d", IFNAMSIZ);
+		ret = register_netdevice(net);
+	}
+
 	if (ret != 0) {
 		pr_err("Unable to register netdev.\n");
 		goto register_failed;
@@ -2407,7 +2428,7 @@ static int netvsc_remove(struct hv_device *dev)
 	.probe = netvsc_probe,
 	.remove = netvsc_remove,
 	.driver = {
-		.probe_type = PROBE_FORCE_SYNCHRONOUS,
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
 };
 
@@ -2473,6 +2494,12 @@ static int __init netvsc_drv_init(void)
 	}
 	netvsc_ring_bytes = ring_size * PAGE_SIZE;
 
+	if (probe_type != PROBE_PREFER_ASYNCHRONOUS)
+		probe_type = PROBE_FORCE_SYNCHRONOUS;
+
+	netvsc_drv.driver.probe_type = probe_type;
+	pr_info("probe_type: %u\n", probe_type);
+
 	ret = vmbus_driver_register(&netvsc_drv);
 	if (ret)
 		return ret;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 6256cc3..12fc5ea 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -841,6 +841,10 @@ struct vmbus_channel {
 	 */
 	struct vmbus_channel *primary_channel;
 	/*
+	 * Used for device naming based on channel offer sequence.
+	 */
+	int dev_num;
+	/*
 	 * Support per-channel state for use by vmbus drivers.
 	 */
 	void *per_channel_state;
-- 
1.8.3.1


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23 19:02 [PATCH net-next] Name NICs based on vmbus offer and enable async probe by default Haiyang Zhang
2019-07-25 18:46 ` David Miller
  -- strict thread matches above, loose matches on Subject: below --
2019-07-09 22:56 Haiyang Zhang
2019-07-09 23:47 ` Stephen Hemminger
2019-07-10  1:03   ` Haiyang Zhang
2019-07-10  0:29 ` David Miller
2019-07-10  1:04   ` Haiyang Zhang

Linux-HyperV Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hyperv/0 linux-hyperv/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 linux-hyperv linux-hyperv/ https://lore.kernel.org/linux-hyperv \
		linux-hyperv@vger.kernel.org linux-hyperv@archiver.kernel.org
	public-inbox-index linux-hyperv


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hyperv


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