Re: gso: Attempt to handle mega-GRO packets

Eric Dumazet
2013-11-07T01:21:39Z

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 ;)

I'll take a look, thanks !