From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: paul@xen.org
Cc: "Durrant, Paul" <pdurrant@amazon.co.uk>,
Michael Brown <mbrown@fensystems.co.uk>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"wei.liu@kernel.org" <wei.liu@kernel.org>,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
Anthony PERARD <anthony.perard@citrix.com>
Subject: Re: [PATCH] xen-netback: Check for hotplug-status existence before watching
Date: Tue, 18 May 2021 11:18:35 +0200 [thread overview]
Message-ID: <YKOGayhGghjfgNXZ@mail-itl> (raw)
In-Reply-To: <8b7a9cd5-3696-65c2-5656-a1c8eb174344@xen.org>
[-- Attachment #1: Type: text/plain, Size: 3263 bytes --]
On Tue, May 18, 2021 at 07:57:16AM +0100, Paul Durrant wrote:
> On 17/05/2021 22:43, Marek Marczykowski-Górecki wrote:
> > On Tue, May 11, 2021 at 12:46:38PM +0000, Durrant, Paul wrote:
> > > I really can't remember any detail. Perhaps try reverting both patches then and check that the unbind/rmmod/modprobe/bind sequence still works (and the backend actually makes it into connected state).
> >
> > Ok, I've tried this. I've reverted both commits, then used your test
> > script from the 9476654bd5e8ad42abe8ee9f9e90069ff8e60c17:
> > This has been tested by running iperf as a server in the test VM and
> > then running a client against it in a continuous loop, whilst also
> > running:
> > while true;
> > do echo vif-$DOMID-$VIF >unbind;
> > echo down;
> > rmmod xen-netback;
> > echo unloaded;
> > modprobe xen-netback;
> > cd $(pwd);
> > brctl addif xenbr0 vif$DOMID.$VIF;
> > ip link set vif$DOMID.$VIF up;
> > echo up;
> > sleep 5;
> > done
> > in dom0 from /sys/bus/xen-backend/drivers/vif to continuously unbind,
> > unload, re-load, re-bind and re-plumb the backend.
> > In fact, the need to call `brctl` and `ip link` manually is exactly
> > because the hotplug script isn't executed. When I execute it manually,
> > the backend properly gets back to working. So, removing 'hotplug-status'
> > was in the correct place (netback_remove). The missing part is the toolstack
> > calling the hotplug script on xen-netback re-bind.
> >
>
> Why is that missing? We're going behind the back of the toolstack to do the
> unbind and bind so why should we expect it to re-execute a hotplug script?
Ok, then simply execute the whole hotplug script (instead of its subset)
after re-loading the backend module and everything will be fine.
For example like this:
XENBUS_PATH=backend/vif/$DOMID/$VIF \
XENBUS_TYPE=vif \
XENBUS_BASE_PATH=backend \
script=/etc/xen/scripts/vif-bridge \
vif=vif.$DOMID.$VIF \
/etc/xen/scripts/vif-bridge online
(...)
> > In short: if device gets XenbusStateInitWait for the first time (ddev ==
> > NULL case), it goes to add_device() which executes the hotplug script
> > and stores the device.
> > Then, if device goes to XenbusStateClosed + online==0 state, then it
> > executes hotplug script again (with "offline" parameter) and forgets the
> > device. If you unbind the driver, the device stays in
> > XenbusStateConnected state (in xenstore), and after you bind it again,
> > it goes to XenbusStateInitWait. It don't think it goes through
> > XenbusStateClosed, and online stays at 1 too, so libxl doesn't execute
> > the hotplug script again.
>
> This is pretty key. The frontend should not notice an unbind/bind i.e. there
> should be no evidence of it happening by examining states in xenstore (from
> the guest side).
If you update the backend module, I think the frontend needs at least to
re-evaluate feature-* nodes. In case of applying just a bug fix, they
should not change (in theory), but technically that would be the correct
thing to do.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-05-18 9:18 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-10 18:25 xen-netback hotplug-status regression bug Michael Brown
2021-04-13 7:12 ` Paul Durrant
2021-04-13 10:48 ` Michael Brown
2021-04-13 10:55 ` Paul Durrant
2021-04-13 15:14 ` Michael Brown
2021-04-13 15:25 ` [PATCH] xen-netback: Check for hotplug-status existence before watching Michael Brown
2021-04-13 19:12 ` Paul Durrant
2021-04-13 22:30 ` patchwork-bot+netdevbpf
2021-05-10 18:32 ` Marek Marczykowski-Górecki
2021-05-10 18:47 ` Michael Brown
2021-05-10 18:53 ` Marek Marczykowski-Górecki
2021-05-10 19:06 ` Michael Brown
2021-05-10 19:42 ` Marek Marczykowski-Górecki
2021-05-11 7:06 ` Durrant, Paul
2021-05-11 10:40 ` Marek Marczykowski-Górecki
2021-05-11 10:45 ` Marek Marczykowski-Górecki
2021-05-11 12:46 ` Durrant, Paul
2021-05-17 21:43 ` Marek Marczykowski-Górecki
2021-05-17 21:51 ` Michael Brown
2021-05-17 21:58 ` Marek Marczykowski-Górecki
2021-05-18 6:57 ` Paul Durrant
2021-05-18 9:18 ` Marek Marczykowski-Górecki [this message]
[not found] ` <887f9533f5c54bfabfbff7231eb99b08@EX13D32EUC003.ant.amazon.com>
[not found] ` <YKOMpXwcnr9QiXy8@mail-itl>
[not found] ` <2c23e102b6254e42877eb1e8fe68a4f7@EX13D32EUC003.ant.amazon.com>
2021-05-18 10:42 ` Marek Marczykowski-Górecki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YKOGayhGghjfgNXZ@mail-itl \
--to=marmarek@invisiblethingslab.com \
--cc=anthony.perard@citrix.com \
--cc=iwj@xenproject.org \
--cc=mbrown@fensystems.co.uk \
--cc=netdev@vger.kernel.org \
--cc=paul@xen.org \
--cc=pdurrant@amazon.co.uk \
--cc=wei.liu@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).