Subject: Re: Regression: Recent networking (qdisc?) patches break irda_get_next_speed()

From: Stephen Hemminger <[email-masked]>
Date: Thu, 23 Oct 2008 16:16:41 -0700

On Thu, 23 Oct 2008 15:13:02 -0700
"Brandeburg, Jesse" <[email-masked]> wrote:

> Alex Villacís Lasso wrote:
> <snip>
> 
> >> The SKB control block is not aliased.
> >> 
> 
> ...
> >> IRDA cannot depend upon the SKB control block not changing across
> >> the dev_queue_xmit() call. 
> >> 
> >> 
> > Let me see if I understood. So the particular illegal thing the IRDA
> > stack is doing is the access of the control block in the middle of the
> > driver transmit routine (via irda_get_next_speed() and friends). This
> > information should be stored somewhere else. Exactly *where* to store
> > it is the main problem to solve.
> > 
> > What is the proper way (if any) to store per-packet parameters (other
> > than the payload itself) which are specific to a particular layer
> > (IrDA in this case) and which are needed by drivers in order to work
> > correctly? The control block gets overwritten by the time the driver
> > proc (hard_start_xmit) is called, so this approach is now ruled out. I
> > was thinking about storing a copy of the parameters (struct
> > irda_skb_cb) as a header within the payload itself (skb->data[]), but
> > I am not sure about whether this approach is a good design decision.
> > I am open to suggestions on where to place the parameters.
> 
> Isn't this what the data that is skb_reserve'd at the beginning of skb's allocated by netdev_alloc_skb is for?  If you take an extra reference to the skb and/or use a destructor hook you should be good, right?
> 
> you should just be able to push ->data using skb_reserve(sizeof your private data) in the beginning of the skb, or is that a horrible idea Dave?

That space is reserved for a copy of the ethernet header when doing bridge/filtering.
Yes, its a stupid, undocumented hack. 

If irda needs additional protocol space, it could advertise a larger hardware header
size and use the additional space for hidden protocol info.