1. Patchew
  2. Libvirt
  3. View series

[PATCH] util: stop probing for IFF_VNET_HDR

Daniel P. Berrangé posted 1 patch 3 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200924092952.2100454-1-berrange@redhat.com
src/util/virnetdevtap.c | 63 +----------------------------------------
1 file changed, 1 insertion(+), 62 deletions(-)
[PATCH] util: stop probing for IFF_VNET_HDR
Posted by Daniel P. Berrangé 3 years, 7 months ago 
This flag was added by Linux with:

  commit f43798c27684ab925adde7d8acc34c78c6e50df8
  Author: Rusty Russell <rusty@rustcorp.com.au>
  Date:   Thu Jul 3 03:48:02 2008 -0700

    tun: Allow GSO using virtio_net_hdr

so we can assume all Linux distros we support have this flag available
and thus the compile time check is sufficient.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/util/virnetdevtap.c | 63 +----------------------------------------
 1 file changed, 1 insertion(+), 62 deletions(-)

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index cbce5c71b7..77c4d1c52c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
 }
 
 
-/**
- * virNetDevProbeVnetHdr:
- * @tapfd: a tun/tap file descriptor
- *
- * Check whether it is safe to enable the IFF_VNET_HDR flag on the
- * tap interface.
- *
- * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
- * guests to pass larger (GSO) packets, with partial checksums, to
- * the host. This greatly increases the achievable throughput.
- *
- * It is only useful to enable this when we're setting up a virtio
- * interface. And it is only *safe* to enable it when we know for
- * sure that a) qemu has support for IFF_VNET_HDR and b) the running
- * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
- * the supplied tapfd.
- *
- * Returns 1 if VnetHdr is supported, 0 if not supported
- */
-#ifdef IFF_VNET_HDR
-static int
-virNetDevProbeVnetHdr(int tapfd)
-{
-# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
-    unsigned int features;
-    struct ifreq dummy;
-
-    if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) {
-        VIR_INFO("Not enabling IFF_VNET_HDR; "
-                 "TUNGETFEATURES ioctl() not implemented");
-        return 0;
-    }
-
-    if (!(features & IFF_VNET_HDR)) {
-        VIR_INFO("Not enabling IFF_VNET_HDR; "
-                 "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
-        return 0;
-    }
-
-    /* The kernel will always return -1 at this point.
-     * If TUNGETIFF is not implemented then errno == EBADFD.
-     */
-    if (ioctl(tapfd, TUNGETIFF, &dummy) != -1 || errno != EBADFD) {
-        VIR_INFO("Not enabling IFF_VNET_HDR; "
-                 "TUNGETIFF ioctl() not implemented");
-        return 0;
-    }
-
-    VIR_INFO("Enabling IFF_VNET_HDR");
-
-    return 1;
-# else
-    (void) tapfd;
-    VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
-    return 0;
-# endif
-}
-#endif
-
-
 #ifdef TUNSETIFF
 /**
  * virNetDevTapGenerateName:
@@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
         }
 
 # ifdef IFF_VNET_HDR
-        if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
-            virNetDevProbeVnetHdr(fd))
+        if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
             ifr.ifr_flags |= IFF_VNET_HDR;
 # endif
 
-- 
2.26.2
Re: [PATCH] util: stop probing for IFF_VNET_HDR
Posted by Ján Tomko 3 years, 7 months ago 
On a Thursday in 2020, Daniel P. Berrangé wrote:
>This flag was added by Linux with:
>
>  commit f43798c27684ab925adde7d8acc34c78c6e50df8
>  Author: Rusty Russell <rusty@rustcorp.com.au>
>  Date:   Thu Jul 3 03:48:02 2008 -0700
>
>    tun: Allow GSO using virtio_net_hdr
>
>so we can assume all Linux distros we support have this flag available
>and thus the compile time check is sufficient.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> src/util/virnetdevtap.c | 63 +----------------------------------------
> 1 file changed, 1 insertion(+), 62 deletions(-)
>
>diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
>index cbce5c71b7..77c4d1c52c 100644
>--- a/src/util/virnetdevtap.c
>+++ b/src/util/virnetdevtap.c
>@@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
> }
>
>
>-/**
>- * virNetDevProbeVnetHdr:
>- * @tapfd: a tun/tap file descriptor
>- *
>- * Check whether it is safe to enable the IFF_VNET_HDR flag on the
>- * tap interface.
>- *
>- * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
>- * guests to pass larger (GSO) packets, with partial checksums, to
>- * the host. This greatly increases the achievable throughput.
>- *
>- * It is only useful to enable this when we're setting up a virtio
>- * interface. And it is only *safe* to enable it when we know for
>- * sure that a) qemu has support for IFF_VNET_HDR and b) the running
>- * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
>- * the supplied tapfd.
>- *
>- * Returns 1 if VnetHdr is supported, 0 if not supported
>- */
>-#ifdef IFF_VNET_HDR
>-static int
>-virNetDevProbeVnetHdr(int tapfd)
>-{
>-# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
>-    unsigned int features;
>-    struct ifreq dummy;
>-
>-    if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) {
>-        VIR_INFO("Not enabling IFF_VNET_HDR; "
>-                 "TUNGETFEATURES ioctl() not implemented");
>-        return 0;
>-    }
>-
>-    if (!(features & IFF_VNET_HDR)) {
>-        VIR_INFO("Not enabling IFF_VNET_HDR; "
>-                 "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
>-        return 0;
>-    }
>-
>-    /* The kernel will always return -1 at this point.
>-     * If TUNGETIFF is not implemented then errno == EBADFD.
>-     */
>-    if (ioctl(tapfd, TUNGETIFF, &dummy) != -1 || errno != EBADFD) {
>-        VIR_INFO("Not enabling IFF_VNET_HDR; "
>-                 "TUNGETIFF ioctl() not implemented");
>-        return 0;
>-    }
>-
>-    VIR_INFO("Enabling IFF_VNET_HDR");
>-
>-    return 1;
>-# else
>-    (void) tapfd;
>-    VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
>-    return 0;
>-# endif
>-}
>-#endif
>-
>-
> #ifdef TUNSETIFF
> /**
>  * virNetDevTapGenerateName:
>@@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
>         }
>
> # ifdef IFF_VNET_HDR

The TUNSETIFF guard above (which was introduced in Linux eons ago)
seems to be enough according to our CI:
https://gitlab.com/janotomko/libvirt/-/pipelines/193942161

It builds even with the IFF_MULTI_QUEUE guard removed.
commit bbb009941efaece3898910a862f6d23aa55d6ba8
CommitDate: 2012-11-01 11:14:08 -0400
     tuntap: introduce multiqueue flags

But #ifdef removal is out of scope of this patch.

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano

>-        if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
>-            virNetDevProbeVnetHdr(fd))
>+        if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
>             ifr.ifr_flags |= IFF_VNET_HDR;
> # endif
>
>-- 
>2.26.2
>
Re: [PATCH] util: stop probing for IFF_VNET_HDR
Posted by Daniel P. Berrangé 3 years, 7 months ago 
On Thu, Sep 24, 2020 at 01:05:48PM +0200, Ján Tomko wrote:
> On a Thursday in 2020, Daniel P. Berrangé wrote:
> > This flag was added by Linux with:
> > 
> >  commit f43798c27684ab925adde7d8acc34c78c6e50df8
> >  Author: Rusty Russell <rusty@rustcorp.com.au>
> >  Date:   Thu Jul 3 03:48:02 2008 -0700
> > 
> >    tun: Allow GSO using virtio_net_hdr
> > 
> > so we can assume all Linux distros we support have this flag available
> > and thus the compile time check is sufficient.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > src/util/virnetdevtap.c | 63 +----------------------------------------
> > 1 file changed, 1 insertion(+), 62 deletions(-)
> > 
> > diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> > index cbce5c71b7..77c4d1c52c 100644
> > --- a/src/util/virnetdevtap.c
> > +++ b/src/util/virnetdevtap.c
> > @@ -183,66 +183,6 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
> > }
> > 
> > 
> > -/**
> > - * virNetDevProbeVnetHdr:
> > - * @tapfd: a tun/tap file descriptor
> > - *
> > - * Check whether it is safe to enable the IFF_VNET_HDR flag on the
> > - * tap interface.
> > - *
> > - * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
> > - * guests to pass larger (GSO) packets, with partial checksums, to
> > - * the host. This greatly increases the achievable throughput.
> > - *
> > - * It is only useful to enable this when we're setting up a virtio
> > - * interface. And it is only *safe* to enable it when we know for
> > - * sure that a) qemu has support for IFF_VNET_HDR and b) the running
> > - * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
> > - * the supplied tapfd.
> > - *
> > - * Returns 1 if VnetHdr is supported, 0 if not supported
> > - */
> > -#ifdef IFF_VNET_HDR
> > -static int
> > -virNetDevProbeVnetHdr(int tapfd)
> > -{
> > -# if defined(IFF_VNET_HDR) && defined(TUNGETFEATURES) && defined(TUNGETIFF)
> > -    unsigned int features;
> > -    struct ifreq dummy;
> > -
> > -    if (ioctl(tapfd, TUNGETFEATURES, &features) != 0) {
> > -        VIR_INFO("Not enabling IFF_VNET_HDR; "
> > -                 "TUNGETFEATURES ioctl() not implemented");
> > -        return 0;
> > -    }
> > -
> > -    if (!(features & IFF_VNET_HDR)) {
> > -        VIR_INFO("Not enabling IFF_VNET_HDR; "
> > -                 "TUNGETFEATURES ioctl() reports no IFF_VNET_HDR");
> > -        return 0;
> > -    }
> > -
> > -    /* The kernel will always return -1 at this point.
> > -     * If TUNGETIFF is not implemented then errno == EBADFD.
> > -     */
> > -    if (ioctl(tapfd, TUNGETIFF, &dummy) != -1 || errno != EBADFD) {
> > -        VIR_INFO("Not enabling IFF_VNET_HDR; "
> > -                 "TUNGETIFF ioctl() not implemented");
> > -        return 0;
> > -    }
> > -
> > -    VIR_INFO("Enabling IFF_VNET_HDR");
> > -
> > -    return 1;
> > -# else
> > -    (void) tapfd;
> > -    VIR_INFO("Not enabling IFF_VNET_HDR; disabled at build time");
> > -    return 0;
> > -# endif
> > -}
> > -#endif
> > -
> > -
> > #ifdef TUNSETIFF
> > /**
> >  * virNetDevTapGenerateName:
> > @@ -363,8 +303,7 @@ int virNetDevTapCreate(char **ifname,
> >         }
> > 
> > # ifdef IFF_VNET_HDR
> 
> The TUNSETIFF guard above (which was introduced in Linux eons ago)
> seems to be enough according to our CI:
> https://gitlab.com/janotomko/libvirt/-/pipelines/193942161
> 
> It builds even with the IFF_MULTI_QUEUE guard removed.
> commit bbb009941efaece3898910a862f6d23aa55d6ba8
> CommitDate: 2012-11-01 11:14:08 -0400
>     tuntap: introduce multiqueue flags
> 
> But #ifdef removal is out of scope of this patch.

Oh right, I was thinking this code was used on BSD/MacOS, but
I forget there was a competely seperate impl. I'll do a followup
for the ifdefs.

> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> Jano
> 
> > -        if ((flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
> > -            virNetDevProbeVnetHdr(fd))
> > +        if (flags &  VIR_NETDEV_TAP_CREATE_VNET_HDR)
> >             ifr.ifr_flags |= IFF_VNET_HDR;
> > # endif
> > 
> > -- 
> > 2.26.2
> > 



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.