netdev - Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201002142013.08745.arnd@arndb.de>
Date:	Sun, 14 Feb 2010 20:13:08 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Sridhar Samudrala <sri@...ibm.com>
Cc:	David Miller <davem@...emloft.net>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support

On Saturday 13 February 2010 21:55:37 Sridhar Samudrala wrote:
> > Also, what about IFF_TAP and IFF_NO_PI, should those be always set?
> >    
> Atleast it is not required for qemu to have these flags set. If we are 
> not doing anything different based on
> these flags, i felt we don't need to have them.

The point is that other applications might depend on them. We only support
IFF_TAP operation (not IFF_TUN), and we do not understand the !IFF_NO_PI
frame format, so any program that tries to use the PI header or use cooked
IP packets will get incorrect data.

> >> -            /* TODO: add support for these, so far we don't
> >> -                     support any offload */
> >> -            if (arg&  (TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> >> -                     TUN_F_TSO_ECN | TUN_F_UFO))
> >> -                    return -EINVAL;
> >> -
> >> -            return 0;
> >> +            q = macvtap_file_get_queue(file);
> >> +            if (!q)
> >> +                    return -ENOLINK;
> >> +            ret = 0;
> >> +            if (!(q->flags&  IFF_VNET_HDR))
> >> +                    ret =  -EINVAL;
> >> +            macvtap_file_put_queue(q);
> >> +            return ret;
> >>
> >>      default:
> >>              return -EINVAL;
> >>      
> > At least the first check needs to be in there, in case we are running with
> > new user space that knows additional flags. Moreover, shouldn't we check
> > the flags against the capabilities of vlan->lowerdev? I though it would be
> > best to report the capabilities of the real hardware to the guest kernel
> > so it can do the right thing.
> >    
> Originally, i also thought we should check these based on the real 
> device capabilities. But later i realized,
> that it is not really required as we fall back to software offload via 
> dev_gso_segment() call in dev_hard_start_xmit()
> if the real device doesn't support any of the offloads. So we can 
> advertise that all the offloads are supported to
> the guest and let host deal with any offloads that are not supported by 
> the real device.

Ah, good point. Will that also work for the checksumming? Also, should the
host really be doing segmentation and checksumming for the guest if the
hardware can't do it? So even if everything works correctly, we might
want to let the guest do the work in order to get accounting of the CPU
cycles right.
OTOH, for data transfers between guests, we can probably use all the offloads
independent of what the HW can do, so when optimizing for inter-guest
communication, we might want to ignore the accounting problems.

I'd like to hear other opinions on this.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html