netdev - Re: gso: Attempt to handle mega-GRO packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1383788068.2878.26.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Wed, 06 Nov 2013 17:34:28 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	Ben Hutchings <bhutchings@...arflare.com>,
	David Miller <davem@...emloft.net>,
	christoph.paasch@...ouvain.be, netdev@...r.kernel.org,
	hkchu@...gle.com, mwdalton@...gle.com
Subject: Re: gso: Attempt to handle mega-GRO packets

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;



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html