Re: gso: Attempt to handle mega-GRO packets

Eric Dumazet
2013-11-07T01:34:32Z

On Wed, 2013-11-06 at 17:21 -0800, Eric Dumazet wrote:
> On Thu, 2013-11-07 at 02:13 +0100, Hannes Frederic Sowa wrote:
> > On Wed, Nov 06, 2013 at 04:15:21PM -0800, Eric Dumazet wrote:
> > > On Wed, 2013-11-06 at 11:47 -0800, Eric Dumazet wrote:
> > > > I'll try a different way.
> > > >
> > > > The frag_list would contain a bunch of frags, that we logically add to the bunch
> > > > of frags found in the first skb shared_info structure.
> > >
> > > Here is the patch I came into (I tested it and it works very fine)
> > >
> > > The theory is that in GRO stack, all skbs use the head_frag trick,
> > > so even if one NIC pulled some payload into skb->head, we do not have to
> > > copy anything. Outside of GRO stack, we are not supposed to provide data
> > > in skb->head (I am speaking of the skb found on the frag_list extension,
> > > not the skb_head)
> > >
> > > I put a fallback code, just in case, with a WARN_ON_ONCE() so that we
> > > can catch the offenders (if any) to fix them.
> > >
> > > I renamed @skb to @skb_head to more clearly document this code.
> > > Same for @i renamed to @cur_frag
> >
> > I wanted to understand this code more closely and tried it with a test case I
> > used for the UDP_CORK bugs and also for the tbf panic.
> >
> > The packet is allocated as an UFO one and gets segmented by tbf.
> >
> > # tc qdisc replace dev eth0 root tbf rate 200kbit latency 20ms burst 5kb
> > # ./udptest
> > (Just doing two writes of 200 bytes, then a write of 4096 bytes on a udp
> > socket. I can send you the source (or a stripped down version, because it got
> > realy noisy.))
>
> Interesting :
>
> if (cskb == head_skb)
> cskb = skb_shinfo(head_skb)->frag_list;
> else
> cskb = cskb->next;
> if (!cskb) {
> WARN_ON_ONCE(1);
> goto err;
> }
>
> So here either head_skb->frag_list is NULL, or the frag_list chain finishes too early.
>
> More probably I have a bug in the code ;)

Oh yes, I missed a : data_len += remain;

at line 2906 :

offset = remain;
+ data_len += remain;
continue;