From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1NDDi4-0003KY-Q1
	for qemu-devel@nongnu.org; Wed, 25 Nov 2009 03:58:48 -0500
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1NDDi0-0003Ht-6c
	for qemu-devel@nongnu.org; Wed, 25 Nov 2009 03:58:48 -0500
Received: from [199.232.76.173] (port=42304 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1NDDi0-0003Hm-3z
	for qemu-devel@nongnu.org; Wed, 25 Nov 2009 03:58:44 -0500
Received: from mx1.redhat.com ([209.132.183.28]:6163)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <markmc@redhat.com>) id 1NDDhz-0002WT-L1
	for qemu-devel@nongnu.org; Wed, 25 Nov 2009 03:58:43 -0500
Subject: Re: [Qemu-devel] Re: [PATCH] Fix TAP networking on host kernels
	without IFF_VNET_HDR support
From: Mark McLoughlin <markmc@redhat.com>
In-Reply-To: <14A83280-2A2D-4DCC-BD75-5DB6B43883EB@irisa.fr>
References: <1259053593-15362-1-git-send-email-Pierre.Riteau@irisa.fr>
	<1259058521.8935.21.camel@blaa>
	<DB148A1F-377F-41EF-B10F-693E628837FB@irisa.fr>
	<1259061772.8935.29.camel@blaa>
	<14A83280-2A2D-4DCC-BD75-5DB6B43883EB@irisa.fr>
Content-Type: text/plain; charset="UTF-8"
Date: Wed, 25 Nov 2009 08:55:55 +0000
Message-Id: <1259139355.2514.5.camel@blaa>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Reply-To: Mark McLoughlin <markmc@redhat.com>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Pierre Riteau <Pierre.Riteau@irisa.fr>
Cc: qemu-devel@nongnu.org

On Tue, 2009-11-24 at 22:27 +0100, Pierre Riteau wrote:
> On 24 nov. 2009, at 12:22, Mark McLoughlin wrote:
> 
> > On Tue, 2009-11-24 at 12:17 +0100, Pierre Riteau wrote:

> >> Isn't there a way to detect whether the kernel supports the
> >> TUNSETOFFLOAD ioctl at all?
> > 
> > The kernel will set errno to EINVAL if TUNSETOFFLOAD isn't supported, so
> > we could just ignore that case:
> > 
> >     if (ioctl(fd, TUNSETOFFLOAD, offload) != 0) {
> >          offload &= ~TUN_F_UFO;
> >          if (ioctl(fd, TUNSETOFFLOAD, offload) != 0 && errno != EINVAL) {
> >              fprintf(stderr, "TUNSETOFFLOAD ioctl() failed: %s\n",
> >                      strerror(errno));
> >          }
> >      }
> > 
> > The only concern is that we'll also miss out on an error message if
> > EINVAL is set for another reason. Currently, the only other reason if we
> > pass a offload flag not supported by the kernel, but that should never
> > happen.
> > 
> > Feel free to send a patch with that change and I'll ack it
....
> 
> Couldn't we probe the kernel with a 0 offload value to check if it supports TUNSETOFFLOAD?
> I tried the following and it works on my 2.6.26 Debian kernel.
> What do you think? I will send a proper patch if you agree.
> 
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 0f621a2..e038e1a 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -129,6 +129,11 @@ void tap_fd_set_offload(int fd, int csum, int tso4,
>  {
>      unsigned int offload = 0;
>  
> +    /* Check if our kernel supports TUNSETOFFLOAD */
> +    if (ioctl(fd, TUNSETOFFLOAD, 0) != 0 && errno == EINVAL) {
> +        return;
> +    }
> +

It's not ideal because a) it's another syscall and b) you're briefly
disabling any flags that may have been set previously ... but in our
case, neither is a real concern and it's a nice, simple solution.

So, sounds good to me :)

Cheers,
Mark.