* Race condition on device add hanling in xl devd @ 2018-12-16 1:47 Marek Marczykowski-Górecki 2018-12-17 9:40 ` Roger Pau Monné 0 siblings, 1 reply; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2018-12-16 1:47 UTC (permalink / raw) To: xen-devel [-- Attachment #1.1: Type: text/plain, Size: 1459 bytes --] Hi, I've found a race condition with handling new devices in driver domain. xl devd calls hotplug script when new device is detected in xenstore. At the same time, asynchronously, kernel create actual backend device (vif in my case). In rare circumstances (especially under high system load) it may happen that hotplug script is executed before kernel create the device, and the hotplug script fails. When hotplug scripts were called by udev, that race didn't existed as udev was informed about the device by the kernel. I'm not sure if the race applies to backend in dom0 - haven't happened to me, but that doesn't really prove anything. Can you remind me why in driver domain xl devd is used now, instead of udev? A workaround could be implemented in hotplug script itself - wait for the device there. I'm not sure how proper solution could look like. Some synchronization between xl devd and the kernel (like xl devd monitoring uevents)? The setup: - Xen 4.8.4, but I believe the same would happen in xen-unstable - Linux 4.19.2 (dom0), Linux 4.14.74 (domU) - problem happens when starting a domU with network backend in another domU - happen more often when Xen run nested in KVM (-> slow), but happened to me on bare metal too -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-16 1:47 Race condition on device add hanling in xl devd Marek Marczykowski-Górecki @ 2018-12-17 9:40 ` Roger Pau Monné 2018-12-17 12:00 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 9:40 UTC (permalink / raw) To: Marek Marczykowski-Górecki; +Cc: xen-devel On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > Hi, > > I've found a race condition with handling new devices in driver domain. > xl devd calls hotplug script when new device is detected in xenstore. At > the same time, asynchronously, kernel create actual backend device (vif > in my case). In rare circumstances (especially under high system load) > it may happen that hotplug script is executed before kernel create the > device, and the hotplug script fails. When hotplug scripts were called > by udev, that race didn't existed as udev was informed about the device > by the kernel. > I'm not sure if the race applies to backend in dom0 - haven't happened > to me, but that doesn't really prove anything. > > Can you remind me why in driver domain xl devd is used now, instead of > udev? udev is Linux specific, while the current code works for Linux, NetBSD and FreeBSD. > > A workaround could be implemented in hotplug script itself - wait for > the device there. I'm not sure how proper solution could look like. Some > synchronization between xl devd and the kernel (like xl devd monitoring > uevents)? There's already a synchronization mechanism, libxl waits for the backend to switch to state 2 (XenbusStateInitWait) before running the hotplug scripts [0]. Maybe netback sets state 2 before creating the backend device? It looks to me like the backend needs to be sure everything needed by the hotplug script is in place before switching to state 2. Thanks, Roger. [0] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_device.c;h=a4a8e9ac323e9d3804d36573181c74b7b5c63bc6;hb=refs/heads/staging#l934 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 9:40 ` Roger Pau Monné @ 2018-12-17 12:00 ` Marek Marczykowski-Górecki 2018-12-17 12:18 ` Roger Pau Monné 0 siblings, 1 reply; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2018-12-17 12:00 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel [-- Attachment #1.1: Type: text/plain, Size: 2305 bytes --] On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > Hi, > > > > I've found a race condition with handling new devices in driver domain. > > xl devd calls hotplug script when new device is detected in xenstore. At > > the same time, asynchronously, kernel create actual backend device (vif > > in my case). In rare circumstances (especially under high system load) > > it may happen that hotplug script is executed before kernel create the > > device, and the hotplug script fails. When hotplug scripts were called > > by udev, that race didn't existed as udev was informed about the device > > by the kernel. > > I'm not sure if the race applies to backend in dom0 - haven't happened > > to me, but that doesn't really prove anything. > > > > Can you remind me why in driver domain xl devd is used now, instead of > > udev? > > udev is Linux specific, while the current code works for Linux, NetBSD > and FreeBSD. > > > > > A workaround could be implemented in hotplug script itself - wait for > > the device there. I'm not sure how proper solution could look like. Some > > synchronization between xl devd and the kernel (like xl devd monitoring > > uevents)? > > There's already a synchronization mechanism, libxl waits for the > backend to switch to state 2 (XenbusStateInitWait) before running the > hotplug scripts [0]. > > Maybe netback sets state 2 before creating the backend device? > > It looks to me like the backend needs to be sure everything needed by > the hotplug script is in place before switching to state 2. I've done some more tests and I think that's something else. I've added a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed out (5s). I don't see _any_ kernel messages related to the device. It may be some bug in nested virtualization in KVM... > Thanks, Roger. > > [0] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/libxl/libxl_device.c;h=a4a8e9ac323e9d3804d36573181c74b7b5c63bc6;hb=refs/heads/staging#l934 -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 12:00 ` Marek Marczykowski-Górecki @ 2018-12-17 12:18 ` Roger Pau Monné 2018-12-17 12:23 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 12:18 UTC (permalink / raw) To: Marek Marczykowski-Górecki; +Cc: xen-devel On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > > A workaround could be implemented in hotplug script itself - wait for > > > the device there. I'm not sure how proper solution could look like. Some > > > synchronization between xl devd and the kernel (like xl devd monitoring > > > uevents)? > > > > There's already a synchronization mechanism, libxl waits for the > > backend to switch to state 2 (XenbusStateInitWait) before running the > > hotplug scripts [0]. > > > > Maybe netback sets state 2 before creating the backend device? > > > > It looks to me like the backend needs to be sure everything needed by > > the hotplug script is in place before switching to state 2. > > I've done some more tests and I think that's something else. I've added > a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed > out (5s). I don't see _any_ kernel messages related to the device. > > It may be some bug in nested virtualization in KVM... In your message you said you have also observed this behavior when running on bare metal, so it's likely not related to nested virtualization? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 12:18 ` Roger Pau Monné @ 2018-12-17 12:23 ` Marek Marczykowski-Górecki 2018-12-17 13:05 ` Roger Pau Monné 0 siblings, 1 reply; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2018-12-17 12:23 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel [-- Attachment #1.1: Type: text/plain, Size: 1801 bytes --] On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki wrote: > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > > > A workaround could be implemented in hotplug script itself - wait for > > > > the device there. I'm not sure how proper solution could look like. Some > > > > synchronization between xl devd and the kernel (like xl devd monitoring > > > > uevents)? > > > > > > There's already a synchronization mechanism, libxl waits for the > > > backend to switch to state 2 (XenbusStateInitWait) before running the > > > hotplug scripts [0]. > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > It looks to me like the backend needs to be sure everything needed by > > > the hotplug script is in place before switching to state 2. > > > > I've done some more tests and I think that's something else. I've added > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed > > out (5s). I don't see _any_ kernel messages related to the device. > > > > It may be some bug in nested virtualization in KVM... > > In your message you said you have also observed this behavior when > running on bare metal, so it's likely not related to nested > virtualization? Yes, but on bare metal is so hard to reproduce (like 0.1% or even less startups), I'm not really sure if that was the same problem, as the problem doesn't leave that much logs... -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 12:23 ` Marek Marczykowski-Górecki @ 2018-12-17 13:05 ` Roger Pau Monné 2018-12-17 13:11 ` Paul Durrant 2018-12-17 13:23 ` Marek Marczykowski-Górecki 0 siblings, 2 replies; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 13:05 UTC (permalink / raw) To: Marek Marczykowski-Górecki; +Cc: xen-devel, Paul Durrant, Wei Liu On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki wrote: > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > > > > A workaround could be implemented in hotplug script itself - wait for > > > > > the device there. I'm not sure how proper solution could look like. Some > > > > > synchronization between xl devd and the kernel (like xl devd monitoring > > > > > uevents)? > > > > > > > > There's already a synchronization mechanism, libxl waits for the > > > > backend to switch to state 2 (XenbusStateInitWait) before running the > > > > hotplug scripts [0]. > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > It looks to me like the backend needs to be sure everything needed by > > > > the hotplug script is in place before switching to state 2. > > > > > > I've done some more tests and I think that's something else. I've added > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed > > > out (5s). I don't see _any_ kernel messages related to the device. > > > > > > It may be some bug in nested virtualization in KVM... > > > > In your message you said you have also observed this behavior when > > running on bare metal, so it's likely not related to nested > > virtualization? > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even less > startups), I'm not really sure if that was the same problem, as the > problem doesn't leave that much logs... I'm not very familiar with netback, but I think it's indeed possible for netback to switch to state 2 without having created the vif. Netback switching from state 1 -> 2 seems to be solely controlled by the frontend state (see frontend_changed). I think the patch below could solve this issue, but I haven't even compile tested it, could you give it a spin? I would also like to hear the opinion of netback maintainers, since I might be completely wrong. Thanks, Roger. ---8<--- diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c index cd51492ae6c2..791c2c0b788f 100644 --- a/drivers/net/xen-netback/xenbus.c +++ b/drivers/net/xen-netback/xenbus.c @@ -427,6 +427,10 @@ static int netback_probe(struct xenbus_device *dev, if (err) goto fail; + err = xenbus_switch_state(dev, XenbusStateInitWait); + if (err) + goto fail; + return 0; abort_transaction: @@ -650,7 +654,10 @@ static void frontend_changed(struct xenbus_device *dev, switch (frontend_state) { case XenbusStateInitialising: - set_backend_state(be, XenbusStateInitWait); + if (dev->state == XenbusStateClosed) { + pr_info("%s: prepare for reconnect\n", dev->nodename); + set_backend_state(be, XenbusStateInitWait); + } break; case XenbusStateInitialised: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 13:05 ` Roger Pau Monné @ 2018-12-17 13:11 ` Paul Durrant 2018-12-17 14:32 ` Roger Pau Monné 2018-12-17 13:23 ` Marek Marczykowski-Górecki 1 sibling, 1 reply; 15+ messages in thread From: Paul Durrant @ 2018-12-17 13:11 UTC (permalink / raw) To: Roger Pau Monne, Marek Marczykowski-Górecki; +Cc: xen-devel, Wei Liu > -----Original Message----- > From: Roger Pau Monne > Sent: 17 December 2018 13:06 > To: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > Cc: xen-devel <xen-devel@lists.xenproject.org>; Wei Liu > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com> > Subject: Re: [Xen-devel] Race condition on device add hanling in xl devd > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki > wrote: > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki > wrote: > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski- > Górecki wrote: > > > > > > A workaround could be implemented in hotplug script itself - > wait for > > > > > > the device there. I'm not sure how proper solution could look > like. Some > > > > > > synchronization between xl devd and the kernel (like xl devd > monitoring > > > > > > uevents)? > > > > > > > > > > There's already a synchronization mechanism, libxl waits for the > > > > > backend to switch to state 2 (XenbusStateInitWait) before running > the > > > > > hotplug scripts [0]. > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > It looks to me like the backend needs to be sure everything needed > by > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > I've done some more tests and I think that's something else. I've > added > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it > timed > > > > out (5s). I don't see _any_ kernel messages related to the device. > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > In your message you said you have also observed this behavior when > > > running on bare metal, so it's likely not related to nested > > > virtualization? > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even less > > startups), I'm not really sure if that was the same problem, as the > > problem doesn't leave that much logs... > > I'm not very familiar with netback, but I think it's indeed possible > for netback to switch to state 2 without having created the vif. > Netback switching from state 1 -> 2 seems to be solely controlled by > the frontend state (see frontend_changed). > > I think the patch below could solve this issue, but I haven't even > compile tested it, could you give it a spin? > > I would also like to hear the opinion of netback maintainers, since I > might be completely wrong. IIRC there is a good reason why netback doesn't want the hotplug script to run before moving into state 2... the script adds the vif to the bridge and, if this is done on the 1 -> 2 transition then you may end up with a load of vifs sat on the bridge for which there is no frontend (at least yet, but maybe never)... so the bridge wastes time in every packet sent to such a vif. Paul > > Thanks, Roger. > ---8<--- > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen- > netback/xenbus.c > index cd51492ae6c2..791c2c0b788f 100644 > --- a/drivers/net/xen-netback/xenbus.c > +++ b/drivers/net/xen-netback/xenbus.c > @@ -427,6 +427,10 @@ static int netback_probe(struct xenbus_device *dev, > if (err) > goto fail; > > + err = xenbus_switch_state(dev, XenbusStateInitWait); > + if (err) > + goto fail; > + > return 0; > > abort_transaction: > @@ -650,7 +654,10 @@ static void frontend_changed(struct xenbus_device > *dev, > > switch (frontend_state) { > case XenbusStateInitialising: > - set_backend_state(be, XenbusStateInitWait); > + if (dev->state == XenbusStateClosed) { > + pr_info("%s: prepare for reconnect\n", dev->nodename); > + set_backend_state(be, XenbusStateInitWait); > + } > break; > > case XenbusStateInitialised: _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 13:11 ` Paul Durrant @ 2018-12-17 14:32 ` Roger Pau Monné 2018-12-17 14:42 ` Paul Durrant 0 siblings, 1 reply; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 14:32 UTC (permalink / raw) To: Paul Durrant; +Cc: xen-devel, Wei Liu, Marek Marczykowski-Górecki On Mon, Dec 17, 2018 at 01:11:11PM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Roger Pau Monne > > Sent: 17 December 2018 13:06 > > To: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Wei Liu > > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com> > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl devd > > > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki > > wrote: > > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki > > wrote: > > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski- > > Górecki wrote: > > > > > > > A workaround could be implemented in hotplug script itself - > > wait for > > > > > > > the device there. I'm not sure how proper solution could look > > like. Some > > > > > > > synchronization between xl devd and the kernel (like xl devd > > monitoring > > > > > > > uevents)? > > > > > > > > > > > > There's already a synchronization mechanism, libxl waits for the > > > > > > backend to switch to state 2 (XenbusStateInitWait) before running > > the > > > > > > hotplug scripts [0]. > > > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > > > It looks to me like the backend needs to be sure everything needed > > by > > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > > > I've done some more tests and I think that's something else. I've > > added > > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it > > timed > > > > > out (5s). I don't see _any_ kernel messages related to the device. > > > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > > > In your message you said you have also observed this behavior when > > > > running on bare metal, so it's likely not related to nested > > > > virtualization? > > > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even less > > > startups), I'm not really sure if that was the same problem, as the > > > problem doesn't leave that much logs... > > > > I'm not very familiar with netback, but I think it's indeed possible > > for netback to switch to state 2 without having created the vif. > > Netback switching from state 1 -> 2 seems to be solely controlled by > > the frontend state (see frontend_changed). > > > > I think the patch below could solve this issue, but I haven't even > > compile tested it, could you give it a spin? > > > > I would also like to hear the opinion of netback maintainers, since I > > might be completely wrong. > > IIRC there is a good reason why netback doesn't want the hotplug script to run before moving into state 2... the script adds the vif to the bridge and, if this is done on the 1 -> 2 transition then you may end up with a load of vifs sat on the bridge for which there is no frontend (at least yet, but maybe never)... so the bridge wastes time in every packet sent to such a vif. I don't think netback has ever waited for a frontend before running hotplug scripts. In the udev times the hotplug script would be run upon vif creation, which happens in netback_probe, and when launching hotplug scripts from libxl the script is executed when the backend changes to state 2, which happens almost immediately because netback switches to state 2 when the frotnend is in state 1 which is the initial frontend state. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 14:32 ` Roger Pau Monné @ 2018-12-17 14:42 ` Paul Durrant 2018-12-17 16:09 ` Roger Pau Monné 0 siblings, 1 reply; 15+ messages in thread From: Paul Durrant @ 2018-12-17 14:42 UTC (permalink / raw) To: Roger Pau Monne; +Cc: xen-devel, Wei Liu, Marek Marczykowski-Górecki > -----Original Message----- > From: Roger Pau Monne > Sent: 17 December 2018 14:32 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>; xen- > devel <xen-devel@lists.xenproject.org>; Wei Liu <wei.liu2@citrix.com> > Subject: Re: [Xen-devel] Race condition on device add hanling in xl devd > > On Mon, Dec 17, 2018 at 01:11:11PM +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Roger Pau Monne > > > Sent: 17 December 2018 13:06 > > > To: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Wei Liu > > > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com> > > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl > devd > > > > > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki > > > wrote: > > > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski- > Górecki > > > wrote: > > > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski- > > > Górecki wrote: > > > > > > > > A workaround could be implemented in hotplug script itself - > > > wait for > > > > > > > > the device there. I'm not sure how proper solution could > look > > > like. Some > > > > > > > > synchronization between xl devd and the kernel (like xl devd > > > monitoring > > > > > > > > uevents)? > > > > > > > > > > > > > > There's already a synchronization mechanism, libxl waits for > the > > > > > > > backend to switch to state 2 (XenbusStateInitWait) before > running > > > the > > > > > > > hotplug scripts [0]. > > > > > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > > > > > It looks to me like the backend needs to be sure everything > needed > > > by > > > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > > > > > I've done some more tests and I think that's something else. > I've > > > added > > > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but > it > > > timed > > > > > > out (5s). I don't see _any_ kernel messages related to the > device. > > > > > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > > > > > In your message you said you have also observed this behavior when > > > > > running on bare metal, so it's likely not related to nested > > > > > virtualization? > > > > > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even > less > > > > startups), I'm not really sure if that was the same problem, as the > > > > problem doesn't leave that much logs... > > > > > > I'm not very familiar with netback, but I think it's indeed possible > > > for netback to switch to state 2 without having created the vif. > > > Netback switching from state 1 -> 2 seems to be solely controlled by > > > the frontend state (see frontend_changed). > > > > > > I think the patch below could solve this issue, but I haven't even > > > compile tested it, could you give it a spin? > > > > > > I would also like to hear the opinion of netback maintainers, since I > > > might be completely wrong. > > > > IIRC there is a good reason why netback doesn't want the hotplug script > to run before moving into state 2... the script adds the vif to the bridge > and, if this is done on the 1 -> 2 transition then you may end up with a > load of vifs sat on the bridge for which there is no frontend (at least > yet, but maybe never)... so the bridge wastes time in every packet sent to > such a vif. > > I don't think netback has ever waited for a frontend before running > hotplug scripts. > > In the udev times the hotplug script would be run upon vif creation, > which happens in netback_probe, and when launching hotplug scripts > from libxl the script is executed when the backend changes to state 2, > which happens almost immediately because netback switches to state 2 > when the frotnend is in state 1 which is the initial frontend state. > I suspect I must be remembering a XenServer-specific hack^Wpatch then. I'd have to dig... it's been a while since I messed with the netif state model, which is of course different the blkif state model. Paul > Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 14:42 ` Paul Durrant @ 2018-12-17 16:09 ` Roger Pau Monné 2019-02-24 23:14 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 16:09 UTC (permalink / raw) To: Paul Durrant; +Cc: xen-devel, Wei Liu, Marek Marczykowski-Górecki On Mon, Dec 17, 2018 at 02:42:23PM +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Roger Pau Monne > > Sent: 17 December 2018 14:32 > > To: Paul Durrant <Paul.Durrant@citrix.com> > > Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>; xen- > > devel <xen-devel@lists.xenproject.org>; Wei Liu <wei.liu2@citrix.com> > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl devd > > > > On Mon, Dec 17, 2018 at 01:11:11PM +0000, Paul Durrant wrote: > > > > -----Original Message----- > > > > From: Roger Pau Monne > > > > Sent: 17 December 2018 13:06 > > > > To: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Wei Liu > > > > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com> > > > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl > > devd > > > > > > > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki > > > > wrote: > > > > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski- > > Górecki > > > > wrote: > > > > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski- > > > > Górecki wrote: > > > > > > > > > A workaround could be implemented in hotplug script itself - > > > > wait for > > > > > > > > > the device there. I'm not sure how proper solution could > > look > > > > like. Some > > > > > > > > > synchronization between xl devd and the kernel (like xl devd > > > > monitoring > > > > > > > > > uevents)? > > > > > > > > > > > > > > > > There's already a synchronization mechanism, libxl waits for > > the > > > > > > > > backend to switch to state 2 (XenbusStateInitWait) before > > running > > > > the > > > > > > > > hotplug scripts [0]. > > > > > > > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > > > > > > > It looks to me like the backend needs to be sure everything > > needed > > > > by > > > > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > > > > > > > I've done some more tests and I think that's something else. > > I've > > > > added > > > > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but > > it > > > > timed > > > > > > > out (5s). I don't see _any_ kernel messages related to the > > device. > > > > > > > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > > > > > > > In your message you said you have also observed this behavior when > > > > > > running on bare metal, so it's likely not related to nested > > > > > > virtualization? > > > > > > > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even > > less > > > > > startups), I'm not really sure if that was the same problem, as the > > > > > problem doesn't leave that much logs... > > > > > > > > I'm not very familiar with netback, but I think it's indeed possible > > > > for netback to switch to state 2 without having created the vif. > > > > Netback switching from state 1 -> 2 seems to be solely controlled by > > > > the frontend state (see frontend_changed). > > > > > > > > I think the patch below could solve this issue, but I haven't even > > > > compile tested it, could you give it a spin? > > > > > > > > I would also like to hear the opinion of netback maintainers, since I > > > > might be completely wrong. > > > > > > IIRC there is a good reason why netback doesn't want the hotplug script > > to run before moving into state 2... the script adds the vif to the bridge > > and, if this is done on the 1 -> 2 transition then you may end up with a > > load of vifs sat on the bridge for which there is no frontend (at least > > yet, but maybe never)... so the bridge wastes time in every packet sent to > > such a vif. > > > > I don't think netback has ever waited for a frontend before running > > hotplug scripts. > > > > In the udev times the hotplug script would be run upon vif creation, > > which happens in netback_probe, and when launching hotplug scripts > > from libxl the script is executed when the backend changes to state 2, > > which happens almost immediately because netback switches to state 2 > > when the frotnend is in state 1 which is the initial frontend state. > > > > I suspect I must be remembering a XenServer-specific hack^Wpatch then. I'd have to dig... it's been a while since I messed with the netif state model, which is of course different the blkif state model. Quite likely. With udev scripts is was feasible to only execute hotplug scripts for vifs with an attached frontend. With libxl this is not possible, since hotplug scripts are run during domain creation, at which point the guest is completely paused. I'm not that familiar with bridges and vifs, but maybe the vifs status can be set to offline until there's a frontend attached in order to reduce the bridge distributor load? (if that's not already the case). Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 16:09 ` Roger Pau Monné @ 2019-02-24 23:14 ` Marek Marczykowski-Górecki 2019-02-28 10:08 ` Roger Pau Monné 0 siblings, 1 reply; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2019-02-24 23:14 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel, Paul Durrant, Wei Liu [-- Attachment #1.1: Type: text/plain, Size: 6595 bytes --] On Mon, Dec 17, 2018 at 05:09:19PM +0100, Roger Pau Monné wrote: > On Mon, Dec 17, 2018 at 02:42:23PM +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Roger Pau Monne > > > Sent: 17 December 2018 14:32 > > > To: Paul Durrant <Paul.Durrant@citrix.com> > > > Cc: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>; xen- > > > devel <xen-devel@lists.xenproject.org>; Wei Liu <wei.liu2@citrix.com> > > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl devd > > > > > > On Mon, Dec 17, 2018 at 01:11:11PM +0000, Paul Durrant wrote: > > > > > -----Original Message----- > > > > > From: Roger Pau Monne > > > > > Sent: 17 December 2018 13:06 > > > > > To: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > > > > Cc: xen-devel <xen-devel@lists.xenproject.org>; Wei Liu > > > > > <wei.liu2@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com> > > > > > Subject: Re: [Xen-devel] Race condition on device add hanling in xl > > > devd > > > > > > > > > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki > > > > > wrote: > > > > > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > > > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski- > > > Górecki > > > > > wrote: > > > > > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski- > > > > > Górecki wrote: > > > > > > > > > > A workaround could be implemented in hotplug script itself - > > > > > wait for > > > > > > > > > > the device there. I'm not sure how proper solution could > > > look > > > > > like. Some > > > > > > > > > > synchronization between xl devd and the kernel (like xl devd > > > > > monitoring > > > > > > > > > > uevents)? > > > > > > > > > > > > > > > > > > There's already a synchronization mechanism, libxl waits for > > > the > > > > > > > > > backend to switch to state 2 (XenbusStateInitWait) before > > > running > > > > > the > > > > > > > > > hotplug scripts [0]. > > > > > > > > > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > > > > > > > > > It looks to me like the backend needs to be sure everything > > > needed > > > > > by > > > > > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > > > > > > > > > I've done some more tests and I think that's something else. > > > I've > > > > > added > > > > > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but > > > it > > > > > timed > > > > > > > > out (5s). I don't see _any_ kernel messages related to the > > > device. > > > > > > > > > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > > > > > > > > > In your message you said you have also observed this behavior when > > > > > > > running on bare metal, so it's likely not related to nested > > > > > > > virtualization? > > > > > > > > > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even > > > less > > > > > > startups), I'm not really sure if that was the same problem, as the > > > > > > problem doesn't leave that much logs... > > > > > > > > > > I'm not very familiar with netback, but I think it's indeed possible > > > > > for netback to switch to state 2 without having created the vif. > > > > > Netback switching from state 1 -> 2 seems to be solely controlled by > > > > > the frontend state (see frontend_changed). > > > > > > > > > > I think the patch below could solve this issue, but I haven't even > > > > > compile tested it, could you give it a spin? > > > > > > > > > > I would also like to hear the opinion of netback maintainers, since I > > > > > might be completely wrong. > > > > > > > > IIRC there is a good reason why netback doesn't want the hotplug script > > > to run before moving into state 2... the script adds the vif to the bridge > > > and, if this is done on the 1 -> 2 transition then you may end up with a > > > load of vifs sat on the bridge for which there is no frontend (at least > > > yet, but maybe never)... so the bridge wastes time in every packet sent to > > > such a vif. > > > > > > I don't think netback has ever waited for a frontend before running > > > hotplug scripts. > > > > > > In the udev times the hotplug script would be run upon vif creation, > > > which happens in netback_probe, and when launching hotplug scripts > > > from libxl the script is executed when the backend changes to state 2, > > > which happens almost immediately because netback switches to state 2 > > > when the frotnend is in state 1 which is the initial frontend state. > > > > > > > I suspect I must be remembering a XenServer-specific hack^Wpatch then. I'd have to dig... it's been a while since I messed with the netif state model, which is of course different the blkif state model. > > Quite likely. With udev scripts is was feasible to only execute > hotplug scripts for vifs with an attached frontend. > > With libxl this is not possible, since hotplug scripts are run during > domain creation, at which point the guest is completely paused. > > I'm not that familiar with bridges and vifs, but maybe the vifs status > can be set to offline until there's a frontend attached in order to > reduce the bridge distributor load? (if that's not already the case). I've found was the problem, and with some definition of "race condition" it could be named this way. The problem is that for some reason xenstore watch on device add sometimes does not fire in xl devd. But then, when libxl in dom0 timeouts and remove the device, the xenstore watch in xl devd fire and hotplug script is called. At this point device is already gone, so it fails. xl devd then quickly calls hotplug script the second time, for device removal. I have no idea why this xenstore watch do not fire, but triggering a no-op write into watched path (to trigger the watch again) workarounds the problem. I use a xenstore watch in dom0 for that[1] - which works. I suspect something related to KVM nested virtualization (lost interrupt?)... [1] https://github.com/marmarek/openqa-tests-qubesos/blob/3e604b521eb4d4e1b8ff40ad9e278d63d9a3baa3/extra-files/system-tests/xenstore-watch-trigger.py -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2019-02-24 23:14 ` Marek Marczykowski-Górecki @ 2019-02-28 10:08 ` Roger Pau Monné 2019-02-28 12:38 ` Marek Marczykowski-Górecki 0 siblings, 1 reply; 15+ messages in thread From: Roger Pau Monné @ 2019-02-28 10:08 UTC (permalink / raw) To: Marek Marczykowski-Górecki; +Cc: xen-devel, Paul Durrant, Wei Liu On Mon, Feb 25, 2019 at 12:14:02AM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Dec 17, 2018 at 05:09:19PM +0100, Roger Pau Monné wrote: > > On Mon, Dec 17, 2018 at 02:42:23PM +0000, Paul Durrant wrote: > > > I suspect I must be remembering a XenServer-specific hack^Wpatch then. I'd have to dig... it's been a while since I messed with the netif state model, which is of course different the blkif state model. > > > > Quite likely. With udev scripts is was feasible to only execute > > hotplug scripts for vifs with an attached frontend. > > > > With libxl this is not possible, since hotplug scripts are run during > > domain creation, at which point the guest is completely paused. > > > > I'm not that familiar with bridges and vifs, but maybe the vifs status > > can be set to offline until there's a frontend attached in order to > > reduce the bridge distributor load? (if that's not already the case). > > I've found was the problem, and with some definition of "race condition" > it could be named this way. > The problem is that for some reason xenstore watch on device add > sometimes does not fire in xl devd. But then, when libxl in dom0 > timeouts and remove the device, the xenstore watch in xl devd fire and > hotplug script is called. At this point device is already gone, so > it fails. xl devd then quickly calls hotplug script the second time, for > device removal. > > I have no idea why this xenstore watch do not fire, but triggering a > no-op write into watched path (to trigger the watch again) workarounds > the problem. I use a xenstore watch in dom0 for that[1] - which works. > I suspect something related to KVM nested virtualization (lost > interrupt?)... That's very weird, could you try to run xenstored in dom0 with trace enabled [0] in order to try to figure out what's happening? I assume this only happens when running nested in KVM? Thanks, Roger. [0] http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/hotplug/Linux/launch-xenstore.in _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2019-02-28 10:08 ` Roger Pau Monné @ 2019-02-28 12:38 ` Marek Marczykowski-Górecki 0 siblings, 0 replies; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2019-02-28 12:38 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel, Paul Durrant, Wei Liu [-- Attachment #1.1: Type: text/plain, Size: 2702 bytes --] On Thu, Feb 28, 2019 at 11:08:37AM +0100, Roger Pau Monné wrote: > On Mon, Feb 25, 2019 at 12:14:02AM +0100, Marek Marczykowski-Górecki wrote: > > On Mon, Dec 17, 2018 at 05:09:19PM +0100, Roger Pau Monné wrote: > > > On Mon, Dec 17, 2018 at 02:42:23PM +0000, Paul Durrant wrote: > > > > I suspect I must be remembering a XenServer-specific hack^Wpatch then. I'd have to dig... it's been a while since I messed with the netif state model, which is of course different the blkif state model. > > > > > > Quite likely. With udev scripts is was feasible to only execute > > > hotplug scripts for vifs with an attached frontend. > > > > > > With libxl this is not possible, since hotplug scripts are run during > > > domain creation, at which point the guest is completely paused. > > > > > > I'm not that familiar with bridges and vifs, but maybe the vifs status > > > can be set to offline until there's a frontend attached in order to > > > reduce the bridge distributor load? (if that's not already the case). > > > > I've found was the problem, and with some definition of "race condition" > > it could be named this way. > > The problem is that for some reason xenstore watch on device add > > sometimes does not fire in xl devd. But then, when libxl in dom0 > > timeouts and remove the device, the xenstore watch in xl devd fire and > > hotplug script is called. At this point device is already gone, so > > it fails. xl devd then quickly calls hotplug script the second time, for > > device removal. > > > > I have no idea why this xenstore watch do not fire, but triggering a > > no-op write into watched path (to trigger the watch again) workarounds > > the problem. I use a xenstore watch in dom0 for that[1] - which works. > > I suspect something related to KVM nested virtualization (lost > > interrupt?)... > > That's very weird, could you try to run xenstored in dom0 with trace > enabled [0] in order to try to figure out what's happening? I've tried already, but it was way too slow (remember it's nested KVM, it doesn't really improve the performance). I hit multiple timeouts even without hitting this problem. Unfortunately I don't have logs from that experiment anymore. I can try again... > I assume this only happens when running nested in KVM? I'd say so. I'm not entirely sure, because I've seen similar symptoms on bare metal Xen too in the past, but I think it could be a different problem and also I haven't seen it in past 3 months. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 13:05 ` Roger Pau Monné 2018-12-17 13:11 ` Paul Durrant @ 2018-12-17 13:23 ` Marek Marczykowski-Górecki 2018-12-17 14:44 ` Roger Pau Monné 1 sibling, 1 reply; 15+ messages in thread From: Marek Marczykowski-Górecki @ 2018-12-17 13:23 UTC (permalink / raw) To: Roger Pau Monné; +Cc: xen-devel, Paul Durrant, Wei Liu [-- Attachment #1.1: Type: text/plain, Size: 3549 bytes --] On Mon, Dec 17, 2018 at 02:05:34PM +0100, Roger Pau Monné wrote: > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki wrote: > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki wrote: > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > > > > > A workaround could be implemented in hotplug script itself - wait for > > > > > > the device there. I'm not sure how proper solution could look like. Some > > > > > > synchronization between xl devd and the kernel (like xl devd monitoring > > > > > > uevents)? > > > > > > > > > > There's already a synchronization mechanism, libxl waits for the > > > > > backend to switch to state 2 (XenbusStateInitWait) before running the > > > > > hotplug scripts [0]. > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > It looks to me like the backend needs to be sure everything needed by > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > I've done some more tests and I think that's something else. I've added > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed > > > > out (5s). I don't see _any_ kernel messages related to the device. > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > In your message you said you have also observed this behavior when > > > running on bare metal, so it's likely not related to nested > > > virtualization? > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even less > > startups), I'm not really sure if that was the same problem, as the > > problem doesn't leave that much logs... > > I'm not very familiar with netback, but I think it's indeed possible > for netback to switch to state 2 without having created the vif. > Netback switching from state 1 -> 2 seems to be solely controlled by > the frontend state (see frontend_changed). Isn't frontend_changed guaranteed to be called after netback_probe? > I think the patch below could solve this issue, but I haven't even > compile tested it, could you give it a spin? > > I would also like to hear the opinion of netback maintainers, since I > might be completely wrong. > > Thanks, Roger. > ---8<--- > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c > index cd51492ae6c2..791c2c0b788f 100644 > --- a/drivers/net/xen-netback/xenbus.c > +++ b/drivers/net/xen-netback/xenbus.c > @@ -427,6 +427,10 @@ static int netback_probe(struct xenbus_device *dev, > if (err) > goto fail; > > + err = xenbus_switch_state(dev, XenbusStateInitWait); > + if (err) > + goto fail; > + > return 0; > > abort_transaction: > @@ -650,7 +654,10 @@ static void frontend_changed(struct xenbus_device *dev, > > switch (frontend_state) { > case XenbusStateInitialising: > - set_backend_state(be, XenbusStateInitWait); > + if (dev->state == XenbusStateClosed) { > + pr_info("%s: prepare for reconnect\n", dev->nodename); > + set_backend_state(be, XenbusStateInitWait); > + } > break; > > case XenbusStateInitialised: > -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Race condition on device add hanling in xl devd 2018-12-17 13:23 ` Marek Marczykowski-Górecki @ 2018-12-17 14:44 ` Roger Pau Monné 0 siblings, 0 replies; 15+ messages in thread From: Roger Pau Monné @ 2018-12-17 14:44 UTC (permalink / raw) To: Marek Marczykowski-Górecki; +Cc: xen-devel, Paul Durrant, Wei Liu On Mon, Dec 17, 2018 at 02:23:41PM +0100, Marek Marczykowski-Górecki wrote: > On Mon, Dec 17, 2018 at 02:05:34PM +0100, Roger Pau Monné wrote: > > On Mon, Dec 17, 2018 at 01:23:15PM +0100, Marek Marczykowski-Górecki wrote: > > > On Mon, Dec 17, 2018 at 01:18:55PM +0100, Roger Pau Monné wrote: > > > > On Mon, Dec 17, 2018 at 01:00:01PM +0100, Marek Marczykowski-Górecki wrote: > > > > > On Mon, Dec 17, 2018 at 10:40:59AM +0100, Roger Pau Monné wrote: > > > > > > On Sun, Dec 16, 2018 at 02:47:43AM +0100, Marek Marczykowski-Górecki wrote: > > > > > > > A workaround could be implemented in hotplug script itself - wait for > > > > > > > the device there. I'm not sure how proper solution could look like. Some > > > > > > > synchronization between xl devd and the kernel (like xl devd monitoring > > > > > > > uevents)? > > > > > > > > > > > > There's already a synchronization mechanism, libxl waits for the > > > > > > backend to switch to state 2 (XenbusStateInitWait) before running the > > > > > > hotplug scripts [0]. > > > > > > > > > > > > Maybe netback sets state 2 before creating the backend device? > > > > > > > > > > > > It looks to me like the backend needs to be sure everything needed by > > > > > > the hotplug script is in place before switching to state 2. > > > > > > > > > > I've done some more tests and I think that's something else. I've added > > > > > a loop waiting for /sys/class/net/$vif to a hotplug script, but it timed > > > > > out (5s). I don't see _any_ kernel messages related to the device. > > > > > > > > > > It may be some bug in nested virtualization in KVM... > > > > > > > > In your message you said you have also observed this behavior when > > > > running on bare metal, so it's likely not related to nested > > > > virtualization? > > > > > > Yes, but on bare metal is so hard to reproduce (like 0.1% or even less > > > startups), I'm not really sure if that was the same problem, as the > > > problem doesn't leave that much logs... > > > > I'm not very familiar with netback, but I think it's indeed possible > > for netback to switch to state 2 without having created the vif. > > Netback switching from state 1 -> 2 seems to be solely controlled by > > the frontend state (see frontend_changed). > > Isn't frontend_changed guaranteed to be called after netback_probe? Yes, that seems to be correct, in which case the patch is moot. The otherend watch is only setup after calling the probe driver method in xenbus_dev_probe. Do you think you can add some instrumentation code to netback in order to figure out what's going on? Interesting events would be backend switching to state 2 and vif creation (xenvif_alloc and the call to register_netdev AFAICT). Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-02-28 12:38 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-16 1:47 Race condition on device add hanling in xl devd Marek Marczykowski-Górecki 2018-12-17 9:40 ` Roger Pau Monné 2018-12-17 12:00 ` Marek Marczykowski-Górecki 2018-12-17 12:18 ` Roger Pau Monné 2018-12-17 12:23 ` Marek Marczykowski-Górecki 2018-12-17 13:05 ` Roger Pau Monné 2018-12-17 13:11 ` Paul Durrant 2018-12-17 14:32 ` Roger Pau Monné 2018-12-17 14:42 ` Paul Durrant 2018-12-17 16:09 ` Roger Pau Monné 2019-02-24 23:14 ` Marek Marczykowski-Górecki 2019-02-28 10:08 ` Roger Pau Monné 2019-02-28 12:38 ` Marek Marczykowski-Górecki 2018-12-17 13:23 ` Marek Marczykowski-Górecki 2018-12-17 14:44 ` Roger Pau Monné
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.