All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Hannes Frederic Sowa <hannes@stressinduktion.org>,
	Eric Dumazet <edumazet@google.com>
Subject: Re: [PATCH net] tuntap: raise EPOLLOUT on device up
Date: Fri, 18 May 2018 17:06:23 +0300	[thread overview]
Message-ID: <20180518170155-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <6159b23e-eeda-1d4b-6a30-63b1a30666eb@redhat.com>

On Fri, May 18, 2018 at 10:00:31PM +0800, Jason Wang wrote:
> 
> 
> On 2018年05月18日 21:26, Jason Wang wrote:
> > 
> > 
> > On 2018年05月18日 21:13, Michael S. Tsirkin wrote:
> > > On Fri, May 18, 2018 at 09:00:43PM +0800, Jason Wang wrote:
> > > > We return -EIO on device down but can not raise EPOLLOUT after it was
> > > > up. This may confuse user like vhost which expects tuntap to raise
> > > > EPOLLOUT to re-enable its TX routine after tuntap is down. This could
> > > > be easily reproduced by transmitting packets from VM while down and up
> > > > the tap device. Fixing this by set SOCKWQ_ASYNC_NOSPACE on -EIO.
> > > > 
> > > > Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Fixes: 1bd4978a88ac2 ("tun: honor IFF_UP in tun_get_user()")
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > > ---
> > > >   drivers/net/tun.c | 4 +++-
> > > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > > index d45ac37..1b29761 100644
> > > > --- a/drivers/net/tun.c
> > > > +++ b/drivers/net/tun.c
> > > > @@ -1734,8 +1734,10 @@ static ssize_t tun_get_user(struct
> > > > tun_struct *tun, struct tun_file *tfile,
> > > >       int skb_xdp = 1;
> > > >       bool frags = tun_napi_frags_enabled(tun);
> > > >   -    if (!(tun->dev->flags & IFF_UP))
> > > > +    if (!(tun->dev->flags & IFF_UP)) {
> > > Isn't this racy?  What if flag is cleared at this point?
> > 
> > I think you mean "set at this point"? Then yes, so we probably need to
> > set the bit during tun_net_close().
> > 
> > Thanks
> 
> Looks no need, vhost will poll socket after it see EIO. So we are ok here?
> 
> Thanks

In fact I don't even understand why does this help any longer.

-- 
MST

  reply	other threads:[~2018-05-18 14:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-18 13:00 [PATCH net] tuntap: raise EPOLLOUT on device up Jason Wang
2018-05-18 13:13 ` Michael S. Tsirkin
2018-05-18 13:26   ` Jason Wang
2018-05-18 14:00     ` Jason Wang
2018-05-18 14:06       ` Michael S. Tsirkin [this message]
2018-05-18 14:11         ` Jason Wang
2018-05-18 14:46           ` Michael S. Tsirkin
2018-05-19  1:09             ` Jason Wang
2018-05-21 22:06               ` Michael S. Tsirkin
2018-05-21 15:47 ` David Miller
2018-05-21 22:08   ` Michael S. Tsirkin
2018-05-22  3:22     ` Jason Wang
2018-05-22  3:45       ` Michael S. Tsirkin
2018-05-22  3:46       ` Michael S. Tsirkin
2018-05-22  4:00         ` Jason Wang

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=20180518170155-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=edumazet@google.com \
    --cc=hannes@stressinduktion.org \
    --cc=jasowang@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.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 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.