On Mon, 2008-05-05 at 13:44 -0700, David Miller wrote:
quoted text
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Mon, 05 May 2008 22:10:14 +0200
>=20
> > So you're saying the check there to see if we can add 802.11 headers
> > should depend on skb_header_cloned() and not skb_cloned()?
>=20
> If you're pushing things in front (adding headers), you should basically
> always be OK.

We're not, we need to strip off the ethernet header and replace it with
802.11.

quoted text
> The problem is the skb->data pointer, that has to remain stable
> once dev_queue_xmit() gets the frame because taps like AF_PACKET
> can have access to the packet (via net/core/dev.c:dev_queue_xmit_nit()).
>=20
> (If you read the AF_PACKET code, and notice how it munges the
>  headers, note that it only does so temporarily and restores the
>  skb->data and skb->len state before returning back up to
>  it's caller, dev_hard_start_xmit())
>=20
> I think the AF_PACKET stuff is actually superfluous on the transmit
> side, we always give it clones so it can modify skb->data however it
> pleases without having to restore anything.  Someone with some time
> should look more closely into this :-)  The problem here seems to
> be that pt->func is used for both receive and transmit paths, so it
> must be mindful to handle both cases properly.

Interesting, but I'm not really interested in looking into it right
now :)

quoted text
> BTW, this gets back to the topic of the pain caused by pretending this
> stuff is ethernet when it isn't.  If we really made these wireless
> devices look the way they should, tcpdump could see the headers
> correctly.  I'm sure you have all sorts of hacks in the 80211 code to
> make 802.11 header capture possible via side-band stuff, but it would
> have been so much nicer (and transparently solved these header space
> issues) if we didn't try to put lipstick on a pig :-)

We can discuss this to no end and already have; I just don't see how
you'd want to handle the BSSID or QoS fields with things like dhclient.
As for header capture, that is actually not very interesting, the
interesting stuff is the transmit indication (was the frame ack'ed, ...)
and that's still not possible that way.

quoted text
> Anyways, back to the original topic.  I think you can avoid COW'ing
> clones if you don't modify skb->data and give the drivers some other
> way to know where the wireless headers really start.
>=20
> Alternatively, you can just clone the packet.  At that point the
> skb->data etc. members are your's to modify to your heart's content.
> And because nobody can look below the original skb->data value, you
> can stick your headers there.

But I cannot modify the ethernet header that'll live on in the skb data
so that's not useful. Also, if two mac80211 netdevs are bridged together
they'd stomp on each other no?

johannes