From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH RFC 2/2] veth: propagate bridge GSO to peer Date: Thu, 30 Nov 2017 10:10:50 -0800 Message-ID: <1512065450.19682.25.camel@gmail.com> 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> <20171130100825.01ea1d14@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Cc: Solio Sarabia , David Ahern , davem@davemloft.net, netdev@vger.kernel.org, sthemmin@microsoft.com, shiny.sebastian@intel.com To: Stephen Hemminger Return-path: Received: from mail-io0-f196.google.com ([209.85.223.196]:36917 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752415AbdK3SKx (ORCPT ); Thu, 30 Nov 2017 13:10:53 -0500 Received: by mail-io0-f196.google.com with SMTP id d16so8483337iob.4 for ; Thu, 30 Nov 2017 10:10:53 -0800 (PST) In-Reply-To: <20171130100825.01ea1d14@xeon-e3> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, 2017-11-30 at 10:08 -0800, Stephen Hemminger wrote: > 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: > > >    > > > > On Thu, 2017-11-30 at 09:10 -0800, Stephen Hemminger wrote:   > > > > > > > > > > > > > > > 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.     > > > > > > > > > > > > Since very few drivers have these gso_max_segs / gso_max_size, > > > > check > > > > could be done in their ndo_features_check()   > > > > > > Actually, we already check for max_segs, just missing check for > > > size > > > here: > > > > > > 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 > > > > > > 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. > > > > > > 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. > > > > > > Signed-off-by: Stephen Hemminger > > > --- > > >  net/core/dev.c | 4 +++- > > >  1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > 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, > > >       struct net_device > > > *dev, > > >       netdev_features_t > > > features) > > >  { > > > + unsigned int gso_size = skb_shinfo(skb)->gso_size; > > >   u16 gso_segs = skb_shinfo(skb)->gso_segs; > > >   > > > - if (gso_segs > dev->gso_max_segs) > > > + if (gso_segs > dev->gso_max_segs || > > > +     gso_size > dev->gso_max_size) > > >   return features & ~NETIF_F_GSO_MASK; > > >   > > >   /* Support for GSO partial features requires software   > > > > > > Yes, but check commit 743b03a83297690f0bd38c452a3bbb47d2be300a > > ("net: remove netdevice gso_min_segs") > > > > 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. > Sure, the netdev->gso_max_{size|segs} are staying. I was simply trying to not add another check in fast path :/ Thanks.