[mini-os master] mini-os: netfront: fix suspend/resume handling

[patchbot@xxxxxxx]

commit 4821876fcd2ff30bdfd77835731e9a0f1eaa33db
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Tue Sep 22 12:58:26 2020 +0200
Commit:     Wei Liu <wl@xxxxxxx>
CommitDate: Thu Oct 1 09:36:35 2020 +0000

    mini-os: netfront: fix suspend/resume handling
    
    Suspend/resume handling of netfront is completely broken from the
    beginning. Commit d225f4012d69a1 ("Save/Restore Support: Add
    suspend/restore support for netfront") introduced a new structure
    netfront_dev_list referencing the real struct netfront_dev elements.
    This list is used to setup the devices when resuming again.
    
    Unfortunately the netfront_dev elements are released during suspend,
    so at resume time those references will be stale.
    
    Fix this whole mess by dropping struct netfront_dev_list again and
    link the netfront_dev elements directly into a list. When suspending
    don't free those elements.
    
    The ip-address, netmask and gateway strings can just be released when
    suspending and reread from xenstore at resume time.
    
    Fixes: d225f4012d69a1 ("Save/Restore Support: Add suspend/restore support 
for netfront")
    Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
    Reviewed-by: Samuel Thibault <samuel.thibault@xxxxxxxxxxxx>
---
 netfront.c | 162 +++++++++++++++++++++++++------------------------------------
 1 file changed, 67 insertions(+), 95 deletions(-)

diff --git a/netfront.c b/netfront.c
index 9057908..2075410 100644
--- a/netfront.c
+++ b/netfront.c
@@ -36,6 +36,8 @@ struct net_buffer {
 };
 
 struct netfront_dev {
+    int refcount;
+
     domid_t dom;
 
     unsigned short tx_freelist[NET_TX_RING_SIZE + 1];
@@ -66,27 +68,19 @@ struct netfront_dev {
     void (*netif_rx)(unsigned char* data, int len, void* arg);
     void *netif_rx_arg;
 
-    struct netfront_dev_list *ldev;
-};
-
-struct netfront_dev_list {
-    struct netfront_dev *dev;
     unsigned char rawmac[6];
     char *ip;
     char *mask;
     char *gw;
 
-    int refcount;
-
-    struct netfront_dev_list *next;
+    struct netfront_dev *next;
 };
 
-static struct netfront_dev_list *dev_list = NULL;
+static struct netfront_dev *dev_list = NULL;
 
 void init_rx_buffers(struct netfront_dev *dev);
-static struct netfront_dev *_init_netfront(struct netfront_dev *dev,
-                                           unsigned char rawmac[6], char **ip, 
char **mask, char **gw);
-static void _shutdown_netfront(struct netfront_dev *dev);
+static struct netfront_dev *_init_netfront(struct netfront_dev *dev);
+static int _shutdown_netfront(struct netfront_dev *dev);
 void netfront_set_rx_handler(struct netfront_dev *dev,
                              void (*thenetif_rx)(unsigned char *data, int len,
                                                  void *arg),
@@ -276,6 +270,7 @@ static void free_netfront(struct netfront_dev *dev)
     mask_evtchn(dev->evtchn);
 
     free(dev->mac);
+    free(dev->ip);
     free(dev->backend);
 
     gnttab_end_access(dev->rx_ring_ref);
@@ -309,8 +304,7 @@ struct netfront_dev *init_netfront(char *_nodename,
 {
     char nodename[256];
     struct netfront_dev *dev;
-    struct netfront_dev_list *ldev = NULL;
-    struct netfront_dev_list *list = NULL;
+    struct netfront_dev *list;
     static int netfrontends = 0;
 
     if (!_nodename)
@@ -321,10 +315,9 @@ struct netfront_dev *init_netfront(char *_nodename,
     }
 
     /* Check if the device is already initialized */
-    for (list = dev_list; list != NULL; list = list->next) {
-        if (strcmp(nodename, list->dev->nodename) == 0) {
-            list->refcount++;
-            dev = list->dev;
+    for (dev = dev_list; dev != NULL; dev = dev->next) {
+        if (strcmp(nodename, dev->nodename) == 0) {
+            dev->refcount++;
             if (thenetif_rx)
                 netfront_set_rx_handler(dev, thenetif_rx, NULL);
             goto out;
@@ -345,40 +338,34 @@ struct netfront_dev *init_netfront(char *_nodename,
     dev->netif_rx = thenetif_rx;
     dev->netif_rx_arg = NULL;
 
-    ldev = malloc(sizeof(struct netfront_dev_list));
-    memset(ldev, 0, sizeof(struct netfront_dev_list));
-
-    if (_init_netfront(dev, ldev->rawmac, &(ldev->ip), &(ldev->mask), 
&(ldev->gw))) {
-        dev->ldev = ldev;
-        ldev->dev = dev;
-        ldev->refcount = 1;
-        ldev->next = NULL;
+    if (_init_netfront(dev)) {
+        dev->refcount = 1;
+        dev->next = NULL;
 
         if (!dev_list) {
-            dev_list = ldev;
+            dev_list = dev;
         } else {
             for (list = dev_list; list->next != NULL; list = list->next)
                 ;
-            list->next = ldev;
-               }
+            list->next = dev;
+        }
         netfrontends++;
     } else {
-        free(ldev);
         dev = NULL;
         goto err;
     }
 
 out:
     if (rawmac) {
-        rawmac[0] = ldev->rawmac[0];
-        rawmac[1] = ldev->rawmac[1];
-        rawmac[2] = ldev->rawmac[2];
-        rawmac[3] = ldev->rawmac[3];
-        rawmac[4] = ldev->rawmac[4];
-        rawmac[5] = ldev->rawmac[5];
+        rawmac[0] = dev->rawmac[0];
+        rawmac[1] = dev->rawmac[1];
+        rawmac[2] = dev->rawmac[2];
+        rawmac[3] = dev->rawmac[3];
+        rawmac[4] = dev->rawmac[4];
+        rawmac[5] = dev->rawmac[5];
        }
     if (ip)
-        *ip = strdup(ldev->ip);
+        *ip = strdup(dev->ip);
 
 err:
     return dev;
@@ -386,17 +373,15 @@ err:
 
 char *netfront_get_netmask(struct netfront_dev *dev)
 {
-    return dev->ldev->mask ? strdup(dev->ldev->mask) : NULL;
+    return dev->mask ? strdup(dev->mask) : NULL;
 }
 
 char *netfront_get_gateway(struct netfront_dev *dev)
 {
-    return dev->ldev->gw ? strdup(dev->ldev->gw) : NULL;
+    return dev->gw ? strdup(dev->gw) : NULL;
 }
 
-static struct netfront_dev *_init_netfront(struct netfront_dev *dev,
-                                          unsigned char rawmac[6],
-                                          char **ip, char **mask, char **gw)
+static struct netfront_dev *_init_netfront(struct netfront_dev *dev)
 {
     xenbus_transaction_t xbt;
     char* err = NULL;
@@ -518,6 +503,8 @@ done:
     {
         XenbusState state;
         char path[strlen(dev->backend) + strlen("/state") + 1];
+        char *p;
+
         snprintf(path, sizeof(path), "%s/state", dev->backend);
 
         xenbus_watch_path_token(XBT_NIL, path, path, &dev->events);
@@ -532,26 +519,18 @@ done:
             goto error;
         }
 
-        if (ip) {
-            char *p;
-
-            snprintf(path, sizeof(path), "%s/ip", dev->backend);
-            xenbus_read(XBT_NIL, path, ip);
-
-            if (mask) {
-                p = strchr(*ip, ' ');
-                if (p) {
-                    *p++ = '\0';
-                    *mask = p;
-
-                    if (gw) {
-                        p = strchr(p, ' ');
-                        if (p) {
-                            *p++ = '\0';
-                            *gw = p;
-                        }
-                    }
-                }
+        snprintf(path, sizeof(path), "%s/ip", dev->backend);
+        xenbus_read(XBT_NIL, path, &dev->ip);
+
+        p = strchr(dev->ip, ' ');
+        if (p) {
+            *p++ = '\0';
+            dev->mask = p;
+
+            p = strchr(p, ' ');
+            if (p) {
+                *p++ = '\0';
+                dev->gw = p;
             }
         }
     }
@@ -563,14 +542,13 @@ done:
     /* Special conversion specifier 'hh' needed for __ia64__. Without
      * this mini-os panics with 'Unaligned reference'.
      */
-    if (rawmac)
-        sscanf(dev->mac,"%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
-               &rawmac[0],
-               &rawmac[1],
-               &rawmac[2],
-               &rawmac[3],
-               &rawmac[4],
-               &rawmac[5]);
+    sscanf(dev->mac,"%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+           &dev->rawmac[0],
+           &dev->rawmac[1],
+           &dev->rawmac[2],
+           &dev->rawmac[3],
+           &dev->rawmac[4],
+           &dev->rawmac[5]);
 
     return dev;
 
@@ -600,38 +578,33 @@ int netfront_tap_open(char *nodename) {
 
 void shutdown_netfront(struct netfront_dev *dev)
 {
-    struct netfront_dev_list *list = NULL;
-    struct netfront_dev_list *to_del = NULL;
+    struct netfront_dev *list;
 
     /* Check this is a valid device */
-    for (list = dev_list; list != NULL; list = list->next) {
-        if (list->dev == dev)
-            break;
-    }
+    for (list = dev_list; list != NULL && list != dev; list = list->next);
 
     if (!list) {
         printk("Trying to shutdown an invalid netfront device (%p)\n", dev);
         return;
     }
 
-    list->refcount--;
-    if (list->refcount == 0) {
-        _shutdown_netfront(dev);
+    dev->refcount--;
+    if (dev->refcount == 0) {
+        if (_shutdown_netfront(dev))
+            return;
 
-        to_del = list;
-        if (to_del == dev_list) {
-            free(to_del);
-                       dev_list = NULL;
+        if (dev == dev_list) {
+            dev_list = NULL;
         } else {
-            for (list = dev_list; list->next != to_del; list = list->next)
+            for (list = dev_list; list->next != dev; list = list->next)
                 ;
-            list->next = to_del->next;
-            free(to_del);
+            list->next = dev->next;
         }
+        free_netfront(dev);
     }
 }
 
-static void _shutdown_netfront(struct netfront_dev *dev)
+static int _shutdown_netfront(struct netfront_dev *dev)
 {
     char* err = NULL, *err2;
     XenbusState state;
@@ -692,24 +665,23 @@ close:
     err2 = xenbus_rm(XBT_NIL, nodename);
     free(err2);
 
-    if (!err)
-        free_netfront(dev);
+    return err ? -EBUSY : 0;
 }
 
 void suspend_netfront(void)
 {
-    struct netfront_dev_list *list;
+    struct netfront_dev *dev;
 
-    for (list = dev_list; list != NULL; list = list->next)
-        _shutdown_netfront(list->dev);
+    for (dev = dev_list; dev != NULL; dev = dev->next)
+        _shutdown_netfront(dev);
 }
 
 void resume_netfront(void)
 {
-    struct netfront_dev_list *list;
+    struct netfront_dev *dev;
 
-    for (list = dev_list; list != NULL; list = list->next)
-        _init_netfront(list->dev, NULL, NULL, NULL, NULL);
+    for (dev = dev_list; dev != NULL; dev = dev->next)
+        _init_netfront(dev);
 }
 
 void init_rx_buffers(struct netfront_dev *dev)
--
generated by git-patchbot for /home/xen/git/mini-os.git#master