From: "Durrant, Paul" <firstname.lastname@example.org> To: "Marek Marczykowski-Górecki" <email@example.com> Cc: Michael Brown <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org> Subject: RE: [PATCH] xen-netback: Check for hotplug-status existence before watching Date: Tue, 11 May 2021 12:46:38 +0000 [thread overview] Message-ID: <9edd6873034f474baafd70b1df693001@EX13D32EUC003.ant.amazon.com> (raw) In-Reply-To: <YJpgNvOmDL9SuRye@mail-itl> > -----Original Message----- > From: Marek Marczykowski-Górecki <email@example.com> > Sent: 11 May 2021 11:45 > To: Durrant, Paul <firstname.lastname@example.org> > Cc: Michael Brown <email@example.com>; firstname.lastname@example.org; email@example.com; > firstname.lastname@example.org; email@example.com > Subject: RE: [EXTERNAL] [PATCH] xen-netback: Check for hotplug-status existence before watching > > On Tue, May 11, 2021 at 12:40:54PM +0200, Marek Marczykowski-Górecki wrote: > > On Tue, May 11, 2021 at 07:06:55AM +0000, Durrant, Paul wrote: > > > > -----Original Message----- > > > > From: Marek Marczykowski-Górecki <firstname.lastname@example.org> > > > > Sent: 10 May 2021 20:43 > > > > To: Michael Brown <email@example.com>; firstname.lastname@example.org > > > > Cc: email@example.com; firstname.lastname@example.org; email@example.com; firstname.lastname@example.org; > Durrant, > > > > Paul <email@example.com> > > > > Subject: RE: [EXTERNAL] [PATCH] xen-netback: Check for hotplug-status existence before watching > > > > > > > > On Mon, May 10, 2021 at 08:06:55PM +0100, Michael Brown wrote: > > > > > If you have a suggested patch, I'm happy to test that it doesn't reintroduce > > > > > the regression bug that was fixed by this commit. > > > > > > > > Actually, I've just tested with a simple reloading xen-netfront module. It > > > > seems in this case, the hotplug script is not re-executed. In fact, I > > > > think it should not be re-executed at all, since the vif interface > > > > remains in place (it just gets NO-CARRIER flag). > > > > > > > > This brings a question, why removing hotplug-status in the first place? > > > > The interface remains correctly configured by the hotplug script after > > > > all. From the commit message: > > > > > > > > xen-netback: remove 'hotplug-status' once it has served its purpose > > > > > > > > Removing the 'hotplug-status' node in netback_remove() is wrong; the script > > > > may not have completed. Only remove the node once the watch has fired and > > > > has been unregistered. > > > > > > > > I think the intention was to remove 'hotplug-status' node _later_ in > > > > case of quickly adding and removing the interface. Is that right, Paul? > > > > > > The removal was done to allow unbind/bind to function correctly. IIRC before the original patch > doing a bind would stall forever waiting for the hotplug status to change, which would never happen. > > > > Hmm, in that case maybe don't remove it at all in the backend, and let > > it be cleaned up by the toolstack, when it removes other backend-related > > nodes? > > No, unbind/bind _does_ require hotplug script to be called again. > Yes, sorry I was misremembering. My memory is hazy but there was definitely a problem at the time with leaving the node in place. > When exactly vif interface appears in the system (starts to be available > for the hotplug script)? Maybe remove 'hotplug-status' just before that > point? > I really can't remember any detail. Perhaps try reverting both patches then and check that the unbind/rmmod/modprobe/bind sequence still works (and the backend actually makes it into connected state). Paul
next prev parent reply other threads:[~2021-05-11 12:47 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-10 18:25 xen-netback hotplug-status regression bug Michael Brown 2021-04-13 7:12 ` Paul Durrant 2021-04-13 10:48 ` Michael Brown 2021-04-13 10:55 ` Paul Durrant 2021-04-13 15:14 ` Michael Brown 2021-04-13 15:25 ` [PATCH] xen-netback: Check for hotplug-status existence before watching Michael Brown 2021-04-13 19:12 ` Paul Durrant 2021-04-13 22:30 ` patchwork-bot+netdevbpf 2021-05-10 18:32 ` Marek Marczykowski-Górecki 2021-05-10 18:47 ` Michael Brown 2021-05-10 18:53 ` Marek Marczykowski-Górecki 2021-05-10 19:06 ` Michael Brown 2021-05-10 19:42 ` Marek Marczykowski-Górecki 2021-05-11 7:06 ` Durrant, Paul 2021-05-11 10:40 ` Marek Marczykowski-Górecki 2021-05-11 10:45 ` Marek Marczykowski-Górecki 2021-05-11 12:46 ` Durrant, Paul [this message] 2021-05-17 21:43 ` Marek Marczykowski-Górecki 2021-05-17 21:51 ` Michael Brown 2021-05-17 21:58 ` Marek Marczykowski-Górecki 2021-05-18 6:57 ` Paul Durrant 2021-05-18 9:18 ` Marek Marczykowski-Górecki [not found] ` <887f9533f5c54bfabfbff7231eb99b08@EX13D32EUC003.ant.amazon.com> [not found] ` <YKOMpXwcnr9QiXy8@mail-itl> [not found] ` <2c23e102b6254e42877eb1e8fe68a4f7@EX13D32EUC003.ant.amazon.com> 2021-05-18 10:42 ` Marek Marczykowski-Górecki
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=9edd6873034f474baafd70b1df693001@EX13D32EUC003.ant.amazon.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='RE: [PATCH] xen-netback: Check for hotplug-status existence before watching' \ /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: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).