Re: [PATCH] rt2x00: rt2x00queue: avoid using more headroom then driver requested

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Wed, Oct 08, 2014 at 11:34:22PM -0400, Mark Asselstine wrote:
> On Wed, Oct 8, 2014 at 3:52 PM, Mark Asselstine <asselsm@gmail.com> wrote:
> > On Wed, Oct 8, 2014 at 9:00 AM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> >>
> >> On Wed, Oct 08, 2014 at 07:46:33AM -0400, Mark Asselstine wrote:
> >> > > If rt2x00 does not remove the alignment from the frame before giving it back
> >> > > to mac80211 and the same frame comes into rt2x00 again it should be correctly
> >> > > aligned and no additional header space is required. So this should be fine.
> >> >
> >> > Then I would say this definitely hints at a design flaw in
> >> > rt2x00queue_insert_l2pad(). Take the following scenario.
> >> >
> >> > * skb's first arrival in rt2x00queue_insert_l2pad(), 3 bytes needed
> >> > for frame alignment, 2 bytes for l2pad results in 3 bytes of headroom
> >> > taken.
> >> Not quite realistic assumption - header length will have to be odd then.
> >>
> >> But if such situation would happen we will have:
> >>
> >> header_align=2, payload_align=3, l2pad=3
> >>
> >> Since payload_align will be bigger than header_align, header_align will
> >> be increased to 6.
> >>
> >> Header will be moved by 6 bytes, frame will be moved by 3 bytes,
> >> between header and frame there will be l2pad equal to 3.
> >>
> >> > * rt2x00lib_txdone() returns 2 bytes of headroom
> >> Return 3 bytes.
> >>
> >> > * skb's second arrival in rt2x00queue_insert_l2pad(), 0 bytes needed
> >> > for frame alignment, 2 bytes for l2pad results in 4 bytes of headroom
> >> > taken.
> >> Header will be moved by 3 bytes.
> >>
> >> > * rt2x00lib_txdone() returns 2 bytes of headroom
> >> Return 3 bytes.
> >>
> >> > Basically as long as any bytes are required for l2pad the headroom
> >> > will lose 4 bytes again and again, never being returned by
> >> > rt2x00lib_txdone().
> >>
> >> I think that's not true - you made a few mistakes in your scenario,
> >> but perhaps I'm wrong :-)
> >
> > No just me being an idiot. I had thought
> >  frame == header + l2pad + payload
> > not
> >  frame == payload
> >
> 
> By the way, I assumed that this due to the name and contents of
> rt2x00queue_align_frame(). Where all the data (header and payload) are
> aligned to a 4-byte boundary and the function is name 'align_frame'. I
> assume your interpretation is the correct one, can you confirm?

Frame term is used mostly to describe all data including header and
payload.

Stanislaw