| Message ID | 1524700768-38627-5-git-send-email-sridhar.samudrala@intel.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Enable virtio_net to act as a standby for a passthru device | expand |
On Wed, 25 Apr 2018 16:59:28 -0700 Sridhar Samudrala <sridhar.samudrala@intel.com> wrote: > Use the registration/notification framework supported by the generic > failover infrastructure. > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> NAK unless you prove this works on legacy distributions and with DPDK 18.05 without modification.
On Wed, Apr 25, 2018 at 05:08:37PM -0700, Stephen Hemminger wrote: > On Wed, 25 Apr 2018 16:59:28 -0700 > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote: > > > Use the registration/notification framework supported by the generic > > failover infrastructure. > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > > NAK unless you prove this works on legacy distributions and with DPDK 18.05 > without modification. It looks like it should work. What kind of proof are you looking for?
On Thu, 26 Apr 2018 05:30:05 +0300 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Apr 25, 2018 at 05:08:37PM -0700, Stephen Hemminger wrote: > > On Wed, 25 Apr 2018 16:59:28 -0700 > > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote: > > > > > Use the registration/notification framework supported by the generic > > > failover infrastructure. > > > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > > > > NAK unless you prove this works on legacy distributions and with DPDK 18.05 > > without modification. > > It looks like it should work. What kind of proof are you looking for? > I tried this with working Ubuntu 17 on WS2016. It boots if the failover driver is configured in (as module). But if the configuration has: $ grep FAILOVER .config # CONFIG_NET_FAILOVER is not set CONFIG_MAY_USE_NET_FAILOVER=y The netvsc driver fails on boot with: [ 0.826447] hv_vmbus: registering driver hv_netvsc [ 0.829616] scsi 0:0:0:0: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 5 [ 0.836291] input: Microsoft Vmbus HID-compliant Mouse as /devices/0006:045E:0621.0001/input/input1 [ 0.839139] hid-generic 0006:045E:0621.0001: input: <UNKNOWN> HID v0.01 Mouse [Microsoft Vmbus HID-compliant Mouse] on [ 0.964897] hv_vmbus: probe failed for device 849a776e-8120-4e4a-9a36-7e3d95ac75b3 (-95) [ 0.968039] hv_netvsc: probe of 849a776e-8120-4e4a-9a36-7e3d95ac75b3 failed with error -95 [ 1.112877] hv_vmbus: probe failed for device 53557f8e-057d-425b-9265-01c0fd7e273e (-95) [ 1.116064] hv_netvsc: probe of 53557f8e-057d-425b-9265-01c0fd7e273e failed with error -95 The system has two virtual networks. eth0 is on vswitch for management. eth1 is on vswitch with SR-IOV for performance tests. You probably need to just put the failover part in net/core and select it. It is trivial to get an evaluation version of Windows Server 2016 and setup a Linux VM. Please try it.
On Thu, Apr 26, 2018 at 03:33:26PM -0700, Stephen Hemminger wrote: > On Thu, 26 Apr 2018 05:30:05 +0300 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Wed, Apr 25, 2018 at 05:08:37PM -0700, Stephen Hemminger wrote: > > > On Wed, 25 Apr 2018 16:59:28 -0700 > > > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote: > > > > > > > Use the registration/notification framework supported by the generic > > > > failover infrastructure. > > > > > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > > > > > > NAK unless you prove this works on legacy distributions and with DPDK 18.05 > > > without modification. > > > > It looks like it should work. What kind of proof are you looking for? > > > > > I tried this with working Ubuntu 17 on WS2016. > It boots if the failover driver is configured in (as module). Oh so by "unless you prove" you meant "unless you test"? > But if the configuration has: > > $ grep FAILOVER .config > # CONFIG_NET_FAILOVER is not set > CONFIG_MAY_USE_NET_FAILOVER=y So the new option works, what's broken is when it's *not* selected. Looks like a bug. > The netvsc driver fails on boot with: > > [ 0.826447] hv_vmbus: registering driver hv_netvsc > [ 0.829616] scsi 0:0:0:0: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 5 > [ 0.836291] input: Microsoft Vmbus HID-compliant Mouse as /devices/0006:045E:0621.0001/input/input1 > [ 0.839139] hid-generic 0006:045E:0621.0001: input: <UNKNOWN> HID v0.01 Mouse [Microsoft Vmbus HID-compliant Mouse] on > [ 0.964897] hv_vmbus: probe failed for device 849a776e-8120-4e4a-9a36-7e3d95ac75b3 (-95) > [ 0.968039] hv_netvsc: probe of 849a776e-8120-4e4a-9a36-7e3d95ac75b3 failed with error -95 > [ 1.112877] hv_vmbus: probe failed for device 53557f8e-057d-425b-9265-01c0fd7e273e (-95) > [ 1.116064] hv_netvsc: probe of 53557f8e-057d-425b-9265-01c0fd7e273e failed with error -95 > > The system has two virtual networks. eth0 is on vswitch for management. > eth1 is on vswitch with SR-IOV for performance tests. > > You probably need to just put the failover part in net/core and select it. From all drivers. Yes. And it does not need to be visible in the menu imho. > It is trivial to get an evaluation version of Windows Server 2016 and setup a Linux VM. > Please try it. I presume some kind of test was done here. Sridhar, do you think you could include some notes about testing of this patch? Or in case you only build-tested this part, it's a good idea to notice this after --- in the patch, or in the cover letter.
On 4/26/2018 5:09 PM, Michael S. Tsirkin wrote: > On Thu, Apr 26, 2018 at 03:33:26PM -0700, Stephen Hemminger wrote: >> On Thu, 26 Apr 2018 05:30:05 +0300 >> "Michael S. Tsirkin" <mst@redhat.com> wrote: >> >>> On Wed, Apr 25, 2018 at 05:08:37PM -0700, Stephen Hemminger wrote: >>>> On Wed, 25 Apr 2018 16:59:28 -0700 >>>> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote: >>>> >>>>> Use the registration/notification framework supported by the generic >>>>> failover infrastructure. >>>>> >>>>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >>>> NAK unless you prove this works on legacy distributions and with DPDK 18.05 >>>> without modification. >>> It looks like it should work. What kind of proof are you looking for? >>> >> >> I tried this with working Ubuntu 17 on WS2016. >> It boots if the failover driver is configured in (as module). > Oh so by "unless you prove" you meant "unless you test"? > >> But if the configuration has: >> >> $ grep FAILOVER .config >> # CONFIG_NET_FAILOVER is not set >> CONFIG_MAY_USE_NET_FAILOVER=y > So the new option works, what's broken is when it's *not* selected. > Looks like a bug. Yes. Looks like i need to make NET_FAILOVER to be enabled automatically when VIRTIO_NET or HYPERV_NET are selected. Thanks Stephen for confirming that it works when NET_FAILOVER is enabled. > >> The netvsc driver fails on boot with: >> >> [ 0.826447] hv_vmbus: registering driver hv_netvsc >> [ 0.829616] scsi 0:0:0:0: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 5 >> [ 0.836291] input: Microsoft Vmbus HID-compliant Mouse as /devices/0006:045E:0621.0001/input/input1 >> [ 0.839139] hid-generic 0006:045E:0621.0001: input: <UNKNOWN> HID v0.01 Mouse [Microsoft Vmbus HID-compliant Mouse] on >> [ 0.964897] hv_vmbus: probe failed for device 849a776e-8120-4e4a-9a36-7e3d95ac75b3 (-95) >> [ 0.968039] hv_netvsc: probe of 849a776e-8120-4e4a-9a36-7e3d95ac75b3 failed with error -95 >> [ 1.112877] hv_vmbus: probe failed for device 53557f8e-057d-425b-9265-01c0fd7e273e (-95) >> [ 1.116064] hv_netvsc: probe of 53557f8e-057d-425b-9265-01c0fd7e273e failed with error -95 >> >> The system has two virtual networks. eth0 is on vswitch for management. >> eth1 is on vswitch with SR-IOV for performance tests. >> >> You probably need to just put the failover part in net/core and select it. > From all drivers. Yes. And it does not need to be visible in the menu > imho. > > >> It is trivial to get an evaluation version of Windows Server 2016 and setup a Linux VM. >> Please try it. > I presume some kind of test was done here. Sridhar, do you think you > could include some notes about testing of this patch? Or in case you > only build-tested this part, it's a good idea to notice this after --- > in the patch, or in the cover letter. I only did compile and build test of netvsc. I don't have a hyperv setup to test it out. Would appreciate if Stephen or someone who has this setup will test it out when i submit the next revision with the config fixes. >
diff --git a/drivers/net/hyperv/Kconfig b/drivers/net/hyperv/Kconfig index 0765d5f61714..20e70d4855a9 100644 --- a/drivers/net/hyperv/Kconfig +++ b/drivers/net/hyperv/Kconfig @@ -1,6 +1,7 @@ config HYPERV_NET tristate "Microsoft Hyper-V virtual network driver" depends on HYPERV + depends on MAY_USE_NET_FAILOVER select UCS2_STRING help Select this option to enable the Hyper-V virtual network driver. diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h index 6ebe39a3dde6..2ec18344c0e8 100644 --- a/drivers/net/hyperv/hyperv_net.h +++ b/drivers/net/hyperv/hyperv_net.h @@ -932,6 +932,8 @@ struct net_device_context { u32 vf_alloc; /* Serial number of the VF to team with */ u32 vf_serial; + + struct net_failover *failover; }; /* Per channel data */ diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index ecc84954c511..fa446234bc11 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -43,6 +43,7 @@ #include <net/pkt_sched.h> #include <net/checksum.h> #include <net/ip6_checksum.h> +#include <net/net_failover.h> #include "hyperv_net.h" @@ -1763,46 +1764,6 @@ static void netvsc_link_change(struct work_struct *w) rtnl_unlock(); } -static struct net_device *get_netvsc_bymac(const u8 *mac) -{ - struct net_device *dev; - - ASSERT_RTNL(); - - for_each_netdev(&init_net, dev) { - if (dev->netdev_ops != &device_ops) - continue; /* not a netvsc device */ - - if (ether_addr_equal(mac, dev->perm_addr)) - return dev; - } - - return NULL; -} - -static struct net_device *get_netvsc_byref(struct net_device *vf_netdev) -{ - struct net_device *dev; - - ASSERT_RTNL(); - - for_each_netdev(&init_net, dev) { - struct net_device_context *net_device_ctx; - - if (dev->netdev_ops != &device_ops) - continue; /* not a netvsc device */ - - net_device_ctx = netdev_priv(dev); - if (!rtnl_dereference(net_device_ctx->nvdev)) - continue; /* device is removed */ - - if (rtnl_dereference(net_device_ctx->vf_netdev) == vf_netdev) - return dev; /* a match */ - } - - return NULL; -} - /* Called when VF is injecting data into network stack. * Change the associated network device from VF to netvsc. * note: already called with rcu_read_lock @@ -1914,24 +1875,15 @@ static void netvsc_vf_setup(struct work_struct *w) rtnl_unlock(); } -static int netvsc_register_vf(struct net_device *vf_netdev) +static int netvsc_register_vf(struct net_device *vf_netdev, + struct net_device *ndev) { - struct net_device *ndev; struct net_device_context *net_device_ctx; struct netvsc_device *netvsc_dev; if (vf_netdev->addr_len != ETH_ALEN) return NOTIFY_DONE; - /* - * We will use the MAC address to locate the synthetic interface to - * associate with the VF interface. If we don't find a matching - * synthetic interface, move on. - */ - ndev = get_netvsc_bymac(vf_netdev->perm_addr); - if (!ndev) - return NOTIFY_DONE; - net_device_ctx = netdev_priv(ndev); netvsc_dev = rtnl_dereference(net_device_ctx->nvdev); if (!netvsc_dev || rtnl_dereference(net_device_ctx->vf_netdev)) @@ -1948,17 +1900,13 @@ static int netvsc_register_vf(struct net_device *vf_netdev) } /* VF up/down change detected, schedule to change data path */ -static int netvsc_vf_changed(struct net_device *vf_netdev) +static int netvsc_vf_changed(struct net_device *vf_netdev, + struct net_device *ndev) { struct net_device_context *net_device_ctx; struct netvsc_device *netvsc_dev; - struct net_device *ndev; bool vf_is_up = netif_running(vf_netdev); - ndev = get_netvsc_byref(vf_netdev); - if (!ndev) - return NOTIFY_DONE; - net_device_ctx = netdev_priv(ndev); netvsc_dev = rtnl_dereference(net_device_ctx->nvdev); if (!netvsc_dev) @@ -1971,15 +1919,11 @@ static int netvsc_vf_changed(struct net_device *vf_netdev) return NOTIFY_OK; } -static int netvsc_unregister_vf(struct net_device *vf_netdev) +static int netvsc_unregister_vf(struct net_device *vf_netdev, + struct net_device *ndev) { - struct net_device *ndev; struct net_device_context *net_device_ctx; - ndev = get_netvsc_byref(vf_netdev); - if (!ndev) - return NOTIFY_DONE; - net_device_ctx = netdev_priv(ndev); cancel_delayed_work_sync(&net_device_ctx->vf_takeover); @@ -1993,6 +1937,12 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev) return NOTIFY_OK; } +static struct net_failover_ops netvsc_failover_ops = { + .slave_register = netvsc_register_vf, + .slave_unregister = netvsc_unregister_vf, + .slave_link_change = netvsc_vf_changed, +}; + static int netvsc_probe(struct hv_device *dev, const struct hv_vmbus_device_id *dev_id) { @@ -2082,8 +2032,15 @@ static int netvsc_probe(struct hv_device *dev, goto register_failed; } + ret = net_failover_register(net, &netvsc_failover_ops, + &net_device_ctx->failover); + if (ret != 0) + goto err_failover; + return ret; +err_failover: + unregister_netdev(net); register_failed: rndis_filter_device_remove(dev, nvdev); rndis_failed: @@ -2124,13 +2081,15 @@ static int netvsc_remove(struct hv_device *dev) rtnl_lock(); vf_netdev = rtnl_dereference(ndev_ctx->vf_netdev); if (vf_netdev) - netvsc_unregister_vf(vf_netdev); + net_failover_slave_unregister(vf_netdev); if (nvdev) rndis_filter_device_remove(dev, nvdev); unregister_netdevice(net); + net_failover_unregister(ndev_ctx->failover); + rtnl_unlock(); rcu_read_unlock(); @@ -2157,54 +2116,8 @@ static struct hv_driver netvsc_drv = { .remove = netvsc_remove, }; -/* - * On Hyper-V, every VF interface is matched with a corresponding - * synthetic interface. The synthetic interface is presented first - * to the guest. When the corresponding VF instance is registered, - * we will take care of switching the data path. - */ -static int netvsc_netdev_event(struct notifier_block *this, - unsigned long event, void *ptr) -{ - struct net_device *event_dev = netdev_notifier_info_to_dev(ptr); - - /* Skip our own events */ - if (event_dev->netdev_ops == &device_ops) - return NOTIFY_DONE; - - /* Avoid non-Ethernet type devices */ - if (event_dev->type != ARPHRD_ETHER) - return NOTIFY_DONE; - - /* Avoid Vlan dev with same MAC registering as VF */ - if (is_vlan_dev(event_dev)) - return NOTIFY_DONE; - - /* Avoid Bonding master dev with same MAC registering as VF */ - if ((event_dev->priv_flags & IFF_BONDING) && - (event_dev->flags & IFF_MASTER)) - return NOTIFY_DONE; - - switch (event) { - case NETDEV_REGISTER: - return netvsc_register_vf(event_dev); - case NETDEV_UNREGISTER: - return netvsc_unregister_vf(event_dev); - case NETDEV_UP: - case NETDEV_DOWN: - return netvsc_vf_changed(event_dev); - default: - return NOTIFY_DONE; - } -} - -static struct notifier_block netvsc_netdev_notifier = { - .notifier_call = netvsc_netdev_event, -}; - static void __exit netvsc_drv_exit(void) { - unregister_netdevice_notifier(&netvsc_netdev_notifier); vmbus_driver_unregister(&netvsc_drv); } @@ -2224,7 +2137,6 @@ static int __init netvsc_drv_init(void) if (ret) return ret; - register_netdevice_notifier(&netvsc_netdev_notifier); return 0; }
Use the registration/notification framework supported by the generic failover infrastructure. Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> --- drivers/net/hyperv/Kconfig | 1 + drivers/net/hyperv/hyperv_net.h | 2 + drivers/net/hyperv/netvsc_drv.c | 134 +++++++--------------------------------- 3 files changed, 26 insertions(+), 111 deletions(-)