11 messages, 3 authors, 2017-08-09 · open the first message on its own page
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-07 18:30:06
Previous fix was incomplete. Stephen Hemminger (1): netvsc: make sure and unregister datapath drivers/net/hyperv/hyperv_net.h | 3 -- drivers/net/hyperv/netvsc.c | 2 -- drivers/net/hyperv/netvsc_drv.c | 71 ++++++++++++++++------------------------- 3 files changed, 28 insertions(+), 48 deletions(-) -- 2.11.0
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-07 18:30:07
Go back to switching datapath directly in the notifier callback.
Otherwise datapath might not get switched on unregister.
No need for calling the NOTIFY_PEERS notifier since that is only for
a gratitious ARP/ND packet; but that is not required with Hyper-V
because both VF and synthetic NIC have the same MAC address.
Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Fixes: 0c195567a8f6 ("netvsc: transparent VF management")
Signed-off-by: Stephen Hemminger <redacted>
---
drivers/net/hyperv/hyperv_net.h | 3 --
drivers/net/hyperv/netvsc.c | 2 --
drivers/net/hyperv/netvsc_drv.c | 71 ++++++++++++++++-------------------------
3 files changed, 28 insertions(+), 48 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index c701b059c5ac..d1ea99a12cf2 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h@@ -724,14 +724,11 @@ struct net_device_context { struct net_device __rcu *vf_netdev; struct netvsc_vf_pcpu_stats __percpu *vf_stats; struct work_struct vf_takeover; - struct work_struct vf_notify; /* 1: allocated, serial number is valid. 0: not allocated */ u32 vf_alloc; /* Serial number of the VF to team with */ u32 vf_serial; - - bool datapath; /* 0 - synthetic, 1 - VF nic */ }; /* Per channel data */
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 9598220b3bcc..208f03aa83de 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c@@ -60,8 +60,6 @@ void netvsc_switch_datapath(struct net_device *ndev, bool vf) sizeof(struct nvsp_message), (unsigned long)init_pkt, VM_PKT_DATA_INBAND, 0); - - net_device_ctx->datapath = vf; } static struct netvsc_device *alloc_net_device(void)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index e75c0f852a63..eb0023f55fe1 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c@@ -1649,55 +1649,35 @@ static int netvsc_register_vf(struct net_device *vf_netdev) return NOTIFY_OK; } -/* Change datapath */ -static void netvsc_vf_update(struct work_struct *w) +static int netvsc_vf_up(struct net_device *vf_netdev) { - struct net_device_context *ndev_ctx - = container_of(w, struct net_device_context, vf_notify); - struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx); + struct net_device_context *net_device_ctx; struct netvsc_device *netvsc_dev; - struct net_device *vf_netdev; - bool vf_is_up; - - if (!rtnl_trylock()) { - schedule_work(w); - return; - } + struct net_device *ndev; - vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev); - if (!vf_netdev) - goto unlock; + ndev = get_netvsc_byref(vf_netdev); + if (!ndev) + return NOTIFY_DONE; - netvsc_dev = rtnl_dereference(ndev_ctx->nvdev); + net_device_ctx = netdev_priv(ndev); + netvsc_dev = rtnl_dereference(net_device_ctx->nvdev); if (!netvsc_dev) - goto unlock; - - vf_is_up = netif_running(vf_netdev); - if (vf_is_up != ndev_ctx->datapath) { - if (vf_is_up) { - netdev_info(ndev, "VF up: %s\n", vf_netdev->name); - rndis_filter_open(netvsc_dev); - netvsc_switch_datapath(ndev, true); - netdev_info(ndev, "Data path switched to VF: %s\n", - vf_netdev->name); - } else { - netdev_info(ndev, "VF down: %s\n", vf_netdev->name); - netvsc_switch_datapath(ndev, false); - rndis_filter_close(netvsc_dev); - netdev_info(ndev, "Data path switched from VF: %s\n", - vf_netdev->name); - } + return NOTIFY_DONE; - /* Now notify peers through VF device. */ - call_netdevice_notifiers(NETDEV_NOTIFY_PEERS, ndev); - } -unlock: - rtnl_unlock(); + /* Bump refcount when datapath is acvive - Why? */ + rndis_filter_open(netvsc_dev); + + /* notify the host to switch the data path. */ + netvsc_switch_datapath(ndev, true); + netdev_info(ndev, "Data path switched to VF: %s\n", vf_netdev->name); + + return NOTIFY_OK; } -static int netvsc_vf_notify(struct net_device *vf_netdev) +static int netvsc_vf_down(struct net_device *vf_netdev) { struct net_device_context *net_device_ctx; + struct netvsc_device *netvsc_dev; struct net_device *ndev; ndev = get_netvsc_byref(vf_netdev);
@@ -1705,7 +1685,13 @@ static int netvsc_vf_notify(struct net_device *vf_netdev) return NOTIFY_DONE; net_device_ctx = netdev_priv(ndev); - schedule_work(&net_device_ctx->vf_notify); + netvsc_dev = rtnl_dereference(net_device_ctx->nvdev); + if (!netvsc_dev) + return NOTIFY_DONE; + + netvsc_switch_datapath(ndev, false); + netdev_info(ndev, "Data path switched from VF: %s\n", vf_netdev->name); + rndis_filter_close(netvsc_dev); return NOTIFY_OK; }
@@ -1721,7 +1707,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev) net_device_ctx = netdev_priv(ndev); cancel_work_sync(&net_device_ctx->vf_takeover); - cancel_work_sync(&net_device_ctx->vf_notify); netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
@@ -1764,7 +1749,6 @@ static int netvsc_probe(struct hv_device *dev, spin_lock_init(&net_device_ctx->lock); INIT_LIST_HEAD(&net_device_ctx->reconfig_events); INIT_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup); - INIT_WORK(&net_device_ctx->vf_notify, netvsc_vf_update); net_device_ctx->vf_stats = netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
@@ -1915,8 +1899,9 @@ static int netvsc_netdev_event(struct notifier_block *this, case NETDEV_UNREGISTER: return netvsc_unregister_vf(event_dev); case NETDEV_UP: + return netvsc_vf_up(event_dev); case NETDEV_DOWN: - return netvsc_vf_notify(event_dev); + return netvsc_vf_down(event_dev); default: return NOTIFY_DONE; }
--
2.11.0
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: 2017-08-08 14:03:59
Stephen Hemminger [off-list ref] writes:
Previous fix was incomplete.
Not really related to this patch series (which btw fixes my issue, thanks!), but I found one addition issue. Systemd fails to rename VF interface: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 kernel: mlx4_en: eth2: Link Up NetworkManager[750]: <info> [1502200557.0821] device (eth2): link connected NetworkManager[750]: <info> [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6) systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy With some debug added I figured out what's wrong: __netvsc_vf_setup() does dev_open() which sets IFF_UP flag on the interface. When systemd tries to rename the interface we get into dev_change_name() and this fails with -EBUSY when (dev->flags & IFF_UP). The issue is of less importance as we're not supposed to configure VF interface now. However, we may still want to get a stable name for it. Any idea how this can be fixed? -- Vitaly
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-08 15:15:08
On Tue, 08 Aug 2017 16:03:56 +0200 Vitaly Kuznetsov [off-list ref] wrote:
Stephen Hemminger [off-list ref] writes:quoted
Previous fix was incomplete.Not really related to this patch series (which btw fixes my issue, thanks!), but I found one addition issue. Systemd fails to rename VF interface: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 kernel: mlx4_en: eth2: Link Up NetworkManager[750]: <info> [1502200557.0821] device (eth2): link connected NetworkManager[750]: <info> [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6) systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy With some debug added I figured out what's wrong: __netvsc_vf_setup() does dev_open() which sets IFF_UP flag on the interface. When systemd tries to rename the interface we get into dev_change_name() and this fails with -EBUSY when (dev->flags & IFF_UP). The issue is of less importance as we're not supposed to configure VF interface now. However, we may still want to get a stable name for it. Any idea how this can be fixed?
The problem is Network Manager should ignore the VF device. I don't run NM on these interfaces because it causes more issues than it helps (dueling userspace). The driver needs to have slave track the master interface. Relying on userspace to bring interface up leads to all the issues the bonding script had. One option would be to delay the work of bringing up the slave device to allow small window for renaming to run.
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: 2017-08-08 15:24:06
Stephen Hemminger [off-list ref] writes:
On Tue, 08 Aug 2017 16:03:56 +0200 Vitaly Kuznetsov [off-list ref] wrote:quoted
Stephen Hemminger [off-list ref] writes:quoted
Previous fix was incomplete.Not really related to this patch series (which btw fixes my issue, thanks!), but I found one addition issue. Systemd fails to rename VF interface: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 kernel: mlx4_en: eth2: Link Up NetworkManager[750]: <info> [1502200557.0821] device (eth2): link connected NetworkManager[750]: <info> [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6) systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy With some debug added I figured out what's wrong: __netvsc_vf_setup() does dev_open() which sets IFF_UP flag on the interface. When systemd tries to rename the interface we get into dev_change_name() and this fails with -EBUSY when (dev->flags & IFF_UP). The issue is of less importance as we're not supposed to configure VF interface now. However, we may still want to get a stable name for it. Any idea how this can be fixed?The problem is Network Manager should ignore the VF device. I don't run NM on these interfaces because it causes more issues than it helps (dueling userspace). The driver needs to have slave track the master interface. Relying on userspace to bring interface up leads to all the issues the bonding script had. One option would be to delay the work of bringing up the slave device to allow small window for renaming to run.
Actually, I tried removing 'if (dev->flags & IFF_UP)' check from dev_change_name() and everything seems to work fine. The history of this code predates git so I have no idea why it's forbiden to change names of IFF_UP interfaces... I can send an RFC removing the check to figure out the truth :-) -- Vitaly
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-08 15:29:30
On Tue, 08 Aug 2017 17:24:03 +0200 Vitaly Kuznetsov [off-list ref] wrote:
Stephen Hemminger [off-list ref] writes:quoted
On Tue, 08 Aug 2017 16:03:56 +0200 Vitaly Kuznetsov [off-list ref] wrote:quoted
Stephen Hemminger [off-list ref] writes:quoted
Previous fix was incomplete.Not really related to this patch series (which btw fixes my issue, thanks!), but I found one addition issue. Systemd fails to rename VF interface: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 kernel: mlx4_en: eth2: Link Up NetworkManager[750]: <info> [1502200557.0821] device (eth2): link connected NetworkManager[750]: <info> [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6) systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy With some debug added I figured out what's wrong: __netvsc_vf_setup() does dev_open() which sets IFF_UP flag on the interface. When systemd tries to rename the interface we get into dev_change_name() and this fails with -EBUSY when (dev->flags & IFF_UP). The issue is of less importance as we're not supposed to configure VF interface now. However, we may still want to get a stable name for it. Any idea how this can be fixed?The problem is Network Manager should ignore the VF device. I don't run NM on these interfaces because it causes more issues than it helps (dueling userspace). The driver needs to have slave track the master interface. Relying on userspace to bring interface up leads to all the issues the bonding script had. One option would be to delay the work of bringing up the slave device to allow small window for renaming to run.Actually, I tried removing 'if (dev->flags & IFF_UP)' check from dev_change_name() and everything seems to work fine. The history of this code predates git so I have no idea why it's forbiden to change names of IFF_UP interfaces... I can send an RFC removing the check to figure out the truth :-)
That only works because NM by default brings everything up. Many users don't use NM on servers.
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: 2017-08-08 15:42:10
Stephen Hemminger [off-list ref] writes:
On Tue, 08 Aug 2017 17:24:03 +0200 Vitaly Kuznetsov [off-list ref] wrote:quoted
Stephen Hemminger [off-list ref] writes:quoted
On Tue, 08 Aug 2017 16:03:56 +0200 Vitaly Kuznetsov [off-list ref] wrote:quoted
Stephen Hemminger [off-list ref] writes:quoted
Previous fix was incomplete.Not really related to this patch series (which btw fixes my issue, thanks!), but I found one addition issue. Systemd fails to rename VF interface: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 kernel: mlx4_en: eth2: Link Up NetworkManager[750]: <info> [1502200557.0821] device (eth2): link connected NetworkManager[750]: <info> [1502200557.1004] manager: (eth2): new Ethernet device (/org/freedesktop/NetworkManager/Devices/6) systemd-udevd[6942]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[6942]: could not rename interface '6' from 'eth2' to 'enP2p0s2': Device or resource busy With some debug added I figured out what's wrong: __netvsc_vf_setup() does dev_open() which sets IFF_UP flag on the interface. When systemd tries to rename the interface we get into dev_change_name() and this fails with -EBUSY when (dev->flags & IFF_UP). The issue is of less importance as we're not supposed to configure VF interface now. However, we may still want to get a stable name for it. Any idea how this can be fixed?The problem is Network Manager should ignore the VF device. I don't run NM on these interfaces because it causes more issues than it helps (dueling userspace). The driver needs to have slave track the master interface. Relying on userspace to bring interface up leads to all the issues the bonding script had. One option would be to delay the work of bringing up the slave device to allow small window for renaming to run.Actually, I tried removing 'if (dev->flags & IFF_UP)' check from dev_change_name() and everything seems to work fine. The history of this code predates git so I have no idea why it's forbiden to change names of IFF_UP interfaces... I can send an RFC removing the check to figure out the truth :-)That only works because NM by default brings everything up. Many users don't use NM on servers.
I'm not sure NetworkManager is related here: renaming is done by systemd/udev according to the "predictable interface names" scheme: https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/ Basically, systemd/udev tries to rename all new interfaces in the system according to this scheme. And systemd/udev is ubiquitous nowdays. I tried disabling NetworkManager in my VM and the behavior is not any different: kernel: mlx4_core 0002:00:02.0 eth2: joined to eth1 kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: VF registering: eth2 kernel: mlx4_en: eth2: Link Up kernel: hv_netvsc 33b7a6f9-6736-451f-8fce-b382eaa50bee eth1: Data path switched to VF: eth2 systemd-udevd[1785]: Error changing net interface name 'eth2' to 'enP2p0s2': Device or resource busy systemd-udevd[1785]: could not rename interface '5' from 'eth2' to 'enP2p0s2': Device or resource busy The 'if (dev->flags & IFF_UP)' check in dev_change_name() function I mentioned prevents renaming interfaces which are already up but I don't know an obvious reason for it to exist. -- Vitaly
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-08 15:53:57
The following would allow udev a chance at the device. From 37fb240a6107834c3dd3f57caede9d73b807f414 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger <redacted> Date: Tue, 8 Aug 2017 08:39:24 -0700 Subject: [PATCH] netvsc: delay setup of VF device When VF device is discovered, delay bring it automatically up in order to allow userspace to some simple changes (like renaming). Signed-off-by: Stephen Hemminger <redacted> --- drivers/net/hyperv/hyperv_net.h | 2 +- drivers/net/hyperv/netvsc_drv.c | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index d1ea99a12cf2..f620c90307ed 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h@@ -723,7 +723,7 @@ struct net_device_context { /* State to manage the associated VF interface. */ struct net_device __rcu *vf_netdev; struct netvsc_vf_pcpu_stats __percpu *vf_stats; - struct work_struct vf_takeover; + struct delayed_work vf_takeover; /* 1: allocated, serial number is valid. 0: not allocated */ u32 vf_alloc;
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7ebf0e10e62b..1dff160368a3 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c@@ -47,6 +47,7 @@ #define RING_SIZE_MIN 64 #define LINKCHANGE_INT (2 * HZ) +#define VF_TAKEOVER_INT (HZ / 10) static int ring_size = 128; module_param(ring_size, int, S_IRUGO);
@@ -1570,7 +1571,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev, /* set slave flag before open to prevent IPv6 addrconf */ vf_netdev->flags |= IFF_SLAVE; - schedule_work(&ndev_ctx->vf_takeover); + schedule_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT); netdev_info(vf_netdev, "joined to %s\n", ndev->name); return 0;
@@ -1608,12 +1609,12 @@ static void __netvsc_vf_setup(struct net_device *ndev, static void netvsc_vf_setup(struct work_struct *w) { struct net_device_context *ndev_ctx - = container_of(w, struct net_device_context, vf_takeover); + = container_of(w, struct net_device_context, vf_takeover.work); struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx); struct net_device *vf_netdev; if (!rtnl_trylock()) { - schedule_work(w); + schedule_delayed_work(&ndev_ctx->vf_takeover, 0); return; }
@@ -1717,7 +1718,7 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev) return NOTIFY_DONE; net_device_ctx = netdev_priv(ndev); - cancel_work_sync(&net_device_ctx->vf_takeover); + cancel_delayed_work_sync(&net_device_ctx->vf_takeover); netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
@@ -1759,7 +1760,7 @@ static int netvsc_probe(struct hv_device *dev, spin_lock_init(&net_device_ctx->lock); INIT_LIST_HEAD(&net_device_ctx->reconfig_events); - INIT_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup); + INIT_DELAYED_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup); net_device_ctx->vf_stats = netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
--
2.11.0
From: David Miller <davem@davemloft.net>
Date: 2017-08-09 01:10:18
From: Stephen Hemminger <stephen@networkplumber.org> Date: Mon, 7 Aug 2017 11:30:00 -0700
Go back to switching datapath directly in the notifier callback.
Otherwise datapath might not get switched on unregister.
No need for calling the NOTIFY_PEERS notifier since that is only for
a gratitious ARP/ND packet; but that is not required with Hyper-V
because both VF and synthetic NIC have the same MAC address.
Reported-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Fixes: 0c195567a8f6 ("netvsc: transparent VF management")
Signed-off-by: Stephen Hemminger <redacted>Applied, thanks Stephen.
From: Vitaly Kuznetsov <vkuznets@redhat.com>
Date: 2017-08-09 09:03:08
Stephen Hemminger [off-list ref] writes:
The following would allow udev a chance at the device.
This would of course work but the approach is a bit hackish and can still fail on a loaded system. Raising the pause interval would be an option, but again, probably not the best one. Let me try to send an RFC removing the check it dev_change_name() and if it turns out that it can't be removed we can go back to your patch. But in case it can we can leave without it. Thanks,
From 37fb240a6107834c3dd3f57caede9d73b807f414 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger <redacted> Date: Tue, 8 Aug 2017 08:39:24 -0700 Subject: [PATCH] netvsc: delay setup of VF device When VF device is discovered, delay bring it automatically up in order to allow userspace to some simple changes (like renaming). Signed-off-by: Stephen Hemminger <redacted> --- drivers/net/hyperv/hyperv_net.h | 2 +- drivers/net/hyperv/netvsc_drv.c | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-)diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index d1ea99a12cf2..f620c90307ed 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h@@ -723,7 +723,7 @@ struct net_device_context { /* State to manage the associated VF interface. */ struct net_device __rcu *vf_netdev; struct netvsc_vf_pcpu_stats __percpu *vf_stats; - struct work_struct vf_takeover; + struct delayed_work vf_takeover; /* 1: allocated, serial number is valid. 0: not allocated */ u32 vf_alloc;diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 7ebf0e10e62b..1dff160368a3 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c@@ -47,6 +47,7 @@ #define RING_SIZE_MIN 64 #define LINKCHANGE_INT (2 * HZ) +#define VF_TAKEOVER_INT (HZ / 10) static int ring_size = 128; module_param(ring_size, int, S_IRUGO);@@ -1570,7 +1571,7 @@ static int netvsc_vf_join(struct net_device *vf_netdev, /* set slave flag before open to prevent IPv6 addrconf */ vf_netdev->flags |= IFF_SLAVE; - schedule_work(&ndev_ctx->vf_takeover); + schedule_delayed_work(&ndev_ctx->vf_takeover, VF_TAKEOVER_INT); netdev_info(vf_netdev, "joined to %s\n", ndev->name); return 0;@@ -1608,12 +1609,12 @@ static void __netvsc_vf_setup(struct net_device *ndev, static void netvsc_vf_setup(struct work_struct *w) { struct net_device_context *ndev_ctx - = container_of(w, struct net_device_context, vf_takeover); + = container_of(w, struct net_device_context, vf_takeover.work); struct net_device *ndev = hv_get_drvdata(ndev_ctx->device_ctx); struct net_device *vf_netdev; if (!rtnl_trylock()) { - schedule_work(w); + schedule_delayed_work(&ndev_ctx->vf_takeover, 0); return; }@@ -1717,7 +1718,7 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev) return NOTIFY_DONE; net_device_ctx = netdev_priv(ndev); - cancel_work_sync(&net_device_ctx->vf_takeover); + cancel_delayed_work_sync(&net_device_ctx->vf_takeover); netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);@@ -1759,7 +1760,7 @@ static int netvsc_probe(struct hv_device *dev, spin_lock_init(&net_device_ctx->lock); INIT_LIST_HEAD(&net_device_ctx->reconfig_events); - INIT_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup); + INIT_DELAYED_WORK(&net_device_ctx->vf_takeover, netvsc_vf_setup); net_device_ctx->vf_stats = netdev_alloc_pcpu_stats(struct netvsc_vf_pcpu_stats);
-- Vitaly
From: Stephen Hemminger <stephen@networkplumber.org>
Date: 2017-08-09 14:41:43
On Wed, 09 Aug 2017 11:03:05 +0200 Vitaly Kuznetsov [off-list ref] wrote:
Stephen Hemminger [off-list ref] writes:quoted
The following would allow udev a chance at the device.This would of course work but the approach is a bit hackish and can still fail on a loaded system. Raising the pause interval would be an option, but again, probably not the best one. Let me try to send an RFC removing the check it dev_change_name() and if it turns out that it can't be removed we can go back to your patch. But in case it can we can leave without it. Thanks,
I don't want to require change of semantics of core networking code for one driver. Changing name of up device has been blocked for so long that allowing it might break existing userspace apps. It might be ok in the future, but netvsc needs to work without that change.