2009-10-30 19:53:23

by Nikolai ZHUBR

[permalink] [raw]
Subject: ieee80211_tx_status: headroom too small

Hello people,

I'm using compat-wireless-2009-09-14 with 2.6.25.20 and I have
recently noticed lots of these in syslog:

ieee80211_tx_status: headroom too small
ieee80211_tx_status: headroom too small
ieee80211_tx_status: headroom too small

The wireless (access point) works fine, though maybe not very
fast. However, this warning somehow bothers me. Is it important?
Does it mean something critical? Maybe I should just comment
it out in the code? I don't like syslog being filled up this
way...

Thank you.

Nikolai ZHUBR




2009-10-31 15:37:09

by Johannes Berg

[permalink] [raw]
Subject: Re: Re[2]: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 17:24 +0300, Nikolai ZHUBR wrote:
> Saturday, October 31, 2009, 12:34:37 PM, Johannes Berg wrote:
> >> > > 4 actually, wasn't there some cases where the header could have an odd
> >> > > number of bytes and would require 3 bytes for alignment?
> >> >
> >> > Hm, is there? I thought they changed the 11s draft to no longer have
> >> > that.
> >>
> >> Well 2 is fine in that case, (I was basing my comment on something said on this mailinglist
> >> a long time ago). :)
>
> > However, it really all depends on the hardware too ... Maybe we should
> > put more of the logic into drivers, and have mac80211 only export its
> > required headroom?
>
> > Or maybe we should just not bother with having drivers do the alignment
> > any more -- mac80211 does that now and it only does it if necessary
> > (i.e. not on powerpc/x86)
>
> I'm having this issue on mips platform, if it matters. I cannot test
> it on x86 yet.
>
> Now, searching for "extra_tx_headroom" gives me essentially the
> following two:
>
> net/mac80211/main.c:
> local->tx_headroom = max_t(unsigned int , local->hw.extra_tx_headroom,
> sizeof(struct ieee80211_tx_status_rtap_hdr));
>
> drivers/net/wireless/rt*pci.c:
> rt2x00dev->hw->extra_tx_headroom = 0;
>
> So, as a quick-and-dirty fix, should I replace "max_t(..." by "4 + max_t(..."
> or "= 0" by "= 4" ?

4 + max ... should work if it's what we think it is. Let us know.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 08:51:30

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 00:56 +0300, Nikolai ZHUBR wrote:
> > Friday, October 30, 2009, 11:59:01 PM, Johannes Berg wrote:
> > >>
> > >> ieee80211_tx_status: headroom too small
> >
> > > What driver are you using?
> >
> > Ah, sorry, forgot to mention. It is rt61pci (RaLink mini-PCI card)
>
> Ok. I looked at the problem and it's a very odd problem -- mac80211
> should always reserve enough headroom in buffers it passes to the
> driver. Does rt61pci somehow realloc frames?

It does use skb_push. but will/should always stay within the limits
of rt2x00dev->hw->extra_tx_headroom. Although I just realize there
might be a situation where it needs to align the frame to a 4-byte boundary,
it could exceed that amount. :(

Ivo

2009-10-31 10:33:14

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 11:17 +0100, Ivo van Doorn wrote:
>
> > At the least the beacons, but I am not sure about if there are other frame types
> > which also have this requirement.
>
> So you align them all right now? Just wondering since beacons shouldn't
> be status-reported.

Currently I do all, but that is because I am not sure which specific frames
require it. I might just fix this for beacons only and just see what happens
with the other frames.

Ivo

2009-10-30 21:49:08

by Nikolai ZHUBR

[permalink] [raw]
Subject: Re[2]: ieee80211_tx_status: headroom too small

Friday, October 30, 2009, 11:59:01 PM, Johannes Berg wrote:
>>
>> ieee80211_tx_status: headroom too small

> What driver are you using?

Ah, sorry, forgot to mention. It is rt61pci (RaLink mini-PCI card)

Nikolai ZHUBR

> johannes



2009-10-31 05:46:04

by Johannes Berg

[permalink] [raw]
Subject: Re: Re[2]: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 00:56 +0300, Nikolai ZHUBR wrote:
> Friday, October 30, 2009, 11:59:01 PM, Johannes Berg wrote:
> >>
> >> ieee80211_tx_status: headroom too small
>
> > What driver are you using?
>
> Ah, sorry, forgot to mention. It is rt61pci (RaLink mini-PCI card)

Ok. I looked at the problem and it's a very odd problem -- mac80211
should always reserve enough headroom in buffers it passes to the
driver. Does rt61pci somehow realloc frames?

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 10:17:21

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 10:44 +0100, Ivo van Doorn wrote:
>
> > > Or maybe we should just not bother with having drivers do the alignment
> > > any more -- mac80211 does that now and it only does it if necessary
> > > (i.e. not on powerpc/x86)
> >
> > I think you mean the RX alignment, while I am talking about the TX alignment.
> > Beacons and some othe frames must be 4-byte alignment because the wireleless
> > hardware requires it.
>
> Oh, indeed, I got confused.
>
> Hmm. TX alignment is weird ... no driver really needs that except rt2x00
> afaik. Which frames require this?

At the least the beacons, but I am not sure about if there are other frame types
which also have this requirement.

Users had reported that if they align beacon frames, rt2500pci finally starts
beaconing correctly in master mode.

Ivo

2009-10-31 09:31:34

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 09:59 +0100, Ivo van Doorn wrote:
> > On Saturday 31 October 2009, Johannes Berg wrote:
> > > On Sat, 2009-10-31 at 09:51 +0100, Ivo van Doorn wrote:
> > >
> > > > > Ok. I looked at the problem and it's a very odd problem -- mac80211
> > > > > should always reserve enough headroom in buffers it passes to the
> > > > > driver. Does rt61pci somehow realloc frames?
> > > >
> > > > It does use skb_push. but will/should always stay within the limits
> > > > of rt2x00dev->hw->extra_tx_headroom. Although I just realize there
> > > > might be a situation where it needs to align the frame to a 4-byte boundary,
> > > > it could exceed that amount. :(
> > >
> > > extra_tx_headroom is set to max_t(driver, mac80211) so that there's
> > > always some headroom. This doesn't take into account alignment though.
> > > Should we simply add 2 to that value?
> >
> > 4 actually, wasn't there some cases where the header could have an odd
> > number of bytes and would require 3 bytes for alignment?
>
> Hm, is there? I thought they changed the 11s draft to no longer have
> that.

Well 2 is fine in that case, (I was basing my comment on something said on this mailinglist
a long time ago). :)

Ivo


2009-10-31 08:56:09

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 09:51 +0100, Ivo van Doorn wrote:

> > Ok. I looked at the problem and it's a very odd problem -- mac80211
> > should always reserve enough headroom in buffers it passes to the
> > driver. Does rt61pci somehow realloc frames?
>
> It does use skb_push. but will/should always stay within the limits
> of rt2x00dev->hw->extra_tx_headroom. Although I just realize there
> might be a situation where it needs to align the frame to a 4-byte boundary,
> it could exceed that amount. :(

extra_tx_headroom is set to max_t(driver, mac80211) so that there's
always some headroom. This doesn't take into account alignment though.
Should we simply add 2 to that value?

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-30 20:03:35

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Fri, Oct 30, 2009 at 1:00 PM, Nikolai ZHUBR <[email protected]> wrote:
> Hello people,
>
> I'm using compat-wireless-2009-09-14 with 2.6.25.20 and I have
> recently noticed lots of these in syslog:
>
> ieee80211_tx_status: headroom too small
> ieee80211_tx_status: headroom too small
> ieee80211_tx_status: headroom too small
>
> The wireless (access point) works fine, though maybe not very
> fast. However, this warning somehow bothers me. Is it important?
> Does it mean something critical? Maybe I should just comment
> it out in the code? I don't like syslog being filled up this
> way...

You're lucky it even worked on 2.6.25.

Luis

2009-10-31 09:34:38

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 10:30 +0100, Ivo van Doorn wrote:

> > > 4 actually, wasn't there some cases where the header could have an odd
> > > number of bytes and would require 3 bytes for alignment?
> >
> > Hm, is there? I thought they changed the 11s draft to no longer have
> > that.
>
> Well 2 is fine in that case, (I was basing my comment on something said on this mailinglist
> a long time ago). :)

However, it really all depends on the hardware too ... Maybe we should
put more of the logic into drivers, and have mac80211 only export its
required headroom?

Or maybe we should just not bother with having drivers do the alignment
any more -- mac80211 does that now and it only does it if necessary
(i.e. not on powerpc/x86)

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 09:44:43

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 10:30 +0100, Ivo van Doorn wrote:
>
> > > > 4 actually, wasn't there some cases where the header could have an odd
> > > > number of bytes and would require 3 bytes for alignment?
> > >
> > > Hm, is there? I thought they changed the 11s draft to no longer have
> > > that.
> >
> > Well 2 is fine in that case, (I was basing my comment on something said on this mailinglist
> > a long time ago). :)
>
> However, it really all depends on the hardware too ... Maybe we should
> put more of the logic into drivers, and have mac80211 only export its
> required headroom?
>
> Or maybe we should just not bother with having drivers do the alignment
> any more -- mac80211 does that now and it only does it if necessary
> (i.e. not on powerpc/x86)

I think you mean the RX alignment, while I am talking about the TX alignment.
Beacons and some othe frames must be 4-byte alignment because the wireleless
hardware requires it.

Ivo

2009-10-31 14:17:02

by Nikolai ZHUBR

[permalink] [raw]
Subject: Re[2]: ieee80211_tx_status: headroom too small

Saturday, October 31, 2009, 12:34:37 PM, Johannes Berg wrote:
>> > > 4 actually, wasn't there some cases where the header could have an odd
>> > > number of bytes and would require 3 bytes for alignment?
>> >
>> > Hm, is there? I thought they changed the 11s draft to no longer have
>> > that.
>>
>> Well 2 is fine in that case, (I was basing my comment on something said on this mailinglist
>> a long time ago). :)

> However, it really all depends on the hardware too ... Maybe we should
> put more of the logic into drivers, and have mac80211 only export its
> required headroom?

> Or maybe we should just not bother with having drivers do the alignment
> any more -- mac80211 does that now and it only does it if necessary
> (i.e. not on powerpc/x86)

I'm having this issue on mips platform, if it matters. I cannot test
it on x86 yet.

Now, searching for "extra_tx_headroom" gives me essentially the
following two:

net/mac80211/main.c:
local->tx_headroom = max_t(unsigned int , local->hw.extra_tx_headroom,
sizeof(struct ieee80211_tx_status_rtap_hdr));

drivers/net/wireless/rt*pci.c:
rt2x00dev->hw->extra_tx_headroom = 0;

So, as a quick-and-dirty fix, should I replace "max_t(..." by "4 + max_t(..."
or "= 0" by "= 4" ?

Thank you!

Nikolai ZHUBR

> johannes



2009-10-31 10:03:50

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 10:44 +0100, Ivo van Doorn wrote:

> > Or maybe we should just not bother with having drivers do the alignment
> > any more -- mac80211 does that now and it only does it if necessary
> > (i.e. not on powerpc/x86)
>
> I think you mean the RX alignment, while I am talking about the TX alignment.
> Beacons and some othe frames must be 4-byte alignment because the wireleless
> hardware requires it.

Oh, indeed, I got confused.

Hmm. TX alignment is weird ... no driver really needs that except rt2x00
afaik. Which frames require this?

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 09:25:15

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 09:59 +0100, Ivo van Doorn wrote:
> On Saturday 31 October 2009, Johannes Berg wrote:
> > On Sat, 2009-10-31 at 09:51 +0100, Ivo van Doorn wrote:
> >
> > > > Ok. I looked at the problem and it's a very odd problem -- mac80211
> > > > should always reserve enough headroom in buffers it passes to the
> > > > driver. Does rt61pci somehow realloc frames?
> > >
> > > It does use skb_push. but will/should always stay within the limits
> > > of rt2x00dev->hw->extra_tx_headroom. Although I just realize there
> > > might be a situation where it needs to align the frame to a 4-byte boundary,
> > > it could exceed that amount. :(
> >
> > extra_tx_headroom is set to max_t(driver, mac80211) so that there's
> > always some headroom. This doesn't take into account alignment though.
> > Should we simply add 2 to that value?
>
> 4 actually, wasn't there some cases where the header could have an odd
> number of bytes and would require 3 bytes for alignment?

Hm, is there? I thought they changed the 11s draft to no longer have
that.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 17:34:33

by Nikolai ZHUBR

[permalink] [raw]
Subject: Re[4]: ieee80211_tx_status: headroom too small

Saturday, October 31, 2009, 6:35:44 PM, Johannes Berg wrote:
>> net/mac80211/main.c:
>> local->tx_headroom = max_t(unsigned int , local->hw.extra_tx_headroom,
>> sizeof(struct ieee80211_tx_status_rtap_hdr));
>>
>> drivers/net/wireless/rt*pci.c:
>> rt2x00dev->hw->extra_tx_headroom = 0;
>>
>> So, as a quick-and-dirty fix, should I replace "max_t(..." by "4 + max_t(...."
>> or "= 0" by "= 4" ?

> 4 + max ... should work if it's what we think it is. Let us know.

Yes, it removed those messages.
The actual speed is still not very high (like at 10Mbit maybe,
though 54Mbit is shown and the distance from the AP is very small,
so signal quality should be good I suppose) but it is ok for me.

Thank you!

Nikolai ZHUBR


> johannes



2009-10-31 10:23:34

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Sat, 2009-10-31 at 11:17 +0100, Ivo van Doorn wrote:

> At the least the beacons, but I am not sure about if there are other frame types
> which also have this requirement.

So you align them all right now? Just wondering since beacons shouldn't
be status-reported.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-10-31 09:02:07

by Ivo Van Doorn

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Saturday 31 October 2009, Johannes Berg wrote:
> On Sat, 2009-10-31 at 09:51 +0100, Ivo van Doorn wrote:
>
> > > Ok. I looked at the problem and it's a very odd problem -- mac80211
> > > should always reserve enough headroom in buffers it passes to the
> > > driver. Does rt61pci somehow realloc frames?
> >
> > It does use skb_push. but will/should always stay within the limits
> > of rt2x00dev->hw->extra_tx_headroom. Although I just realize there
> > might be a situation where it needs to align the frame to a 4-byte boundary,
> > it could exceed that amount. :(
>
> extra_tx_headroom is set to max_t(driver, mac80211) so that there's
> always some headroom. This doesn't take into account alignment though.
> Should we simply add 2 to that value?

4 actually, wasn't there some cases where the header could have an odd
number of bytes and would require 3 bytes for alignment?

Ivo

2009-10-30 20:41:47

by Nikolai ZHUBR

[permalink] [raw]
Subject: Re[2]: ieee80211_tx_status: headroom too small

Friday, October 30, 2009, 11:03:20 PM, Luis R. Rodriguez wrote:

>> ieee80211_tx_status: headroom too small
>> ieee80211_tx_status: headroom too small
>> ieee80211_tx_status: headroom too small
>>
>> The wireless (access point) works fine, though maybe not very
>> fast. However, this warning somehow bothers me. Is it important?
>> Does it mean something critical? Maybe I should just comment
>> it out in the code? I don't like syslog being filled up this
>> way...

> You're lucky it even worked on 2.6.25.

Yeah but... 2.6.25 is the kernel used in the latest official release
of OpenWRT and in OpenSuse 11.0 as well (which is still their latest
reasonable release, as of now, too)

Ok, I'll probably wait for some updates.

Nikolai ZHUBR

> Luis



2009-10-30 20:59:00

by Johannes Berg

[permalink] [raw]
Subject: Re: ieee80211_tx_status: headroom too small

On Fri, 2009-10-30 at 23:00 +0300, Nikolai ZHUBR wrote:
> Hello people,
>
> I'm using compat-wireless-2009-09-14 with 2.6.25.20 and I have
> recently noticed lots of these in syslog:
>
> ieee80211_tx_status: headroom too small

What driver are you using?

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part