* [Xen-devel] [PATCH net-next 0/3] xen-netback: clean-up @ 2019-12-17 13:32 ` Paul Durrant 0 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel; +Cc: Paul Durrant Paul Durrant (3): xen-netback: move netback_probe() and netback_remove() to the end... xen-netback: switch state to InitWait at the end of netback_probe()... xen-netback: remove 'hotplug-status' once it has served its purpose drivers/net/xen-netback/xenbus.c | 350 +++++++++++++++---------------- 1 file changed, 171 insertions(+), 179 deletions(-) -- 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH net-next 0/3] xen-netback: clean-up @ 2019-12-17 13:32 ` Paul Durrant 0 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel; +Cc: Paul Durrant Paul Durrant (3): xen-netback: move netback_probe() and netback_remove() to the end... xen-netback: switch state to InitWait at the end of netback_probe()... xen-netback: remove 'hotplug-status' once it has served its purpose drivers/net/xen-netback/xenbus.c | 350 +++++++++++++++---------------- 1 file changed, 171 insertions(+), 179 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Xen-devel] [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 13:32 ` Paul Durrant -1 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, David S. Miller, Wei Liu, Paul Durrant ...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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 353 +++++++++++++++---------------- 1 file changed, 174 insertions(+), 179 deletions(-) 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" }, { "" } -- 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... @ 2019-12-17 13:32 ` Paul Durrant 0 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller ...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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 353 +++++++++++++++---------------- 1 file changed, 174 insertions(+), 179 deletions(-) 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" }, { "" } -- 2.20.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 15:23 ` Wei Liu -1 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: Wei Liu, Paul Durrant, netdev, linux-kernel, xen-devel, David S. Miller On Tue, Dec 17, 2019 at 01:32:16PM +0000, Paul Durrant wrote: > ...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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... @ 2019-12-17 15:23 ` Wei Liu 0 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller On Tue, Dec 17, 2019 at 01:32:16PM +0000, Paul Durrant wrote: > ...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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-18 7:03 ` David Miller -1 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:03 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, wei.liu, paul, linux-kernel, netdev From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:16 +0000 > ...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 <pdurrant@amazon.com> Applied. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end... @ 2019-12-18 7:03 ` David Miller 0 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:03 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:16 +0000 > ...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 <pdurrant@amazon.com> Applied. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Xen-devel] [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 13:32 ` Paul Durrant -1 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, David S. Miller, Wei Liu, Paul Durrant ...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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) 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); -- 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... @ 2019-12-17 13:32 ` Paul Durrant 0 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller ...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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) 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); -- 2.20.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 15:23 ` Wei Liu -1 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: Wei Liu, Paul Durrant, netdev, linux-kernel, xen-devel, David S. Miller On Tue, Dec 17, 2019 at 01:32:17PM +0000, Paul Durrant wrote: > ...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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... @ 2019-12-17 15:23 ` Wei Liu 0 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller On Tue, Dec 17, 2019 at 01:32:17PM +0000, Paul Durrant wrote: > ...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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... 2019-12-17 13:32 ` Paul Durrant @ 2019-12-18 7:03 ` David Miller -1 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:03 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, wei.liu, paul, linux-kernel, netdev From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:17 +0000 > ...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 <pdurrant@amazon.com> Applied. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe()... @ 2019-12-18 7:03 ` David Miller 0 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:03 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:17 +0000 > ...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 <pdurrant@amazon.com> Applied. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Xen-devel] [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 13:32 ` Paul Durrant -1 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, David S. Miller, Wei Liu, Paul Durrant 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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; } -- 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose @ 2019-12-17 13:32 ` Paul Durrant 0 siblings, 0 replies; 20+ messages in thread From: Paul Durrant @ 2019-12-17 13:32 UTC (permalink / raw) To: xen-devel, netdev, linux-kernel Cc: Paul Durrant, Wei Liu, Paul Durrant, David S. Miller 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 <pdurrant@amazon.com> --- Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> --- drivers/net/xen-netback/xenbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose 2019-12-17 13:32 ` Paul Durrant @ 2019-12-17 15:23 ` Wei Liu -1 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: Wei Liu, Paul Durrant, netdev, linux-kernel, xen-devel, David S. Miller On Tue, Dec 17, 2019 at 01:32:18PM +0000, Paul Durrant wrote: > 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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose @ 2019-12-17 15:23 ` Wei Liu 0 siblings, 0 replies; 20+ messages in thread From: Wei Liu @ 2019-12-17 15:23 UTC (permalink / raw) To: Paul Durrant Cc: xen-devel, netdev, linux-kernel, Wei Liu, Paul Durrant, David S. Miller On Tue, Dec 17, 2019 at 01:32:18PM +0000, Paul Durrant wrote: > 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 <pdurrant@amazon.com> Acked-by: Wei Liu <wei.liu@kernel.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Xen-devel] [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose 2019-12-17 13:32 ` Paul Durrant @ 2019-12-18 7:04 ` David Miller -1 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:04 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, wei.liu, paul, linux-kernel, netdev From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:18 +0000 > 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 <pdurrant@amazon.com> Applied. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose @ 2019-12-18 7:04 ` David Miller 0 siblings, 0 replies; 20+ messages in thread From: David Miller @ 2019-12-18 7:04 UTC (permalink / raw) To: pdurrant; +Cc: xen-devel, netdev, linux-kernel, wei.liu, paul From: Paul Durrant <pdurrant@amazon.com> Date: Tue, 17 Dec 2019 13:32:18 +0000 > 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 <pdurrant@amazon.com> Applied. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-12-18 7:04 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-12-17 13:32 [Xen-devel] [PATCH net-next 0/3] xen-netback: clean-up Paul Durrant 2019-12-17 13:32 ` Paul Durrant 2019-12-17 13:32 ` [Xen-devel] [PATCH net-next 1/3] xen-netback: move netback_probe() and netback_remove() to the end Paul Durrant 2019-12-17 13:32 ` Paul Durrant 2019-12-17 15:23 ` [Xen-devel] " Wei Liu 2019-12-17 15:23 ` Wei Liu 2019-12-18 7:03 ` [Xen-devel] " David Miller 2019-12-18 7:03 ` David Miller 2019-12-17 13:32 ` [Xen-devel] [PATCH net-next 2/3] xen-netback: switch state to InitWait at the end of netback_probe() Paul Durrant 2019-12-17 13:32 ` Paul Durrant 2019-12-17 15:23 ` [Xen-devel] " Wei Liu 2019-12-17 15:23 ` Wei Liu 2019-12-18 7:03 ` [Xen-devel] " David Miller 2019-12-18 7:03 ` David Miller 2019-12-17 13:32 ` [Xen-devel] [PATCH net-next 3/3] xen-netback: remove 'hotplug-status' once it has served its purpose Paul Durrant 2019-12-17 13:32 ` Paul Durrant 2019-12-17 15:23 ` [Xen-devel] " Wei Liu 2019-12-17 15:23 ` Wei Liu 2019-12-18 7:04 ` [Xen-devel] " David Miller 2019-12-18 7:04 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.