From: annie li <annie.li@oracle•com>
To: Paul Durrant <paul.durrant@citrix•com>
Cc: netdev@vger•kernel.org, xen-devel@lists•xen.org,
Wei Liu <wei.liu2@citrix•com>,
David Vrabel <david.vrabel@citrix•com>,
Ian Campbell <ian.campbell@citrix•com>
Subject: Re: [Xen-devel] [PATCH net-next v2 1/2] xen-netback: add a vif-is-connected flag
Date: Sun, 22 Sep 2013 10:56:35 +0800 [thread overview]
Message-ID: <523E5C63.9080804@oracle.com> (raw)
In-Reply-To: <1379685460-25032-2-git-send-email-paul.durrant@citrix.com>
On 2013-9-20 21:57, 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.
Or it is better to do more implements in windows netfront? For example,
when the windows vm hibernates, disconnect the vif as required by
netback: connect-> closing-> closed.
Thanks
Annie
> 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)
next prev parent reply other threads:[~2013-09-22 2:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-20 13:57 [PATCH net-next 0/2] xen-netback: windows frontend compatibility fixes Paul Durrant
2013-09-20 13:57 ` [PATCH net-next v2 1/2] xen-netback: add a vif-is-connected flag Paul Durrant
2013-09-20 18:40 ` David Miller
2013-09-22 2:56 ` annie li [this message]
2013-09-23 8:51 ` [Xen-devel] " Paul Durrant
2013-09-20 13:57 ` [PATCH net-next v2 2/2] xen-netback: handle frontends that fail to transition through Closing Paul Durrant
2013-09-20 16:14 ` Bastian Blank
2013-09-23 8:59 ` [Xen-devel] " Paul Durrant
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=523E5C63.9080804@oracle.com \
--to=annie.li@oracle$(echo .)com \
--cc=david.vrabel@citrix$(echo .)com \
--cc=ian.campbell@citrix$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=paul.durrant@citrix$(echo .)com \
--cc=wei.liu2@citrix$(echo .)com \
--cc=xen-devel@lists$(echo .)xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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