Messages in this thread | | | From | Roman Kagan <> | Subject | Re: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus offer sequence and use async probe | Date | Tue, 31 Dec 2019 11:34:44 +0000 |
| |
On Mon, Dec 30, 2019 at 12:13:34PM -0800, Haiyang Zhang wrote: > The dev_num field in vmbus channel structure is assigned to the first > available number when the channel is offered. So netvsc driver uses it > for NIC naming based on channel offer sequence. Now re-enable the async > probing mode for faster probing. > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com> > --- > drivers/net/hyperv/netvsc_drv.c | 18 +++++++++++++++--- > 1 file changed, 15 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index f3f9eb8..39c412f 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -2267,10 +2267,14 @@ 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); > + snprintf(name, IFNAMSIZ, "eth%d", dev->channel->dev_num);
How is this supposed to work when there are other ethernet device types on the system, which may claim the same device names?
> + net = alloc_netdev_mqs(sizeof(struct net_device_context), name, > + NET_NAME_ENUM, ether_setup, > + VRSS_CHANNEL_MAX, VRSS_CHANNEL_MAX); > + > if (!net) > goto no_net; > > @@ -2355,6 +2359,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); > + }
IOW you want the device naming to be predictable, but don't guarantee this?
I think the problem this patchset is trying to solve is much better solved with a udev rule, similar to how it's done for PCI net devices. And IMO the primary channel number, being a device's "hardware" property, is more suited to be used in the device name, than this completely ephemeral device number.
Thanks, Roman.
> + > if (ret != 0) { > pr_err("Unable to register netdev.\n"); > goto register_failed; > @@ -2496,7 +2508,7 @@ static int netvsc_resume(struct hv_device *dev) > .suspend = netvsc_suspend, > .resume = netvsc_resume, > .driver = { > - .probe_type = PROBE_FORCE_SYNCHRONOUS, > + .probe_type = PROBE_PREFER_ASYNCHRONOUS, > }, > }; > > -- > 1.8.3.1 >
| |