From: "Durrant, Paul" <pdurrant@amazon.com> To: "jandryuk@gmail.com" <jandryuk@gmail.com> Cc: xen-devel <xen-devel@lists.xenproject.org>, "netdev@vger.kernel.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 Date: Thu, 12 Dec 2019 16:45:15 +0000 [thread overview] Message-ID: <34de94e87020467fac84434194809894@EX13D32EUC003.ant.amazon.com> (raw) In-Reply-To: <CAKf6xptNRAuvjqzqFwbPmetYsTdPOMgTT0AWEouwjsHq1iCV6w@mail.gmail.com> > -----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
WARNING: multiple messages have this Message-ID (diff)
From: "Durrant, Paul" <pdurrant@amazon.com> To: "jandryuk@gmail.com" <jandryuk@gmail.com> Cc: xen-devel <xen-devel@lists.xenproject.org>, Wei Liu <wei.liu@kernel.org>, "David S. Miller" <davem@davemloft.net>, open list <linux-kernel@vger.kernel.org>, "netdev@vger.kernel.org" <netdev@vger.kernel.org> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code Date: Thu, 12 Dec 2019 16:45:15 +0000 [thread overview] Message-ID: <34de94e87020467fac84434194809894@EX13D32EUC003.ant.amazon.com> (raw) In-Reply-To: <CAKf6xptNRAuvjqzqFwbPmetYsTdPOMgTT0AWEouwjsHq1iCV6w@mail.gmail.com> > -----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
next prev parent reply other threads:[~2019-12-12 16:45 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-12 13:54 [PATCH net-next] xen-netback: get rid of old udev related code Paul Durrant 2019-12-12 13:54 ` [Xen-devel] " Paul Durrant 2019-12-12 16:31 ` Jason Andryuk 2019-12-12 16:31 ` Jason Andryuk 2019-12-12 16:45 ` Durrant, Paul [this message] 2019-12-12 16:45 ` Durrant, Paul 2019-12-12 19:05 ` David Miller 2019-12-12 19:05 ` [Xen-devel] " David Miller 2019-12-13 5:40 ` Jürgen Groß 2019-12-13 5:40 ` Jürgen Groß 2019-12-13 9:24 ` Durrant, Paul 2019-12-13 9:24 ` Durrant, Paul 2019-12-13 10:02 ` Jürgen Groß 2019-12-13 10:02 ` Jürgen Groß 2019-12-13 10:12 ` Durrant, Paul 2019-12-13 10:12 ` Durrant, Paul 2019-12-16 8:10 ` Jürgen Groß 2019-12-16 8:10 ` Jürgen Groß 2019-12-16 8:18 ` Durrant, Paul 2019-12-16 8:18 ` Durrant, Paul 2019-12-16 8:29 ` Jürgen Groß 2019-12-16 8:29 ` Jürgen Groß 2020-01-09 13:55 ` Rich Persaud 2020-01-09 13:55 ` Rich Persaud
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=34de94e87020467fac84434194809894@EX13D32EUC003.ant.amazon.com \ --to=pdurrant@amazon.com \ --cc=davem@davemloft.net \ --cc=jandryuk@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=wei.liu@kernel.org \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.