RE: [PATCH] xen-netback: Check for hotplug-status existence before watching

-----Original Message-----
From: Marek Marczykowski-Górecki <redacted>
Sent: 11 May 2021 11:45
To: Durrant, Paul <redacted>
Cc: Michael Brown <redacted>; paul@xen.org; xen-devel@lists.xenproject.org;
netdev@vger.kernel.org; wei.liu@kernel.org
Subject: RE: [EXTERNAL] [PATCH] xen-netback: Check for hotplug-status existence before watching

On Tue, May 11, 2021 at 12:40:54PM +0200, Marek Marczykowski-Górecki wrote:

quoted

On Tue, May 11, 2021 at 07:06:55AM +0000, Durrant, Paul wrote:

quoted

quoted

-----Original Message-----
From: Marek Marczykowski-Górecki <redacted>
Sent: 10 May 2021 20:43
To: Michael Brown <redacted>; paul@xen.org
Cc: paul@xen.org; xen-devel@lists.xenproject.org; netdev@vger.kernel.org; wei.liu@kernel.org;

Durrant,

quoted

quoted

quoted

Paul [off-list ref]
Subject: RE: [EXTERNAL] [PATCH] xen-netback: Check for hotplug-status existence before watching

On Mon, May 10, 2021 at 08:06:55PM +0100, Michael Brown wrote:

quoted

If you have a suggested patch, I'm happy to test that it doesn't reintroduce
the regression bug that was fixed by this commit.

Actually, I've just tested with a simple reloading xen-netfront module. It
seems in this case, the hotplug script is not re-executed. In fact, I
think it should not be re-executed at all, since the vif interface
remains in place (it just gets NO-CARRIER flag).

This brings a question, why removing hotplug-status in the first place?
The interface remains correctly configured by the hotplug script after
all. From the commit message:

    xen-netback: remove 'hotplug-status' once it has served its purpose

    Removing the 'hotplug-status' node in netback_remove() is wrong; the script
    may not have completed. Only remove the node once the watch has fired and
    has been unregistered.

I think the intention was to remove 'hotplug-status' node _later_ in
case of quickly adding and removing the interface. Is that right, Paul?

The removal was done to allow unbind/bind to function correctly. IIRC before the original patch

doing a bind would stall forever waiting for the hotplug status to change, which would never happen.

quoted

Hmm, in that case maybe don't remove it at all in the backend, and let
it be cleaned up by the toolstack, when it removes other backend-related
nodes?

No, unbind/bind _does_ require hotplug script to be called again.

Yes, sorry I was misremembering. My memory is hazy but there was definitely a problem at the time with leaving the node in place.

When exactly vif interface appears in the system (starts to be available
for the hotplug script)? Maybe remove 'hotplug-status' just before that
point?

I really can't remember any detail. Perhaps try reverting both patches then and check that the unbind/rmmod/modprobe/bind sequence still works (and the backend actually makes it into connected state).

  Paul