* [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 12:56 [PATCH net-next 0/2] xen-netback: windows frontend compatibility fixes Paul Durrant
@ 2013-09-20 12:56 ` Paul Durrant
2013-09-20 12:56 ` Paul Durrant
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 12:56 UTC (permalink / raw)
To: netdev, xen-devel; +Cc: Paul Durrant, Wei Liu, David Vrabel, Ian Campbell
Having applied my patch to separate vif disconnect and free, I ran into a
BUG when testing resume from S3 with a Windows frontend because the vif task
pointer was not cleared by xenvif_disconnect() and so a double call to this
function tries to stop the thread twice.
Rather than applying a point fix for that issue it seems better to introduce
a boolean to indicate whether the vif is connected or not such that repeated
calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 24 +++++++++++++-----------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 5715318..860d92c 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -169,6 +169,7 @@ struct xenvif {
/* Miscellaneous private stuff. */
struct net_device *dev;
+ bool connected;
};
static inline struct xenbus_device *xenvif_to_xenbus_device(struct xenvif *vif)
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 01bb854..94b47f5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -366,7 +366,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
int err = -ENOMEM;
/* Already connected through? */
- if (vif->tx_irq)
+ if (vif->connected)
return 0;
err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
@@ -425,6 +425,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
wake_up_process(vif->task);
+ vif->connected = 1;
return 0;
err_rx_unbind:
@@ -453,23 +454,24 @@ void xenvif_carrier_off(struct xenvif *vif)
void xenvif_disconnect(struct xenvif *vif)
{
+ if (!vif->connected)
+ return;
+
if (netif_carrier_ok(vif->dev))
xenvif_carrier_off(vif);
- if (vif->tx_irq) {
- if (vif->tx_irq == vif->rx_irq)
- unbind_from_irqhandler(vif->tx_irq, vif);
- else {
- unbind_from_irqhandler(vif->tx_irq, vif);
- unbind_from_irqhandler(vif->rx_irq, vif);
- }
- vif->tx_irq = 0;
+ if (vif->tx_irq == vif->rx_irq)
+ unbind_from_irqhandler(vif->tx_irq, vif);
+ else {
+ unbind_from_irqhandler(vif->tx_irq, vif);
+ unbind_from_irqhandler(vif->rx_irq, vif);
}
- if (vif->task)
- kthread_stop(vif->task);
+ kthread_stop(vif->task);
xenvif_unmap_frontend_rings(vif);
+
+ vif->connected = 0;
}
void xenvif_free(struct xenvif *vif)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 12:56 [PATCH net-next 0/2] xen-netback: windows frontend compatibility fixes Paul Durrant
2013-09-20 12:56 ` [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag Paul Durrant
@ 2013-09-20 12:56 ` Paul Durrant
2013-09-20 13:31 ` Wei Liu
2013-09-20 13:31 ` Wei Liu
2013-09-20 12:56 ` [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing Paul Durrant
2013-09-20 12:56 ` Paul Durrant
3 siblings, 2 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 12:56 UTC (permalink / raw)
To: netdev, xen-devel; +Cc: Paul Durrant, David Vrabel, Wei Liu, Ian Campbell
Having applied my patch to separate vif disconnect and free, I ran into a
BUG when testing resume from S3 with a Windows frontend because the vif task
pointer was not cleared by xenvif_disconnect() and so a double call to this
function tries to stop the thread twice.
Rather than applying a point fix for that issue it seems better to introduce
a boolean to indicate whether the vif is connected or not such that repeated
calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/xen-netback/common.h | 1 +
drivers/net/xen-netback/interface.c | 24 +++++++++++++-----------
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 5715318..860d92c 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -169,6 +169,7 @@ struct xenvif {
/* Miscellaneous private stuff. */
struct net_device *dev;
+ bool connected;
};
static inline struct xenbus_device *xenvif_to_xenbus_device(struct xenvif *vif)
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 01bb854..94b47f5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -366,7 +366,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
int err = -ENOMEM;
/* Already connected through? */
- if (vif->tx_irq)
+ if (vif->connected)
return 0;
err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
@@ -425,6 +425,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
wake_up_process(vif->task);
+ vif->connected = 1;
return 0;
err_rx_unbind:
@@ -453,23 +454,24 @@ void xenvif_carrier_off(struct xenvif *vif)
void xenvif_disconnect(struct xenvif *vif)
{
+ if (!vif->connected)
+ return;
+
if (netif_carrier_ok(vif->dev))
xenvif_carrier_off(vif);
- if (vif->tx_irq) {
- if (vif->tx_irq == vif->rx_irq)
- unbind_from_irqhandler(vif->tx_irq, vif);
- else {
- unbind_from_irqhandler(vif->tx_irq, vif);
- unbind_from_irqhandler(vif->rx_irq, vif);
- }
- vif->tx_irq = 0;
+ if (vif->tx_irq == vif->rx_irq)
+ unbind_from_irqhandler(vif->tx_irq, vif);
+ else {
+ unbind_from_irqhandler(vif->tx_irq, vif);
+ unbind_from_irqhandler(vif->rx_irq, vif);
}
- if (vif->task)
- kthread_stop(vif->task);
+ kthread_stop(vif->task);
xenvif_unmap_frontend_rings(vif);
+
+ vif->connected = 0;
}
void xenvif_free(struct xenvif *vif)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 12:56 ` Paul Durrant
@ 2013-09-20 13:31 ` Wei Liu
2013-09-20 13:31 ` Wei Liu
1 sibling, 0 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:31 UTC (permalink / raw)
To: Paul Durrant; +Cc: netdev, Ian Campbell, Wei Liu, David Vrabel, xen-devel
On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
> Having applied my patch to separate vif disconnect and free, I ran into a
> BUG when testing resume from S3 with a Windows frontend because the vif task
> pointer was not cleared by xenvif_disconnect() and so a double call to this
> function tries to stop the thread twice.
> Rather than applying a point fix for that issue it seems better to introduce
> a boolean to indicate whether the vif is connected or not such that repeated
> calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
Reasonable change.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Thanks
Wei.
> ---
> drivers/net/xen-netback/common.h | 1 +
> drivers/net/xen-netback/interface.c | 24 +++++++++++++-----------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 5715318..860d92c 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -169,6 +169,7 @@ struct xenvif {
>
> /* Miscellaneous private stuff. */
> struct net_device *dev;
> + bool connected;
> };
>
> static inline struct xenbus_device *xenvif_to_xenbus_device(struct xenvif *vif)
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 01bb854..94b47f5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -366,7 +366,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
> int err = -ENOMEM;
>
> /* Already connected through? */
> - if (vif->tx_irq)
> + if (vif->connected)
> return 0;
>
> err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
> @@ -425,6 +425,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>
> wake_up_process(vif->task);
>
> + vif->connected = 1;
> return 0;
>
> err_rx_unbind:
> @@ -453,23 +454,24 @@ void xenvif_carrier_off(struct xenvif *vif)
>
> void xenvif_disconnect(struct xenvif *vif)
> {
> + if (!vif->connected)
> + return;
> +
> if (netif_carrier_ok(vif->dev))
> xenvif_carrier_off(vif);
>
> - if (vif->tx_irq) {
> - if (vif->tx_irq == vif->rx_irq)
> - unbind_from_irqhandler(vif->tx_irq, vif);
> - else {
> - unbind_from_irqhandler(vif->tx_irq, vif);
> - unbind_from_irqhandler(vif->rx_irq, vif);
> - }
> - vif->tx_irq = 0;
> + if (vif->tx_irq == vif->rx_irq)
> + unbind_from_irqhandler(vif->tx_irq, vif);
> + else {
> + unbind_from_irqhandler(vif->tx_irq, vif);
> + unbind_from_irqhandler(vif->rx_irq, vif);
> }
>
> - if (vif->task)
> - kthread_stop(vif->task);
> + kthread_stop(vif->task);
>
> xenvif_unmap_frontend_rings(vif);
> +
> + vif->connected = 0;
> }
>
> void xenvif_free(struct xenvif *vif)
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 12:56 ` Paul Durrant
2013-09-20 13:31 ` Wei Liu
@ 2013-09-20 13:31 ` Wei Liu
2013-09-20 14:28 ` David Vrabel
2013-09-20 14:28 ` David Vrabel
1 sibling, 2 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:31 UTC (permalink / raw)
To: Paul Durrant; +Cc: netdev, xen-devel, David Vrabel, Wei Liu, Ian Campbell
On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
> Having applied my patch to separate vif disconnect and free, I ran into a
> BUG when testing resume from S3 with a Windows frontend because the vif task
> pointer was not cleared by xenvif_disconnect() and so a double call to this
> function tries to stop the thread twice.
> Rather than applying a point fix for that issue it seems better to introduce
> a boolean to indicate whether the vif is connected or not such that repeated
> calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
Reasonable change.
Acked-by: Wei Liu <wei.liu2@citrix.com>
Thanks
Wei.
> ---
> drivers/net/xen-netback/common.h | 1 +
> drivers/net/xen-netback/interface.c | 24 +++++++++++++-----------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
> index 5715318..860d92c 100644
> --- a/drivers/net/xen-netback/common.h
> +++ b/drivers/net/xen-netback/common.h
> @@ -169,6 +169,7 @@ struct xenvif {
>
> /* Miscellaneous private stuff. */
> struct net_device *dev;
> + bool connected;
> };
>
> static inline struct xenbus_device *xenvif_to_xenbus_device(struct xenvif *vif)
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 01bb854..94b47f5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -366,7 +366,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
> int err = -ENOMEM;
>
> /* Already connected through? */
> - if (vif->tx_irq)
> + if (vif->connected)
> return 0;
>
> err = xenvif_map_frontend_rings(vif, tx_ring_ref, rx_ring_ref);
> @@ -425,6 +425,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
>
> wake_up_process(vif->task);
>
> + vif->connected = 1;
> return 0;
>
> err_rx_unbind:
> @@ -453,23 +454,24 @@ void xenvif_carrier_off(struct xenvif *vif)
>
> void xenvif_disconnect(struct xenvif *vif)
> {
> + if (!vif->connected)
> + return;
> +
> if (netif_carrier_ok(vif->dev))
> xenvif_carrier_off(vif);
>
> - if (vif->tx_irq) {
> - if (vif->tx_irq == vif->rx_irq)
> - unbind_from_irqhandler(vif->tx_irq, vif);
> - else {
> - unbind_from_irqhandler(vif->tx_irq, vif);
> - unbind_from_irqhandler(vif->rx_irq, vif);
> - }
> - vif->tx_irq = 0;
> + if (vif->tx_irq == vif->rx_irq)
> + unbind_from_irqhandler(vif->tx_irq, vif);
> + else {
> + unbind_from_irqhandler(vif->tx_irq, vif);
> + unbind_from_irqhandler(vif->rx_irq, vif);
> }
>
> - if (vif->task)
> - kthread_stop(vif->task);
> + kthread_stop(vif->task);
>
> xenvif_unmap_frontend_rings(vif);
> +
> + vif->connected = 0;
> }
>
> void xenvif_free(struct xenvif *vif)
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 13:31 ` Wei Liu
@ 2013-09-20 14:28 ` David Vrabel
2013-09-20 14:28 ` David Vrabel
1 sibling, 0 replies; 21+ messages in thread
From: David Vrabel @ 2013-09-20 14:28 UTC (permalink / raw)
To: Wei Liu; +Cc: netdev, Paul Durrant, Ian Campbell, xen-devel
On 20/09/13 14:31, Wei Liu wrote:
> On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
>> Having applied my patch to separate vif disconnect and free, I ran into a
>> BUG when testing resume from S3 with a Windows frontend because the vif task
>> pointer was not cleared by xenvif_disconnect() and so a double call to this
>> function tries to stop the thread twice.
>> Rather than applying a point fix for that issue it seems better to introduce
>> a boolean to indicate whether the vif is connected or not such that repeated
>> calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
We already have a backend state of CONNECTED/CLOSED/etc. why do we need
an additional bit of state outside of this?
Does something like this in frontend_changed() fix it?
case XenbusStateClosing:
switch (dev->state) {
case XenbusStateClosed;
break;
case XenbusStateConnected:
disconnect_backend(dev);
/* fall through */
default:
xenbus_switch_state(dev, XenbusStateClosing);
break;
}
break;
case XenbusStateClosed:
switch (dev->state) {
case XenbusStateConnected;
disconnect_backend(dev);
/* fall through */
default:
xenbus_switch_state(dev, XenbusStateClosed);
break;
}
if (xenbus_dev_is_online(dev))
break;
/* fall through if not online */
Can you also remove destroy_backend()? It's not needed any more.
I'd also recommend waiting a bit for other review feedback before
posting an updated series.
David
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 13:31 ` Wei Liu
2013-09-20 14:28 ` David Vrabel
@ 2013-09-20 14:28 ` David Vrabel
2013-09-20 15:02 ` Paul Durrant
2013-09-20 15:02 ` Paul Durrant
1 sibling, 2 replies; 21+ messages in thread
From: David Vrabel @ 2013-09-20 14:28 UTC (permalink / raw)
To: Wei Liu; +Cc: Paul Durrant, netdev, xen-devel, Ian Campbell
On 20/09/13 14:31, Wei Liu wrote:
> On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
>> Having applied my patch to separate vif disconnect and free, I ran into a
>> BUG when testing resume from S3 with a Windows frontend because the vif task
>> pointer was not cleared by xenvif_disconnect() and so a double call to this
>> function tries to stop the thread twice.
>> Rather than applying a point fix for that issue it seems better to introduce
>> a boolean to indicate whether the vif is connected or not such that repeated
>> calls to either xenvif_connect() or xenvif_disconnect() behave appropriately.
We already have a backend state of CONNECTED/CLOSED/etc. why do we need
an additional bit of state outside of this?
Does something like this in frontend_changed() fix it?
case XenbusStateClosing:
switch (dev->state) {
case XenbusStateClosed;
break;
case XenbusStateConnected:
disconnect_backend(dev);
/* fall through */
default:
xenbus_switch_state(dev, XenbusStateClosing);
break;
}
break;
case XenbusStateClosed:
switch (dev->state) {
case XenbusStateConnected;
disconnect_backend(dev);
/* fall through */
default:
xenbus_switch_state(dev, XenbusStateClosed);
break;
}
if (xenbus_dev_is_online(dev))
break;
/* fall through if not online */
Can you also remove destroy_backend()? It's not needed any more.
I'd also recommend waiting a bit for other review feedback before
posting an updated series.
David
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 14:28 ` David Vrabel
@ 2013-09-20 15:02 ` Paul Durrant
2013-09-20 15:02 ` Paul Durrant
1 sibling, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 15:02 UTC (permalink / raw)
To: David Vrabel, Wei Liu
Cc: netdev@vger.kernel.org, xen-devel@lists.xen.org, Ian Campbell
> -----Original Message-----
> From: David Vrabel
> Sent: 20 September 2013 15:29
> To: Wei Liu
> Cc: Paul Durrant; netdev@vger.kernel.org; xen-devel@lists.xen.org; Ian
> Campbell
> Subject: Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
>
> On 20/09/13 14:31, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
> >> Having applied my patch to separate vif disconnect and free, I ran into a
> >> BUG when testing resume from S3 with a Windows frontend because the
> vif task
> >> pointer was not cleared by xenvif_disconnect() and so a double call to this
> >> function tries to stop the thread twice.
> >> Rather than applying a point fix for that issue it seems better to introduce
> >> a boolean to indicate whether the vif is connected or not such that
> repeated
> >> calls to either xenvif_connect() or xenvif_disconnect() behave
> appropriately.
>
> We already have a backend state of CONNECTED/CLOSED/etc. why do we
> need
> an additional bit of state outside of this?
>
It's not really additional state; we were essentially inferring connected-ness from the value of tx_irq. This patch really just removes that inference and created something with the intended meaning.
> Does something like this in frontend_changed() fix it?
>
It may well do, but it's a far more invasive change and would require more testing. It certainly sounds like a good thing to do in the longer term.
Paul
> case XenbusStateClosing:
> switch (dev->state) {
> case XenbusStateClosed;
> break;
> case XenbusStateConnected:
> disconnect_backend(dev);
> /* fall through */
> default:
> xenbus_switch_state(dev, XenbusStateClosing);
> break;
> }
> break;
>
> case XenbusStateClosed:
> switch (dev->state) {
> case XenbusStateConnected;
> disconnect_backend(dev);
> /* fall through */
> default:
> xenbus_switch_state(dev, XenbusStateClosed);
> break;
> }
> if (xenbus_dev_is_online(dev))
> break;
> /* fall through if not online */
>
> Can you also remove destroy_backend()? It's not needed any more.
>
> I'd also recommend waiting a bit for other review feedback before
> posting an updated series.
>
> David
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
2013-09-20 14:28 ` David Vrabel
2013-09-20 15:02 ` Paul Durrant
@ 2013-09-20 15:02 ` Paul Durrant
1 sibling, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 15:02 UTC (permalink / raw)
To: David Vrabel, Wei Liu
Cc: netdev@vger.kernel.org, Ian Campbell, xen-devel@lists.xen.org
> -----Original Message-----
> From: David Vrabel
> Sent: 20 September 2013 15:29
> To: Wei Liu
> Cc: Paul Durrant; netdev@vger.kernel.org; xen-devel@lists.xen.org; Ian
> Campbell
> Subject: Re: [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag
>
> On 20/09/13 14:31, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:30PM +0100, Paul Durrant wrote:
> >> Having applied my patch to separate vif disconnect and free, I ran into a
> >> BUG when testing resume from S3 with a Windows frontend because the
> vif task
> >> pointer was not cleared by xenvif_disconnect() and so a double call to this
> >> function tries to stop the thread twice.
> >> Rather than applying a point fix for that issue it seems better to introduce
> >> a boolean to indicate whether the vif is connected or not such that
> repeated
> >> calls to either xenvif_connect() or xenvif_disconnect() behave
> appropriately.
>
> We already have a backend state of CONNECTED/CLOSED/etc. why do we
> need
> an additional bit of state outside of this?
>
It's not really additional state; we were essentially inferring connected-ness from the value of tx_irq. This patch really just removes that inference and created something with the intended meaning.
> Does something like this in frontend_changed() fix it?
>
It may well do, but it's a far more invasive change and would require more testing. It certainly sounds like a good thing to do in the longer term.
Paul
> case XenbusStateClosing:
> switch (dev->state) {
> case XenbusStateClosed;
> break;
> case XenbusStateConnected:
> disconnect_backend(dev);
> /* fall through */
> default:
> xenbus_switch_state(dev, XenbusStateClosing);
> break;
> }
> break;
>
> case XenbusStateClosed:
> switch (dev->state) {
> case XenbusStateConnected;
> disconnect_backend(dev);
> /* fall through */
> default:
> xenbus_switch_state(dev, XenbusStateClosed);
> break;
> }
> if (xenbus_dev_is_online(dev))
> break;
> /* fall through if not online */
>
> Can you also remove destroy_backend()? It's not needed any more.
>
> I'd also recommend waiting a bit for other review feedback before
> posting an updated series.
>
> David
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 12:56 [PATCH net-next 0/2] xen-netback: windows frontend compatibility fixes Paul Durrant
2013-09-20 12:56 ` [PATCH net-next 1/2] xen-netback: add a vif-is-connected flag Paul Durrant
2013-09-20 12:56 ` Paul Durrant
@ 2013-09-20 12:56 ` Paul Durrant
2013-09-20 13:34 ` Wei Liu
2013-09-20 13:34 ` Wei Liu
2013-09-20 12:56 ` Paul Durrant
3 siblings, 2 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 12:56 UTC (permalink / raw)
To: netdev, xen-devel; +Cc: Paul Durrant, David Vrabel, Wei Liu, Ian Campbell
Some old Windows frontends fail to transition through the xenbus Closing
state and move directly from Connected to Closed. Handle this case properly.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/xen-netback/xenbus.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index a53782e..bcaa25b 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
break;
case XenbusStateClosed:
+ if (dev->state == XenbusStateConnected)
+ disconnect_backend(dev);
xenbus_switch_state(dev, XenbusStateClosed);
if (xenbus_dev_is_online(dev))
break;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 12:56 ` [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing Paul Durrant
@ 2013-09-20 13:34 ` Wei Liu
2013-09-20 13:34 ` Wei Liu
1 sibling, 0 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:34 UTC (permalink / raw)
To: Paul Durrant; +Cc: netdev, Ian Campbell, Wei Liu, David Vrabel, xen-devel
On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> Some old Windows frontends fail to transition through the xenbus Closing
> state and move directly from Connected to Closed. Handle this case properly.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> drivers/net/xen-netback/xenbus.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index a53782e..bcaa25b 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
> break;
>
> case XenbusStateClosed:
> + if (dev->state == XenbusStateConnected)
> + disconnect_backend(dev);
Could you please add a comment above this change stating that this is a
workaround for some old frontend that we cannot fix / upgrade.
We would still like to later frontend goes through the normal connected
-> closing -> closed path.
Wei.
> xenbus_switch_state(dev, XenbusStateClosed);
> if (xenbus_dev_is_online(dev))
> break;
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 12:56 ` [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing Paul Durrant
2013-09-20 13:34 ` Wei Liu
@ 2013-09-20 13:34 ` Wei Liu
2013-09-20 13:38 ` Paul Durrant
` (3 more replies)
1 sibling, 4 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:34 UTC (permalink / raw)
To: Paul Durrant; +Cc: netdev, xen-devel, David Vrabel, Wei Liu, Ian Campbell
On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> Some old Windows frontends fail to transition through the xenbus Closing
> state and move directly from Connected to Closed. Handle this case properly.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
> drivers/net/xen-netback/xenbus.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index a53782e..bcaa25b 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
> break;
>
> case XenbusStateClosed:
> + if (dev->state == XenbusStateConnected)
> + disconnect_backend(dev);
Could you please add a comment above this change stating that this is a
workaround for some old frontend that we cannot fix / upgrade.
We would still like to later frontend goes through the normal connected
-> closing -> closed path.
Wei.
> xenbus_switch_state(dev, XenbusStateClosed);
> if (xenbus_dev_is_online(dev))
> break;
> --
> 1.7.10.4
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:34 ` Wei Liu
@ 2013-09-20 13:38 ` Paul Durrant
2013-09-20 13:38 ` Paul Durrant
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 13:38 UTC (permalink / raw)
Cc: netdev@vger.kernel.org, Ian Campbell, Wei Liu, David Vrabel,
xen-devel@lists.xen.org
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 20 September 2013 14:35
> To: Paul Durrant
> Cc: netdev@vger.kernel.org; xen-devel@lists.xen.org; David Vrabel; Wei Liu;
> Ian Campbell
> Subject: Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to
> transition through Closing
>
> On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> > Some old Windows frontends fail to transition through the xenbus Closing
> > state and move directly from Connected to Closed. Handle this case
> properly.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > drivers/net/xen-netback/xenbus.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> > index a53782e..bcaa25b 100644
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device
> *dev,
> > break;
> >
> > case XenbusStateClosed:
> > + if (dev->state == XenbusStateConnected)
> > + disconnect_backend(dev);
>
> Could you please add a comment above this change stating that this is a
> workaround for some old frontend that we cannot fix / upgrade.
>
> We would still like to later frontend goes through the normal connected
> -> closing -> closed path.
>
Sure. I'll re-post the series with that change.
Paul
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:34 ` Wei Liu
2013-09-20 13:38 ` Paul Durrant
@ 2013-09-20 13:38 ` Paul Durrant
2013-09-20 13:38 ` David Vrabel
2013-09-20 13:38 ` David Vrabel
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 13:38 UTC (permalink / raw)
To: Wei Liu
Cc: netdev@vger.kernel.org, xen-devel@lists.xen.org, David Vrabel,
Wei Liu, Ian Campbell
> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 20 September 2013 14:35
> To: Paul Durrant
> Cc: netdev@vger.kernel.org; xen-devel@lists.xen.org; David Vrabel; Wei Liu;
> Ian Campbell
> Subject: Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to
> transition through Closing
>
> On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> > Some old Windows frontends fail to transition through the xenbus Closing
> > state and move directly from Connected to Closed. Handle this case
> properly.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > ---
> > drivers/net/xen-netback/xenbus.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> > index a53782e..bcaa25b 100644
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device
> *dev,
> > break;
> >
> > case XenbusStateClosed:
> > + if (dev->state == XenbusStateConnected)
> > + disconnect_backend(dev);
>
> Could you please add a comment above this change stating that this is a
> workaround for some old frontend that we cannot fix / upgrade.
>
> We would still like to later frontend goes through the normal connected
> -> closing -> closed path.
>
Sure. I'll re-post the series with that change.
Paul
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:34 ` Wei Liu
2013-09-20 13:38 ` Paul Durrant
2013-09-20 13:38 ` Paul Durrant
@ 2013-09-20 13:38 ` David Vrabel
2013-09-20 13:40 ` Paul Durrant
` (3 more replies)
2013-09-20 13:38 ` David Vrabel
3 siblings, 4 replies; 21+ messages in thread
From: David Vrabel @ 2013-09-20 13:38 UTC (permalink / raw)
To: Wei Liu; +Cc: Paul Durrant, netdev, xen-devel, Ian Campbell
On 20/09/13 14:34, Wei Liu wrote:
> On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
>> Some old Windows frontends fail to transition through the xenbus Closing
>> state and move directly from Connected to Closed. Handle this case properly.
>>
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
>> break;
>>
>> case XenbusStateClosed:
>> + if (dev->state == XenbusStateConnected)
>> + disconnect_backend(dev);
>
> Could you please add a comment above this change stating that this is a
> workaround for some old frontend that we cannot fix / upgrade.
Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
to do regardless of whether there are old frontends that do this or not.
> We would still like to later frontend goes through the normal connected
> -> closing -> closed path.
This should be documented as a full description of the two state
machines in public/io/netif.h in Xen. Not scattered about in comments
in a particular backend implementation.
David
^ permalink raw reply [flat|nested] 21+ messages in thread* RE: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:38 ` David Vrabel
@ 2013-09-20 13:40 ` Paul Durrant
2013-09-20 13:40 ` Paul Durrant
` (2 subsequent siblings)
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 13:40 UTC (permalink / raw)
To: David Vrabel, Wei Liu
Cc: netdev@vger.kernel.org, xen-devel@lists.xen.org, Ian Campbell
> -----Original Message-----
> From: David Vrabel
> Sent: 20 September 2013 14:39
> To: Wei Liu
> Cc: Paul Durrant; netdev@vger.kernel.org; xen-devel@lists.xen.org; Ian
> Campbell
> Subject: Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to
> transition through Closing
>
> On 20/09/13 14:34, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> >> Some old Windows frontends fail to transition through the xenbus Closing
> >> state and move directly from Connected to Closed. Handle this case
> properly.
> >>
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -265,6 +265,8 @@ static void frontend_changed(struct
> xenbus_device *dev,
> >> break;
> >>
> >> case XenbusStateClosed:
> >> + if (dev->state == XenbusStateConnected)
> >> + disconnect_backend(dev);
> >
> > Could you please add a comment above this change stating that this is a
> > workaround for some old frontend that we cannot fix / upgrade.
>
> Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
> to do regardless of whether there are old frontends that do this or not.
>
Agreed, but probably still worth the comment in case someone gets the wrong idea.
Paul
> > We would still like to later frontend goes through the normal connected
> > -> closing -> closed path.
>
> This should be documented as a full description of the two state
> machines in public/io/netif.h in Xen. Not scattered about in comments
> in a particular backend implementation.
>
> David
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:38 ` David Vrabel
2013-09-20 13:40 ` Paul Durrant
@ 2013-09-20 13:40 ` Paul Durrant
2013-09-20 13:48 ` Wei Liu
2013-09-20 13:48 ` Wei Liu
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 13:40 UTC (permalink / raw)
To: David Vrabel, Wei Liu
Cc: netdev@vger.kernel.org, Ian Campbell, xen-devel@lists.xen.org
> -----Original Message-----
> From: David Vrabel
> Sent: 20 September 2013 14:39
> To: Wei Liu
> Cc: Paul Durrant; netdev@vger.kernel.org; xen-devel@lists.xen.org; Ian
> Campbell
> Subject: Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to
> transition through Closing
>
> On 20/09/13 14:34, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> >> Some old Windows frontends fail to transition through the xenbus Closing
> >> state and move directly from Connected to Closed. Handle this case
> properly.
> >>
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -265,6 +265,8 @@ static void frontend_changed(struct
> xenbus_device *dev,
> >> break;
> >>
> >> case XenbusStateClosed:
> >> + if (dev->state == XenbusStateConnected)
> >> + disconnect_backend(dev);
> >
> > Could you please add a comment above this change stating that this is a
> > workaround for some old frontend that we cannot fix / upgrade.
>
> Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
> to do regardless of whether there are old frontends that do this or not.
>
Agreed, but probably still worth the comment in case someone gets the wrong idea.
Paul
> > We would still like to later frontend goes through the normal connected
> > -> closing -> closed path.
>
> This should be documented as a full description of the two state
> machines in public/io/netif.h in Xen. Not scattered about in comments
> in a particular backend implementation.
>
> David
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:38 ` David Vrabel
2013-09-20 13:40 ` Paul Durrant
2013-09-20 13:40 ` Paul Durrant
@ 2013-09-20 13:48 ` Wei Liu
2013-09-20 13:48 ` Wei Liu
3 siblings, 0 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:48 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, Paul Durrant, Wei Liu, Ian Campbell, xen-devel
On Fri, Sep 20, 2013 at 02:38:46PM +0100, David Vrabel wrote:
> On 20/09/13 14:34, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> >> Some old Windows frontends fail to transition through the xenbus Closing
> >> state and move directly from Connected to Closed. Handle this case properly.
> >>
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
> >> break;
> >>
> >> case XenbusStateClosed:
> >> + if (dev->state == XenbusStateConnected)
> >> + disconnect_backend(dev);
> >
> > Could you please add a comment above this change stating that this is a
> > workaround for some old frontend that we cannot fix / upgrade.
>
> Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
> to do regardless of whether there are old frontends that do this or not.
>
I agree handling connected -> closed is sensible here based on the fact
that old frontends could do such state change. However If the state
machine was documented well enough then I think connected -> closed
would not be considered sensible.
This code snippet without comment will cause confusion / encourage wrong
usage of state machine if someone comes here for reference.
> > We would still like to later frontend goes through the normal connected
> > -> closing -> closed path.
>
> This should be documented as a full description of the two state
> machines in public/io/netif.h in Xen. Not scattered about in comments
Sure.
> in a particular backend implementation.
>
The comment in implementation is still worthwhile in case someone comes
here for reference and gets confused.
Wei.
> David
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:38 ` David Vrabel
` (2 preceding siblings ...)
2013-09-20 13:48 ` Wei Liu
@ 2013-09-20 13:48 ` Wei Liu
3 siblings, 0 replies; 21+ messages in thread
From: Wei Liu @ 2013-09-20 13:48 UTC (permalink / raw)
To: David Vrabel; +Cc: Wei Liu, Paul Durrant, netdev, xen-devel, Ian Campbell
On Fri, Sep 20, 2013 at 02:38:46PM +0100, David Vrabel wrote:
> On 20/09/13 14:34, Wei Liu wrote:
> > On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
> >> Some old Windows frontends fail to transition through the xenbus Closing
> >> state and move directly from Connected to Closed. Handle this case properly.
> >>
> >> --- a/drivers/net/xen-netback/xenbus.c
> >> +++ b/drivers/net/xen-netback/xenbus.c
> >> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
> >> break;
> >>
> >> case XenbusStateClosed:
> >> + if (dev->state == XenbusStateConnected)
> >> + disconnect_backend(dev);
> >
> > Could you please add a comment above this change stating that this is a
> > workaround for some old frontend that we cannot fix / upgrade.
>
> Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
> to do regardless of whether there are old frontends that do this or not.
>
I agree handling connected -> closed is sensible here based on the fact
that old frontends could do such state change. However If the state
machine was documented well enough then I think connected -> closed
would not be considered sensible.
This code snippet without comment will cause confusion / encourage wrong
usage of state machine if someone comes here for reference.
> > We would still like to later frontend goes through the normal connected
> > -> closing -> closed path.
>
> This should be documented as a full description of the two state
> machines in public/io/netif.h in Xen. Not scattered about in comments
Sure.
> in a particular backend implementation.
>
The comment in implementation is still worthwhile in case someone comes
here for reference and gets confused.
Wei.
> David
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 13:34 ` Wei Liu
` (2 preceding siblings ...)
2013-09-20 13:38 ` David Vrabel
@ 2013-09-20 13:38 ` David Vrabel
3 siblings, 0 replies; 21+ messages in thread
From: David Vrabel @ 2013-09-20 13:38 UTC (permalink / raw)
To: Wei Liu; +Cc: netdev, Paul Durrant, Ian Campbell, xen-devel
On 20/09/13 14:34, Wei Liu wrote:
> On Fri, Sep 20, 2013 at 01:56:31PM +0100, Paul Durrant wrote:
>> Some old Windows frontends fail to transition through the xenbus Closing
>> state and move directly from Connected to Closed. Handle this case properly.
>>
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
>> break;
>>
>> case XenbusStateClosed:
>> + if (dev->state == XenbusStateConnected)
>> + disconnect_backend(dev);
>
> Could you please add a comment above this change stating that this is a
> workaround for some old frontend that we cannot fix / upgrade.
Handling frontend CONNECTED -> CLOSED is a sensible thing for a backend
to do regardless of whether there are old frontends that do this or not.
> We would still like to later frontend goes through the normal connected
> -> closing -> closed path.
This should be documented as a full description of the two state
machines in public/io/netif.h in Xen. Not scattered about in comments
in a particular backend implementation.
David
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing
2013-09-20 12:56 [PATCH net-next 0/2] xen-netback: windows frontend compatibility fixes Paul Durrant
` (2 preceding siblings ...)
2013-09-20 12:56 ` [PATCH net-next 2/2] xen-netback: handle frontends that fail to transition through Closing Paul Durrant
@ 2013-09-20 12:56 ` Paul Durrant
3 siblings, 0 replies; 21+ messages in thread
From: Paul Durrant @ 2013-09-20 12:56 UTC (permalink / raw)
To: netdev, xen-devel; +Cc: Paul Durrant, Wei Liu, David Vrabel, Ian Campbell
Some old Windows frontends fail to transition through the xenbus Closing
state and move directly from Connected to Closed. Handle this case properly.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
drivers/net/xen-netback/xenbus.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index a53782e..bcaa25b 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -265,6 +265,8 @@ static void frontend_changed(struct xenbus_device *dev,
break;
case XenbusStateClosed:
+ if (dev->state == XenbusStateConnected)
+ disconnect_backend(dev);
xenbus_switch_state(dev, XenbusStateClosed);
if (xenbus_dev_is_online(dev))
break;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread