RE: [PATCH net,v3] hyperv: Fix the carrier status setting

2014-02-13 Thread Haiyang Zhang


 -Original Message-
 From: Jason Wang [mailto:jasow...@redhat.com]
 Sent: Wednesday, February 12, 2014 10:52 PM
 To: Haiyang Zhang; da...@davemloft.net; net...@vger.kernel.org
 Cc: KY Srinivasan; o...@aepfle.de; linux-ker...@vger.kernel.org; driverdev-
 de...@linuxdriverproject.org
 Subject: Re: [PATCH net,v3] hyperv: Fix the carrier status setting
 
 On 02/13/2014 08:54 AM, Haiyang Zhang wrote:
  Without this patch, the cat /sys/class/net/ethN/operstate shows
  unknown, and ethtool ethN shows Link detected: yes, when VM
  boots up with or without vNIC connected.
 
  This patch fixed the problem.
 
  Signed-off-by: Haiyang Zhang haiya...@microsoft.com
  Reviewed-by: K. Y. Srinivasan k...@microsoft.com
  ---
   drivers/net/hyperv/netvsc_drv.c |   53
 ---
   1 files changed, 38 insertions(+), 15 deletions(-)
 
  diff --git a/drivers/net/hyperv/netvsc_drv.c
  b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644
  --- a/drivers/net/hyperv/netvsc_drv.c
  +++ b/drivers/net/hyperv/netvsc_drv.c
  @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net)  {
  struct net_device_context *net_device_ctx = netdev_priv(net);
  struct hv_device *device_obj = net_device_ctx-device_ctx;
  +   struct netvsc_device *nvdev;
  +   struct rndis_device *rdev;
  int ret = 0;
 
  +   netif_carrier_off(net);
  +
  /* Open up the device */
  ret = rndis_filter_open(device_obj);
  if (ret != 0) {
  @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net)
 
  netif_start_queue(net);
 
  +   nvdev = hv_get_drvdata(device_obj);
  +   rdev = nvdev-extension;
  +   if (!rdev-link_state)
  +   netif_carrier_on(net);
  +
 
 Maybe you can just schedule the work here and then you can drop the
 rtnl_lock in netvsc_link_change() ?

The rtnl_lock will still be necessary in the netvsc_link_change(), because 
we want to prevent it getting wrong rdev pointer when netvsc_change_mtu
is removing/adding rndis device.

  +
  +   if (notify)
  +   netdev_notify_peers(net);
   }
 
 
 Looks like this forces arp_notify here. Is it expected?

Yes, this is expected. It's required after live migration.

Thanks,
- Haiyang


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH net,v3] hyperv: Fix the carrier status setting

2014-02-13 Thread Jason Wang
On 02/13/2014 11:04 PM, Haiyang Zhang wrote:

 -Original Message-
 From: Jason Wang [mailto:jasow...@redhat.com]
 Sent: Wednesday, February 12, 2014 10:52 PM
 To: Haiyang Zhang; da...@davemloft.net; net...@vger.kernel.org
 Cc: KY Srinivasan; o...@aepfle.de; linux-ker...@vger.kernel.org; driverdev-
 de...@linuxdriverproject.org
 Subject: Re: [PATCH net,v3] hyperv: Fix the carrier status setting

 On 02/13/2014 08:54 AM, Haiyang Zhang wrote:
 Without this patch, the cat /sys/class/net/ethN/operstate shows
 unknown, and ethtool ethN shows Link detected: yes, when VM
 boots up with or without vNIC connected.

 This patch fixed the problem.

 Signed-off-by: Haiyang Zhang haiya...@microsoft.com
 Reviewed-by: K. Y. Srinivasan k...@microsoft.com
 ---
  drivers/net/hyperv/netvsc_drv.c |   53
 ---
  1 files changed, 38 insertions(+), 15 deletions(-)

 diff --git a/drivers/net/hyperv/netvsc_drv.c
 b/drivers/net/hyperv/netvsc_drv.c index 7756118..7141a19 100644
 --- a/drivers/net/hyperv/netvsc_drv.c
 +++ b/drivers/net/hyperv/netvsc_drv.c
 @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net)  {
 struct net_device_context *net_device_ctx = netdev_priv(net);
 struct hv_device *device_obj = net_device_ctx-device_ctx;
 +   struct netvsc_device *nvdev;
 +   struct rndis_device *rdev;
 int ret = 0;

 +   netif_carrier_off(net);
 +
 /* Open up the device */
 ret = rndis_filter_open(device_obj);
 if (ret != 0) {
 @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net)

 netif_start_queue(net);

 +   nvdev = hv_get_drvdata(device_obj);
 +   rdev = nvdev-extension;
 +   if (!rdev-link_state)
 +   netif_carrier_on(net);
 +
 Maybe you can just schedule the work here and then you can drop the
 rtnl_lock in netvsc_link_change() ?
 The rtnl_lock will still be necessary in the netvsc_link_change(), because 
 we want to prevent it getting wrong rdev pointer when netvsc_change_mtu
 is removing/adding rndis device.

Ok.
 +
 +   if (notify)
 +   netdev_notify_peers(net);
  }

 Looks like this forces arp_notify here. Is it expected?
 Yes, this is expected. It's required after live migration.

 Thanks,
 - Haiyang

Yes, this does not change the current behaviour. (arp_notify is
meaningless for netvsc).

Acked-by: Jason Wang jasow...@redhat.com

The patch is also needed for stable.

Thanks

 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH net,v3] hyperv: Fix the carrier status setting

2014-02-13 Thread David Miller
From: Haiyang Zhang haiya...@microsoft.com
Date: Wed, 12 Feb 2014 16:54:27 -0800

 Without this patch, the cat /sys/class/net/ethN/operstate shows
 unknown, and ethtool ethN shows Link detected: yes, when VM
 boots up with or without vNIC connected.
 
 This patch fixed the problem.
 
 Signed-off-by: Haiyang Zhang haiya...@microsoft.com
 Reviewed-by: K. Y. Srinivasan k...@microsoft.com

Applied and queued up for -stable, thanks.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH net,v3] hyperv: Fix the carrier status setting

2014-02-12 Thread Haiyang Zhang
Without this patch, the cat /sys/class/net/ethN/operstate shows
unknown, and ethtool ethN shows Link detected: yes, when VM
boots up with or without vNIC connected.

This patch fixed the problem.

Signed-off-by: Haiyang Zhang haiya...@microsoft.com
Reviewed-by: K. Y. Srinivasan k...@microsoft.com
---
 drivers/net/hyperv/netvsc_drv.c |   53 ---
 1 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 7756118..7141a19 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net)
 {
struct net_device_context *net_device_ctx = netdev_priv(net);
struct hv_device *device_obj = net_device_ctx-device_ctx;
+   struct netvsc_device *nvdev;
+   struct rndis_device *rdev;
int ret = 0;
 
+   netif_carrier_off(net);
+
/* Open up the device */
ret = rndis_filter_open(device_obj);
if (ret != 0) {
@@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net)
 
netif_start_queue(net);
 
+   nvdev = hv_get_drvdata(device_obj);
+   rdev = nvdev-extension;
+   if (!rdev-link_state)
+   netif_carrier_on(net);
+
return ret;
 }
 
@@ -229,23 +238,24 @@ void netvsc_linkstatus_callback(struct hv_device 
*device_obj,
struct net_device *net;
struct net_device_context *ndev_ctx;
struct netvsc_device *net_device;
+   struct rndis_device *rdev;
 
net_device = hv_get_drvdata(device_obj);
+   rdev = net_device-extension;
+
+   rdev-link_state = status != 1;
+
net = net_device-ndev;
 
-   if (!net) {
-   netdev_err(net, got link status but net device 
-   not initialized yet\n);
+   if (!net || net-reg_state != NETREG_REGISTERED)
return;
-   }
 
+   ndev_ctx = netdev_priv(net);
if (status == 1) {
-   netif_carrier_on(net);
-   ndev_ctx = netdev_priv(net);
schedule_delayed_work(ndev_ctx-dwork, 0);
schedule_delayed_work(ndev_ctx-dwork, msecs_to_jiffies(20));
} else {
-   netif_carrier_off(net);
+   schedule_delayed_work(ndev_ctx-dwork, 0);
}
 }
 
@@ -388,17 +398,35 @@ static const struct net_device_ops device_ops = {
  * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add
  * another netif_notify_peers() into a delayed work, otherwise GARP packet
  * will not be sent after quick migration, and cause network disconnection.
+ * Also, we update the carrier status here.
  */
-static void netvsc_send_garp(struct work_struct *w)
+static void netvsc_link_change(struct work_struct *w)
 {
struct net_device_context *ndev_ctx;
struct net_device *net;
struct netvsc_device *net_device;
+   struct rndis_device *rdev;
+   bool notify;
+
+   rtnl_lock();
 
ndev_ctx = container_of(w, struct net_device_context, dwork.work);
net_device = hv_get_drvdata(ndev_ctx-device_ctx);
+   rdev = net_device-extension;
net = net_device-ndev;
-   netdev_notify_peers(net);
+
+   if (rdev-link_state) {
+   netif_carrier_off(net);
+   notify = false;
+   } else {
+   netif_carrier_on(net);
+   notify = true;
+   }
+
+   rtnl_unlock();
+
+   if (notify)
+   netdev_notify_peers(net);
 }
 
 
@@ -414,13 +442,10 @@ static int netvsc_probe(struct hv_device *dev,
if (!net)
return -ENOMEM;
 
-   /* Set initial state */
-   netif_carrier_off(net);
-
net_device_ctx = netdev_priv(net);
net_device_ctx-device_ctx = dev;
hv_set_drvdata(dev, net);
-   INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_send_garp);
+   INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_link_change);
INIT_WORK(net_device_ctx-work, do_set_multicast);
 
net-netdev_ops = device_ops;
@@ -443,8 +468,6 @@ static int netvsc_probe(struct hv_device *dev,
}
memcpy(net-dev_addr, device_info.mac_adr, ETH_ALEN);
 
-   netif_carrier_on(net);
-
ret = register_netdev(net);
if (ret != 0) {
pr_err(Unable to register netdev.\n);
-- 
1.7.4.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH net,v3] hyperv: Fix the carrier status setting

2014-02-12 Thread Jason Wang
On 02/13/2014 08:54 AM, Haiyang Zhang wrote:
 Without this patch, the cat /sys/class/net/ethN/operstate shows
 unknown, and ethtool ethN shows Link detected: yes, when VM
 boots up with or without vNIC connected.

 This patch fixed the problem.

 Signed-off-by: Haiyang Zhang haiya...@microsoft.com
 Reviewed-by: K. Y. Srinivasan k...@microsoft.com
 ---
  drivers/net/hyperv/netvsc_drv.c |   53 
 ---
  1 files changed, 38 insertions(+), 15 deletions(-)

 diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
 index 7756118..7141a19 100644
 --- a/drivers/net/hyperv/netvsc_drv.c
 +++ b/drivers/net/hyperv/netvsc_drv.c
 @@ -88,8 +88,12 @@ static int netvsc_open(struct net_device *net)
  {
   struct net_device_context *net_device_ctx = netdev_priv(net);
   struct hv_device *device_obj = net_device_ctx-device_ctx;
 + struct netvsc_device *nvdev;
 + struct rndis_device *rdev;
   int ret = 0;
  
 + netif_carrier_off(net);
 +
   /* Open up the device */
   ret = rndis_filter_open(device_obj);
   if (ret != 0) {
 @@ -99,6 +103,11 @@ static int netvsc_open(struct net_device *net)
  
   netif_start_queue(net);
  
 + nvdev = hv_get_drvdata(device_obj);
 + rdev = nvdev-extension;
 + if (!rdev-link_state)
 + netif_carrier_on(net);
 +

Maybe you can just schedule the work here and then you can drop the
rtnl_lock in netvsc_link_change() ?
   return ret;
  }
  
 @@ -229,23 +238,24 @@ void netvsc_linkstatus_callback(struct hv_device 
 *device_obj,
   struct net_device *net;
   struct net_device_context *ndev_ctx;
   struct netvsc_device *net_device;
 + struct rndis_device *rdev;
  
   net_device = hv_get_drvdata(device_obj);
 + rdev = net_device-extension;
 +
 + rdev-link_state = status != 1;
 +
   net = net_device-ndev;
  
 - if (!net) {
 - netdev_err(net, got link status but net device 
 - not initialized yet\n);
 + if (!net || net-reg_state != NETREG_REGISTERED)
   return;
 - }
  
 + ndev_ctx = netdev_priv(net);
   if (status == 1) {
 - netif_carrier_on(net);
 - ndev_ctx = netdev_priv(net);
   schedule_delayed_work(ndev_ctx-dwork, 0);
   schedule_delayed_work(ndev_ctx-dwork, msecs_to_jiffies(20));
   } else {
 - netif_carrier_off(net);
 + schedule_delayed_work(ndev_ctx-dwork, 0);
   }
  }
  
 @@ -388,17 +398,35 @@ static const struct net_device_ops device_ops = {
   * current context when receiving RNDIS_STATUS_MEDIA_CONNECT event. So, add
   * another netif_notify_peers() into a delayed work, otherwise GARP packet
   * will not be sent after quick migration, and cause network disconnection.
 + * Also, we update the carrier status here.
   */
 -static void netvsc_send_garp(struct work_struct *w)
 +static void netvsc_link_change(struct work_struct *w)
  {
   struct net_device_context *ndev_ctx;
   struct net_device *net;
   struct netvsc_device *net_device;
 + struct rndis_device *rdev;
 + bool notify;
 +
 + rtnl_lock();
  
   ndev_ctx = container_of(w, struct net_device_context, dwork.work);
   net_device = hv_get_drvdata(ndev_ctx-device_ctx);
 + rdev = net_device-extension;
   net = net_device-ndev;
 - netdev_notify_peers(net);
 +
 + if (rdev-link_state) {
 + netif_carrier_off(net);
 + notify = false;
 + } else {
 + netif_carrier_on(net);
 + notify = true;
 + }
 +
 + rtnl_unlock();
 +
 + if (notify)
 + netdev_notify_peers(net);
  }
  

Looks like this forces arp_notify here. Is it expected?

Other looks good.
  
 @@ -414,13 +442,10 @@ static int netvsc_probe(struct hv_device *dev,
   if (!net)
   return -ENOMEM;
  
 - /* Set initial state */
 - netif_carrier_off(net);
 -
   net_device_ctx = netdev_priv(net);
   net_device_ctx-device_ctx = dev;
   hv_set_drvdata(dev, net);
 - INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_send_garp);
 + INIT_DELAYED_WORK(net_device_ctx-dwork, netvsc_link_change);
   INIT_WORK(net_device_ctx-work, do_set_multicast);
  
   net-netdev_ops = device_ops;
 @@ -443,8 +468,6 @@ static int netvsc_probe(struct hv_device *dev,
   }
   memcpy(net-dev_addr, device_info.mac_adr, ETH_ALEN);
  
 - netif_carrier_on(net);
 -
   ret = register_netdev(net);
   if (ret != 0) {
   pr_err(Unable to register netdev.\n);

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel