From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 06 May 2008 01:23:55 +0200

[ Herbert CC:'d ]

quoted text
> On Mon, 2008-05-05 at 16:14 -0700, David Miller wrote:
> 
> > If it is cloned, it makes sure the data reference count allows
> > for modification of the buffer.
> 
> Which buffer? skb->data..skb->tail? And isn't that, in mac80211's case,
> at least currently all the buffer anyway?

Right.

For TSO or cases using frag lists of frag page vectors, it gets
more complicated.

Basically, skb_header_cloned()==false means that you can modify
anything covered by skb->data..skb->tail

quoted text
> Interesting point. I'll have to see when socket filters are run,
> wpa_supplicant could have a tap open I think. Maybe that's why I'm
> seeing so many cloned packets. I think I'll stacktrace skb_clone and
> print that out.

Note that we can even optimize that case even further if we really
have to.

But try to see why the filter used by WPA Supplicant is not effective.
If the filter fails to match, the packet is free'd immediately using
kfree_skb() which should undo clone'age.

Actually, I think I see a potential problem with how the
->dataref bit fields are managed in this situation.  This is
Herbert's creation, so let's ask him :-)

It seems that if we have a TCP packet for which skb_header_cloned()
is false, and this enters dev_hard_start_xmit(), if any network
taps take the packet, we'll never return to skb_header_cloned()==false
state even if every tap immediately kfree_skb(skb)'s the packet.

When we clone, we'll increment only the low 16-bit part of ->dataref.

On free, since skb->nohdr is set, we'll decrement both of (1 <<
SKB_DATAREF_SHIFT) and the low 16-bit counter.

Therefore the packet never becomes re-writable, right?  Is there some
easy way to cure this Herbert?  Apparently having a tap registered
constantly merely to watch for filtered traffic is quite common. :-/

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