Re: [RFC PATCH 1/5] uapi/linux/if_tun.h: Added new ioctl for tun/tap.

[Jason Wang]

On Wed, Feb 23, 2022 at 9:31 PM Yuri Benditovich
<yuri.benditovich@xxxxxxxxxx> wrote:
>
> Hi Jason,
> We agree that the same can be done also using the old way, i.e. try to
> set specific offload - if failed, probably it is not supported.
> We think this is a little not scalable and we suggest adding the ioctl
> that will allow us to query allo the supported features in a single
> call.

Possibly but then you need some kind of probing. E.g we need endup
with probing TUNGETSUPPORTEDOFFLOADS iotctl itself.

> We think this will make QEMU code more simple also in future.

We can discuss this when qemu patches were sent.

> Do I understand correctly that you suggest to skip this new ioctl and
> use the old way of query for this (USO) feature and all future
> extensions?

Yes, since it's not a must. And we can do the TUNGETSUPPORTEDOFFLOADS
in a separate series.

Thanks

>
> Thanks
>
>
> On Wed, Feb 23, 2022 at 5:53 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
> >
> > On Tue, Feb 22, 2022 at 9:28 PM Andrew Melnichenko <andrew@xxxxxxxxxx> wrote:
> > >
> > > Hi all,
> > >
> > > On Wed, Feb 9, 2022 at 6:26 AM Jason Wang <jasowang@xxxxxxxxxx> wrote:
> > > >
> > > >
> > > > 在 2022/1/25 下午4:46, Andrew Melnychenko 写道:
> > > > > Added TUNGETSUPPORTEDOFFLOADS that should allow
> > > > > to get bits of supported offloads.
> > > >
> > > >
> > > > So we don't use dedicated ioctls in the past, instead, we just probing
> > > > by checking the return value of TUNSETOFFLOADS.
> > > >
> > > > E.g qemu has the following codes:
> > > >
> > > > int tap_probe_has_ufo(int fd)
> > > > {
> > > >      unsigned offload;
> > > >
> > > >      offload = TUN_F_CSUM | TUN_F_UFO;
> > > >
> > > >      if (ioctl(fd, TUNSETOFFLOAD, offload) < 0)
> > > >          return 0;
> > > >
> > > >      return 1;
> > > > }
> > > >
> > > > Any reason we can't keep using that?
> > > >
> > > > Thanks
> > > >
> > >
> > > Well, even in this example. To check the ufo feature, we trying to set it.
> > > What if we don't need to "enable" UFO and/or do not change its state?
> >
> > So at least Qemu doesn't have such a requirement since during the
> > probe the virtual networking backend is not even started.
> >
> > > I think it's a good idea to have the ability to get supported offloads
> > > without changing device behavior.
> >
> > Do you see a real user for this?
> >
> > Btw, we still need to probe this new ioctl anyway.
> >
> > Thanks
> >
> > >
> > > >
> > > > > Added 2 additional offlloads for USO(IPv4 & IPv6).
> > > > > Separate offloads are required for Windows VM guests,
> > > > > g.e. Windows may set USO rx only for IPv4.
> > > > >
> > > > > Signed-off-by: Andrew Melnychenko <andrew@xxxxxxxxxx>
> > > > > ---
> > > > >   include/uapi/linux/if_tun.h | 3 +++
> > > > >   1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> > > > > index 454ae31b93c7..07680fae6e18 100644
> > > > > --- a/include/uapi/linux/if_tun.h
> > > > > +++ b/include/uapi/linux/if_tun.h
> > > > > @@ -61,6 +61,7 @@
> > > > >   #define TUNSETFILTEREBPF _IOR('T', 225, int)
> > > > >   #define TUNSETCARRIER _IOW('T', 226, int)
> > > > >   #define TUNGETDEVNETNS _IO('T', 227)
> > > > > +#define TUNGETSUPPORTEDOFFLOADS _IOR('T', 228, unsigned int)
> > > > >
> > > > >   /* TUNSETIFF ifr flags */
> > > > >   #define IFF_TUN             0x0001
> > > > > @@ -88,6 +89,8 @@
> > > > >   #define TUN_F_TSO6  0x04    /* I can handle TSO for IPv6 packets */
> > > > >   #define TUN_F_TSO_ECN       0x08    /* I can handle TSO with ECN bits. */
> > > > >   #define TUN_F_UFO   0x10    /* I can handle UFO packets */
> > > > > +#define TUN_F_USO4   0x20    /* I can handle USO for IPv4 packets */
> > > > > +#define TUN_F_USO6   0x40    /* I can handle USO for IPv6 packets */
> > > > >
> > > > >   /* Protocol info prepended to the packets (when IFF_NO_PI is not set) */
> > > > >   #define TUN_PKT_STRIP       0x0001
> > > >
> > >
> >
>