netdev - Re: [PATCH RFC 2/2] veth: propagate bridge GSO to peer

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:   Thu, 30 Nov 2017 09:10:21 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Solio Sarabia <solio.sarabia@...el.com>
Cc:     David Ahern <dsahern@...il.com>, davem@...emloft.net,
        netdev@...r.kernel.org, sthemmin@...rosoft.com,
        shiny.sebastian@...el.com
Subject: Re: [PATCH RFC 2/2] veth: propagate bridge GSO to peer

On Wed, 29 Nov 2017 16:35:37 -0800
Solio Sarabia <solio.sarabia@...el.com> wrote:

> On Mon, Nov 27, 2017 at 07:02:01PM -0700, David Ahern wrote:
> > On 11/27/17 6:42 PM, Solio Sarabia wrote:  
> > > Adding ioctl support for 'ip link set' would work. I'm still concerned
> > > how to enforce the upper limit to not exceed that of the lower devices.
> > >   
> Actually, giving the user control to change gso doesn't solve the issue.
> In a VM, user could simple ignore setting the gso, still hurting host
> perf. We need to enforce the lower gso on the bridge/veth.
> 
> Should this issue be fixed at hv_netvsc level? Why is the driver passing
> down gso buffer sizes greater than what synthetic interface allows.
> 
> > > Consider a system with three NICs, each reporting values in the range
> > > [60,000 - 62,780]. Users could set virtual interfaces' gso to 65,536,
> > > exceeding the limit, and having the host do sw gso (vms settings must
> > > not affect host performance.)
> > > 
> > > Looping through interfaces?  With the difference that now it'd be
> > > trigger upon user's request, not every time a veth is created (like one
> > > previous patch discussed.)
> > >   
> > 
> > You are concerned about the routed case right? One option is to have VRF
> > devices propagate gso sizes to all devices (veth, vlan, etc) enslaved to
> > it. VRF devices are Layer 3 master devices so an L3 parallel to a bridge.  
> Having the VRF device propagate the gso to its slaves is opposite of
> what we do now: get minimum of all ports and assign it to bridge
> (net/bridge/br_if.c, br_min_mtu, br_set_gso_limits.)
> 
> Would it be right to change the logic flow? If so, this this could work:
> 
> (1) bridge gets gso from lower devices upon init/setup
> (2) when new device is attached to bridge, bridge sets gso for this new
>     slave (and its peer if it's veth.)
> (3) as the code is now, there's an optimization opportunity: for every
>     new interface attached to bridge, bridge loops through all ports to
>     set gso, mtu. It's not necessary as bridge already has the minimum
>     from previous interfaces attached. Could be O(1) instead of O(n).


The problem goes back into the core GSO networking code.
Something like this is needed.

static inline bool netif_needs_gso(struct sk_buff *skb,
				   const struct net_device *dev,
				   netdev_features_t features)
{
	return skb_is_gso(skb) &&
		(!skb_gso_ok(skb, features) ||
		 unlikely(skb_shinfo(skb)->gso_segs > dev->gso_max_segs) ||  << new
		 unlikely(skb_shinfo(skb)->gso_size > dev->gso_max_size) ||  << new
		 unlikely((skb->ip_summed != CHECKSUM_PARTIAL) &&
			  (skb->ip_summed != CHECKSUM_UNNECESSARY)));
}

What that will do is split up the monster GSO packets if they ever bleed
across from one device to another through the twisty mazes of packet
processing paths.