From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH RFC 2/2] veth: propagate bridge GSO to peer Date: Thu, 30 Nov 2017 10:08:25 -0800 Message-ID: <20171130100825.01ea1d14@xeon-e3> References: <20171126181749.19288-1-sthemmin@microsoft.com> <20171126181749.19288-3-sthemmin@microsoft.com> <20171126230725.1fcc3b51@xeon-e3> <20171127201419.GA79@intel.com> <20171127131502.1fbfaa66@xeon-e3> <20171128014222.GA503@intel.com> <91628267-2e48-a231-7cc2-4830eb95ceef@gmail.com> <20171130003534.GA60@intel.com> <20171130091021.658869b0@xeon-e3> <1512062799.19682.19.camel@gmail.com> <20171130094932.6d9e7c61@xeon-e3> <1512064763.19682.23.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Solio Sarabia , David Ahern , davem@davemloft.net, netdev@vger.kernel.org, sthemmin@microsoft.com, shiny.sebastian@intel.com To: Eric Dumazet Return-path: Received: from mail-pl0-f68.google.com ([209.85.160.68]:38436 "EHLO mail-pl0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752241AbdK3SIe (ORCPT ); Thu, 30 Nov 2017 13:08:34 -0500 Received: by mail-pl0-f68.google.com with SMTP id s10so4702582plj.5 for ; Thu, 30 Nov 2017 10:08:33 -0800 (PST) In-Reply-To: <1512064763.19682.23.camel@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 30 Nov 2017 09:59:23 -0800 Eric Dumazet wrote: > On Thu, 2017-11-30 at 09:49 -0800, Stephen Hemminger wrote: > > On Thu, 30 Nov 2017 09:26:39 -0800 > > Eric Dumazet wrote: > > =20 > > > On Thu, 2017-11-30 at 09:10 -0800, Stephen Hemminger wrote: =20 > > > >=20 > > > >=20 > > > > The problem goes back into the core GSO networking code. > > > > Something like this is needed. > > > >=20 > > > > static inline bool netif_needs_gso(struct sk_buff *skb, > > > > =C2=A0=C2=A0=C2=A0const struct net_device *dev, > > > > =C2=A0=C2=A0=C2=A0netdev_features_t features) > > > > { > > > > return skb_is_gso(skb) && > > > > (!skb_gso_ok(skb, features) || > > > > =C2=A0unlikely(skb_shinfo(skb)->gso_segs > dev-=C2=A0=C2=A0 =20 > > > > > gso_max_segs) ||=C2=A0=C2=A0<< new=C2=A0=C2=A0 =20 > > > >=20 > > > > =C2=A0unlikely(skb_shinfo(skb)->gso_size > dev-=C2=A0=C2=A0 =20 > > > > > gso_max_size) ||=C2=A0=C2=A0<< new=C2=A0=C2=A0 =20 > > > >=20 > > > > =C2=A0unlikely((skb->ip_summed !=3D CHECKSUM_PARTIAL) && > > > > =C2=A0=C2=A0(skb->ip_summed !=3D CHECKSUM_UNNECESSARY))); > > > > } > > > >=20 > > > > 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.=C2=A0=C2=A0 =20 > > >=20 > > >=20 > > > Since very few drivers have these gso_max_segs / gso_max_size, > > > check > > > could be done in their ndo_features_check() =20 > >=20 > > Actually, we already check for max_segs, just missing check for size > > here: > >=20 > > From 71a134f41c4aae8947241091300d21745aa237f2 Mon Sep 17 00:00:00 > > 2001 > > From: Stephen Hemminger > > Date: Thu, 30 Nov 2017 09:45:11 -0800 > > Subject: [PATCH] net: do not GSO if frame is too large > >=20 > > This adds an additional check to breakup skb's that exceed a devices > > GSO maximum size. The code was already checking for too many segments > > but did not check size. > >=20 > > This has been observed to be a problem when using containers on > > Hyper-V/Azure where the allowed GSO maximum size is less than > > maximum and skb's have gone through multiple layers to arrive > > at the virtual device. > >=20 > > Signed-off-by: Stephen Hemminger > > --- > > =C2=A0net/core/dev.c | 4 +++- > > =C2=A01 file changed, 3 insertions(+), 1 deletion(-) > >=20 > > diff --git a/net/core/dev.c b/net/core/dev.c > > index 07ed21d64f92..0bb398f3bfa3 100644 > > --- a/net/core/dev.c > > +++ b/net/core/dev.c > > @@ -2918,9 +2918,11 @@ static netdev_features_t > > gso_features_check(const struct sk_buff *skb, > > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0struct net_device *dev, > > =C2=A0 =C2=A0=C2=A0=C2=A0=C2=A0netdev_features_t > > features) > > =C2=A0{ > > + unsigned int gso_size =3D skb_shinfo(skb)->gso_size; > > =C2=A0 u16 gso_segs =3D skb_shinfo(skb)->gso_segs; > > =C2=A0 > > - if (gso_segs > dev->gso_max_segs) > > + if (gso_segs > dev->gso_max_segs || > > + =C2=A0=C2=A0=C2=A0=C2=A0gso_size > dev->gso_max_size) > > =C2=A0 return features & ~NETIF_F_GSO_MASK; > > =C2=A0 > > =C2=A0 /* Support for GSO partial features requires software =20 >=20 >=20 > Yes, but check commit 743b03a83297690f0bd38c452a3bbb47d2be300a > ("net: remove netdevice gso_min_segs") >=20 > Plan was to get rid of the existing check, not adding new ones :/ Sure can do it in the driver and that has other benefits like ability to backport to older distributions. Still need gso_max_size though since want to tell TCP to avoid generating mega-jumbo frames.