Re: [PATCH 4/4] tun: indicate support for USO feature

[Yuri Benditovich]

On Thu, May 13, 2021 at 11:43 PM Willem de Bruijn
<willemdebruijn.kernel@xxxxxxxxx> wrote:
>
> > > > > > So the question is what to do now:
> > > > > > A)
> > > > > > Finalize patches for guest TX and respective QEMU patches
> > > > > > Prepare RFC patches for guest RX, get ack on them
> > > > > > Change the spec
> > > > > > Finalize patches for guest RX according to the spec
> > > > > >
> > > > > > B)
> > > > > > Reject the patches for guest TX
> > > > > > Prepare RFC patches for everything, get ack on them
> > > > > > Change the spec
> > > > > > Finalize patches for everything according to the spec
> > > > > >
> > > > > > I'm for A) of course :)
> > > > >
> > > > > I'm for B :)
> > > > >
> > > > > The reasons are:
> > > > >
> > > > > 1) keep the assumption of tun_set_offload() to simply the logic and
> > > > > compatibility
> > > > > 2) it's hard or tricky to touch guest TX path only (e.g the
> > > > > virtio_net_hdr_from_skb() is called in both RX and TX)
> > > >
> > > > I suspect there is _some_ misunderstanding here.
> > > > I did not touch virtio_net_hdr_from_skb at all.
> > > >
> > >
> > > Typo, actually I meant virtio_net_hdr_to_skb().
> > OK.
> > 2) tun_get_user() which is guest TX - this is covered
> > 3) tap_get_user() which is guest TX - this is covered
> > 4) {t}packet_send() which is userspace TX - this is OK, the userspace
> > does not have this feature, it will never use USO
>
> What do you mean exactly? I can certainly imagine packet socket users
> that could benefit from using udp gso.
I've just tried to understand whether we have a real functional
problem due to the fact that I define the USO feature only for guest
TX path.
This set of patches modifies virtio_net_hdr_to_skb and Jason's comment
was that this procedure is called in both guest TX and RX, there are 4
places where the virtio_net_hdr_to_skb is called, userspace TX is one
of them.
AFAIU userspace 'socket' and 'user' backends of qemu do not have any
offloads at all so they will never use USO also.
Sorry for misunderstanding if any.
>
> When adding support for a new GSO type in virtio_net_hdr, it ideally
> is supported by all users of that interface. Alternatively, if some
> users do not support the flag, a call that sets the flag has to
> (continue to) fail hard, so that we can enable it at a later time.
I agree of course. IMO this is what I've tried to do. I did not have
in the initial plan to make Linux virtio-net to use the USO at all but
this should not present any problem (if I'm not mistaken).