[PATCH net-next] xen-netback: get rid of old udev related code

[PATCH net-next] xen-netback: get rid of old udev related code

[Paul Durrant]

In the past it used to be the case that the Xen toolstack relied upon
udev to execute backend hotplug scripts. However this has not been the
case for many releases now and removal of the associated code in
xen-netback shortens the source by more than 100 lines, and removes much
complexity in the interaction with the xenstore backend state.

NOTE: xen-netback is the only xenbus driver to have a functional uevent()
      method. The only other driver to have a method at all is
      pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
      Hence this patch also facilitates further cleanup.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wei.liu@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netback/common.h |  11 ---
 drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
 2 files changed, 14 insertions(+), 122 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb91a1b..e48da004c1a3 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,17 +251,6 @@ struct xenvif_hash {
 struct backend_info {
 	struct xenbus_device *dev;
 	struct xenvif *vif;
-
-	/* This is the state that will be reflected in xenstore when any
-	 * active hotplug script completes.
-	 */
-	enum xenbus_state state;
-
-	enum xenbus_state frontend_state;
-	struct xenbus_watch hotplug_status_watch;
-	u8 have_hotplug_status_watch:1;
-
-	const char *hotplug_script;
 };
 
 struct xenvif {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f533b7372d59..4e89393d5dd8 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -15,7 +15,6 @@ static int connect_data_rings(struct backend_info *be,
 static void connect(struct backend_info *be);
 static int read_xenbus_vif_flags(struct backend_info *be);
 static int backend_create_xenvif(struct backend_info *be);
-static void unregister_hotplug_status_watch(struct backend_info *be);
 static void xen_unregister_watchers(struct xenvif *vif);
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state);
@@ -199,17 +198,11 @@ 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;
@@ -227,7 +220,6 @@ static int netback_probe(struct xenbus_device *dev,
 	struct xenbus_transaction xbt;
 	int err;
 	int sg;
-	const char *script;
 	struct backend_info *be = kzalloc(sizeof(struct backend_info),
 					  GFP_KERNEL);
 	if (!be) {
@@ -239,7 +231,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;
@@ -347,21 +338,12 @@ static int netback_probe(struct xenbus_device *dev,
 	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;
 
+	set_backend_state(be, XenbusStateInitWait);
+
 	return 0;
 
 abort_transaction:
@@ -374,29 +356,6 @@ static int netback_probe(struct xenbus_device *dev,
 }
 
 
-/*
- * 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
- * scripts.
- */
-static int netback_uevent(struct xenbus_device *xdev,
-			  struct kobj_uevent_env *env)
-{
-	struct backend_info *be = dev_get_drvdata(&xdev->dev);
-
-	if (!be)
-		return 0;
-
-	if (add_uevent_var(env, "script=%s", be->hotplug_script))
-		return -ENOMEM;
-
-	if (!be->vif)
-		return 0;
-
-	return add_uevent_var(env, "vif=%s", be->vif->dev->name);
-}
-
-
 static int backend_create_xenvif(struct backend_info *be)
 {
 	int err;
@@ -422,7 +381,6 @@ static int backend_create_xenvif(struct backend_info *be)
 	be->vif = vif;
 	vif->be = be;
 
-	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 	return 0;
 }
 
@@ -462,21 +420,6 @@ static void backend_connect(struct backend_info *be)
 		connect(be);
 }
 
-static inline void backend_switch_state(struct backend_info *be,
-					enum xenbus_state state)
-{
-	struct xenbus_device *dev = be->dev;
-
-	pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
-	be->state = state;
-
-	/* If we are waiting for a hotplug script then defer the
-	 * actual xenbus state change.
-	 */
-	if (!be->have_hotplug_status_watch)
-		xenbus_switch_state(dev, state);
-}
-
 /* Handle backend state transitions:
  *
  * The backend state starts in Initialising and the following transitions are
@@ -500,17 +443,19 @@ static inline void backend_switch_state(struct backend_info *be,
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state)
 {
-	while (be->state != state) {
-		switch (be->state) {
+	struct xenbus_device *dev = be->dev;
+
+	while (dev->state != state) {
+		switch (dev->state) {
 		case XenbusStateInitialising:
 			switch (state) {
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
 			case XenbusStateClosing:
-				backend_switch_state(be, XenbusStateInitWait);
+				xenbus_switch_state(dev, XenbusStateInitWait);
 				break;
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosed);
+				xenbus_switch_state(dev, XenbusStateClosed);
 				break;
 			default:
 				BUG();
@@ -520,10 +465,10 @@ static void set_backend_state(struct backend_info *be,
 			switch (state) {
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
-				backend_switch_state(be, XenbusStateInitWait);
+				xenbus_switch_state(dev, XenbusStateInitWait);
 				break;
 			case XenbusStateClosing:
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -533,11 +478,11 @@ static void set_backend_state(struct backend_info *be,
 			switch (state) {
 			case XenbusStateConnected:
 				backend_connect(be);
-				backend_switch_state(be, XenbusStateConnected);
+				xenbus_switch_state(dev, XenbusStateConnected);
 				break;
 			case XenbusStateClosing:
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -549,7 +494,7 @@ static void set_backend_state(struct backend_info *be,
 			case XenbusStateClosing:
 			case XenbusStateClosed:
 				backend_disconnect(be);
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -560,7 +505,7 @@ static void set_backend_state(struct backend_info *be,
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosed);
+				xenbus_switch_state(dev, XenbusStateClosed);
 				break;
 			default:
 				BUG();
@@ -582,8 +527,6 @@ static void frontend_changed(struct xenbus_device *dev,
 
 	pr_debug("%s -> %s\n", dev->otherend, xenbus_strstate(frontend_state));
 
-	be->frontend_state = frontend_state;
-
 	switch (frontend_state) {
 	case XenbusStateInitialising:
 		set_backend_state(be, XenbusStateInitWait);
@@ -799,38 +742,6 @@ static void xen_unregister_watchers(struct xenvif *vif)
 	xen_unregister_credit_watch(vif);
 }
 
-static void unregister_hotplug_status_watch(struct backend_info *be)
-{
-	if (be->have_hotplug_status_watch) {
-		unregister_xenbus_watch(&be->hotplug_status_watch);
-		kfree(be->hotplug_status_watch.node);
-	}
-	be->have_hotplug_status_watch = 0;
-}
-
-static void hotplug_status_changed(struct xenbus_watch *watch,
-				   const char *path,
-				   const char *token)
-{
-	struct backend_info *be = container_of(watch,
-					       struct backend_info,
-					       hotplug_status_watch);
-	char *str;
-	unsigned int len;
-
-	str = xenbus_read(XBT_NIL, be->dev->nodename, "hotplug-status", &len);
-	if (IS_ERR(str))
-		return;
-	if (len == sizeof("connected")-1 && !memcmp(str, "connected", len)) {
-		/* Complete any pending state change */
-		xenbus_switch_state(be->dev, be->state);
-
-		/* Not interested in this watch anymore. */
-		unregister_hotplug_status_watch(be);
-	}
-	kfree(str);
-}
-
 static int connect_ctrl_ring(struct backend_info *be)
 {
 	struct xenbus_device *dev = be->dev;
@@ -974,13 +885,6 @@ static void connect(struct backend_info *be)
 
 	xenvif_carrier_on(be->vif);
 
-	unregister_hotplug_status_watch(be);
-	err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch,
-				   hotplug_status_changed,
-				   "%s/%s", dev->nodename, "hotplug-status");
-	if (!err)
-		be->have_hotplug_status_watch = 1;
-
 	netif_tx_wake_all_queues(be->vif->dev);
 
 	return;
@@ -1137,7 +1041,6 @@ static struct xenbus_driver netback_driver = {
 	.ids = netback_ids,
 	.probe = netback_probe,
 	.remove = netback_remove,
-	.uevent = netback_uevent,
 	.otherend_changed = frontend_changed,
 };
 
-- 
2.20.1

[Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Paul Durrant]

In the past it used to be the case that the Xen toolstack relied upon
udev to execute backend hotplug scripts. However this has not been the
case for many releases now and removal of the associated code in
xen-netback shortens the source by more than 100 lines, and removes much
complexity in the interaction with the xenstore backend state.

NOTE: xen-netback is the only xenbus driver to have a functional uevent()
      method. The only other driver to have a method at all is
      pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
      Hence this patch also facilitates further cleanup.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Wei Liu <wei.liu@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netback/common.h |  11 ---
 drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
 2 files changed, 14 insertions(+), 122 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb91a1b..e48da004c1a3 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,17 +251,6 @@ struct xenvif_hash {
 struct backend_info {
 	struct xenbus_device *dev;
 	struct xenvif *vif;
-
-	/* This is the state that will be reflected in xenstore when any
-	 * active hotplug script completes.
-	 */
-	enum xenbus_state state;
-
-	enum xenbus_state frontend_state;
-	struct xenbus_watch hotplug_status_watch;
-	u8 have_hotplug_status_watch:1;
-
-	const char *hotplug_script;
 };
 
 struct xenvif {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f533b7372d59..4e89393d5dd8 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -15,7 +15,6 @@ static int connect_data_rings(struct backend_info *be,
 static void connect(struct backend_info *be);
 static int read_xenbus_vif_flags(struct backend_info *be);
 static int backend_create_xenvif(struct backend_info *be);
-static void unregister_hotplug_status_watch(struct backend_info *be);
 static void xen_unregister_watchers(struct xenvif *vif);
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state);
@@ -199,17 +198,11 @@ 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;
@@ -227,7 +220,6 @@ static int netback_probe(struct xenbus_device *dev,
 	struct xenbus_transaction xbt;
 	int err;
 	int sg;
-	const char *script;
 	struct backend_info *be = kzalloc(sizeof(struct backend_info),
 					  GFP_KERNEL);
 	if (!be) {
@@ -239,7 +231,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;
@@ -347,21 +338,12 @@ static int netback_probe(struct xenbus_device *dev,
 	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;
 
+	set_backend_state(be, XenbusStateInitWait);
+
 	return 0;
 
 abort_transaction:
@@ -374,29 +356,6 @@ static int netback_probe(struct xenbus_device *dev,
 }
 
 
-/*
- * 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
- * scripts.
- */
-static int netback_uevent(struct xenbus_device *xdev,
-			  struct kobj_uevent_env *env)
-{
-	struct backend_info *be = dev_get_drvdata(&xdev->dev);
-
-	if (!be)
-		return 0;
-
-	if (add_uevent_var(env, "script=%s", be->hotplug_script))
-		return -ENOMEM;
-
-	if (!be->vif)
-		return 0;
-
-	return add_uevent_var(env, "vif=%s", be->vif->dev->name);
-}
-
-
 static int backend_create_xenvif(struct backend_info *be)
 {
 	int err;
@@ -422,7 +381,6 @@ static int backend_create_xenvif(struct backend_info *be)
 	be->vif = vif;
 	vif->be = be;
 
-	kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
 	return 0;
 }
 
@@ -462,21 +420,6 @@ static void backend_connect(struct backend_info *be)
 		connect(be);
 }
 
-static inline void backend_switch_state(struct backend_info *be,
-					enum xenbus_state state)
-{
-	struct xenbus_device *dev = be->dev;
-
-	pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
-	be->state = state;
-
-	/* If we are waiting for a hotplug script then defer the
-	 * actual xenbus state change.
-	 */
-	if (!be->have_hotplug_status_watch)
-		xenbus_switch_state(dev, state);
-}
-
 /* Handle backend state transitions:
  *
  * The backend state starts in Initialising and the following transitions are
@@ -500,17 +443,19 @@ static inline void backend_switch_state(struct backend_info *be,
 static void set_backend_state(struct backend_info *be,
 			      enum xenbus_state state)
 {
-	while (be->state != state) {
-		switch (be->state) {
+	struct xenbus_device *dev = be->dev;
+
+	while (dev->state != state) {
+		switch (dev->state) {
 		case XenbusStateInitialising:
 			switch (state) {
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
 			case XenbusStateClosing:
-				backend_switch_state(be, XenbusStateInitWait);
+				xenbus_switch_state(dev, XenbusStateInitWait);
 				break;
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosed);
+				xenbus_switch_state(dev, XenbusStateClosed);
 				break;
 			default:
 				BUG();
@@ -520,10 +465,10 @@ static void set_backend_state(struct backend_info *be,
 			switch (state) {
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
-				backend_switch_state(be, XenbusStateInitWait);
+				xenbus_switch_state(dev, XenbusStateInitWait);
 				break;
 			case XenbusStateClosing:
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -533,11 +478,11 @@ static void set_backend_state(struct backend_info *be,
 			switch (state) {
 			case XenbusStateConnected:
 				backend_connect(be);
-				backend_switch_state(be, XenbusStateConnected);
+				xenbus_switch_state(dev, XenbusStateConnected);
 				break;
 			case XenbusStateClosing:
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -549,7 +494,7 @@ static void set_backend_state(struct backend_info *be,
 			case XenbusStateClosing:
 			case XenbusStateClosed:
 				backend_disconnect(be);
-				backend_switch_state(be, XenbusStateClosing);
+				xenbus_switch_state(dev, XenbusStateClosing);
 				break;
 			default:
 				BUG();
@@ -560,7 +505,7 @@ static void set_backend_state(struct backend_info *be,
 			case XenbusStateInitWait:
 			case XenbusStateConnected:
 			case XenbusStateClosed:
-				backend_switch_state(be, XenbusStateClosed);
+				xenbus_switch_state(dev, XenbusStateClosed);
 				break;
 			default:
 				BUG();
@@ -582,8 +527,6 @@ static void frontend_changed(struct xenbus_device *dev,
 
 	pr_debug("%s -> %s\n", dev->otherend, xenbus_strstate(frontend_state));
 
-	be->frontend_state = frontend_state;
-
 	switch (frontend_state) {
 	case XenbusStateInitialising:
 		set_backend_state(be, XenbusStateInitWait);
@@ -799,38 +742,6 @@ static void xen_unregister_watchers(struct xenvif *vif)
 	xen_unregister_credit_watch(vif);
 }
 
-static void unregister_hotplug_status_watch(struct backend_info *be)
-{
-	if (be->have_hotplug_status_watch) {
-		unregister_xenbus_watch(&be->hotplug_status_watch);
-		kfree(be->hotplug_status_watch.node);
-	}
-	be->have_hotplug_status_watch = 0;
-}
-
-static void hotplug_status_changed(struct xenbus_watch *watch,
-				   const char *path,
-				   const char *token)
-{
-	struct backend_info *be = container_of(watch,
-					       struct backend_info,
-					       hotplug_status_watch);
-	char *str;
-	unsigned int len;
-
-	str = xenbus_read(XBT_NIL, be->dev->nodename, "hotplug-status", &len);
-	if (IS_ERR(str))
-		return;
-	if (len == sizeof("connected")-1 && !memcmp(str, "connected", len)) {
-		/* Complete any pending state change */
-		xenbus_switch_state(be->dev, be->state);
-
-		/* Not interested in this watch anymore. */
-		unregister_hotplug_status_watch(be);
-	}
-	kfree(str);
-}
-
 static int connect_ctrl_ring(struct backend_info *be)
 {
 	struct xenbus_device *dev = be->dev;
@@ -974,13 +885,6 @@ static void connect(struct backend_info *be)
 
 	xenvif_carrier_on(be->vif);
 
-	unregister_hotplug_status_watch(be);
-	err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch,
-				   hotplug_status_changed,
-				   "%s/%s", dev->nodename, "hotplug-status");
-	if (!err)
-		be->have_hotplug_status_watch = 1;
-
 	netif_tx_wake_all_queues(be->vif->dev);
 
 	return;
@@ -1137,7 +1041,6 @@ static struct xenbus_driver netback_driver = {
 	.ids = netback_ids,
 	.probe = netback_probe,
 	.remove = netback_remove,
-	.uevent = netback_uevent,
 	.otherend_changed = frontend_changed,
 };
 
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jason Andryuk]

On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <pdurrant@amazon.com> wrote:
>
> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
>
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>       method. The only other driver to have a method at all is
>       pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>       Hence this patch also facilitates further cleanup.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> ---
> Cc: Wei Liu <wei.liu@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  drivers/net/xen-netback/common.h |  11 ---
>  drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
>  2 files changed, 14 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 05847eb91a1b..e48da004c1a3 100644

<snip>

> -static inline void backend_switch_state(struct backend_info *be,
> -                                       enum xenbus_state state)
> -{
> -       struct xenbus_device *dev = be->dev;
> -
> -       pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> -       be->state = state;
> -
> -       /* If we are waiting for a hotplug script then defer the
> -        * actual xenbus state change.
> -        */
> -       if (!be->have_hotplug_status_watch)
> -               xenbus_switch_state(dev, state);

have_hotplug_status_watch prevents xen-netback from switching to
connected state unless the the backend scripts have written
"hotplug-status" "success".  I had always thought that was intentional
so the frontend doesn't connect when the backend is unconnected.  i.e.
if the backend scripts fails, it writes "hotplug-status" "error" and
the frontend doesn't connect.

That behavior is independent of using udev to run the scripts.  I'm
not opposed to removing it, but I think it at least warrants
mentioning in the commit message.

Regards,
Jason

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jason Andryuk]

On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <pdurrant@amazon.com> wrote:
>
> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
>
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>       method. The only other driver to have a method at all is
>       pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>       Hence this patch also facilitates further cleanup.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> ---
> Cc: Wei Liu <wei.liu@kernel.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  drivers/net/xen-netback/common.h |  11 ---
>  drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
>  2 files changed, 14 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 05847eb91a1b..e48da004c1a3 100644

<snip>

> -static inline void backend_switch_state(struct backend_info *be,
> -                                       enum xenbus_state state)
> -{
> -       struct xenbus_device *dev = be->dev;
> -
> -       pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> -       be->state = state;
> -
> -       /* If we are waiting for a hotplug script then defer the
> -        * actual xenbus state change.
> -        */
> -       if (!be->have_hotplug_status_watch)
> -               xenbus_switch_state(dev, state);

have_hotplug_status_watch prevents xen-netback from switching to
connected state unless the the backend scripts have written
"hotplug-status" "success".  I had always thought that was intentional
so the frontend doesn't connect when the backend is unconnected.  i.e.
if the backend scripts fails, it writes "hotplug-status" "error" and
the frontend doesn't connect.

That behavior is independent of using udev to run the scripts.  I'm
not opposed to removing it, but I think it at least warrants
mentioning in the commit message.

Regards,
Jason

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: jandryuk@gmail.com <jandryuk@gmail.com>
> Sent: 12 December 2019 16:32
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>; netdev@vger.kernel.org;
> open list <linux-kernel@vger.kernel.org>; Wei Liu <wei.liu@kernel.org>;
> David S. Miller <davem@davemloft.net>
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <pdurrant@amazon.com> wrote:
> >
> > In the past it used to be the case that the Xen toolstack relied upon
> > udev to execute backend hotplug scripts. However this has not been the
> > case for many releases now and removal of the associated code in
> > xen-netback shortens the source by more than 100 lines, and removes much
> > complexity in the interaction with the xenstore backend state.
> >
> > NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >       method. The only other driver to have a method at all is
> >       pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >       Hence this patch also facilitates further cleanup.
> >
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> > ---
> > Cc: Wei Liu <wei.liu@kernel.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > ---
> >  drivers/net/xen-netback/common.h |  11 ---
> >  drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
> >  2 files changed, 14 insertions(+), 122 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index 05847eb91a1b..e48da004c1a3 100644
> 
> <snip>
> 
> > -static inline void backend_switch_state(struct backend_info *be,
> > -                                       enum xenbus_state state)
> > -{
> > -       struct xenbus_device *dev = be->dev;
> > -
> > -       pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > -       be->state = state;
> > -
> > -       /* If we are waiting for a hotplug script then defer the
> > -        * actual xenbus state change.
> > -        */
> > -       if (!be->have_hotplug_status_watch)
> > -               xenbus_switch_state(dev, state);
> 
> have_hotplug_status_watch prevents xen-netback from switching to
> connected state unless the the backend scripts have written
> "hotplug-status" "success".  I had always thought that was intentional
> so the frontend doesn't connect when the backend is unconnected.  i.e.
> if the backend scripts fails, it writes "hotplug-status" "error" and
> the frontend doesn't connect.
> 
> That behavior is independent of using udev to run the scripts.  I'm
> not opposed to removing it, but I think it at least warrants
> mentioning in the commit message.

True, but it's probably related. The netback probe would previously kick udev, the hotplug script would then run, and then the state would go connected. I think, because the hotplug is invoked directly by the toolstack now, these things really ought not to be tied together. TBH I can't see any harm in the frontend seeing the network connection before the backend plumbing is done... If the frontend should have any sort of indication of whether the backend is plumbed or not then IMO it ought to be as a virtual carrier/link status, because unplumbing and re-plumbing could be done at any time really without any need for the shared ring to go away (and in fact I will be following up at some point with a patch to allow unbind and re-bind of netback).

I'll elaborate in the commit message as you suggest :-)

Cheers,

  Paul

> 
> Regards,
> Jason

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: jandryuk@gmail.com <jandryuk@gmail.com>
> Sent: 12 December 2019 16:32
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: xen-devel <xen-devel@lists.xenproject.org>; netdev@vger.kernel.org;
> open list <linux-kernel@vger.kernel.org>; Wei Liu <wei.liu@kernel.org>;
> David S. Miller <davem@davemloft.net>
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant <pdurrant@amazon.com> wrote:
> >
> > In the past it used to be the case that the Xen toolstack relied upon
> > udev to execute backend hotplug scripts. However this has not been the
> > case for many releases now and removal of the associated code in
> > xen-netback shortens the source by more than 100 lines, and removes much
> > complexity in the interaction with the xenstore backend state.
> >
> > NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >       method. The only other driver to have a method at all is
> >       pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >       Hence this patch also facilitates further cleanup.
> >
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> > ---
> > Cc: Wei Liu <wei.liu@kernel.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > ---
> >  drivers/net/xen-netback/common.h |  11 ---
> >  drivers/net/xen-netback/xenbus.c | 125 ++++---------------------------
> >  2 files changed, 14 insertions(+), 122 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index 05847eb91a1b..e48da004c1a3 100644
> 
> <snip>
> 
> > -static inline void backend_switch_state(struct backend_info *be,
> > -                                       enum xenbus_state state)
> > -{
> > -       struct xenbus_device *dev = be->dev;
> > -
> > -       pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > -       be->state = state;
> > -
> > -       /* If we are waiting for a hotplug script then defer the
> > -        * actual xenbus state change.
> > -        */
> > -       if (!be->have_hotplug_status_watch)
> > -               xenbus_switch_state(dev, state);
> 
> have_hotplug_status_watch prevents xen-netback from switching to
> connected state unless the the backend scripts have written
> "hotplug-status" "success".  I had always thought that was intentional
> so the frontend doesn't connect when the backend is unconnected.  i.e.
> if the backend scripts fails, it writes "hotplug-status" "error" and
> the frontend doesn't connect.
> 
> That behavior is independent of using udev to run the scripts.  I'm
> not opposed to removing it, but I think it at least warrants
> mentioning in the commit message.

True, but it's probably related. The netback probe would previously kick udev, the hotplug script would then run, and then the state would go connected. I think, because the hotplug is invoked directly by the toolstack now, these things really ought not to be tied together. TBH I can't see any harm in the frontend seeing the network connection before the backend plumbing is done... If the frontend should have any sort of indication of whether the backend is plumbed or not then IMO it ought to be as a virtual carrier/link status, because unplumbing and re-plumbing could be done at any time really without any need for the shared ring to go away (and in fact I will be following up at some point with a patch to allow unbind and re-bind of netback).

I'll elaborate in the commit message as you suggest :-)

Cheers,

  Paul

> 
> Regards,
> Jason
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH net-next] xen-netback: get rid of old udev related code

[David Miller]

From: Paul Durrant <pdurrant@amazon.com>
Date: Thu, 12 Dec 2019 13:54:06 +0000

> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
> 
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>       method. The only other driver to have a method at all is
>       pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>       Hence this patch also facilitates further cleanup.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

If userspace ever used this stuff, I seriously doubt you can remove this
even if it hasn't been used in 5+ years.

Sorry.

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[David Miller]

From: Paul Durrant <pdurrant@amazon.com>
Date: Thu, 12 Dec 2019 13:54:06 +0000

> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
> 
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>       method. The only other driver to have a method at all is
>       pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>       Hence this patch also facilitates further cleanup.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

If userspace ever used this stuff, I seriously doubt you can remove this
even if it hasn't been used in 5+ years.

Sorry.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 12.12.19 20:05, David Miller wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> Date: Thu, 12 Dec 2019 13:54:06 +0000
> 
>> In the past it used to be the case that the Xen toolstack relied upon
>> udev to execute backend hotplug scripts. However this has not been the
>> case for many releases now and removal of the associated code in
>> xen-netback shortens the source by more than 100 lines, and removes much
>> complexity in the interaction with the xenstore backend state.
>>
>> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>>        method. The only other driver to have a method at all is
>>        pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>>        Hence this patch also facilitates further cleanup.
>>
>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> 
> If userspace ever used this stuff, I seriously doubt you can remove this
> even if it hasn't been used in 5+ years.

Hmm, depends.

This has been used by Xen tools in dom0 only. If the last usage has been
in a Xen version which is no longer able to run with current Linux in
dom0 it could be removed. But I guess this would have to be a rather old
version of Xen (like 3.x?).

Paul, can you give a hint since which Xen version the toolstack no
longer relies on udev to start the hotplug scripts?


Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 12.12.19 20:05, David Miller wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> Date: Thu, 12 Dec 2019 13:54:06 +0000
> 
>> In the past it used to be the case that the Xen toolstack relied upon
>> udev to execute backend hotplug scripts. However this has not been the
>> case for many releases now and removal of the associated code in
>> xen-netback shortens the source by more than 100 lines, and removes much
>> complexity in the interaction with the xenstore backend state.
>>
>> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>>        method. The only other driver to have a method at all is
>>        pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>>        Hence this patch also facilitates further cleanup.
>>
>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> 
> If userspace ever used this stuff, I seriously doubt you can remove this
> even if it hasn't been used in 5+ years.

Hmm, depends.

This has been used by Xen tools in dom0 only. If the last usage has been
in a Xen version which is no longer able to run with current Linux in
dom0 it could be removed. But I guess this would have to be a rather old
version of Xen (like 3.x?).

Paul, can you give a hint since which Xen version the toolstack no
longer relies on udev to start the hotplug scripts?


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 13 December 2019 05:41
> To: David Miller <davem@davemloft.net>; Durrant, Paul
> <pdurrant@amazon.com>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 12.12.19 20:05, David Miller wrote:
> > From: Paul Durrant <pdurrant@amazon.com>
> > Date: Thu, 12 Dec 2019 13:54:06 +0000
> >
> >> In the past it used to be the case that the Xen toolstack relied upon
> >> udev to execute backend hotplug scripts. However this has not been the
> >> case for many releases now and removal of the associated code in
> >> xen-netback shortens the source by more than 100 lines, and removes
> much
> >> complexity in the interaction with the xenstore backend state.
> >>
> >> NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >>        method. The only other driver to have a method at all is
> >>        pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >>        Hence this patch also facilitates further cleanup.
> >>
> >> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >
> > If userspace ever used this stuff, I seriously doubt you can remove this
> > even if it hasn't been used in 5+ years.
> 
> Hmm, depends.
> 
> This has been used by Xen tools in dom0 only. If the last usage has been
> in a Xen version which is no longer able to run with current Linux in
> dom0 it could be removed. But I guess this would have to be a rather old
> version of Xen (like 3.x?).
> 
> Paul, can you give a hint since which Xen version the toolstack no
> longer relies on udev to start the hotplug scripts?
> 

The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:

commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
Author: Roger Pau Monne <roger.pau@citrix.com>
Date:   Thu Jul 26 16:47:35 2012 +0100

    libxl: call hotplug scripts for nic devices from libxl

    Since most of the needed work is already done in previous patches,
    this patch only contains the necessary code to call hotplug scripts
    for nic devices, that should be called when the device is added or
    removed from a guest.

    Added another parameter to libxl__get_hotplug_script_info, that is
    used to know the number of times hotplug scripts have been called for
    that device. This is currently used by IOEMU nics on Linux.

    Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
    Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
    Committed-by: Ian Campbell <ian.campbell@citrix.com>

The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:

commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
Author: George Dunlap <george.dunlap@eu.citrix.com>
Date:   Mon Jul 6 11:51:39 2015 +0100

    libxl: Remove linux udev rules

    They are no longer needed, having been replaced by a daemon for
    driverdomains which will run scripts as necessary.

    Worse yet, they seem to be broken for script-based block devices, such
    as block-iscsi.  This wouldn't matter so much if they were never run
    by default; but if you run block-attach without having created a
    domain, then the appropriate node to disable running udev scripts will
    not have been written yet, and the attach will silently fail.

    Rather than try to sort out that issue, just remove them entirely.

    Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
    Acked-by: Wei Liu <wei.liu2@citrix.com>

So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).

  Paul



> 
> Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 13 December 2019 05:41
> To: David Miller <davem@davemloft.net>; Durrant, Paul
> <pdurrant@amazon.com>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 12.12.19 20:05, David Miller wrote:
> > From: Paul Durrant <pdurrant@amazon.com>
> > Date: Thu, 12 Dec 2019 13:54:06 +0000
> >
> >> In the past it used to be the case that the Xen toolstack relied upon
> >> udev to execute backend hotplug scripts. However this has not been the
> >> case for many releases now and removal of the associated code in
> >> xen-netback shortens the source by more than 100 lines, and removes
> much
> >> complexity in the interaction with the xenstore backend state.
> >>
> >> NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >>        method. The only other driver to have a method at all is
> >>        pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >>        Hence this patch also facilitates further cleanup.
> >>
> >> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >
> > If userspace ever used this stuff, I seriously doubt you can remove this
> > even if it hasn't been used in 5+ years.
> 
> Hmm, depends.
> 
> This has been used by Xen tools in dom0 only. If the last usage has been
> in a Xen version which is no longer able to run with current Linux in
> dom0 it could be removed. But I guess this would have to be a rather old
> version of Xen (like 3.x?).
> 
> Paul, can you give a hint since which Xen version the toolstack no
> longer relies on udev to start the hotplug scripts?
> 

The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:

commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
Author: Roger Pau Monne <roger.pau@citrix.com>
Date:   Thu Jul 26 16:47:35 2012 +0100

    libxl: call hotplug scripts for nic devices from libxl

    Since most of the needed work is already done in previous patches,
    this patch only contains the necessary code to call hotplug scripts
    for nic devices, that should be called when the device is added or
    removed from a guest.

    Added another parameter to libxl__get_hotplug_script_info, that is
    used to know the number of times hotplug scripts have been called for
    that device. This is currently used by IOEMU nics on Linux.

    Signed-off-by: Roger Pau Monne <roger.pau@citrix.com>
    Acked-by: Ian Jackson<ian.jackson@eu.citrix.com>
    Committed-by: Ian Campbell <ian.campbell@citrix.com>

The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:

commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
Author: George Dunlap <george.dunlap@eu.citrix.com>
Date:   Mon Jul 6 11:51:39 2015 +0100

    libxl: Remove linux udev rules

    They are no longer needed, having been replaced by a daemon for
    driverdomains which will run scripts as necessary.

    Worse yet, they seem to be broken for script-based block devices, such
    as block-iscsi.  This wouldn't matter so much if they were never run
    by default; but if you run block-attach without having created a
    domain, then the appropriate node to disable running udev scripts will
    not have been written yet, and the attach will silently fail.

    Rather than try to sort out that issue, just remove them entirely.

    Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
    Acked-by: Wei Liu <wei.liu2@citrix.com>

So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).

  Paul



> 
> Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 13.12.19 10:24, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 13 December 2019 05:41
>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>> <pdurrant@amazon.com>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 12.12.19 20:05, David Miller wrote:
>>> From: Paul Durrant <pdurrant@amazon.com>
>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>
>>>> In the past it used to be the case that the Xen toolstack relied upon
>>>> udev to execute backend hotplug scripts. However this has not been the
>>>> case for many releases now and removal of the associated code in
>>>> xen-netback shortens the source by more than 100 lines, and removes
>> much
>>>> complexity in the interaction with the xenstore backend state.
>>>>
>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>> uevent()
>>>>         method. The only other driver to have a method at all is
>>>>         pvcalls-back, and currently pvcalls_back_uevent() simply returns
>> 0.
>>>>         Hence this patch also facilitates further cleanup.
>>>>
>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>
>>> If userspace ever used this stuff, I seriously doubt you can remove this
>>> even if it hasn't been used in 5+ years.
>>
>> Hmm, depends.
>>
>> This has been used by Xen tools in dom0 only. If the last usage has been
>> in a Xen version which is no longer able to run with current Linux in
>> dom0 it could be removed. But I guess this would have to be a rather old
>> version of Xen (like 3.x?).
>>
>> Paul, can you give a hint since which Xen version the toolstack no
>> longer relies on udev to start the hotplug scripts?
>>
> 
> The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:
> 
> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53

Xen 4.2

> The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:
> 
> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b

Xen 4.6

> So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).

The problem are systems with either old Xen versions (before Xen 4.2) or
with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
dom0 kernel.

And I'm not sure there aren't such systems (especially in case someone
wants to stick with xend).


Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 13.12.19 10:24, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 13 December 2019 05:41
>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>> <pdurrant@amazon.com>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 12.12.19 20:05, David Miller wrote:
>>> From: Paul Durrant <pdurrant@amazon.com>
>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>
>>>> In the past it used to be the case that the Xen toolstack relied upon
>>>> udev to execute backend hotplug scripts. However this has not been the
>>>> case for many releases now and removal of the associated code in
>>>> xen-netback shortens the source by more than 100 lines, and removes
>> much
>>>> complexity in the interaction with the xenstore backend state.
>>>>
>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>> uevent()
>>>>         method. The only other driver to have a method at all is
>>>>         pvcalls-back, and currently pvcalls_back_uevent() simply returns
>> 0.
>>>>         Hence this patch also facilitates further cleanup.
>>>>
>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>
>>> If userspace ever used this stuff, I seriously doubt you can remove this
>>> even if it hasn't been used in 5+ years.
>>
>> Hmm, depends.
>>
>> This has been used by Xen tools in dom0 only. If the last usage has been
>> in a Xen version which is no longer able to run with current Linux in
>> dom0 it could be removed. But I guess this would have to be a rather old
>> version of Xen (like 3.x?).
>>
>> Paul, can you give a hint since which Xen version the toolstack no
>> longer relies on udev to start the hotplug scripts?
>>
> 
> The udev rules were in a file called tools/hotplug/Linux/xen-backend.rules (in xen.git), and a commit from Roger removed the NIC rules in 2012:
> 
> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53

Xen 4.2

> The last commit I could find to that file modified its name to xen-backend.rules.in, and this was finally removed by George in 2015:
> 
> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b

Xen 4.6

> So, I think this means anyone using a version of the Xen tools within recent memory will be having their hotplug scripts called directly by libxl (and having udev rules present would actually be counter-productive, as George's commit states and as I discovered the hard way when the change was originally made).

The problem are systems with either old Xen versions (before Xen 4.2) or
with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
dom0 kernel.

And I'm not sure there aren't such systems (especially in case someone
wants to stick with xend).


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 13 December 2019 10:02
> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> <davem@davemloft.net>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 13.12.19 10:24, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jürgen Groß <jgross@suse.com>
> >> Sent: 13 December 2019 05:41
> >> To: David Miller <davem@davemloft.net>; Durrant, Paul
> >> <pdurrant@amazon.com>
> >> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >> kernel@vger.kernel.org; netdev@vger.kernel.org
> >> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> udev
> >> related code
> >>
> >> On 12.12.19 20:05, David Miller wrote:
> >>> From: Paul Durrant <pdurrant@amazon.com>
> >>> Date: Thu, 12 Dec 2019 13:54:06 +0000
> >>>
> >>>> In the past it used to be the case that the Xen toolstack relied upon
> >>>> udev to execute backend hotplug scripts. However this has not been
> the
> >>>> case for many releases now and removal of the associated code in
> >>>> xen-netback shortens the source by more than 100 lines, and removes
> >> much
> >>>> complexity in the interaction with the xenstore backend state.
> >>>>
> >>>> NOTE: xen-netback is the only xenbus driver to have a functional
> >> uevent()
> >>>>         method. The only other driver to have a method at all is
> >>>>         pvcalls-back, and currently pvcalls_back_uevent() simply
> returns
> >> 0.
> >>>>         Hence this patch also facilitates further cleanup.
> >>>>
> >>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >>>
> >>> If userspace ever used this stuff, I seriously doubt you can remove
> this
> >>> even if it hasn't been used in 5+ years.
> >>
> >> Hmm, depends.
> >>
> >> This has been used by Xen tools in dom0 only. If the last usage has
> been
> >> in a Xen version which is no longer able to run with current Linux in
> >> dom0 it could be removed. But I guess this would have to be a rather
> old
> >> version of Xen (like 3.x?).
> >>
> >> Paul, can you give a hint since which Xen version the toolstack no
> >> longer relies on udev to start the hotplug scripts?
> >>
> >
> > The udev rules were in a file called tools/hotplug/Linux/xen-
> backend.rules (in xen.git), and a commit from Roger removed the NIC rules
> in 2012:
> >
> > commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
> 
> Xen 4.2
> 
> > The last commit I could find to that file modified its name to xen-
> backend.rules.in, and this was finally removed by George in 2015:
> >
> > commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
> 
> Xen 4.6
> 
> > So, I think this means anyone using a version of the Xen tools within
> recent memory will be having their hotplug scripts called directly by
> libxl (and having udev rules present would actually be counter-productive,
> as George's commit states and as I discovered the hard way when the change
> was originally made).
> 
> The problem are systems with either old Xen versions (before Xen 4.2) or
> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
> dom0 kernel.
> 
> And I'm not sure there aren't such systems (especially in case someone
> wants to stick with xend).
> 

But would someone sticking with such an old toolstack expect to run on an unmodified upstream dom0? There has to be some way in which we can retire old code.

Aside from the udev kicks though, I still think the hotplug-status/ring state interaction is just bogus anyway. As I said in a previous thread, the hotplug-status ought to be indicated as carrier status, if at all, so I still think all that code ought to go.

  Paul

> 
> Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 13 December 2019 10:02
> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> <davem@davemloft.net>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 13.12.19 10:24, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jürgen Groß <jgross@suse.com>
> >> Sent: 13 December 2019 05:41
> >> To: David Miller <davem@davemloft.net>; Durrant, Paul
> >> <pdurrant@amazon.com>
> >> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >> kernel@vger.kernel.org; netdev@vger.kernel.org
> >> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> udev
> >> related code
> >>
> >> On 12.12.19 20:05, David Miller wrote:
> >>> From: Paul Durrant <pdurrant@amazon.com>
> >>> Date: Thu, 12 Dec 2019 13:54:06 +0000
> >>>
> >>>> In the past it used to be the case that the Xen toolstack relied upon
> >>>> udev to execute backend hotplug scripts. However this has not been
> the
> >>>> case for many releases now and removal of the associated code in
> >>>> xen-netback shortens the source by more than 100 lines, and removes
> >> much
> >>>> complexity in the interaction with the xenstore backend state.
> >>>>
> >>>> NOTE: xen-netback is the only xenbus driver to have a functional
> >> uevent()
> >>>>         method. The only other driver to have a method at all is
> >>>>         pvcalls-back, and currently pvcalls_back_uevent() simply
> returns
> >> 0.
> >>>>         Hence this patch also facilitates further cleanup.
> >>>>
> >>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >>>
> >>> If userspace ever used this stuff, I seriously doubt you can remove
> this
> >>> even if it hasn't been used in 5+ years.
> >>
> >> Hmm, depends.
> >>
> >> This has been used by Xen tools in dom0 only. If the last usage has
> been
> >> in a Xen version which is no longer able to run with current Linux in
> >> dom0 it could be removed. But I guess this would have to be a rather
> old
> >> version of Xen (like 3.x?).
> >>
> >> Paul, can you give a hint since which Xen version the toolstack no
> >> longer relies on udev to start the hotplug scripts?
> >>
> >
> > The udev rules were in a file called tools/hotplug/Linux/xen-
> backend.rules (in xen.git), and a commit from Roger removed the NIC rules
> in 2012:
> >
> > commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
> 
> Xen 4.2
> 
> > The last commit I could find to that file modified its name to xen-
> backend.rules.in, and this was finally removed by George in 2015:
> >
> > commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
> 
> Xen 4.6
> 
> > So, I think this means anyone using a version of the Xen tools within
> recent memory will be having their hotplug scripts called directly by
> libxl (and having udev rules present would actually be counter-productive,
> as George's commit states and as I discovered the hard way when the change
> was originally made).
> 
> The problem are systems with either old Xen versions (before Xen 4.2) or
> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
> dom0 kernel.
> 
> And I'm not sure there aren't such systems (especially in case someone
> wants to stick with xend).
> 

But would someone sticking with such an old toolstack expect to run on an unmodified upstream dom0? There has to be some way in which we can retire old code.

Aside from the udev kicks though, I still think the hotplug-status/ring state interaction is just bogus anyway. As I said in a previous thread, the hotplug-status ought to be indicated as carrier status, if at all, so I still think all that code ought to go.

  Paul

> 
> Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 13.12.19 11:12, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 13 December 2019 10:02
>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>> <davem@davemloft.net>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>> -----Original Message-----
>>>> From: Jürgen Groß <jgross@suse.com>
>>>> Sent: 13 December 2019 05:41
>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>> <pdurrant@amazon.com>
>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>> udev
>>>> related code
>>>>
>>>> On 12.12.19 20:05, David Miller wrote:
>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>
>>>>>> In the past it used to be the case that the Xen toolstack relied upon
>>>>>> udev to execute backend hotplug scripts. However this has not been
>> the
>>>>>> case for many releases now and removal of the associated code in
>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>> much
>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>
>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>> uevent()
>>>>>>          method. The only other driver to have a method at all is
>>>>>>          pvcalls-back, and currently pvcalls_back_uevent() simply
>> returns
>>>> 0.
>>>>>>          Hence this patch also facilitates further cleanup.
>>>>>>
>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>
>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>> this
>>>>> even if it hasn't been used in 5+ years.
>>>>
>>>> Hmm, depends.
>>>>
>>>> This has been used by Xen tools in dom0 only. If the last usage has
>> been
>>>> in a Xen version which is no longer able to run with current Linux in
>>>> dom0 it could be removed. But I guess this would have to be a rather
>> old
>>>> version of Xen (like 3.x?).
>>>>
>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>> longer relies on udev to start the hotplug scripts?
>>>>
>>>
>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>> backend.rules (in xen.git), and a commit from Roger removed the NIC rules
>> in 2012:
>>>
>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>
>> Xen 4.2
>>
>>> The last commit I could find to that file modified its name to xen-
>> backend.rules.in, and this was finally removed by George in 2015:
>>>
>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>
>> Xen 4.6
>>
>>> So, I think this means anyone using a version of the Xen tools within
>> recent memory will be having their hotplug scripts called directly by
>> libxl (and having udev rules present would actually be counter-productive,
>> as George's commit states and as I discovered the hard way when the change
>> was originally made).
>>
>> The problem are systems with either old Xen versions (before Xen 4.2) or
>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>> dom0 kernel.
>>
>> And I'm not sure there aren't such systems (especially in case someone
>> wants to stick with xend).
>>
> 
> But would someone sticking with such an old toolstack expect to run on an unmodified upstream dom0? There has to be some way in which we can retire old code.

As long as there are no hypervisor interface related issues
prohibiting running dom0 unmodified I think the expectation to be
able to use the kernel in that environment is fine.

Another question coming up would be: how is this handled in a driver
domain running netback? Which component is starting the hotplug script
there? I don't think we can assume a standard Xen toolset in this case.
So I'd rather leave this code as it is instead of breaking some rare
but valid use cases.

> 
> Aside from the udev kicks though, I still think the hotplug-status/ring state interaction is just bogus anyway. As I said in a previous thread, the hotplug-status ought to be indicated as carrier status, if at all, so I still think all that code ought to go.

I agree regarding the future interface, but with the carrier state just
being in the plans to be added now, it is clearly too early to remove
the code with that reasoning.


Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 13.12.19 11:12, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 13 December 2019 10:02
>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>> <davem@davemloft.net>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>> -----Original Message-----
>>>> From: Jürgen Groß <jgross@suse.com>
>>>> Sent: 13 December 2019 05:41
>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>> <pdurrant@amazon.com>
>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>> udev
>>>> related code
>>>>
>>>> On 12.12.19 20:05, David Miller wrote:
>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>
>>>>>> In the past it used to be the case that the Xen toolstack relied upon
>>>>>> udev to execute backend hotplug scripts. However this has not been
>> the
>>>>>> case for many releases now and removal of the associated code in
>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>> much
>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>
>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>> uevent()
>>>>>>          method. The only other driver to have a method at all is
>>>>>>          pvcalls-back, and currently pvcalls_back_uevent() simply
>> returns
>>>> 0.
>>>>>>          Hence this patch also facilitates further cleanup.
>>>>>>
>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>
>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>> this
>>>>> even if it hasn't been used in 5+ years.
>>>>
>>>> Hmm, depends.
>>>>
>>>> This has been used by Xen tools in dom0 only. If the last usage has
>> been
>>>> in a Xen version which is no longer able to run with current Linux in
>>>> dom0 it could be removed. But I guess this would have to be a rather
>> old
>>>> version of Xen (like 3.x?).
>>>>
>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>> longer relies on udev to start the hotplug scripts?
>>>>
>>>
>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>> backend.rules (in xen.git), and a commit from Roger removed the NIC rules
>> in 2012:
>>>
>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>
>> Xen 4.2
>>
>>> The last commit I could find to that file modified its name to xen-
>> backend.rules.in, and this was finally removed by George in 2015:
>>>
>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>
>> Xen 4.6
>>
>>> So, I think this means anyone using a version of the Xen tools within
>> recent memory will be having their hotplug scripts called directly by
>> libxl (and having udev rules present would actually be counter-productive,
>> as George's commit states and as I discovered the hard way when the change
>> was originally made).
>>
>> The problem are systems with either old Xen versions (before Xen 4.2) or
>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>> dom0 kernel.
>>
>> And I'm not sure there aren't such systems (especially in case someone
>> wants to stick with xend).
>>
> 
> But would someone sticking with such an old toolstack expect to run on an unmodified upstream dom0? There has to be some way in which we can retire old code.

As long as there are no hypervisor interface related issues
prohibiting running dom0 unmodified I think the expectation to be
able to use the kernel in that environment is fine.

Another question coming up would be: how is this handled in a driver
domain running netback? Which component is starting the hotplug script
there? I don't think we can assume a standard Xen toolset in this case.
So I'd rather leave this code as it is instead of breaking some rare
but valid use cases.

> 
> Aside from the udev kicks though, I still think the hotplug-status/ring state interaction is just bogus anyway. As I said in a previous thread, the hotplug-status ought to be indicated as carrier status, if at all, so I still think all that code ought to go.

I agree regarding the future interface, but with the carrier state just
being in the plans to be added now, it is clearly too early to remove
the code with that reasoning.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

RE: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 16 December 2019 08:10
> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> <davem@davemloft.net>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 13.12.19 11:12, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jürgen Groß <jgross@suse.com>
> >> Sent: 13 December 2019 10:02
> >> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> >> <davem@davemloft.net>
> >> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >> kernel@vger.kernel.org; netdev@vger.kernel.org
> >> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> udev
> >> related code
> >>
> >> On 13.12.19 10:24, Durrant, Paul wrote:
> >>>> -----Original Message-----
> >>>> From: Jürgen Groß <jgross@suse.com>
> >>>> Sent: 13 December 2019 05:41
> >>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
> >>>> <pdurrant@amazon.com>
> >>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >>>> kernel@vger.kernel.org; netdev@vger.kernel.org
> >>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> >> udev
> >>>> related code
> >>>>
> >>>> On 12.12.19 20:05, David Miller wrote:
> >>>>> From: Paul Durrant <pdurrant@amazon.com>
> >>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
> >>>>>
> >>>>>> In the past it used to be the case that the Xen toolstack relied
> upon
> >>>>>> udev to execute backend hotplug scripts. However this has not been
> >> the
> >>>>>> case for many releases now and removal of the associated code in
> >>>>>> xen-netback shortens the source by more than 100 lines, and removes
> >>>> much
> >>>>>> complexity in the interaction with the xenstore backend state.
> >>>>>>
> >>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
> >>>> uevent()
> >>>>>>          method. The only other driver to have a method at all is
> >>>>>>          pvcalls-back, and currently pvcalls_back_uevent() simply
> >> returns
> >>>> 0.
> >>>>>>          Hence this patch also facilitates further cleanup.
> >>>>>>
> >>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >>>>>
> >>>>> If userspace ever used this stuff, I seriously doubt you can remove
> >> this
> >>>>> even if it hasn't been used in 5+ years.
> >>>>
> >>>> Hmm, depends.
> >>>>
> >>>> This has been used by Xen tools in dom0 only. If the last usage has
> >> been
> >>>> in a Xen version which is no longer able to run with current Linux in
> >>>> dom0 it could be removed. But I guess this would have to be a rather
> >> old
> >>>> version of Xen (like 3.x?).
> >>>>
> >>>> Paul, can you give a hint since which Xen version the toolstack no
> >>>> longer relies on udev to start the hotplug scripts?
> >>>>
> >>>
> >>> The udev rules were in a file called tools/hotplug/Linux/xen-
> >> backend.rules (in xen.git), and a commit from Roger removed the NIC
> rules
> >> in 2012:
> >>>
> >>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
> >>
> >> Xen 4.2
> >>
> >>> The last commit I could find to that file modified its name to xen-
> >> backend.rules.in, and this was finally removed by George in 2015:
> >>>
> >>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
> >>
> >> Xen 4.6
> >>
> >>> So, I think this means anyone using a version of the Xen tools within
> >> recent memory will be having their hotplug scripts called directly by
> >> libxl (and having udev rules present would actually be counter-
> productive,
> >> as George's commit states and as I discovered the hard way when the
> change
> >> was originally made).
> >>
> >> The problem are systems with either old Xen versions (before Xen 4.2)
> or
> >> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
> >> dom0 kernel.
> >>
> >> And I'm not sure there aren't such systems (especially in case someone
> >> wants to stick with xend).
> >>
> >
> > But would someone sticking with such an old toolstack expect to run on
> an unmodified upstream dom0? There has to be some way in which we can
> retire old code.
> 
> As long as there are no hypervisor interface related issues
> prohibiting running dom0 unmodified I think the expectation to be
> able to use the kernel in that environment is fine.
> 

I think we need a better policy in future then otherwise we will only collect baggage.

> Another question coming up would be: how is this handled in a driver
> domain running netback? Which component is starting the hotplug script
> there? I don't think we can assume a standard Xen toolset in this case.
> So I'd rather leave this code as it is instead of breaking some rare
> but valid use cases.

I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.

> 
> >
> > Aside from the udev kicks though, I still think the hotplug-status/ring
> state interaction is just bogus anyway. As I said in a previous thread,
> the hotplug-status ought to be indicated as carrier status, if at all, so
> I still think all that code ought to go.
> 
> I agree regarding the future interface, but with the carrier state just
> being in the plans to be added now, it is clearly too early to remove
> the code with that reasoning.

I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.

  Paul

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Durrant, Paul]

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 16 December 2019 08:10
> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> <davem@davemloft.net>
> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> kernel@vger.kernel.org; netdev@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On 13.12.19 11:12, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jürgen Groß <jgross@suse.com>
> >> Sent: 13 December 2019 10:02
> >> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
> >> <davem@davemloft.net>
> >> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >> kernel@vger.kernel.org; netdev@vger.kernel.org
> >> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> udev
> >> related code
> >>
> >> On 13.12.19 10:24, Durrant, Paul wrote:
> >>>> -----Original Message-----
> >>>> From: Jürgen Groß <jgross@suse.com>
> >>>> Sent: 13 December 2019 05:41
> >>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
> >>>> <pdurrant@amazon.com>
> >>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
> >>>> kernel@vger.kernel.org; netdev@vger.kernel.org
> >>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
> >> udev
> >>>> related code
> >>>>
> >>>> On 12.12.19 20:05, David Miller wrote:
> >>>>> From: Paul Durrant <pdurrant@amazon.com>
> >>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
> >>>>>
> >>>>>> In the past it used to be the case that the Xen toolstack relied
> upon
> >>>>>> udev to execute backend hotplug scripts. However this has not been
> >> the
> >>>>>> case for many releases now and removal of the associated code in
> >>>>>> xen-netback shortens the source by more than 100 lines, and removes
> >>>> much
> >>>>>> complexity in the interaction with the xenstore backend state.
> >>>>>>
> >>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
> >>>> uevent()
> >>>>>>          method. The only other driver to have a method at all is
> >>>>>>          pvcalls-back, and currently pvcalls_back_uevent() simply
> >> returns
> >>>> 0.
> >>>>>>          Hence this patch also facilitates further cleanup.
> >>>>>>
> >>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> >>>>>
> >>>>> If userspace ever used this stuff, I seriously doubt you can remove
> >> this
> >>>>> even if it hasn't been used in 5+ years.
> >>>>
> >>>> Hmm, depends.
> >>>>
> >>>> This has been used by Xen tools in dom0 only. If the last usage has
> >> been
> >>>> in a Xen version which is no longer able to run with current Linux in
> >>>> dom0 it could be removed. But I guess this would have to be a rather
> >> old
> >>>> version of Xen (like 3.x?).
> >>>>
> >>>> Paul, can you give a hint since which Xen version the toolstack no
> >>>> longer relies on udev to start the hotplug scripts?
> >>>>
> >>>
> >>> The udev rules were in a file called tools/hotplug/Linux/xen-
> >> backend.rules (in xen.git), and a commit from Roger removed the NIC
> rules
> >> in 2012:
> >>>
> >>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
> >>
> >> Xen 4.2
> >>
> >>> The last commit I could find to that file modified its name to xen-
> >> backend.rules.in, and this was finally removed by George in 2015:
> >>>
> >>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
> >>
> >> Xen 4.6
> >>
> >>> So, I think this means anyone using a version of the Xen tools within
> >> recent memory will be having their hotplug scripts called directly by
> >> libxl (and having udev rules present would actually be counter-
> productive,
> >> as George's commit states and as I discovered the hard way when the
> change
> >> was originally made).
> >>
> >> The problem are systems with either old Xen versions (before Xen 4.2)
> or
> >> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
> >> dom0 kernel.
> >>
> >> And I'm not sure there aren't such systems (especially in case someone
> >> wants to stick with xend).
> >>
> >
> > But would someone sticking with such an old toolstack expect to run on
> an unmodified upstream dom0? There has to be some way in which we can
> retire old code.
> 
> As long as there are no hypervisor interface related issues
> prohibiting running dom0 unmodified I think the expectation to be
> able to use the kernel in that environment is fine.
> 

I think we need a better policy in future then otherwise we will only collect baggage.

> Another question coming up would be: how is this handled in a driver
> domain running netback? Which component is starting the hotplug script
> there? I don't think we can assume a standard Xen toolset in this case.
> So I'd rather leave this code as it is instead of breaking some rare
> but valid use cases.

I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.

> 
> >
> > Aside from the udev kicks though, I still think the hotplug-status/ring
> state interaction is just bogus anyway. As I said in a previous thread,
> the hotplug-status ought to be indicated as carrier status, if at all, so
> I still think all that code ought to go.
> 
> I agree regarding the future interface, but with the carrier state just
> being in the plans to be added now, it is clearly too early to remove
> the code with that reasoning.

I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.

  Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 16.12.19 09:18, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 16 December 2019 08:10
>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>> <davem@davemloft.net>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 13.12.19 11:12, Durrant, Paul wrote:
>>>> -----Original Message-----
>>>> From: Jürgen Groß <jgross@suse.com>
>>>> Sent: 13 December 2019 10:02
>>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>>> <davem@davemloft.net>
>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>> udev
>>>> related code
>>>>
>>>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>> Sent: 13 December 2019 05:41
>>>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>>>> <pdurrant@amazon.com>
>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>>> udev
>>>>>> related code
>>>>>>
>>>>>> On 12.12.19 20:05, David Miller wrote:
>>>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>>>
>>>>>>>> In the past it used to be the case that the Xen toolstack relied
>> upon
>>>>>>>> udev to execute backend hotplug scripts. However this has not been
>>>> the
>>>>>>>> case for many releases now and removal of the associated code in
>>>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>>>> much
>>>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>>>
>>>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>>>> uevent()
>>>>>>>>           method. The only other driver to have a method at all is
>>>>>>>>           pvcalls-back, and currently pvcalls_back_uevent() simply
>>>> returns
>>>>>> 0.
>>>>>>>>           Hence this patch also facilitates further cleanup.
>>>>>>>>
>>>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>>>
>>>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>>>> this
>>>>>>> even if it hasn't been used in 5+ years.
>>>>>>
>>>>>> Hmm, depends.
>>>>>>
>>>>>> This has been used by Xen tools in dom0 only. If the last usage has
>>>> been
>>>>>> in a Xen version which is no longer able to run with current Linux in
>>>>>> dom0 it could be removed. But I guess this would have to be a rather
>>>> old
>>>>>> version of Xen (like 3.x?).
>>>>>>
>>>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>>>> longer relies on udev to start the hotplug scripts?
>>>>>>
>>>>>
>>>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>>>> backend.rules (in xen.git), and a commit from Roger removed the NIC
>> rules
>>>> in 2012:
>>>>>
>>>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>>>
>>>> Xen 4.2
>>>>
>>>>> The last commit I could find to that file modified its name to xen-
>>>> backend.rules.in, and this was finally removed by George in 2015:
>>>>>
>>>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>>>
>>>> Xen 4.6
>>>>
>>>>> So, I think this means anyone using a version of the Xen tools within
>>>> recent memory will be having their hotplug scripts called directly by
>>>> libxl (and having udev rules present would actually be counter-
>> productive,
>>>> as George's commit states and as I discovered the hard way when the
>> change
>>>> was originally made).
>>>>
>>>> The problem are systems with either old Xen versions (before Xen 4.2)
>> or
>>>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>>>> dom0 kernel.
>>>>
>>>> And I'm not sure there aren't such systems (especially in case someone
>>>> wants to stick with xend).
>>>>
>>>
>>> But would someone sticking with such an old toolstack expect to run on
>> an unmodified upstream dom0? There has to be some way in which we can
>> retire old code.
>>
>> As long as there are no hypervisor interface related issues
>> prohibiting running dom0 unmodified I think the expectation to be
>> able to use the kernel in that environment is fine.
>>
> 
> I think we need a better policy in future then otherwise we will only collect baggage.

The Linux kernel policy regarding user interfaces and existing use cases
is rather clear and we should not deviate without very strong reasons.

> 
>> Another question coming up would be: how is this handled in a driver
>> domain running netback? Which component is starting the hotplug script
>> there? I don't think we can assume a standard Xen toolset in this case.
>> So I'd rather leave this code as it is instead of breaking some rare
>> but valid use cases.
> 
> I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.

I intend in no way to break projects like Qubes. Disaggregation is
one of the very big advantages of Xen over KVM, Hyper-V and VMWare.
We should not give that up "just to get rid of some code". Period.

> 
>>
>>>
>>> Aside from the udev kicks though, I still think the hotplug-status/ring
>> state interaction is just bogus anyway. As I said in a previous thread,
>> the hotplug-status ought to be indicated as carrier status, if at all, so
>> I still think all that code ought to go.
>>
>> I agree regarding the future interface, but with the carrier state just
>> being in the plans to be added now, it is clearly too early to remove
>> the code with that reasoning.
> 
> I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.

If it is being used that way we need to continue supporting it.


Juergen

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Jürgen Groß]

On 16.12.19 09:18, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jürgen Groß <jgross@suse.com>
>> Sent: 16 December 2019 08:10
>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>> <davem@davemloft.net>
>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>> kernel@vger.kernel.org; netdev@vger.kernel.org
>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>> related code
>>
>> On 13.12.19 11:12, Durrant, Paul wrote:
>>>> -----Original Message-----
>>>> From: Jürgen Groß <jgross@suse.com>
>>>> Sent: 13 December 2019 10:02
>>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>>> <davem@davemloft.net>
>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>> udev
>>>> related code
>>>>
>>>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>> Sent: 13 December 2019 05:41
>>>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>>>> <pdurrant@amazon.com>
>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>>> udev
>>>>>> related code
>>>>>>
>>>>>> On 12.12.19 20:05, David Miller wrote:
>>>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>>>
>>>>>>>> In the past it used to be the case that the Xen toolstack relied
>> upon
>>>>>>>> udev to execute backend hotplug scripts. However this has not been
>>>> the
>>>>>>>> case for many releases now and removal of the associated code in
>>>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>>>> much
>>>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>>>
>>>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>>>> uevent()
>>>>>>>>           method. The only other driver to have a method at all is
>>>>>>>>           pvcalls-back, and currently pvcalls_back_uevent() simply
>>>> returns
>>>>>> 0.
>>>>>>>>           Hence this patch also facilitates further cleanup.
>>>>>>>>
>>>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>>>
>>>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>>>> this
>>>>>>> even if it hasn't been used in 5+ years.
>>>>>>
>>>>>> Hmm, depends.
>>>>>>
>>>>>> This has been used by Xen tools in dom0 only. If the last usage has
>>>> been
>>>>>> in a Xen version which is no longer able to run with current Linux in
>>>>>> dom0 it could be removed. But I guess this would have to be a rather
>>>> old
>>>>>> version of Xen (like 3.x?).
>>>>>>
>>>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>>>> longer relies on udev to start the hotplug scripts?
>>>>>>
>>>>>
>>>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>>>> backend.rules (in xen.git), and a commit from Roger removed the NIC
>> rules
>>>> in 2012:
>>>>>
>>>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>>>
>>>> Xen 4.2
>>>>
>>>>> The last commit I could find to that file modified its name to xen-
>>>> backend.rules.in, and this was finally removed by George in 2015:
>>>>>
>>>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>>>
>>>> Xen 4.6
>>>>
>>>>> So, I think this means anyone using a version of the Xen tools within
>>>> recent memory will be having their hotplug scripts called directly by
>>>> libxl (and having udev rules present would actually be counter-
>> productive,
>>>> as George's commit states and as I discovered the hard way when the
>> change
>>>> was originally made).
>>>>
>>>> The problem are systems with either old Xen versions (before Xen 4.2)
>> or
>>>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>>>> dom0 kernel.
>>>>
>>>> And I'm not sure there aren't such systems (especially in case someone
>>>> wants to stick with xend).
>>>>
>>>
>>> But would someone sticking with such an old toolstack expect to run on
>> an unmodified upstream dom0? There has to be some way in which we can
>> retire old code.
>>
>> As long as there are no hypervisor interface related issues
>> prohibiting running dom0 unmodified I think the expectation to be
>> able to use the kernel in that environment is fine.
>>
> 
> I think we need a better policy in future then otherwise we will only collect baggage.

The Linux kernel policy regarding user interfaces and existing use cases
is rather clear and we should not deviate without very strong reasons.

> 
>> Another question coming up would be: how is this handled in a driver
>> domain running netback? Which component is starting the hotplug script
>> there? I don't think we can assume a standard Xen toolset in this case.
>> So I'd rather leave this code as it is instead of breaking some rare
>> but valid use cases.
> 
> I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.

I intend in no way to break projects like Qubes. Disaggregation is
one of the very big advantages of Xen over KVM, Hyper-V and VMWare.
We should not give that up "just to get rid of some code". Period.

> 
>>
>>>
>>> Aside from the udev kicks though, I still think the hotplug-status/ring
>> state interaction is just bogus anyway. As I said in a previous thread,
>> the hotplug-status ought to be indicated as carrier status, if at all, so
>> I still think all that code ought to go.
>>
>> I agree regarding the future interface, but with the carrier state just
>> being in the plans to be added now, it is clearly too early to remove
>> the code with that reasoning.
> 
> I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.

If it is being used that way we need to continue supporting it.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Rich Persaud]

> On Dec 16, 2019, at 03:31, Jürgen Groß <jgross@suse.com> wrote:
> 
> On 16.12.19 09:18, Durrant, Paul wrote:
>>> -----Original Message-----
>>> From: Jürgen Groß <jgross@suse.com>
>>> Sent: 16 December 2019 08:10
>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>> <davem@davemloft.net>
>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>>> related code
>>>> On 13.12.19 11:12, Durrant, Paul wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>> Sent: 13 December 2019 10:02
>>>>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>>>>> <davem@davemloft.net>
>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>> udev
>>>>> related code
>>>>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>>> Sent: 13 December 2019 05:41
>>>>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>>>>> <pdurrant@amazon.com>
>>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>>>> udev
>>>>>>> related code
>>>>>>>> On 12.12.19 20:05, David Miller wrote:
>>>>>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>>>>>> In the past it used to be the case that the Xen toolstack relied
>>>> upon
>>>>>>>>>> udev to execute backend hotplug scripts. However this has not been
>>>>>> the
>>>>>>>>>> case for many releases now and removal of the associated code in
>>>>>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>>>>>> much
>>>>>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>>>>>> uevent()
>>>>>>>>>>         method. The only other driver to have a method at all is
>>>>>>>>>>         pvcalls-back, and currently pvcalls_back_uevent() simply
>>>>>> returns
>>>>>>>> 0.
>>>>>>>>>>         Hence this patch also facilitates further cleanup.
>>>>>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>>>>>> this
>>>>>>>>> even if it hasn't been used in 5+ years.
>>>>>>>> Hmm, depends.
>>>>>>>> This has been used by Xen tools in dom0 only. If the last usage has
>>>>>> been
>>>>>>>> in a Xen version which is no longer able to run with current Linux in
>>>>>>>> dom0 it could be removed. But I guess this would have to be a rather
>>>>>> old
>>>>>>>> version of Xen (like 3.x?).
>>>>>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>>>>>> longer relies on udev to start the hotplug scripts?
>>>>>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>>>>>> backend.rules (in xen.git), and a commit from Roger removed the NIC
>>>> rules
>>>>>> in 2012:
>>>>>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>>>>> Xen 4.2
>>>>>>> The last commit I could find to that file modified its name to xen-
>>>>>> backend.rules.in, and this was finally removed by George in 2015:
>>>>>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>>>>> Xen 4.6
>>>>>>> So, I think this means anyone using a version of the Xen tools within
>>>>>> recent memory will be having their hotplug scripts called directly by
>>>>>> libxl (and having udev rules present would actually be counter-
>>>> productive,
>>>>>> as George's commit states and as I discovered the hard way when the
>>>> change
>>>>>> was originally made).
>>>>>> The problem are systems with either old Xen versions (before Xen 4.2)
>>>> or
>>>>>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>>>>>> dom0 kernel.
>>>>>> And I'm not sure there aren't such systems (especially in case someone
>>>>>> wants to stick with xend).
>>>>> But would someone sticking with such an old toolstack expect to run on
>>>> an unmodified upstream dom0? There has to be some way in which we can
>>>> retire old code.
>>>> As long as there are no hypervisor interface related issues
>>>> prohibiting running dom0 unmodified I think the expectation to be
>>>> able to use the kernel in that environment is fine.
>> I think we need a better policy in future then otherwise we will only collect baggage.
> 
> The Linux kernel policy regarding user interfaces and existing use cases
> is rather clear and we should not deviate without very strong reasons.
> 
>>> Another question coming up would be: how is this handled in a driver
>>> domain running netback? Which component is starting the hotplug script
>>> there? I don't think we can assume a standard Xen toolset in this case.
>>> So I'd rather leave this code as it is instead of breaking some rare
>>> but valid use cases.
>> I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.
> 
> I intend in no way to break projects like Qubes. Disaggregation is
> one of the very big advantages of Xen over KVM, Hyper-V and VMWare.
> We should not give that up "just to get rid of some code". Period.

From a quick poll of Qubes and OpenXT netback usage: neither project would be impacted by the proposed change.  Qubes uses xl devd to call hotplug scripts.  OpenXT uses uevent triggers, but does not rely on any script provided in the uevent.  Ongoing simplification of backend drivers would be welcome.

Rich

> 
>>>> Aside from the udev kicks though, I still think the hotplug-status/ring
>>> state interaction is just bogus anyway. As I said in a previous thread,
>>> the hotplug-status ought to be indicated as carrier status, if at all, so
>>> I still think all that code ought to go.
>>> I agree regarding the future interface, but with the carrier state just
>>> being in the plans to be added now, it is clearly too early to remove
>>> the code with that reasoning.
>> I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.
> 
> If it is being used that way we need to continue supporting it.
> 
> 
> Juergen
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

[Rich Persaud]

> On Dec 16, 2019, at 03:31, Jürgen Groß <jgross@suse.com> wrote:
> 
> On 16.12.19 09:18, Durrant, Paul wrote:
>>> -----Original Message-----
>>> From: Jürgen Groß <jgross@suse.com>
>>> Sent: 16 December 2019 08:10
>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>> <davem@davemloft.net>
>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
>>> related code
>>>> On 13.12.19 11:12, Durrant, Paul wrote:
>>>>>> -----Original Message-----
>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>> Sent: 13 December 2019 10:02
>>>>>> To: Durrant, Paul <pdurrant@amazon.com>; David Miller
>>>>>> <davem@davemloft.net>
>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>> udev
>>>>> related code
>>>>> On 13.12.19 10:24, Durrant, Paul wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Jürgen Groß <jgross@suse.com>
>>>>>>> Sent: 13 December 2019 05:41
>>>>>>> To: David Miller <davem@davemloft.net>; Durrant, Paul
>>>>>>> <pdurrant@amazon.com>
>>>>>>> Cc: xen-devel@lists.xenproject.org; wei.liu@kernel.org; linux-
>>>>>>> kernel@vger.kernel.org; netdev@vger.kernel.org
>>>>>>> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old
>>>>> udev
>>>>>>> related code
>>>>>>>> On 12.12.19 20:05, David Miller wrote:
>>>>>>>>> From: Paul Durrant <pdurrant@amazon.com>
>>>>>>>>> Date: Thu, 12 Dec 2019 13:54:06 +0000
>>>>>>>>>> In the past it used to be the case that the Xen toolstack relied
>>>> upon
>>>>>>>>>> udev to execute backend hotplug scripts. However this has not been
>>>>>> the
>>>>>>>>>> case for many releases now and removal of the associated code in
>>>>>>>>>> xen-netback shortens the source by more than 100 lines, and removes
>>>>>>>> much
>>>>>>>>>> complexity in the interaction with the xenstore backend state.
>>>>>>>>>> NOTE: xen-netback is the only xenbus driver to have a functional
>>>>>>>> uevent()
>>>>>>>>>>         method. The only other driver to have a method at all is
>>>>>>>>>>         pvcalls-back, and currently pvcalls_back_uevent() simply
>>>>>> returns
>>>>>>>> 0.
>>>>>>>>>>         Hence this patch also facilitates further cleanup.
>>>>>>>>>> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
>>>>>>>>> If userspace ever used this stuff, I seriously doubt you can remove
>>>>>> this
>>>>>>>>> even if it hasn't been used in 5+ years.
>>>>>>>> Hmm, depends.
>>>>>>>> This has been used by Xen tools in dom0 only. If the last usage has
>>>>>> been
>>>>>>>> in a Xen version which is no longer able to run with current Linux in
>>>>>>>> dom0 it could be removed. But I guess this would have to be a rather
>>>>>> old
>>>>>>>> version of Xen (like 3.x?).
>>>>>>>> Paul, can you give a hint since which Xen version the toolstack no
>>>>>>>> longer relies on udev to start the hotplug scripts?
>>>>>>> The udev rules were in a file called tools/hotplug/Linux/xen-
>>>>>> backend.rules (in xen.git), and a commit from Roger removed the NIC
>>>> rules
>>>>>> in 2012:
>>>>>>> commit 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53
>>>>>> Xen 4.2
>>>>>>> The last commit I could find to that file modified its name to xen-
>>>>>> backend.rules.in, and this was finally removed by George in 2015:
>>>>>>> commit 2ba368d13893402b2f1fb3c283ddcc714659dd9b
>>>>>> Xen 4.6
>>>>>>> So, I think this means anyone using a version of the Xen tools within
>>>>>> recent memory will be having their hotplug scripts called directly by
>>>>>> libxl (and having udev rules present would actually be counter-
>>>> productive,
>>>>>> as George's commit states and as I discovered the hard way when the
>>>> change
>>>>>> was originally made).
>>>>>> The problem are systems with either old Xen versions (before Xen 4.2)
>>>> or
>>>>>> with other toolstacks (e.g. Xen 4.4 with xend) which want to use a new
>>>>>> dom0 kernel.
>>>>>> And I'm not sure there aren't such systems (especially in case someone
>>>>>> wants to stick with xend).
>>>>> But would someone sticking with such an old toolstack expect to run on
>>>> an unmodified upstream dom0? There has to be some way in which we can
>>>> retire old code.
>>>> As long as there are no hypervisor interface related issues
>>>> prohibiting running dom0 unmodified I think the expectation to be
>>>> able to use the kernel in that environment is fine.
>> I think we need a better policy in future then otherwise we will only collect baggage.
> 
> The Linux kernel policy regarding user interfaces and existing use cases
> is rather clear and we should not deviate without very strong reasons.
> 
>>> Another question coming up would be: how is this handled in a driver
>>> domain running netback? Which component is starting the hotplug script
>>> there? I don't think we can assume a standard Xen toolset in this case.
>>> So I'd rather leave this code as it is instead of breaking some rare
>>> but valid use cases.
>> I am not sure there is a standard. Do we 'support' driver domains with any sort of tools API or do they really just have to notice things via xenstore? I agree Linux running as a driver domain could indeed use udev.
> 
> I intend in no way to break projects like Qubes. Disaggregation is
> one of the very big advantages of Xen over KVM, Hyper-V and VMWare.
> We should not give that up "just to get rid of some code". Period.

From a quick poll of Qubes and OpenXT netback usage: neither project would be impacted by the proposed change.  Qubes uses xl devd to call hotplug scripts.  OpenXT uses uevent triggers, but does not rely on any script provided in the uevent.  Ongoing simplification of backend drivers would be welcome.

Rich

> 
>>>> Aside from the udev kicks though, I still think the hotplug-status/ring
>>> state interaction is just bogus anyway. As I said in a previous thread,
>>> the hotplug-status ought to be indicated as carrier status, if at all, so
>>> I still think all that code ought to go.
>>> I agree regarding the future interface, but with the carrier state just
>>> being in the plans to be added now, it is clearly too early to remove
>>> the code with that reasoning.
>> I don't think so. Like I said, I think the hotplug status has nothing to do with the state of the shared ring. Even with the code as-is, nothing informs the frontend if the netif is subsequently closed or re-plumbed, so why must we continue to maintain this code? AFAICT it is just not fit for purpose.
> 
> If it is being used that way we need to continue supporting it.
> 
> 
> Juergen
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel