lkml.org 
[lkml]   [2019]   [Dec]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus offer sequence and use async probe
Date


> -----Original Message-----
> From: Roman Kagan <rkagan@virtuozzo.com>
> Sent: Tuesday, December 31, 2019 6:35 AM
> 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 V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus
> offer sequence and use async probe
>
> 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.

The vmbus number can be affected by other types of devices and/or subchannel
offerings. They are not stable either. That's why this patch set keeps track of the
offering sequence within the same device type in a new variable "dev_num".

As in my earlier email, to avoid impact by other types of NICs, we should put them
into different naming formats, like "vf*", "enP*", etc. And yes, these can be done in
udev.

But for netvsc (synthetic) NICs, we still want the default naming format "eth*". And
the variable "dev_num" gives them the basis for stable naming with Async probing.

Thanks,
- Haiyang

\
 
 \ /
  Last update: 2019-12-31 17:13    [W:0.687 / U:0.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site