netdev - Re: TCP and reordering

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]

Date:	Wed, 28 Nov 2012 08:09:42 -0800
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Woodhouse <dwmw2@...radead.org>
Cc:	Vijay Subramanian <subramanian.vijay@...il.com>,
	David Miller <davem@...emloft.net>, saku@...i.fi,
	rick.jones2@...com, netdev@...r.kernel.org
Subject: Re: TCP and reordering

On Wed, 2012-11-28 at 15:47 +0000, David Woodhouse wrote:
> On Wed, 2012-11-28 at 04:52 -0800, Eric Dumazet wrote:
> > BQL is nice for high speed adapters.
> 
> For adapters with hugely deep queues, surely? There's a massive
> correlation between the two, of course — but PPP over L2TP or PPPoE
> ought to be included in the classification, right?
> 
> > For slow one, you always can stop the queue for each packet given to
> > start_xmit()
> > 
> > And restart the queue at TX completion.
> 
> Well yes, but only if we get notified of TX completion.
> 
> It's simple enough for the tty-based channels, and we can do it with a
> vcc->pop() function for PPPoATM. But for PPPoE and L2TP, how do we do
> it? We can install a skb destructor... but then we're stomping on TSQ's
> use of the destructor by orphaning it too soon.
> 
> I'm pondering something along the lines of
> 
> 	if (skb->destructor) {
> 		newskb = skb_clone(skb, GFP_KERNEL);
> 		if (newskb) {
> 	             skb_shinfo(newskb) = skb;
> 		     skb = newskb;
> 	        } 
> 	 }
> 	 skb_orphan(skb);
> 	 skb->destructor = ppp_chan_tx_completed;
> 
> 
> ... and then ppp_chan_tx_completed can also destroy the original skb
> (and hence invoke TSQ's destructor too) when the time comes. And in the
> (common?) case where we don't have an existing destructor, we don't
> bother with the skb_clone.
> 
> But I wish there was a nicer way to chain destructors. And no, I don't
> count what GSO does. We can't use the cb here anyway since we're passing
> it down the stack.
> 

My point was that if you limit number of in flight packet to 1,
its relatively easy to add glue in the priv dev data, so that you chain
the destructor without adding yet another fields in all skbs.

At start_xmit() do the following instead of skb_orphan(skb)

if (skb->destructor) {
     BUG_ON(priv->save_destructor);
     priv->save_destructor = skb->destructor;
     priv->save_sk = skb->sk;
     skb->sk = &priv->fake_sk;
}

skb->destructor = my_destructor;
/* stop the queue */
...
}


void my_destructor(struct sk_buff *skb)
{
    struct .... *priv = container_of(skb, struct ..., priv.fake_sk);

    skb->sk = skb->save_sk;
    priv->save_destructor(skb);
    priv->save_destructor = NULL;
    /* restart the queue */
    ...
}

Something like that.

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