All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Yuri Benditovich <yuri.benditovich@daynix.com>
Cc: Jason Wang <jasowang@redhat.com>, Yan Vugenfirer <yan@daynix.com>,
	davem <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>,
	mst <mst@redhat.com>, netdev <netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	virtualization <virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH 4/4] tun: indicate support for USO feature
Date: Fri, 14 May 2021 08:41:06 -0400	[thread overview]
Message-ID: <CA+FuTSeqzN0SPKNEyD=ZntYb6q4u0u_p=VsPfux4mFpPBZzCxQ@mail.gmail.com> (raw)
In-Reply-To: <CAOEp5OcozfSWszTm9VArOAH+wm2Fo8tH1QJDsLPZD8ieBLtg-g@mail.gmail.com>

On Fri, May 14, 2021 at 3:39 AM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> On Fri, May 14, 2021 at 10:16 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, May 14, 2021 at 4:35 AM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > > > But surprisingly when TUN receives TUN_F_UFO it does not propagate it
> > > > > anywhere, there is no corresponding NETIF flag.
> > > >
> > > > (It looks like I drop the community and other ccs accidentally, adding
> > > > them back and sorry)
> > > >
> > > > Actually, there is one, NETIF_F_GSO_UDP.
> > > >
> > > > Kernel used to have NETIF_F_UFO, but it was removed due to bugs and
> > > > the lack of real hardware support. Then we found it breaks uABI, so
> > > > Willem tries to make it appear for userspace again, and then it was
> > > > renamed to NETIF_F_GSO_UDP.
> > > >
> > > > But I think it's a bug that we don't proporate TUN_F_UFO to NETIF
> > > > flag, this is a must for the driver that doesn't support
> > > > VIRTIO_NET_F_GUEST_UFO. I just try to disable all offloads and
> > > > mrg_rxbuf, then netperf UDP_STREAM from host to guest gives me bad
> > > > length packet in the guest.
> > > >
> > > > Willem, I think we probably need to fix this.
> > >
> > > We had to add back support for the kernel to accept UFO packets from
> > > userspace over tuntap.
> > >
> > > The kernel does not generate such packets, so a guest should never be
> > > concerned of receiving UFO packets.
> >
> > That's my feeling as well.
> >
> > But when I:
> >
> > 1) turn off all guest gso feature and mrg rx buffers, in this case
> > virtio-net will only allocate 1500 bytes for each packet
> > 2) doing netperf (UDP_STREAM) from local host to guest, I see packet
> > were truncated in the guest
>
> Is it possible that the virtio-net does not disable UFO offload?
> IMO it sets NETIF_F_LRO too bravely.

After we removed UFO completely, we found that guests may be migrated
from old hosts with UFO support to newer without. And that they do not
renegotiate features, so will continue to send UFO packets.

I added back the absolute minimum support for UFO: for a host to be
able to accept such UFO packets from userspace. But no device can
advertise or negotiate the NETIF_F_USO feature again. If these packets
arrive on the egress path, they will be immediately software segmented
(or fragmented) in skb_segment. So the host will not forward such
packets to another guest.

The behavior that Jason is experiencing, truncated packets received in
a guest from the host, sound unrelated to this feature to me. Can you
see what the original UDP datagram length is? Are these packets just
marginally larger, or indeed clearly U[SF]O packets well beyond any
reasonable MTU size?

Another option is that this is related to the host stack support for
UDP_GRO. The stack can now build large packets, segments these on
demand if needed (e.g., if such a packet arrives at a local socket
that does not advertise UDP_GRO). Perhaps somehow such packets escape
un-segmented to a guest. Do any devices where these packets may
originate have features NETIF_F_GRO_UDP_FWD or NETIF_F_GRO_FRAGLIST
enabled?

WARNING: multiple messages have this Message-ID (diff)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Yuri Benditovich <yuri.benditovich@daynix.com>
Cc: mst <mst@redhat.com>, netdev <netdev@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	virtualization <virtualization@lists.linux-foundation.org>,
	Yan Vugenfirer <yan@daynix.com>, Jakub Kicinski <kuba@kernel.org>,
	davem <davem@davemloft.net>
Subject: Re: [PATCH 4/4] tun: indicate support for USO feature
Date: Fri, 14 May 2021 08:41:06 -0400	[thread overview]
Message-ID: <CA+FuTSeqzN0SPKNEyD=ZntYb6q4u0u_p=VsPfux4mFpPBZzCxQ@mail.gmail.com> (raw)
In-Reply-To: <CAOEp5OcozfSWszTm9VArOAH+wm2Fo8tH1QJDsLPZD8ieBLtg-g@mail.gmail.com>

On Fri, May 14, 2021 at 3:39 AM Yuri Benditovich
<yuri.benditovich@daynix.com> wrote:
>
> On Fri, May 14, 2021 at 10:16 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> > On Fri, May 14, 2021 at 4:35 AM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > > > But surprisingly when TUN receives TUN_F_UFO it does not propagate it
> > > > > anywhere, there is no corresponding NETIF flag.
> > > >
> > > > (It looks like I drop the community and other ccs accidentally, adding
> > > > them back and sorry)
> > > >
> > > > Actually, there is one, NETIF_F_GSO_UDP.
> > > >
> > > > Kernel used to have NETIF_F_UFO, but it was removed due to bugs and
> > > > the lack of real hardware support. Then we found it breaks uABI, so
> > > > Willem tries to make it appear for userspace again, and then it was
> > > > renamed to NETIF_F_GSO_UDP.
> > > >
> > > > But I think it's a bug that we don't proporate TUN_F_UFO to NETIF
> > > > flag, this is a must for the driver that doesn't support
> > > > VIRTIO_NET_F_GUEST_UFO. I just try to disable all offloads and
> > > > mrg_rxbuf, then netperf UDP_STREAM from host to guest gives me bad
> > > > length packet in the guest.
> > > >
> > > > Willem, I think we probably need to fix this.
> > >
> > > We had to add back support for the kernel to accept UFO packets from
> > > userspace over tuntap.
> > >
> > > The kernel does not generate such packets, so a guest should never be
> > > concerned of receiving UFO packets.
> >
> > That's my feeling as well.
> >
> > But when I:
> >
> > 1) turn off all guest gso feature and mrg rx buffers, in this case
> > virtio-net will only allocate 1500 bytes for each packet
> > 2) doing netperf (UDP_STREAM) from local host to guest, I see packet
> > were truncated in the guest
>
> Is it possible that the virtio-net does not disable UFO offload?
> IMO it sets NETIF_F_LRO too bravely.

After we removed UFO completely, we found that guests may be migrated
from old hosts with UFO support to newer without. And that they do not
renegotiate features, so will continue to send UFO packets.

I added back the absolute minimum support for UFO: for a host to be
able to accept such UFO packets from userspace. But no device can
advertise or negotiate the NETIF_F_USO feature again. If these packets
arrive on the egress path, they will be immediately software segmented
(or fragmented) in skb_segment. So the host will not forward such
packets to another guest.

The behavior that Jason is experiencing, truncated packets received in
a guest from the host, sound unrelated to this feature to me. Can you
see what the original UDP datagram length is? Are these packets just
marginally larger, or indeed clearly U[SF]O packets well beyond any
reasonable MTU size?

Another option is that this is related to the host stack support for
UDP_GRO. The stack can now build large packets, segments these on
demand if needed (e.g., if such a packet arrives at a local socket
that does not advertise UDP_GRO). Perhaps somehow such packets escape
un-segmented to a guest. Do any devices where these packets may
originate have features NETIF_F_GRO_UDP_FWD or NETIF_F_GRO_FRAGLIST
enabled?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2021-05-14 12:41 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-11  4:42 [PATCH 0/4] Add host USO support to TUN device Yuri Benditovich
2021-05-11  4:42 ` Yuri Benditovich
2021-05-11  4:42 ` [PATCH 1/4] virtio-net: add definitions for host USO feature Yuri Benditovich
2021-05-11  4:42   ` Yuri Benditovich
2021-05-11  6:47   ` Jason Wang
2021-05-11  6:47     ` Jason Wang
2021-05-11  8:12     ` Yuri Benditovich
2021-05-11  8:12       ` Yuri Benditovich
2021-05-11  8:24       ` Jason Wang
2021-05-11  8:24         ` Jason Wang
2021-05-11  9:21         ` Yuri Benditovich
2021-05-11  9:21           ` Yuri Benditovich
2021-05-12  1:21           ` Jason Wang
2021-05-12  1:21             ` Jason Wang
2021-05-11  4:42 ` [PATCH 2/4] virtio-net: add support of UDP segmentation (USO) on the host Yuri Benditovich
2021-05-11  4:42   ` Yuri Benditovich
2021-05-11  6:47   ` Jason Wang
2021-05-11  6:47     ` Jason Wang
2021-05-11  8:23     ` Yuri Benditovich
2021-05-11  8:23       ` Yuri Benditovich
2021-05-11  8:31       ` Jason Wang
2021-05-11  8:31         ` Jason Wang
2021-05-11 17:47   ` Willem de Bruijn
2021-05-11 17:47     ` Willem de Bruijn
2021-05-12  6:09     ` Yuri Benditovich
2021-05-12  6:09       ` Yuri Benditovich
2021-05-12 14:32       ` Willem de Bruijn
2021-05-12 14:32         ` Willem de Bruijn
2021-05-12 18:56         ` Yuri Benditovich
2021-05-12 18:56           ` Yuri Benditovich
2021-05-12 19:53           ` Willem de Bruijn
2021-05-12 19:53             ` Willem de Bruijn
2021-05-11  4:42 ` [PATCH 3/4] tun: define feature bit for USO support Yuri Benditovich
2021-05-11  4:42   ` Yuri Benditovich
2021-05-11  4:42 ` [PATCH 4/4] tun: indicate support for USO feature Yuri Benditovich
2021-05-11  4:42   ` Yuri Benditovich
2021-05-11  6:50   ` Jason Wang
2021-05-11  6:50     ` Jason Wang
2021-05-11  8:33     ` Yuri Benditovich
2021-05-11  8:33       ` Yuri Benditovich
2021-05-11 19:06       ` Yuri Benditovich
2021-05-11 19:06         ` Yuri Benditovich
2021-05-12  1:33       ` Jason Wang
2021-05-12  1:33         ` Jason Wang
2021-05-12  5:24         ` Yuri Benditovich
2021-05-12  5:24           ` Yuri Benditovich
     [not found]           ` <CACGkMEsZBCzV+d_eLj1aYT+pkS5m1QAy7q8rUkNsdV0C8aL8tQ@mail.gmail.com>
     [not found]             ` <CAOEp5OeSankfA6urXLW_fquSMrZ+WYXDtKNacort1UwR=WgxqA@mail.gmail.com>
     [not found]               ` <CACGkMEt3bZrdqbWtWjSkXvv5v8iCHiN8hkD3T602RZnb6nPd9A@mail.gmail.com>
     [not found]                 ` <CAOEp5Odw=eaQWZCXr+U8PipPtO1Avjw-t3gEdKyvNYxuNa5TfQ@mail.gmail.com>
     [not found]                   ` <CACGkMEuqXaJxGqC+CLoq7k4XDu+W3E3Kk3WvG-D6tnn2K4ZPNA@mail.gmail.com>
     [not found]                     ` <CAOEp5OfB62SQzxMj_GkVD4EM=Z+xf43TPoTZwMbPPa3BsX2ooA@mail.gmail.com>
2021-05-13  7:04                       ` Jason Wang
2021-05-13  7:04                         ` Jason Wang
2021-05-13  8:14                         ` Yuri Benditovich
2021-05-13  8:14                           ` Yuri Benditovich
2021-05-13 20:43                           ` Willem de Bruijn
2021-05-13 20:43                             ` Willem de Bruijn
2021-05-14  5:48                             ` Yuri Benditovich
2021-05-14  5:48                               ` Yuri Benditovich
2021-05-13 20:34                         ` Willem de Bruijn
2021-05-13 20:34                           ` Willem de Bruijn
2021-05-14  7:16                           ` Jason Wang
2021-05-14  7:16                             ` Jason Wang
2021-05-14  7:38                             ` Yuri Benditovich
2021-05-14  7:38                               ` Yuri Benditovich
2021-05-14 12:41                               ` Willem de Bruijn [this message]
2021-05-14 12:41                                 ` Willem de Bruijn

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='CA+FuTSeqzN0SPKNEyD=ZntYb6q4u0u_p=VsPfux4mFpPBZzCxQ@mail.gmail.com' \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=jasowang@redhat.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=yan@daynix.com \
    --cc=yuri.benditovich@daynix.com \
    /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.