hv_netvsc: rework link status change handling

There are several issues in hv_netvsc driver with regards to link status
change handling:
- RNDIS_STATUS_NETWORK_CHANGE results in calling userspace helper doing
  '/etc/init.d/network restart' and this is inappropriate and broken for
  many reasons.
- link_watch infrastructure only sends one notification per second and
  in case of e.g. paired disconnect/connect events we get only one
  notification with last status. This makes it impossible to handle such
  situations in userspace.

Redo link status changes handling in the following way:
- Create a list of reconfig events in network device context.
- On a reconfig event add it to the list of events and schedule
  netvsc_link_change().
- In netvsc_link_change() ensure 2-second delay between link status
  changes.
- Handle RNDIS_STATUS_NETWORK_CHANGE as a paired disconnect/connect event.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 5fa98f599b3dd47996d064fd1685aede8a86bf63..7661a12b00f4b0d25f141b3bb3079fe0278e436c 100644 (file)
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -177,7 +177,6 @@ struct rndis_device {
 
        enum rndis_device_state state;
        bool link_state;
-       bool link_change;
        atomic_t new_req_id;
 
        spinlock_t request_lock;
@@ -644,11 +643,24 @@ struct netvsc_stats {
        struct u64_stats_sync syncp;
 };
 
+struct netvsc_reconfig {
+       struct list_head list;
+       u32 event;
+};
+
 /* The context of the netvsc device  */
 struct net_device_context {
        /* point back to our device context */
        struct hv_device *device_ctx;
+       /* reconfigure work */
        struct delayed_work dwork;
+       /* last reconfig time */
+       unsigned long last_reconfig;
+       /* reconfig events */
+       struct list_head reconfig_events;
+       /* list protection */
+       spinlock_t lock;
+
        struct work_struct work;
        u32 msg_enable; /* debug level */
 

diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 409b48e1e589dfdfa795a56b7369d835fa4643da..268a05821d46c5604f0e44447e8fbe48ccd3b2da 100644 (file)
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -42,6 +42,7 @@
 
 
 #define RING_SIZE_MIN 64
+#define LINKCHANGE_INT (2 * HZ)
 static int ring_size = 128;
 module_param(ring_size, int, S_IRUGO);
 MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
@@ -647,37 +648,33 @@ 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;
+       struct netvsc_reconfig *event;
+       unsigned long flags;
 
-       switch (indicate->status) {
-       case RNDIS_STATUS_MEDIA_CONNECT:
-               rdev->link_state = false;
-               break;
-       case RNDIS_STATUS_MEDIA_DISCONNECT:
-               rdev->link_state = true;
-               break;
-       case RNDIS_STATUS_NETWORK_CHANGE:
-               rdev->link_change = true;
-               break;
-       default:
+       /* Handle link change statuses only */
+       if (indicate->status != RNDIS_STATUS_NETWORK_CHANGE &&
+           indicate->status != RNDIS_STATUS_MEDIA_CONNECT &&
+           indicate->status != RNDIS_STATUS_MEDIA_DISCONNECT)
                return;
-       }
 
+       net_device = hv_get_drvdata(device_obj);
        net = net_device->ndev;
 
        if (!net || net->reg_state != NETREG_REGISTERED)
                return;
 
        ndev_ctx = netdev_priv(net);
-       if (!rdev->link_state) {
-               schedule_delayed_work(&ndev_ctx->dwork, 0);
-               schedule_delayed_work(&ndev_ctx->dwork, msecs_to_jiffies(20));
-       } else {
-               schedule_delayed_work(&ndev_ctx->dwork, 0);
-       }
+
+       event = kzalloc(sizeof(*event), GFP_ATOMIC);
+       if (!event)
+               return;
+       event->event = indicate->status;
+
+       spin_lock_irqsave(&ndev_ctx->lock, flags);
+       list_add_tail(&event->list, &ndev_ctx->reconfig_events);
+       spin_unlock_irqrestore(&ndev_ctx->lock, flags);
+
+       schedule_delayed_work(&ndev_ctx->dwork, 0);
 }
 
 /*
@@ -1009,12 +1006,9 @@ static const struct net_device_ops device_ops = {
 };
 
 /*
- * Send GARP packet to network peers after migrations.
- * After Quick Migration, the network is not immediately operational in the
- * 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.
+ * Handle link status changes. For RNDIS_STATUS_NETWORK_CHANGE emulate link
+ * down/up sequence. In case of RNDIS_STATUS_MEDIA_CONNECT when carrier is
+ * present send GARP packet to network peers with netif_notify_peers().
  */
 static void netvsc_link_change(struct work_struct *w)
 {
@@ -1022,36 +1016,89 @@ static void netvsc_link_change(struct work_struct *w)
        struct net_device *net;
        struct netvsc_device *net_device;
        struct rndis_device *rdev;
-       bool notify, refresh = false;
-       char *argv[] = { "/etc/init.d/network", "restart", NULL };
-       char *envp[] = { "HOME=/", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-
-       rtnl_lock();
+       struct netvsc_reconfig *event = NULL;
+       bool notify = false, reschedule = false;
+       unsigned long flags, next_reconfig, delay;
 
        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;
 
-       if (rdev->link_state) {
-               netif_carrier_off(net);
-               notify = false;
-       } else {
-               netif_carrier_on(net);
-               notify = true;
-               if (rdev->link_change) {
-                       rdev->link_change = false;
-                       refresh = true;
+       next_reconfig = ndev_ctx->last_reconfig + LINKCHANGE_INT;
+       if (time_is_after_jiffies(next_reconfig)) {
+               /* link_watch only sends one notification with current state
+                * per second, avoid doing reconfig more frequently. Handle
+                * wrap around.
+                */
+               delay = next_reconfig - jiffies;
+               delay = delay < LINKCHANGE_INT ? delay : LINKCHANGE_INT;
+               schedule_delayed_work(&ndev_ctx->dwork, delay);
+               return;
+       }
+       ndev_ctx->last_reconfig = jiffies;
+
+       spin_lock_irqsave(&ndev_ctx->lock, flags);
+       if (!list_empty(&ndev_ctx->reconfig_events)) {
+               event = list_first_entry(&ndev_ctx->reconfig_events,
+                                        struct netvsc_reconfig, list);
+               list_del(&event->list);
+               reschedule = !list_empty(&ndev_ctx->reconfig_events);
+       }
+       spin_unlock_irqrestore(&ndev_ctx->lock, flags);
+
+       if (!event)
+               return;
+
+       rtnl_lock();
+
+       switch (event->event) {
+               /* Only the following events are possible due to the check in
+                * netvsc_linkstatus_callback()
+                */
+       case RNDIS_STATUS_MEDIA_CONNECT:
+               if (rdev->link_state) {
+                       rdev->link_state = false;
+                       netif_carrier_on(net);
+                       netif_tx_wake_all_queues(net);
+               } else {
+                       notify = true;
+               }
+               kfree(event);
+               break;
+       case RNDIS_STATUS_MEDIA_DISCONNECT:
+               if (!rdev->link_state) {
+                       rdev->link_state = true;
+                       netif_carrier_off(net);
+                       netif_tx_stop_all_queues(net);
+               }
+               kfree(event);
+               break;
+       case RNDIS_STATUS_NETWORK_CHANGE:
+               /* Only makes sense if carrier is present */
+               if (!rdev->link_state) {
+                       rdev->link_state = true;
+                       netif_carrier_off(net);
+                       netif_tx_stop_all_queues(net);
+                       event->event = RNDIS_STATUS_MEDIA_CONNECT;
+                       spin_lock_irqsave(&ndev_ctx->lock, flags);
+                       list_add_tail(&event->list, &ndev_ctx->reconfig_events);
+                       spin_unlock_irqrestore(&ndev_ctx->lock, flags);
+                       reschedule = true;
                }
+               break;
        }
 
        rtnl_unlock();
 
-       if (refresh)
-               call_usermodehelper(argv[0], argv, envp, UMH_WAIT_EXEC);
-
        if (notify)
                netdev_notify_peers(net);
+
+       /* link_watch only sends one notification with current state per
+        * second, handle next reconfig event in 2 seconds.
+        */
+       if (reschedule)
+               schedule_delayed_work(&ndev_ctx->dwork, LINKCHANGE_INT);
 }
 
 static void netvsc_free_netdev(struct net_device *netdev)
@@ -1106,6 +1153,9 @@ static int netvsc_probe(struct hv_device *dev,
        INIT_DELAYED_WORK(&net_device_ctx->dwork, netvsc_link_change);
        INIT_WORK(&net_device_ctx->work, do_set_multicast);
 
+       spin_lock_init(&net_device_ctx->lock);
+       INIT_LIST_HEAD(&net_device_ctx->reconfig_events);
+
        net->netdev_ops = &device_ops;
 
        net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM |
@@ -1145,8 +1195,6 @@ static int netvsc_probe(struct hv_device *dev,
                pr_err("Unable to register netdev.\n");
                rndis_filter_device_remove(dev);
                netvsc_free_netdev(net);
-       } else {
-               schedule_delayed_work(&net_device_ctx->dwork, 0);
        }
 
        return ret;