From 92fbeb43b6c44fb03e024c4c5b9f3d6b8ebd0cf0 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Tue, 17 Dec 2019 13:32:16 +0000 Subject: xen-netback: move netback_probe() and netback_remove() to the end... ...of xenbus.c This is a cosmetic function re-ordering to reduce churn in a subsequent patch. Some style fix-up was done to make checkpatch.pl happier. No functional change. Signed-off-by: Paul Durrant Acked-by: Wei Liu Signed-off-by: David S. Miller --- drivers/net/xen-netback/xenbus.c | 353 +++++++++++++++++++-------------------- 1 file changed, 174 insertions(+), 179 deletions(-) (limited to 'drivers/net/xen-netback/xenbus.c') diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index f533b7372d59..bb61316d79de 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -195,185 +195,6 @@ static void xenvif_debugfs_delif(struct xenvif *vif) } #endif /* CONFIG_DEBUG_FS */ -static int netback_remove(struct xenbus_device *dev) -{ - struct backend_info *be = dev_get_drvdata(&dev->dev); - - set_backend_state(be, XenbusStateClosed); - - unregister_hotplug_status_watch(be); - if (be->vif) { - kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); - xen_unregister_watchers(be->vif); - xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status"); - xenvif_free(be->vif); - be->vif = NULL; - } - kfree(be->hotplug_script); - kfree(be); - dev_set_drvdata(&dev->dev, NULL); - return 0; -} - - -/** - * Entry point to this code when a new device is created. Allocate the basic - * structures and switch to InitWait. - */ -static int netback_probe(struct xenbus_device *dev, - const struct xenbus_device_id *id) -{ - const char *message; - struct xenbus_transaction xbt; - int err; - int sg; - const char *script; - struct backend_info *be = kzalloc(sizeof(struct backend_info), - GFP_KERNEL); - if (!be) { - xenbus_dev_fatal(dev, -ENOMEM, - "allocating backend structure"); - return -ENOMEM; - } - - be->dev = dev; - dev_set_drvdata(&dev->dev, be); - - be->state = XenbusStateInitialising; - err = xenbus_switch_state(dev, XenbusStateInitialising); - if (err) - goto fail; - - sg = 1; - - do { - err = xenbus_transaction_start(&xbt); - if (err) { - xenbus_dev_fatal(dev, err, "starting transaction"); - goto fail; - } - - err = xenbus_printf(xbt, dev->nodename, "feature-sg", "%d", sg); - if (err) { - message = "writing feature-sg"; - goto abort_transaction; - } - - err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv4", - "%d", sg); - if (err) { - message = "writing feature-gso-tcpv4"; - goto abort_transaction; - } - - err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", - "%d", sg); - if (err) { - message = "writing feature-gso-tcpv6"; - goto abort_transaction; - } - - /* We support partial checksum setup for IPv6 packets */ - err = xenbus_printf(xbt, dev->nodename, - "feature-ipv6-csum-offload", - "%d", 1); - if (err) { - message = "writing feature-ipv6-csum-offload"; - goto abort_transaction; - } - - /* We support rx-copy path. */ - err = xenbus_printf(xbt, dev->nodename, - "feature-rx-copy", "%d", 1); - if (err) { - message = "writing feature-rx-copy"; - goto abort_transaction; - } - - /* - * We don't support rx-flip path (except old guests who don't - * grok this feature flag). - */ - err = xenbus_printf(xbt, dev->nodename, - "feature-rx-flip", "%d", 0); - if (err) { - message = "writing feature-rx-flip"; - goto abort_transaction; - } - - /* We support dynamic multicast-control. */ - err = xenbus_printf(xbt, dev->nodename, - "feature-multicast-control", "%d", 1); - if (err) { - message = "writing feature-multicast-control"; - goto abort_transaction; - } - - err = xenbus_printf(xbt, dev->nodename, - "feature-dynamic-multicast-control", - "%d", 1); - if (err) { - message = "writing feature-dynamic-multicast-control"; - goto abort_transaction; - } - - err = xenbus_transaction_end(xbt, 0); - } while (err == -EAGAIN); - - if (err) { - xenbus_dev_fatal(dev, err, "completing transaction"); - goto fail; - } - - /* - * Split event channels support, this is optional so it is not - * put inside the above loop. - */ - err = xenbus_printf(XBT_NIL, dev->nodename, - "feature-split-event-channels", - "%u", separate_tx_rx_irq); - if (err) - pr_debug("Error writing feature-split-event-channels\n"); - - /* Multi-queue support: This is an optional feature. */ - err = xenbus_printf(XBT_NIL, dev->nodename, - "multi-queue-max-queues", "%u", xenvif_max_queues); - if (err) - pr_debug("Error writing multi-queue-max-queues\n"); - - err = xenbus_printf(XBT_NIL, dev->nodename, - "feature-ctrl-ring", - "%u", true); - if (err) - pr_debug("Error writing feature-ctrl-ring\n"); - - script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL); - if (IS_ERR(script)) { - err = PTR_ERR(script); - xenbus_dev_fatal(dev, err, "reading script"); - goto fail; - } - - be->hotplug_script = script; - - - /* This kicks hotplug scripts, so do it immediately. */ - err = backend_create_xenvif(be); - if (err) - goto fail; - - return 0; - -abort_transaction: - xenbus_transaction_end(xbt, 1); - xenbus_dev_fatal(dev, err, "%s", message); -fail: - pr_debug("failed\n"); - netback_remove(dev); - return err; -} - - /* * Handle the creation of the hotplug script environment. We add the script * and vif variables to the environment, for the benefit of the vif-* hotplug @@ -1128,6 +949,180 @@ static int read_xenbus_vif_flags(struct backend_info *be) return 0; } +static int netback_remove(struct xenbus_device *dev) +{ + struct backend_info *be = dev_get_drvdata(&dev->dev); + + set_backend_state(be, XenbusStateClosed); + + unregister_hotplug_status_watch(be); + if (be->vif) { + kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); + xen_unregister_watchers(be->vif); + xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status"); + xenvif_free(be->vif); + be->vif = NULL; + } + kfree(be->hotplug_script); + kfree(be); + dev_set_drvdata(&dev->dev, NULL); + return 0; +} + +/** + * Entry point to this code when a new device is created. Allocate the basic + * structures and switch to InitWait. + */ +static int netback_probe(struct xenbus_device *dev, + const struct xenbus_device_id *id) +{ + const char *message; + struct xenbus_transaction xbt; + int err; + int sg; + const char *script; + struct backend_info *be = kzalloc(sizeof(*be), GFP_KERNEL); + + if (!be) { + xenbus_dev_fatal(dev, -ENOMEM, + "allocating backend structure"); + return -ENOMEM; + } + + be->dev = dev; + dev_set_drvdata(&dev->dev, be); + + be->state = XenbusStateInitialising; + err = xenbus_switch_state(dev, XenbusStateInitialising); + if (err) + goto fail; + + sg = 1; + + do { + err = xenbus_transaction_start(&xbt); + if (err) { + xenbus_dev_fatal(dev, err, "starting transaction"); + goto fail; + } + + err = xenbus_printf(xbt, dev->nodename, "feature-sg", "%d", sg); + if (err) { + message = "writing feature-sg"; + goto abort_transaction; + } + + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv4", + "%d", sg); + if (err) { + message = "writing feature-gso-tcpv4"; + goto abort_transaction; + } + + err = xenbus_printf(xbt, dev->nodename, "feature-gso-tcpv6", + "%d", sg); + if (err) { + message = "writing feature-gso-tcpv6"; + goto abort_transaction; + } + + /* We support partial checksum setup for IPv6 packets */ + err = xenbus_printf(xbt, dev->nodename, + "feature-ipv6-csum-offload", + "%d", 1); + if (err) { + message = "writing feature-ipv6-csum-offload"; + goto abort_transaction; + } + + /* We support rx-copy path. */ + err = xenbus_printf(xbt, dev->nodename, + "feature-rx-copy", "%d", 1); + if (err) { + message = "writing feature-rx-copy"; + goto abort_transaction; + } + + /* We don't support rx-flip path (except old guests who + * don't grok this feature flag). + */ + err = xenbus_printf(xbt, dev->nodename, + "feature-rx-flip", "%d", 0); + if (err) { + message = "writing feature-rx-flip"; + goto abort_transaction; + } + + /* We support dynamic multicast-control. */ + err = xenbus_printf(xbt, dev->nodename, + "feature-multicast-control", "%d", 1); + if (err) { + message = "writing feature-multicast-control"; + goto abort_transaction; + } + + err = xenbus_printf(xbt, dev->nodename, + "feature-dynamic-multicast-control", + "%d", 1); + if (err) { + message = "writing feature-dynamic-multicast-control"; + goto abort_transaction; + } + + err = xenbus_transaction_end(xbt, 0); + } while (err == -EAGAIN); + + if (err) { + xenbus_dev_fatal(dev, err, "completing transaction"); + goto fail; + } + + /* Split event channels support, this is optional so it is not + * put inside the above loop. + */ + err = xenbus_printf(XBT_NIL, dev->nodename, + "feature-split-event-channels", + "%u", separate_tx_rx_irq); + if (err) + pr_debug("Error writing feature-split-event-channels\n"); + + /* Multi-queue support: This is an optional feature. */ + err = xenbus_printf(XBT_NIL, dev->nodename, + "multi-queue-max-queues", "%u", xenvif_max_queues); + if (err) + pr_debug("Error writing multi-queue-max-queues\n"); + + err = xenbus_printf(XBT_NIL, dev->nodename, + "feature-ctrl-ring", + "%u", true); + if (err) + pr_debug("Error writing feature-ctrl-ring\n"); + + script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL); + if (IS_ERR(script)) { + err = PTR_ERR(script); + xenbus_dev_fatal(dev, err, "reading script"); + goto fail; + } + + be->hotplug_script = script; + + /* This kicks hotplug scripts, so do it immediately. */ + err = backend_create_xenvif(be); + if (err) + goto fail; + + return 0; + +abort_transaction: + xenbus_transaction_end(xbt, 1); + xenbus_dev_fatal(dev, err, "%s", message); +fail: + pr_debug("failed\n"); + netback_remove(dev); + return err; +} + static const struct xenbus_device_id netback_ids[] = { { "vif" }, { "" } -- cgit v1.2.3 From f55c3188df84248558f33b13bba4c2817dda78a7 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Tue, 17 Dec 2019 13:32:17 +0000 Subject: xen-netback: switch state to InitWait at the end of netback_probe()... ...as the comment above the function states. The switch to Initialising at the start of the function is somewhat bogus as the toolstack will have set that initial state anyway. To behave correctly, a backend should switch to InitWait once it has set up all xenstore values that may be required by a initialising frontend. This patch calls backend_switch_state() to make the transition at the appropriate point. NOTE: backend_switch_state() ignores errors from xenbus_switch_state() and so this patch removes an error path from netback_probe(). This means a failure to change state at this stage (in the absence of other failures) will leave the device instantiated. This is highly unlikley to happen as a failure to change state would indicate a failure to write to xenstore, and that will trigger other error paths. Also, a 'stuck' device can still be cleaned up using 'unbind' in any case. Signed-off-by: Paul Durrant Acked-by: Wei Liu Signed-off-by: David S. Miller --- drivers/net/xen-netback/xenbus.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/net/xen-netback/xenbus.c') diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index bb61316d79de..682e5e20971b 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -992,11 +992,6 @@ static int netback_probe(struct xenbus_device *dev, be->dev = dev; dev_set_drvdata(&dev->dev, be); - be->state = XenbusStateInitialising; - err = xenbus_switch_state(dev, XenbusStateInitialising); - if (err) - goto fail; - sg = 1; do { @@ -1098,6 +1093,8 @@ static int netback_probe(struct xenbus_device *dev, if (err) pr_debug("Error writing feature-ctrl-ring\n"); + backend_switch_state(be, XenbusStateInitWait); + script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL); if (IS_ERR(script)) { err = PTR_ERR(script); -- cgit v1.2.3 From 1f2565780e9b7218cf92c7630130e82dcc0fe9c2 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Tue, 17 Dec 2019 13:32:18 +0000 Subject: xen-netback: remove 'hotplug-status' once it has served its purpose Removing the 'hotplug-status' node in netback_remove() is wrong; the script may not have completed. Only remove the node once the watch has fired and has been unregistered. Signed-off-by: Paul Durrant Acked-by: Wei Liu Signed-off-by: David S. Miller --- drivers/net/xen-netback/xenbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/xen-netback/xenbus.c') diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index 682e5e20971b..17b4950ec051 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -648,6 +648,7 @@ static void hotplug_status_changed(struct xenbus_watch *watch, /* Not interested in this watch anymore. */ unregister_hotplug_status_watch(be); + xenbus_rm(XBT_NIL, be->dev->nodename, "hotplug-status"); } kfree(str); } @@ -959,7 +960,6 @@ static int netback_remove(struct xenbus_device *dev) if (be->vif) { kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); xen_unregister_watchers(be->vif); - xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status"); xenvif_free(be->vif); be->vif = NULL; } -- cgit v1.2.3 From 9476654bd5e8ad42abe8ee9f9e90069ff8e60c17 Mon Sep 17 00:00:00 2001 From: Paul Durrant Date: Mon, 23 Dec 2019 09:59:23 +0000 Subject: xen-netback: support dynamic unbind/bind By re-attaching RX, TX, and CTL rings during connect() rather than assuming they are freshly allocated (i.e. assuming the counters are zero), and avoiding forcing state to Closed in netback_remove() it is possible for vif instances to be unbound and re-bound from and to (respectively) a running guest. Dynamic unbind/bind is a highly useful feature for a backend module as it allows it to be unloaded and re-loaded (i.e. updated) without requiring domUs to be halted. This has been tested by running iperf as a server in the test VM and then running a client against it in a continuous loop, whilst also running: while true; do echo vif-$DOMID-$VIF >unbind; echo down; rmmod xen-netback; echo unloaded; modprobe xen-netback; cd $(pwd); brctl addif xenbr0 vif$DOMID.$VIF; ip link set vif$DOMID.$VIF up; echo up; sleep 5; done in dom0 from /sys/bus/xen-backend/drivers/vif to continuously unbind, unload, re-load, re-bind and re-plumb the backend. Clearly a performance drop was seen but no TCP connection resets were observed during this test and moreover a parallel SSH connection into the guest remained perfectly usable throughout. Signed-off-by: Paul Durrant Reviewed-by: Wei Liu Signed-off-by: David S. Miller --- drivers/net/xen-netback/interface.c | 10 +++++++++- drivers/net/xen-netback/netback.c | 20 +++++++++++++++++--- drivers/net/xen-netback/xenbus.c | 5 ++--- 3 files changed, 28 insertions(+), 7 deletions(-) (limited to 'drivers/net/xen-netback/xenbus.c') diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index f15ba3de6195..0c8a02a1ead7 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -585,6 +585,7 @@ int xenvif_connect_ctrl(struct xenvif *vif, grant_ref_t ring_ref, struct net_device *dev = vif->dev; void *addr; struct xen_netif_ctrl_sring *shared; + RING_IDX rsp_prod, req_prod; int err; err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(vif), @@ -593,7 +594,14 @@ int xenvif_connect_ctrl(struct xenvif *vif, grant_ref_t ring_ref, goto err; shared = (struct xen_netif_ctrl_sring *)addr; - BACK_RING_INIT(&vif->ctrl, shared, XEN_PAGE_SIZE); + rsp_prod = READ_ONCE(shared->rsp_prod); + req_prod = READ_ONCE(shared->req_prod); + + BACK_RING_ATTACH(&vif->ctrl, shared, rsp_prod, XEN_PAGE_SIZE); + + err = -EIO; + if (req_prod - rsp_prod > RING_SIZE(&vif->ctrl)) + goto err_unmap; err = bind_interdomain_evtchn_to_irq(vif->domid, evtchn); if (err < 0) diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c index 0020b2e8c279..315dfc6ea297 100644 --- a/drivers/net/xen-netback/netback.c +++ b/drivers/net/xen-netback/netback.c @@ -1453,7 +1453,7 @@ int xenvif_map_frontend_data_rings(struct xenvif_queue *queue, void *addr; struct xen_netif_tx_sring *txs; struct xen_netif_rx_sring *rxs; - + RING_IDX rsp_prod, req_prod; int err = -ENOMEM; err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(queue->vif), @@ -1462,7 +1462,14 @@ int xenvif_map_frontend_data_rings(struct xenvif_queue *queue, goto err; txs = (struct xen_netif_tx_sring *)addr; - BACK_RING_INIT(&queue->tx, txs, XEN_PAGE_SIZE); + rsp_prod = READ_ONCE(txs->rsp_prod); + req_prod = READ_ONCE(txs->req_prod); + + BACK_RING_ATTACH(&queue->tx, txs, rsp_prod, XEN_PAGE_SIZE); + + err = -EIO; + if (req_prod - rsp_prod > RING_SIZE(&queue->tx)) + goto err; err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(queue->vif), &rx_ring_ref, 1, &addr); @@ -1470,7 +1477,14 @@ int xenvif_map_frontend_data_rings(struct xenvif_queue *queue, goto err; rxs = (struct xen_netif_rx_sring *)addr; - BACK_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE); + rsp_prod = READ_ONCE(rxs->rsp_prod); + req_prod = READ_ONCE(rxs->req_prod); + + BACK_RING_ATTACH(&queue->rx, rxs, rsp_prod, XEN_PAGE_SIZE); + + err = -EIO; + if (req_prod - rsp_prod > RING_SIZE(&queue->rx)) + goto err; return 0; diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index 17b4950ec051..286054b60d47 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -954,12 +954,10 @@ static int netback_remove(struct xenbus_device *dev) { struct backend_info *be = dev_get_drvdata(&dev->dev); - set_backend_state(be, XenbusStateClosed); - unregister_hotplug_status_watch(be); if (be->vif) { kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE); - xen_unregister_watchers(be->vif); + backend_disconnect(be); xenvif_free(be->vif); be->vif = NULL; } @@ -1131,6 +1129,7 @@ static struct xenbus_driver netback_driver = { .remove = netback_remove, .uevent = netback_uevent, .otherend_changed = frontend_changed, + .allow_rebind = true, }; int xenvif_xenbus_init(void) -- cgit v1.2.3