Re: [PATCH net-next 05/15] ipv6/gso: remove temporary HBH/jumbo header

From: Alexander Duyck <alexander.duyck@gmail.com>
To: Eric Dumazet <edumazet@google.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
	Coco Li <lixiaoyan@google.com>
Subject: Re: [PATCH net-next 05/15] ipv6/gso: remove temporary HBH/jumbo header
Date: Thu, 3 Feb 2022 16:05:17 -0800	[thread overview]
Message-ID: <CAKgT0UcGoqJ5426JrKeOAhdm5izSAB1_9+X_bbB23Ws34PKASA@mail.gmail.com> (raw)
In-Reply-To: <CANn89iLLgF6f6YQkd26OxL0Fy3hUEx2KQ+PBQ7p6w8zRUpaC_w@mail.gmail.com>

On Thu, Feb 3, 2022 at 1:42 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Feb 3, 2022 at 1:08 PM Alexander H Duyck
> <alexander.duyck@gmail.com> wrote:
> >
> > On Thu, 2022-02-03 at 11:59 -0800, Eric Dumazet wrote:
> > > On Thu, Feb 3, 2022 at 11:45 AM Alexander Duyck
> > > <alexander.duyck@gmail.com> wrote:
> > >
> > > > It is the fact that you are adding IPv6 specific code to the
> > > > net/core/skbuff.c block here. Logically speaking if you are adding the
> > > > header in ipv6_gro_receive then it really seems li:ke the logic to
> > > > remove the header really belongs in ipv6_gso_segment. I suppose this
> > > > is an attempt to optimize it though, since normally updates to the
> > > > header are done after segmentation instead of before.
> > >
> > > Right, doing this at the top level means we do the thing once only,
> > > instead of 45 times if the skb has 45 segments.
> >
> > I'm just wondering if there is a way for us to do it in
> > ipv6_gso_segment directly instead though. With this we essentially end
> > up having to free the skb if the segmentation fails anyway since it
> > won't be able to go out on the wire.
> >
>
> Having a HBH jumbo header in place while the current frame is MTU size
> (typically MTU < 9000) would
> violate the specs. A HBH jumbo header presence implies packet length > 64K.

I get that. What I was getting at was that we might be able to process
it in ipv6_gso_segment before we hand it off to either TCP or UDP gso
handlers to segment.

The general idea being we keep the IPv6 specific bits in the IPv6
specific code instead of having the skb_segment function now have to
understand IPv6 packets. So what we would end up doing is having to do
an skb_cow to replace the skb->head if any clones might be holding on
it, and then just chop off the HBH jumbo header before we start the
segmenting.

The risk would be that we waste cycles removing the HBH header for a
frame that is going to fail, but I am not sure how likely a scenario
that is or if we need to optimize for that.