From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 42A3FC2D0E2 for ; Tue, 22 Sep 2020 10:58:57 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CBECF20BED for ; Tue, 22 Sep 2020 10:58:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=suse.com header.i=@suse.com header.b="dKVaG9Cn" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CBECF20BED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=suse.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kKg0V-0002j1-GQ; Tue, 22 Sep 2020 10:58:47 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1kKg0U-0002eH-HD for xen-devel@lists.xenproject.org; Tue, 22 Sep 2020 10:58:46 +0000 X-Inumbo-ID: 76342151-97be-4589-ba7d-37626c0cc51e Received: from mx2.suse.de (unknown [195.135.220.15]) by us1-rack-iad1.inumbo.com (Halon) with ESMTPS id 76342151-97be-4589-ba7d-37626c0cc51e; Tue, 22 Sep 2020 10:58:29 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1600772309; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=YmJEzG7WE6ySO04WKdM+x0q6b5S1uIVl7WRmYPejCdQ=; b=dKVaG9CnFcHwH5IIYZ4o+wPNPBlTRMa9xHVMaOqg/5YJMT9GBrjcbQjBgdwgOBEujNikM1 RB7G/jp9PAp7vTBkuo/Y5yxgLEOM+IiKJBnaeuvCVQNqLFZaAsR1xhqFpAEFcAjgO8Bp8F 31Xz+Nzqd0t/qisYDRJSl584T7qLtMY= Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id A82E4B22E; Tue, 22 Sep 2020 10:59:05 +0000 (UTC) From: Juergen Gross To: minios-devel@lists.xenproject.org, xen-devel@lists.xenproject.org Cc: samuel.thibault@ens-lyon.org, wl@xen.org, Juergen Gross Subject: [PATCH 2/2] mini-os: netfront: fix suspend/resume handling Date: Tue, 22 Sep 2020 12:58:26 +0200 Message-Id: <20200922105826.26274-3-jgross@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200922105826.26274-1-jgross@suse.com> References: <20200922105826.26274-1-jgross@suse.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" 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 --- 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) -- 2.26.2