netdev - Re: [PATCH 01/10] core: Split out UFO6 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]

Date:	Sat, 20 Dec 2014 23:03:59 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Vlad Yasevich <vyasevic@...hat.com>
Cc:	Vladislav Yasevich <vyasevich@...il.com>, netdev@...r.kernel.org,
	virtualization@...ts.linux-foundation.org, ben@...adent.org.uk,
	stefanha@...hat.com, David Miller <davem@...emloft.net>
Subject: Re: [PATCH 01/10] core: Split out UFO6 support

On Fri, Dec 19, 2014 at 03:13:20PM -0500, Vlad Yasevich wrote:
> On 12/18/2014 12:50 PM, Michael S. Tsirkin wrote:
> > On Thu, Dec 18, 2014 at 07:35:46PM +0200, Michael S. Tsirkin wrote:
> >>>> We should either generate our own ID,
> >>>> like we always did, or make sure we don't accept
> >>>> these packets.
> >>>> Second option is almost sure to break userspace,
> >>>> so it seems we must do the first one.
> >>>>
> >>>
> >>> Right.  This was missing from packet sockets.  I can fix it.
> >>>
> >>> -vlad
> >>
> >> Also, this can't be a patch on top, since we don't
> >> want bisect to give us configurations which
> >> can BUG().
> > 
> > So how doing this in stages:
> > 
> > 1. add helper that checks skb GSO type.
> > If it is SKB_GSO_UDP, check for IPv6, and
> > generate the fragment ID.
> > 
> > Call this helper in
> > 	- virtio net rx path
> 
> Why do we need id on rx path?  Fragment ID should only be generated on tx.

So that all GSO_UDP6 packets have fragment ID as appropriate.
It's similar to how we fill it on rx in tun, is it not?


> > 	- tun rx path (get user)
> > 	- macvtap tx patch (get user)
> > 	- packet socket tx patch (get user)
> 
> The reset makes sense.
> > 
> > 2. Put back UFO flag everywhere: virtio,tun,macvtap,packet,
> > since we can handle IPv6 now even if it's suboptimal.
> > 
> > Above two seem like smalling patches, appropriate for stable.
> 
> OK.
> 
> > 
> > Next, work on a new feature to pass
> > fragment ID in virtio header:
> > 
> > 3. split UFO/UFO6 as you did here, but add code
> > in above 4 places to convert UDP to UDP6,
> 
> Doing so will adversely impact IPv6 UFO performance for legacy
> guests.  I know how many times I've seen mail wrt patch xyz caused
> %X  regression in my setup and how we've reverted or tried to fixed
> things to solve this.  If we go with approach, the only "fix' would be
> to upgrade the guest and that's not available to some users.
> 
> -vlad

I think there's some misunderstanding here.

I merely mean that after split, host should always have
SKB_GSO_UDP6 set for IPv6.

To make sure legacy userspace/guests don't notice changes,
whenever we detect SKB_GSO_UDP6 we should set VIRTIO_NET_HDR_GSO_UDP,
and whenever we get VIRTIO_NET_HDR_GSO_UDP we should set either
SKB_GSO_UDP6 or SKB_GSO_UDP depending on IP type.

Given this clarification there's no reason to think
old guests will regress, correct?

> > additionally, add code in
> > 	- virtio net tx path
> > 	- tun tx path (get user)
> > 	- macvtap rx patch (put user)
> > 	- packet socket rx patch (put user)
> > to convert UDP6 to UDP.
> >
> > 	step 3 needs to be bisect-clean, somehow.
> > 
> > 4. add new field in header, new feature bit for virtio net to gate it,
> > new ioctls to tun,macvtap,packet socket.
> > 
> > These two are more like optimizations, so not stable material.
> > 
> > 
--
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