[PATCH] util: stop probing for IFF_VNET_HDR
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
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
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 :|
participants (2)
-
Daniel P. Berrangé -
Ján Tomko