Linux-HyperV List
 help / color / mirror / Atom feed
* RE: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus offer sequence and use async probe
From: Haiyang Zhang @ 2019-12-31 16:12 UTC (permalink / raw)
  To: Roman Kagan
  Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
	olaf@aepfle.de, vkuznets, davem@davemloft.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <20191231113440.GA380228@rkaganb.sw.ru>



> -----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


^ permalink raw reply

* Re: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus offer sequence and use async probe
From: Stephen Hemminger @ 2019-12-31 17:36 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: Roman Kagan, sashal@kernel.org, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
	olaf@aepfle.de, vkuznets, davem@davemloft.net
In-Reply-To: <MN2PR21MB1375D41039A8A68A2117DDFCCA260@MN2PR21MB1375.namprd21.prod.outlook.com>

On Tue, 31 Dec 2019 16:12:36 +0000
Haiyang Zhang <haiyangz@microsoft.com> wrote:

> > -----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
> 

The primary requirements for network naming are:
  1. Network names must be repeatable on each boot. This was the original problem
     that PCI devices discovered back years ago when parallel probing was enabled.
  2. Network names must be predictable. If new VM is created, the names should
     match a similar VM config.
  3. Names must be persistent. If a NIC is added or deleted, the existing names
     must not change.

The other things which are important (and this proposal breaks):
  1. Don't break it principle: an existing VM should not suddenly get interfaces
     renamed if kernel is upgraded. A corrallary is that a lot of current userspace
     code expects eth0. It doesn't look like first interface would be guaranteed
     to be eth0.

  2. No snowflakes principle: a device driver should follow the current practice
     of other devices. For netvsc, this means VMBus should act like PCI as much
     as possible. Is there another driver doing this already?

  3. Userspace policy principle: Every distribution has its own policy by now.
     The solution must make netvsc work reliably on Redhat (udev), Ubuntu (netplan), SuSE (Yast)
     doing something in the kernel violates #2.

My recommendation would be to take a multi-phase approach:
  1. Expose persistent value in sysfs now.
  2. Work with udev/netplan/... to use that value. 
  3. Make parallel VMBus probing an option. So that when distributions have picked up
     the udev changes they can enable parallel probe. Some will be quick to adopt
     and the enterprise laggards can get to it when they feel the heat.

Long term wish list (requires host side changes):
   1. The interface index could be a host side property; the host networking
       already has the virtual device table and it is persistent.
   2. The Azure NIC name should be visible as a property in guest. 
      Then userspace could do rename based on that property.
      Having multiple disconnected names is leads to confusion.

^ permalink raw reply

* RE: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus offer sequence and use async probe
From: Haiyang Zhang @ 2019-12-31 17:51 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Roman Kagan, sashal@kernel.org, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
	olaf@aepfle.de, vkuznets, davem@davemloft.net
In-Reply-To: <20191231093614.75da9bea@hermes.lan>



> -----Original Message-----
> From: linux-hyperv-owner@vger.kernel.org <linux-hyperv-
> owner@vger.kernel.org> On Behalf Of Stephen Hemminger
> Sent: Tuesday, December 31, 2019 12:36 PM
> To: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Roman Kagan <rkagan@virtuozzo.com>; 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
> Subject: Re: [PATCH V2,net-next, 3/3] hv_netvsc: Name NICs based on vmbus
> offer sequence and use async probe
> 
> On Tue, 31 Dec 2019 16:12:36 +0000
> Haiyang Zhang <haiyangz@microsoft.com> wrote:
> 
> > > -----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
> >
> 
> The primary requirements for network naming are:
>   1. Network names must be repeatable on each boot. This was the original
> problem
>      that PCI devices discovered back years ago when parallel probing was
> enabled.
>   2. Network names must be predictable. If new VM is created, the names
> should
>      match a similar VM config.
>   3. Names must be persistent. If a NIC is added or deleted, the existing names
>      must not change.
> 
> The other things which are important (and this proposal breaks):
>   1. Don't break it principle: an existing VM should not suddenly get interfaces
>      renamed if kernel is upgraded. A corrallary is that a lot of current userspace
>      code expects eth0. It doesn't look like first interface would be guaranteed
>      to be eth0.
> 
>   2. No snowflakes principle: a device driver should follow the current practice
>      of other devices. For netvsc, this means VMBus should act like PCI as much
>      as possible. Is there another driver doing this already?
> 
>   3. Userspace policy principle: Every distribution has its own policy by now.
>      The solution must make netvsc work reliably on Redhat (udev), Ubuntu
> (netplan), SuSE (Yast)
>      doing something in the kernel violates #2.
> 
> My recommendation would be to take a multi-phase approach:
>   1. Expose persistent value in sysfs now.
>   2. Work with udev/netplan/... to use that value.
>   3. Make parallel VMBus probing an option. So that when distributions have
> picked up
>      the udev changes they can enable parallel probe. Some will be quick to
> adopt
>      and the enterprise laggards can get to it when they feel the heat.
> 
> Long term wish list (requires host side changes):
>    1. The interface index could be a host side property; the host networking
>        already has the virtual device table and it is persistent.
>    2. The Azure NIC name should be visible as a property in guest.
>       Then userspace could do rename based on that property.
>       Having multiple disconnected names is leads to confusion.

Thank you for the detailed recommendations!
I will do the step 1. Expose persistent value in sysfs now.
And work on other changes in the future.

Thanks,
- Haiyang

^ permalink raw reply

* [PATCH V3,net-next, 0/3] Add vmbus dev_num and allow netvsc probe options
From: Haiyang Zhang @ 2019-12-31 22:13 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel

Add dev_num for vmbus device based on channel offer sequence.
User programs can use this number for nic naming.
Async probing option is allowed for netvsc. Sync probing is still
the default.

Haiyang Zhang (3):
  Drivers: hv: vmbus: Add a dev_num variable based on channel offer sequence
  Drivers: hv: vmbus: Add dev_num to sysfs
  hv_netvsc: Set probe_type to PROBE_DEFAULT_STRATEGY

 Documentation/ABI/stable/sysfs-bus-vmbus |  8 ++++++
 drivers/hv/channel_mgmt.c                | 46 ++++++++++++++++++++++++++++++--
 drivers/hv/vmbus_drv.c                   | 13 +++++++++
 drivers/net/hyperv/netvsc_drv.c          |  2 +-
 include/linux/hyperv.h                   |  6 +++++
 5 files changed, 72 insertions(+), 3 deletions(-)

-- 
1.8.3.1


^ permalink raw reply

* [PATCH V3,net-next, 3/3] hv_netvsc: Set probe_type to PROBE_DEFAULT_STRATEGY
From: Haiyang Zhang @ 2019-12-31 22:13 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
In-Reply-To: <1577830414-119508-1-git-send-email-haiyangz@microsoft.com>

The dev_num field in VMBus channel structure is assigned to the first
available number when the channel is offered.

Udev or other user programs can use this value from sysfs to set proper
device names.

Set probe_type to PROBE_DEFAULT_STRATEGY, so the user has an option to
enable Async probing when needed.

Sync probing mode is still the default.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/net/hyperv/netvsc_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f3f9eb8..4a2f294 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2496,7 +2496,7 @@ static int netvsc_resume(struct hv_device *dev)
 	.suspend = netvsc_suspend,
 	.resume = netvsc_resume,
 	.driver = {
-		.probe_type = PROBE_FORCE_SYNCHRONOUS,
+		.probe_type = PROBE_DEFAULT_STRATEGY,
 	},
 };
 
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH V3,net-next, 2/3] Drivers: hv: vmbus: Add dev_num to sysfs
From: Haiyang Zhang @ 2019-12-31 22:13 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
In-Reply-To: <1577830414-119508-1-git-send-email-haiyangz@microsoft.com>

It's a number based on the VMBus device offer sequence.
Useful for device naming when using Async probing.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>

---
Changes
V3:
 Use "VMBus" instead of vmbus in text.
---
 Documentation/ABI/stable/sysfs-bus-vmbus |  8 ++++++++
 drivers/hv/vmbus_drv.c                   | 13 +++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 8e8d167..4c4dc86 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -49,6 +49,14 @@ Contact:	Stephen Hemminger <sthemmin@microsoft.com>
 Description:	This NUMA node to which the VMBUS device is
 		attached, or -1 if the node is unknown.
 
+What:           /sys/bus/vmbus/devices/<UUID>/dev_num
+Date:		Dec 2019
+KernelVersion:	5.5
+Contact:	Haiyang Zhang <haiyangz@microsoft.com>
+Description:	A number based on the VMBus device offer sequence.
+		Useful for device naming when using Async probing.
+Users:		Debugging tools and userspace drivers
+
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>
 Date:		September. 2017
 KernelVersion:	4.14
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 4ef5a66..fe7aefa 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -214,6 +214,18 @@ static ssize_t numa_node_show(struct device *dev,
 static DEVICE_ATTR_RO(numa_node);
 #endif
 
+static ssize_t dev_num_show(struct device *dev,
+			      struct device_attribute *attr, char *buf)
+{
+	struct hv_device *hv_dev = device_to_hv_device(dev);
+
+	if (!hv_dev->channel)
+		return -ENODEV;
+
+	return sprintf(buf, "%d\n", hv_dev->channel->dev_num);
+}
+static DEVICE_ATTR_RO(dev_num);
+
 static ssize_t server_monitor_pending_show(struct device *dev,
 					   struct device_attribute *dev_attr,
 					   char *buf)
@@ -598,6 +610,7 @@ static ssize_t driver_override_show(struct device *dev,
 #ifdef CONFIG_NUMA
 	&dev_attr_numa_node.attr,
 #endif
+	&dev_attr_dev_num.attr,
 	&dev_attr_server_monitor_pending.attr,
 	&dev_attr_client_monitor_pending.attr,
 	&dev_attr_server_monitor_latency.attr,
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH V3,net-next, 1/3] Drivers: hv: vmbus: Add a dev_num variable based on channel offer sequence
From: Haiyang Zhang @ 2019-12-31 22:13 UTC (permalink / raw)
  To: sashal, linux-hyperv, netdev
  Cc: haiyangz, kys, sthemmin, olaf, vkuznets, davem, linux-kernel
In-Reply-To: <1577830414-119508-1-git-send-email-haiyangz@microsoft.com>

This number is set to the first available number, starting from zero,
when a VMBus device’s primary channel is offered.
It will be used for stable naming when Async probing is used.

Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
Changes
V3:
 Rename a local var, add code comments.

V2:
 Use nest loops in hv_set_devnum, instead of goto.
---
 drivers/hv/channel_mgmt.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/hyperv.h    |  6 ++++++
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 8eb1675..68adfa1 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -315,6 +315,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);
 
@@ -541,6 +543,44 @@ 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;
+	int i = -1;
+	bool in_use;
+
+	BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
+
+	/*
+	 * Iterate through each possible device number starting at zero.
+	 * If the device number is already in use for a device of this type,
+	 * try the next device number until finding one that is not in use.
+	 * This approach selects the smallest device number that is not in
+	 * use, and so reuses any numbers that are freed by devices that
+	 * have been removed.
+	 */
+	do {
+		i++;
+		in_use = 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)) {
+				in_use = true;
+				break;
+			}
+		}
+	} while (in_use);
+
+	newchannel->dev_num = i;
+}
+
+/*
  * vmbus_process_offer - Process the offer by creating a channel/device
  * associated with this offer
  */
@@ -573,10 +613,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/include/linux/hyperv.h b/include/linux/hyperv.h
index 26f3aee..4f110c5 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -718,6 +718,8 @@ struct vmbus_device {
 	bool perf_device;
 };
 
+#define HV_DEV_NUM_INVALID (-1)
+
 struct vmbus_channel {
 	struct list_head listentry;
 
@@ -849,6 +851,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 related

* Re: [PATCH V3,net-next, 0/3] Add vmbus dev_num and allow netvsc probe options
From: David Miller @ 2020-01-03  0:29 UTC (permalink / raw)
  To: haiyangz
  Cc: sashal, linux-hyperv, netdev, kys, sthemmin, olaf, vkuznets,
	linux-kernel
In-Reply-To: <1577830414-119508-1-git-send-email-haiyangz@microsoft.com>

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Tue, 31 Dec 2019 14:13:31 -0800

> Add dev_num for vmbus device based on channel offer sequence.
> User programs can use this number for nic naming.
> Async probing option is allowed for netvsc. Sync probing is still
> the default.

I don't like this at all, sorry.

If 4 devices get channels we will have IDs 1, 2, 3, 4.

Then if channel 3 is taken down, the next one will get 3 which
is not in order any more.

It is not even clear what semantics these numbers have in any
particular sequence or probe situation.

You have to use something more persistent across boots to number
and strictly identify these virtual devices.

I'm not applying this.

^ permalink raw reply

* RE: [PATCH V3,net-next, 0/3] Add vmbus dev_num and allow netvsc probe options
From: Haiyang Zhang @ 2020-01-03  2:41 UTC (permalink / raw)
  To: David Miller
  Cc: sashal@kernel.org, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, KY Srinivasan, Stephen Hemminger,
	olaf@aepfle.de, vkuznets, linux-kernel@vger.kernel.org
In-Reply-To: <20200102.162941.1933071871521624803.davem@davemloft.net>



> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Thursday, January 2, 2020 7: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 V3,net-next, 0/3] Add vmbus dev_num and allow netvsc
> probe options
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Tue, 31 Dec 2019 14:13:31 -0800
> 
> > Add dev_num for vmbus device based on channel offer sequence.
> > User programs can use this number for nic naming.
> > Async probing option is allowed for netvsc. Sync probing is still the
> > default.
> 
> I don't like this at all, sorry.
> 
> If 4 devices get channels we will have IDs 1, 2, 3, 4.
> 
> Then if channel 3 is taken down, the next one will get 3 which is not in order
> any more.
> 
> It is not even clear what semantics these numbers have in any particular
> sequence or probe situation.
> 
> You have to use something more persistent across boots to number and strictly
> identify these virtual devices.
> 
> I'm not applying this.

The idea of this patch set is to make the naming of async probing same as that
in sync probing.

So, the semantics of this dev_num is actually same as the default "eth%d" naming --
	"Find the smallest number >=0, which is not in use."

In cases:
1) There is no hot add/remove devices in current boot, this scheme does provide 
persistent dev_num across reboot, because Hyper-V hosts offer the primary channels
in order. So the results based on this number with async probing will be the same 
as existing Sync probing.

2) If there is hot add/remove devices, this scheme generates the same results (when
user program use dev_num)  as the default naming mode -- by using the smallest
available number N in the format ethN.

In case of hot add/remove of virtual NICs, the removed NIC are gone, and the newly 
added NIC is a completely new virtual device with new device instance UUID. So if we 
don't reuse the previous numbers, the device name ethN will grow unbounded. For 
example, hot add/remove a virtual NIC 100 times, you will have a name like eth100. This 
not what the default naming scheme does, and we are not doing it for dev_num here 
either.

So the semantics is: "Find smallest number >=0, and not in use".

But if any customer wants to have a 1:1 mapping between the UUID and device name, they
can still implement that in user mode... And that's why this patch set doesn't change
the kernel naming from driver -- it just provides a new variable, "dev_num", so user 
mode program has the option to use it in async mode with knowledge of the channel
offer sequence.

Thanks,
- Haiyang


^ permalink raw reply

* Re: [PATCH v2] tools/hv: async name resolution in kvp_daemon
From: Olaf Hering @ 2020-01-03 13:31 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
	open list:Hyper-V CORE AND DRIVERS, open list
In-Reply-To: <20191113155400.25456-1-olaf@aepfle.de>

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

Am Wed, 13 Nov 2019 16:54:00 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> Adjust the code to resolve the current hostname in a separate thread.

Are you now finally able to compile the patch?
Did you find some documentation about the host-side API of the "FullyQualifiedDomainName" request?
Any further comments about the "aim for success" approach?


Thanks,
Olaf

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH 8/8] net: tipc: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/tipc/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/tipc/core.c b/net/tipc/core.c
index 4f6dc74adf45..98dbc18a0dd8 100644
--- a/net/tipc/core.c
+++ b/net/tipc/core.c
@@ -221,4 +221,3 @@ module_exit(tipc_exit);
 
 MODULE_DESCRIPTION("TIPC: Transparent Inter Process Communication");
 MODULE_LICENSE("Dual BSD/GPL");
-MODULE_VERSION(TIPC_MOD_VER);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 6/8] net: bridge: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/bridge/br.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/bridge/br.c b/net/bridge/br.c
index b6fe30e3768f..6ecd3db3ab59 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -394,5 +394,4 @@ static void __exit br_deinit(void)
 module_init(br_init)
 module_exit(br_deinit)
 MODULE_LICENSE("GPL");
-MODULE_VERSION(BR_VERSION);
 MODULE_ALIAS_RTNL_LINK("bridge");
-- 
2.11.0


^ permalink raw reply related

* [PATCH 5/8] net: bluetooth: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/bluetooth/6lowpan.c      | 3 ---
 net/bluetooth/af_bluetooth.c | 1 -
 net/bluetooth/bnep/core.c    | 1 -
 net/bluetooth/cmtp/core.c    | 1 -
 net/bluetooth/hidp/core.c    | 1 -
 net/bluetooth/rfcomm/core.c  | 1 -
 6 files changed, 8 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index 4febc82a7c76..76fabc65b8a9 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -21,8 +21,6 @@
 
 #include <net/6lowpan.h> /* for the compression support */
 
-#define VERSION "0.1"
-
 static struct dentry *lowpan_enable_debugfs;
 static struct dentry *lowpan_control_debugfs;
 
@@ -1303,5 +1301,4 @@ module_exit(bt_6lowpan_exit);
 
 MODULE_AUTHOR("Jukka Rissanen <jukka.rissanen@linux.intel.com>");
 MODULE_DESCRIPTION("Bluetooth 6LoWPAN");
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index 3fd124927d4d..1a6972fac1f7 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -798,6 +798,5 @@ module_exit(bt_exit);
 
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth Core ver " VERSION);
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_NETPROTO(PF_BLUETOOTH);
diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
index 43c284158f63..96f0eb60deb0 100644
--- a/net/bluetooth/bnep/core.c
+++ b/net/bluetooth/bnep/core.c
@@ -764,6 +764,5 @@ MODULE_PARM_DESC(compress_dst, "Compress destination headers");
 
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth BNEP ver " VERSION);
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("bt-proto-4");
diff --git a/net/bluetooth/cmtp/core.c b/net/bluetooth/cmtp/core.c
index 07cfa3249f83..9a6306d7f738 100644
--- a/net/bluetooth/cmtp/core.c
+++ b/net/bluetooth/cmtp/core.c
@@ -511,6 +511,5 @@ module_exit(cmtp_exit);
 
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth CMTP ver " VERSION);
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("bt-proto-5");
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index bef84b95e2c4..a225ae5f1849 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -1475,6 +1475,5 @@ module_exit(hidp_exit);
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_AUTHOR("David Herrmann <dh.herrmann@gmail.com>");
 MODULE_DESCRIPTION("Bluetooth HIDP ver " VERSION);
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("bt-proto-6");
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 3a9e9d9670be..e621fbb451b3 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -2245,6 +2245,5 @@ MODULE_PARM_DESC(l2cap_ertm, "Use L2CAP ERTM mode for connection");
 
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Bluetooth RFCOMM ver " VERSION);
-MODULE_VERSION(VERSION);
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("bt-proto-3");
-- 
2.11.0


^ permalink raw reply related

* [PATCH 2/8] net: 8021q: use netdev_info()/netdev_warn()
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Use netdev_info() / netdev_warn() instead of pr_info() / pr_warn()
for more consistent log output.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/8021q/vlan.c      | 4 ++--
 net/8021q/vlan_core.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index ded7bf7229cf..7f18c8406ff8 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -123,7 +123,7 @@ int vlan_check_real_dev(struct net_device *real_dev,
 	const char *name = real_dev->name;
 
 	if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
-		pr_info("VLANs not supported on %s\n", name);
+		netdev_info(real_dev, "VLANs not supported on %s\n", name);
 		NL_SET_ERR_MSG_MOD(extack, "VLANs not supported on device");
 		return -EOPNOTSUPP;
 	}
@@ -376,7 +376,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 
 	if ((event == NETDEV_UP) &&
 	    (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)) {
-		pr_info("adding VLAN 0 to HW filter on device %s\n",
+		netdev_info(dev, "adding VLAN 0 to HW filter on device %s\n",
 			dev->name);
 		vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
 	}
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index a313165e7a67..bc32b33e0da8 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -360,7 +360,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
 
 	err = vlan_kill_rx_filter_info(dev, proto, vid);
 	if (err)
-		pr_warn("failed to kill vid %04x/%d for device %s\n",
+		netdev_warn(dev, "failed to kill vid %04x/%d for device %s\n",
 			proto, vid, dev->name);
 
 	list_del(&vid_info->list);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 4/8] net: ipv4: use netdev_info()/netdev_warn()
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Use netdev_info() / netdev_warn() instead of pr_info() / pr_warn()
for more consistent log output.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/ipv4/tcp_cubic.c    | 1 -
 net/ipv4/tcp_illinois.c | 1 -
 net/ipv4/tcp_nv.c       | 1 -
 3 files changed, 3 deletions(-)

diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 1b3d032a4df2..83fda965186d 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -513,4 +513,3 @@ module_exit(cubictcp_unregister);
 MODULE_AUTHOR("Sangtae Ha, Stephen Hemminger");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("CUBIC TCP");
-MODULE_VERSION("2.3");
diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
index 00e54873213e..8cc9967e82ef 100644
--- a/net/ipv4/tcp_illinois.c
+++ b/net/ipv4/tcp_illinois.c
@@ -355,4 +355,3 @@ module_exit(tcp_illinois_unregister);
 MODULE_AUTHOR("Stephen Hemminger, Shao Liu");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("TCP Illinois");
-MODULE_VERSION("1.0");
diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
index 95db7a11ba2a..b3879fb24d33 100644
--- a/net/ipv4/tcp_nv.c
+++ b/net/ipv4/tcp_nv.c
@@ -499,4 +499,3 @@ module_exit(tcpnv_unregister);
 MODULE_AUTHOR("Lawrence Brakmo");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("TCP NV");
-MODULE_VERSION("1.0");
-- 
2.11.0


^ permalink raw reply related

* [PATCH 1/8] net: 8021q: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/8021q/vlan.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index d4bcfd8f95bf..ded7bf7229cf 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -36,15 +36,10 @@
 #include "vlan.h"
 #include "vlanproc.h"
 
-#define DRV_VERSION "1.8"
-
 /* Global VLAN variables */
 
 unsigned int vlan_net_id __read_mostly;
 
-const char vlan_fullname[] = "802.1Q VLAN Support";
-const char vlan_version[] = DRV_VERSION;
-
 /* End of global variables definitions. */
 
 static int vlan_group_prealloc_vid(struct vlan_group *vg,
@@ -683,7 +678,7 @@ static int __init vlan_proto_init(void)
 {
 	int err;
 
-	pr_info("%s v%s\n", vlan_fullname, vlan_version);
+	pr_info("802.1Q VLAN Support\n");
 
 	err = register_pernet_subsys(&vlan_net_ops);
 	if (err < 0)
@@ -739,4 +734,3 @@ module_init(vlan_proto_init);
 module_exit(vlan_cleanup_module);
 
 MODULE_LICENSE("GPL");
-MODULE_VERSION(DRV_VERSION);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 3/8] net: batman-adv: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/batman-adv/main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 4811ec65bc43..8ccca3734b7a 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -748,6 +748,5 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR(BATADV_DRIVER_AUTHOR);
 MODULE_DESCRIPTION(BATADV_DRIVER_DESC);
 MODULE_SUPPORTED_DEVICE(BATADV_DRIVER_DEVICE);
-MODULE_VERSION(BATADV_SOURCE_VERSION);
 MODULE_ALIAS_RTNL_LINK("batadv");
 MODULE_ALIAS_GENL_FAMILY(BATADV_NL_NAME);
-- 
2.11.0


^ permalink raw reply related

* [PATCH 7/8] net: vmw_vsock: remove unneeded MODULE_VERSION() usage
From: Enrico Weigelt, metux IT consult @ 2020-01-04 19:51 UTC (permalink / raw)
  To: linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

Remove MODULE_VERSION(), as it isn't needed at all: the only version
making sense is the kernel version.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 net/vmw_vsock/af_vsock.c         | 1 -
 net/vmw_vsock/hyperv_transport.c | 1 -
 net/vmw_vsock/vmci_transport.c   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 74db4cd637a7..7e0cd3220d42 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -2203,5 +2203,4 @@ module_exit(vsock_exit);
 
 MODULE_AUTHOR("VMware, Inc.");
 MODULE_DESCRIPTION("VMware Virtual Socket Family");
-MODULE_VERSION("1.0.2.0-k");
 MODULE_LICENSE("GPL v2");
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index b3bdae74c243..f6cb66b62479 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -985,6 +985,5 @@ module_init(hvs_init);
 module_exit(hvs_exit);
 
 MODULE_DESCRIPTION("Hyper-V Sockets");
-MODULE_VERSION("1.0.0");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_NETPROTO(PF_VSOCK);
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 644d32e43d23..f2f689b7ca9a 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -2140,7 +2140,6 @@ module_exit(vmci_transport_exit);
 
 MODULE_AUTHOR("VMware, Inc.");
 MODULE_DESCRIPTION("VMCI transport for Virtual Sockets");
-MODULE_VERSION("1.0.5.0-k");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("vmware_vsock");
 MODULE_ALIAS_NETPROTO(PF_VSOCK);
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 4/8] net: ipv4: use netdev_info()/netdev_warn()
From: Sven Eckelmann @ 2020-01-04 19:55 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, davem, mareklindner, sw, a, marcel, johan.hedberg,
	roopa, nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue,
	kafai, songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-4-info@metux.net>

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

On Saturday, 4 January 2020 20:51:27 CET Enrico Weigelt, metux IT consult wrote:
> Use netdev_info() / netdev_warn() instead of pr_info() / pr_warn()
> for more consistent log output.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---

Patch description has nothing to do with the actual patch.

Kind regards,
	Sven

>  net/ipv4/tcp_cubic.c    | 1 -
>  net/ipv4/tcp_illinois.c | 1 -
>  net/ipv4/tcp_nv.c       | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 1b3d032a4df2..83fda965186d 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -513,4 +513,3 @@ module_exit(cubictcp_unregister);
>  MODULE_AUTHOR("Sangtae Ha, Stephen Hemminger");
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("CUBIC TCP");
> -MODULE_VERSION("2.3");
> diff --git a/net/ipv4/tcp_illinois.c b/net/ipv4/tcp_illinois.c
> index 00e54873213e..8cc9967e82ef 100644
> --- a/net/ipv4/tcp_illinois.c
> +++ b/net/ipv4/tcp_illinois.c
> @@ -355,4 +355,3 @@ module_exit(tcp_illinois_unregister);
>  MODULE_AUTHOR("Stephen Hemminger, Shao Liu");
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("TCP Illinois");
> -MODULE_VERSION("1.0");
> diff --git a/net/ipv4/tcp_nv.c b/net/ipv4/tcp_nv.c
> index 95db7a11ba2a..b3879fb24d33 100644
> --- a/net/ipv4/tcp_nv.c
> +++ b/net/ipv4/tcp_nv.c
> @@ -499,4 +499,3 @@ module_exit(tcpnv_unregister);
>  MODULE_AUTHOR("Lawrence Brakmo");
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("TCP NV");
> -MODULE_VERSION("1.0");
> 


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 4/8] net: ipv4: use netdev_info()/netdev_warn()
From: Fabio Estevam @ 2020-01-04 19:56 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, David S. Miller, mareklindner, sw, a, sven,
	Marcel Holtmann, johan.hedberg, roopa, nikolay, edumazet,
	Alexey Kuznetsov, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-4-info@metux.net>

On Sat, Jan 4, 2020 at 4:54 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Use netdev_info() / netdev_warn() instead of pr_info() / pr_warn()
> for more consistent log output.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  net/ipv4/tcp_cubic.c    | 1 -
>  net/ipv4/tcp_illinois.c | 1 -
>  net/ipv4/tcp_nv.c       | 1 -
>  3 files changed, 3 deletions(-)
>
> diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
> index 1b3d032a4df2..83fda965186d 100644
> --- a/net/ipv4/tcp_cubic.c
> +++ b/net/ipv4/tcp_cubic.c
> @@ -513,4 +513,3 @@ module_exit(cubictcp_unregister);
>  MODULE_AUTHOR("Sangtae Ha, Stephen Hemminger");
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("CUBIC TCP");
> -MODULE_VERSION("2.3");

Commit message and code change do not match.

^ permalink raw reply

* Re: [PATCH 3/8] net: batman-adv: remove unneeded MODULE_VERSION() usage
From: Sven Eckelmann @ 2020-01-04 19:59 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-kernel, davem, mareklindner, sw, a, marcel, johan.hedberg,
	roopa, nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue,
	kafai, songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf, Matthias Schiffer,
	b.a.t.m.a.n
In-Reply-To: <20200104195131.16577-3-info@metux.net>

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

On Saturday, 4 January 2020 20:51:26 CET Enrico Weigelt, metux IT consult wrote:
> Remove MODULE_VERSION(), as it isn't needed at all: the only version
> making sense is the kernel version.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---

While it is not strictly required, there are still some consumers that need 
this information to detect the version when installing various modules via 
backports.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 2/8] net: 8021q: use netdev_info()/netdev_warn()
From: Florian Fainelli @ 2020-01-04 20:06 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult, linux-kernel
  Cc: davem, mareklindner, sw, a, sven, marcel, johan.hedberg, roopa,
	nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue, kafai,
	songliubraving, yhs, andriin, netdev, linux-bluetooth,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-2-info@metux.net>



On 1/4/2020 11:51 AM, Enrico Weigelt, metux IT consult wrote:
> Use netdev_info() / netdev_warn() instead of pr_info() / pr_warn()
> for more consistent log output.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
>  net/8021q/vlan.c      | 4 ++--
>  net/8021q/vlan_core.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
> index ded7bf7229cf..7f18c8406ff8 100644
> --- a/net/8021q/vlan.c
> +++ b/net/8021q/vlan.c
> @@ -123,7 +123,7 @@ int vlan_check_real_dev(struct net_device *real_dev,
>  	const char *name = real_dev->name;
>  
>  	if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
> -		pr_info("VLANs not supported on %s\n", name);
> +		netdev_info(real_dev, "VLANs not supported on %s\n", name);

Since we use netdev_info which does internally use net_device::name for
printing, the name argument is now redundant.

>  		NL_SET_ERR_MSG_MOD(extack, "VLANs not supported on device");
>  		return -EOPNOTSUPP;
>  	}
> @@ -376,7 +376,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
>  
>  	if ((event == NETDEV_UP) &&
>  	    (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)) {
> -		pr_info("adding VLAN 0 to HW filter on device %s\n",
> +		netdev_info(dev, "adding VLAN 0 to HW filter on device %s\n",
>  			dev->name);

Likewise.

>  		vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
>  	}
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index a313165e7a67..bc32b33e0da8 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -360,7 +360,7 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
>  
>  	err = vlan_kill_rx_filter_info(dev, proto, vid);
>  	if (err)
> -		pr_warn("failed to kill vid %04x/%d for device %s\n",
> +		netdev_warn(dev, "failed to kill vid %04x/%d for device %s\n",
>  			proto, vid, dev->name);

And here as well.
-- 
Florian

^ permalink raw reply

* Re: [PATCH 1/8] net: 8021q: remove unneeded MODULE_VERSION() usage
From: kbuild test robot @ 2020-01-05  2:40 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: kbuild-all, linux-kernel, davem, mareklindner, sw, a, sven,
	marcel, johan.hedberg, roopa, nikolay, edumazet, kuznet, yoshfuji,
	jon.maloy, ying.xue, kafai, songliubraving, yhs, andriin, netdev,
	linux-bluetooth, tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

[-- Attachment #1: Type: text/plain, Size: 1410 bytes --]

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on net-next/master v5.5-rc4 next-20191220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/net-8021q-remove-unneeded-MODULE_VERSION-usage/20200105-075730
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git b54ef37b1ce892fdf6b632d566246d2f2f539910
config: alpha-defconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=alpha 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ERROR: "vlan_fullname" [net/8021q/8021q.ko] undefined!
>> ERROR: "vlan_version" [net/8021q/8021q.ko] undefined!

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13327 bytes --]

^ permalink raw reply

* Re: [PATCH 1/8] net: 8021q: remove unneeded MODULE_VERSION() usage
From: kbuild test robot @ 2020-01-05  5:56 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: kbuild-all, linux-kernel, davem, mareklindner, sw, a, sven,
	marcel, johan.hedberg, roopa, nikolay, edumazet, kuznet, yoshfuji,
	jon.maloy, ying.xue, kafai, songliubraving, yhs, andriin, netdev,
	linux-bluetooth, tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-1-info@metux.net>

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]
[also build test ERROR on net-next/master v5.5-rc4 next-20191220]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/net-8021q-remove-unneeded-MODULE_VERSION-usage/20200105-075730
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git b54ef37b1ce892fdf6b632d566246d2f2f539910
config: x86_64-randconfig-e001-20200105 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: net/8021q/vlan_dev.o: in function `vlan_ethtool_get_drvinfo':
   vlan_dev.c:(.text+0x10a): undefined reference to `vlan_fullname'
>> ld: vlan_dev.c:(.text+0x11f): undefined reference to `vlan_version'

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40676 bytes --]

^ permalink raw reply

* Re: [PATCH 5/8] net: bluetooth: remove unneeded MODULE_VERSION() usage
From: Marcel Holtmann @ 2020-01-05  9:34 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: lkml, David S. Miller, mareklindner, sw, a, sven, Johan Hedberg,
	roopa, nikolay, edumazet, kuznet, yoshfuji, jon.maloy, ying.xue,
	kafai, songliubraving, yhs, andriin, netdev, BlueZ devel list,
	tipc-discussion, linux-hyperv, bpf
In-Reply-To: <20200104195131.16577-5-info@metux.net>

Hi Enrico,

> Remove MODULE_VERSION(), as it isn't needed at all: the only version
> making sense is the kernel version.

I prefer to keep the MODULE_VERSION info since it provides this information via modinfo.

Unless there is a kernel wide consent to remove MODULE_VERSION altogether, the Bluetooth subsystem is keeping it.

Regards

Marcel


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox