Re: [PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-26 Thread Samudrala, Sridhar


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"  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  wrote:
   

Use the registration/notification framework supported by the generic
failover infrastructure.

Signed-off-by: Sridhar Samudrala 

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





___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-26 Thread Michael S. Tsirkin
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"  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  wrote:
> > >   
> > > > Use the registration/notification framework supported by the generic
> > > > failover infrastructure.
> > > > 
> > > > Signed-off-by: Sridhar Samudrala   
> > > 
> > > 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:  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.

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-26 Thread Stephen Hemminger
On Thu, 26 Apr 2018 05:30:05 +0300
"Michael S. Tsirkin"  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  wrote:
> >   
> > > Use the registration/notification framework supported by the generic
> > > failover infrastructure.
> > > 
> > > Signed-off-by: Sridhar Samudrala   
> > 
> > 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:  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.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-25 Thread Michael S. Tsirkin
On Wed, Apr 25, 2018 at 05:08:37PM -0700, Stephen Hemminger wrote:
> On Wed, 25 Apr 2018 16:59:28 -0700
> Sridhar Samudrala  wrote:
> 
> > Use the registration/notification framework supported by the generic
> > failover infrastructure.
> > 
> > Signed-off-by: Sridhar Samudrala 
> 
> 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?

-- 
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-25 Thread Stephen Hemminger
On Wed, 25 Apr 2018 16:59:28 -0700
Sridhar Samudrala  wrote:

> Use the registration/notification framework supported by the generic
> failover infrastructure.
> 
> Signed-off-by: Sridhar Samudrala 

NAK unless you prove this works on legacy distributions and with DPDK 18.05
without modification.
 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH net-next v8 4/4] netvsc: refactor notifier/event handling code to use the failover framework

2018-04-25 Thread Sridhar Samudrala
Use the registration/notification framework supported by the generic
failover infrastructure.

Signed-off-by: Sridhar Samudrala 
---
 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(-)

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 
 #include 
 #include 
+#include 
 
 #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_dev