All of lore.kernel.org
 help / color / mirror / Atom feed
* 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: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: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 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

* 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

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.