2008-07-07 12:48:07

by Tomas Winkler

[permalink] [raw]
Subject: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

From: Emmanuel Grumbach <[email protected]>

This patch fixes the check at the entrance to ieee80211_rx_reorder_ampdu.
This check has been broken by 511fe3f3c4ba0b5b77421336f64a19b6cd00e65f
'mac80211: rx.c use new helpers'

Letting QoS NULL packet in ieee80211_rx_reorder_ampdu led to packet loss in
RX.

Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
---
include/linux/ieee80211.h | 11 +++++++++++
net/mac80211/rx.c | 2 +-
2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index aa603c3..1ab2480 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -460,6 +460,17 @@ static inline int ieee80211_is_nullfunc(__le16 fc)
cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC);
}

+/**
+ * ieee80211_is_qos_nullfunc - check if FTYPE=IEEE80211_FTYPE_DATA and
+ * STYPE=IEEE80211_STYPE_QOS_NULLFUNC
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline int ieee80211_is_qos_nullfunc(__le16 fc)
+{
+ return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | IEEE80211_FCTL_STYPE)) ==
+ cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_QOS_NULLFUNC);
+}
+
struct ieee80211s_hdr {
u8 flags;
u8 ttl;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6a88e8f..a6e8214 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2052,7 +2052,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];

/* null data frames are excluded */
- if (unlikely(ieee80211_is_nullfunc(hdr->frame_control)))
+ if (unlikely(ieee80211_is_qos_nullfunc(hdr->frame_control)))
goto end_reorder;

/* new un-ordered ampdu frame - process it */
--
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



2008-07-07 17:03:26

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Monday 07 July 2008 18:23:44 Harvey Harrison wrote:
> On Mon, 2008-07-07 at 15:48 +0300, Tomas Winkler wrote:
> > From: Emmanuel Grumbach <[email protected]>
> >
> > This patch fixes the check at the entrance to ieee80211_rx_reorder_ampdu.
> > This check has been broken by 511fe3f3c4ba0b5b77421336f64a19b6cd00e65f
> > 'mac80211: rx.c use new helpers'
> >
> Actually, do we want something like this instead:
>
> > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> > index 6a88e8f..a6e8214 100644
> > --- a/net/mac80211/rx.c
> > +++ b/net/mac80211/rx.c
> > @@ -2052,7 +2052,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
> > tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
> >
> > /* null data frames are excluded */
> > - if (unlikely(ieee80211_is_nullfunc(hdr->frame_control)))
> > + if (unlikely(ieee80211_is_qos_nullfunc(hdr->frame_control)))
> > goto end_reorder;
> >
> > /* new un-ordered ampdu frame - process it */
>
> if (unlikely(ieee80211_is_data(hdr->frame_control) && (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC))))
>
> Or wrap the above in a ieee80211_is_data_has_nullfunc() helper?

I think this helper naming is becoming a bit weird and obfuscating.
I'd say just opencode it, if it's only used in a few places.


--
Greetings Michael.

2008-07-07 21:43:09

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Tue, 2008-07-08 at 00:23 +0300, Tomas Winkler wrote:
> On Mon, Jul 7, 2008 at 10:26 PM, Harvey Harrison
> > Or, after having a coffee and actually engaging brain (similar to
> > ieee80211_is_data_qos)
> >
> > int ieee80211_is_data_nullfunc(__le16 fc)
> > {
> > return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | =EF=BB=BFIEE=
E80211_=EF=BB=BFSTYPE_NULLFUNC)) =3D=3D
> > cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_N=
ULLFUNC);
> > }
> >
> > Tomas, you want a patch?
> >
> > Harvey
>=20
> Maybe it's the late hour but I'm not sure what is wrong with the
> Emmanuel's patch. Mostly it fixes big stall in RX.
> Checked and validated.

Because there are two cases that want to be caught here:

Commit 511fe3f3c4ba0b5b77421336f64a19b6cd00e65f changed:

/* null data frames are excluded */
- if (unlikely(fc & IEEE80211_STYPE_NULLFUNC))
+ if (unlikely(ieee80211_is_nullfunc(hdr->frame_control)))

Which isn't quite equivalent, as it wants to catch just the one bit bei=
ng
set.

How about this:

=46rom: Harvey Harrison <[email protected]>
Subject: [PATCH] mac80211: rx.c fix nullfunc data test

commit 511fe3f3c4ba0b5b77421336f64a19b6cd00e65f mac80211: rx.c use new =
helpers
contained an error when testing for nullfunc data frames. Introduce a
new helper that tests just the NULLFUNC bit as before rather than an ex=
act
match.

Signed-off-by: Harvey Harrison <[email protected]>
---
include/linux/ieee80211.h | 14 ++++++++++++++
net/mac80211/rx.c | 2 +-
2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a1630ba..5ba407e 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -239,6 +239,20 @@ static inline int ieee80211_is_data_qos(__le16 fc)
}
=20
/**
+ * ieee80211_is_data_nullfunc - check if type is IEEE80211_FTYPE_DATA =
and IEEE80211_STYPE_NULLFUNC is set
+ * @fc: frame control bytes in little-endian byteorder
+ */
+static inline int ieee80211_is_data_nullfunc(__le16 fc)
+{
+ /*
+ * mask with STYPE_NULLFUNC rather than IEEE80211_FCTL_STYPE as we ju=
st need
+ * to check the one bit
+ */
+ return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | IEEE80211_STYPE_NULLF=
UNC)) =3D=3D
+ cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC);
+}
+
+/**
* ieee80211_is_data_present - check if type is IEEE80211_FTYPE_DATA a=
nd has data
* @fc: frame control bytes in little-endian byteorder
*/
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index fab443d..ce879e7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2047,7 +2047,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee8=
0211_local *local,
tid_agg_rx =3D sta->ampdu_mlme.tid_rx[tid];
=20
/* null data frames are excluded */
- if (unlikely(ieee80211_is_nullfunc(hdr->frame_control)))
+ if (unlikely(ieee80211_is_data_nullfunc(hdr->frame_control)))
goto end_reorder;
=20
/* new un-ordered ampdu frame - process it */
--=20
1.5.6.1.322.ge904b


2008-07-07 22:16:29

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

T24gVHVlLCBKdWwgOCwgMjAwOCBhdCAxMjo0MyBBTSwgSGFydmV5IEhhcnJpc29uCjxoYXJ2ZXku
aGFycmlzb25AZ21haWwuY29tPiB3cm90ZToKPiBPbiBUdWUsIDIwMDgtMDctMDggYXQgMDA6MjMg
KzAzMDAsIFRvbWFzIFdpbmtsZXIgd3JvdGU6Cj4+IE9uIE1vbiwgSnVsIDcsIDIwMDggYXQgMTA6
MjYgUE0sIEhhcnZleSBIYXJyaXNvbgo+PiA+IE9yLCBhZnRlciBoYXZpbmcgYSBjb2ZmZWUgYW5k
IGFjdHVhbGx5IGVuZ2FnaW5nIGJyYWluIChzaW1pbGFyIHRvCj4+ID4gaWVlZTgwMjExX2lzX2Rh
dGFfcW9zKQo+PiA+Cj4+ID4gaW50IGllZWU4MDIxMV9pc19kYXRhX251bGxmdW5jKF9fbGUxNiBm
YykKPj4gPiB7Cj4+ID4gICAgICAgIHJldHVybiAoZmMgJiBjcHVfdG9fbGUxNihJRUVFODAyMTFf
RkNUTF9GVFlQRSB8IO+7v0lFRUU4MDIxMV/vu79TVFlQRV9OVUxMRlVOQykpID09Cj4+ID4gICAg
ICAgICAgICAgICAgY3B1X3RvX2xlMTYoSUVFRTgwMjExX0ZUWVBFX0RBVEEgfCBJRUVFODAyMTFf
U1RZUEVfTlVMTEZVTkMpOwo+PiA+IH0KPj4gPgo+PiA+IFRvbWFzLCB5b3Ugd2FudCBhIHBhdGNo
Pwo+PiA+Cj4+ID4gSGFydmV5Cj4+Cj4+IE1heWJlIGl0J3MgdGhlIGxhdGUgaG91ciBidXQgSSdt
IG5vdCBzdXJlIHdoYXQgaXMgd3Jvbmcgd2l0aCB0aGUKPj4gRW1tYW51ZWwncyAgcGF0Y2guIE1v
c3RseSBpdCBmaXhlcyBiaWcgc3RhbGwgaW4gUlguCj4+IENoZWNrZWQgYW5kIHZhbGlkYXRlZC4K
Pgo+IEJlY2F1c2UgdGhlcmUgYXJlIHR3byBjYXNlcyB0aGF0IHdhbnQgdG8gYmUgY2F1Z2h0IGhl
cmU6Cj4KPiBDb21taXQgNTExZmUzZjNjNGJhMGI1Yjc3NDIxMzM2ZjY0YTE5YjZjZDAwZTY1ZiBj
aGFuZ2VkOgo+Cj4gICAgICAgIC8qIG51bGwgZGF0YSBmcmFtZXMgYXJlIGV4Y2x1ZGVkICovCj4g
LSAgICAgICBpZiAodW5saWtlbHkoZmMgJiBJRUVFODAyMTFfU1RZUEVfTlVMTEZVTkMpKQo+ICsg
ICAgICAgaWYgKHVubGlrZWx5KGllZWU4MDIxMV9pc19udWxsZnVuYyhoZHItPmZyYW1lX2NvbnRy
b2wpKSkKPgo+IFdoaWNoIGlzbid0IHF1aXRlIGVxdWl2YWxlbnQsIGFzIGl0IHdhbnRzIHRvIGNh
dGNoIGp1c3QgdGhlIG9uZSBiaXQgYmVpbmcKPiBzZXQuCj4KSSBrbm93IGJ1dCwgd2hhdCB5b3Ug
bWlzc2luZyBpcyB0aGUgY2hlY2sgYWJvdmUuIHdlIGJhaWwgb3V0IGZyb20gdGhlCmZ1bmN0aW9u
IGlmIHRoZSBwYWNrZXQgaXMgbm90IHFvcyBkYXRhIGJlZm9yZSBzbyBpdCdzIG5vdCByZWxldmFu
dCBmb3IKY2hlY2tpbmcKb2YgYW55IG51bGwgZGF0YSBqdXN0IHFvcyBudWxsIGRhdGEuIEkgZGlk
bid0IHdhbnQgdG8gdG91Y2ggdGhlIFBTCmNvZGUgdGhhdCdzIHdoeSB3ZSd2ZSBjcmVhdGVkIGEg
c3BlY2lhbCBoYW5kbGVyIGZvciB0aGlzIHBhcnRpY3VsYXIKY2FzZS4KSSB0aGluayB5b3UgcGF0
Y2ggd29ya3MgYXMgd2VsbCAgYW5kIG1heWJlIGNhbiBiZSB1c2VkICBQUyBtb2RlIGFzCndlbGwg
YnV0IEknbSBub3Qgc3VyZSBub3cuLiBJIHdpbGwgbG9vayBhdCB0aGlzIGFsdGVyLgpUb21hcwoK
PiBIb3cgYWJvdXQgdGhpczoKPgo+IEZyb206IEhhcnZleSBIYXJyaXNvbiA8aGFydmV5LmhhcnJp
c29uQGdtYWlsLmNvbT4KPiBTdWJqZWN0OiBbUEFUQ0hdIG1hYzgwMjExOiByeC5jIGZpeCBudWxs
ZnVuYyBkYXRhIHRlc3QKPgo+IGNvbW1pdCA1MTFmZTNmM2M0YmEwYjViNzc0MjEzMzZmNjRhMTli
NmNkMDBlNjVmIG1hYzgwMjExOiByeC5jIHVzZSBuZXcgaGVscGVycwo+IGNvbnRhaW5lZCBhbiBl
cnJvciB3aGVuIHRlc3RpbmcgZm9yIG51bGxmdW5jIGRhdGEgZnJhbWVzLiAgSW50cm9kdWNlIGEK
PiBuZXcgaGVscGVyIHRoYXQgdGVzdHMganVzdCB0aGUgTlVMTEZVTkMgYml0IGFzIGJlZm9yZSBy
YXRoZXIgdGhhbiBhbiBleGFjdAo+IG1hdGNoLgo+Cj4gU2lnbmVkLW9mZi1ieTogSGFydmV5IEhh
cnJpc29uIDxoYXJ2ZXkuaGFycmlzb25AZ21haWwuY29tPgo+IC0tLQo+ICBpbmNsdWRlL2xpbnV4
L2llZWU4MDIxMS5oIHwgICAxNCArKysrKysrKysrKysrKwo+ICBuZXQvbWFjODAyMTEvcnguYyAg
ICAgICAgIHwgICAgMiArLQo+ICAyIGZpbGVzIGNoYW5nZWQsIDE1IGluc2VydGlvbnMoKyksIDEg
ZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9saW51eC9pZWVlODAyMTEuaCBi
L2luY2x1ZGUvbGludXgvaWVlZTgwMjExLmgKPiBpbmRleCBhMTYzMGJhLi41YmE0MDdlIDEwMDY0
NAo+IC0tLSBhL2luY2x1ZGUvbGludXgvaWVlZTgwMjExLmgKPiArKysgYi9pbmNsdWRlL2xpbnV4
L2llZWU4MDIxMS5oCj4gQEAgLTIzOSw2ICsyMzksMjAgQEAgc3RhdGljIGlubGluZSBpbnQgaWVl
ZTgwMjExX2lzX2RhdGFfcW9zKF9fbGUxNiBmYykKPiAgfQo+Cj4gIC8qKgo+ICsgKiBpZWVlODAy
MTFfaXNfZGF0YV9udWxsZnVuYyAtIGNoZWNrIGlmIHR5cGUgaXMgSUVFRTgwMjExX0ZUWVBFX0RB
VEEgYW5kIElFRUU4MDIxMV9TVFlQRV9OVUxMRlVOQyBpcyBzZXQKPiArICogQGZjOiBmcmFtZSBj
b250cm9sIGJ5dGVzIGluIGxpdHRsZS1lbmRpYW4gYnl0ZW9yZGVyCj4gKyAqLwo+ICtzdGF0aWMg
aW5saW5lIGludCBpZWVlODAyMTFfaXNfZGF0YV9udWxsZnVuYyhfX2xlMTYgZmMpCj4gK3sKPiAr
ICAgICAgIC8qCj4gKyAgICAgICAgKiBtYXNrIHdpdGggU1RZUEVfTlVMTEZVTkMgcmF0aGVyIHRo
YW4gSUVFRTgwMjExX0ZDVExfU1RZUEUgYXMgd2UganVzdCBuZWVkCj4gKyAgICAgICAgKiB0byBj
aGVjayB0aGUgb25lIGJpdAo+ICsgICAgICAgICovCj4gKyAgICAgICByZXR1cm4gKGZjICYgY3B1
X3RvX2xlMTYoSUVFRTgwMjExX0ZDVExfRlRZUEUgfCBJRUVFODAyMTFfU1RZUEVfTlVMTEZVTkMp
KSA9PQo+ICsgICAgICAgICAgICAgIGNwdV90b19sZTE2KElFRUU4MDIxMV9GVFlQRV9EQVRBIHwg
SUVFRTgwMjExX1NUWVBFX05VTExGVU5DKTsKPiArfQo+ICsKPiArLyoqCj4gICogaWVlZTgwMjEx
X2lzX2RhdGFfcHJlc2VudCAtIGNoZWNrIGlmIHR5cGUgaXMgSUVFRTgwMjExX0ZUWVBFX0RBVEEg
YW5kIGhhcyBkYXRhCj4gICogQGZjOiBmcmFtZSBjb250cm9sIGJ5dGVzIGluIGxpdHRsZS1lbmRp
YW4gYnl0ZW9yZGVyCj4gICovCj4gZGlmZiAtLWdpdCBhL25ldC9tYWM4MDIxMS9yeC5jIGIvbmV0
L21hYzgwMjExL3J4LmMKPiBpbmRleCBmYWI0NDNkLi5jZTg3OWU3IDEwMDY0NAo+IC0tLSBhL25l
dC9tYWM4MDIxMS9yeC5jCj4gKysrIGIvbmV0L21hYzgwMjExL3J4LmMKPiBAQCAtMjA0Nyw3ICsy
MDQ3LDcgQEAgc3RhdGljIHU4IGllZWU4MDIxMV9yeF9yZW9yZGVyX2FtcGR1KHN0cnVjdCBpZWVl
ODAyMTFfbG9jYWwgKmxvY2FsLAo+ICAgICAgICB0aWRfYWdnX3J4ID0gc3RhLT5hbXBkdV9tbG1l
LnRpZF9yeFt0aWRdOwo+Cj4gICAgICAgIC8qIG51bGwgZGF0YSBmcmFtZXMgYXJlIGV4Y2x1ZGVk
ICovCj4gLSAgICAgICBpZiAodW5saWtlbHkoaWVlZTgwMjExX2lzX251bGxmdW5jKGhkci0+ZnJh
bWVfY29udHJvbCkpKQo+ICsgICAgICAgaWYgKHVubGlrZWx5KGllZWU4MDIxMV9pc19kYXRhX251
bGxmdW5jKGhkci0+ZnJhbWVfY29udHJvbCkpKQo+ICAgICAgICAgICAgICAgIGdvdG8gZW5kX3Jl
b3JkZXI7Cj4KPiAgICAgICAgLyogbmV3IHVuLW9yZGVyZWQgYW1wZHUgZnJhbWUgLSBwcm9jZXNz
IGl0ICovCj4gLS0KPiAxLjUuNi4xLjMyMi5nZTkwNGIKPgo+Cj4KPgo=

2008-07-07 22:44:48

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Tue, 2008-07-08 at 01:16 +0300, Tomas Winkler wrote:
> On Tue, Jul 8, 2008 at 12:43 AM, Harvey Harrison
> > set.
> >
> I know but, what you missing is the check above. we bail out from the
> function if the packet is not qos data before so it's not relevant for
> checking
> of any null data just qos null data. I didn't want to touch the PS
> code that's why we've created a special handler for this particular
> case.
> I think you patch works as well and maybe can be used PS mode as
> well but I'm not sure now.. I will look at this alter.
> Tomas
>

Good point, a helper probably isn't right for this as it's already been
checked that it's a data packet with qos, so opencoding the nullfunc
test is probably the right thing (as it was before my change).

if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))

Cheers,

Harvey


2008-07-07 16:23:39

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Mon, 2008-07-07 at 15:48 +0300, Tomas Winkler wrote:
> From: Emmanuel Grumbach <[email protected]>
>
> This patch fixes the check at the entrance to ieee80211_rx_reorder_ampdu.
> This check has been broken by 511fe3f3c4ba0b5b77421336f64a19b6cd00e65f
> 'mac80211: rx.c use new helpers'
>
Actually, do we want something like this instead:

> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 6a88e8f..a6e8214 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -2052,7 +2052,7 @@ static u8 ieee80211_rx_reorder_ampdu(struct ieee80211_local *local,
> tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
>
> /* null data frames are excluded */
> - if (unlikely(ieee80211_is_nullfunc(hdr->frame_control)))
> + if (unlikely(ieee80211_is_qos_nullfunc(hdr->frame_control)))
> goto end_reorder;
>
> /* new un-ordered ampdu frame - process it */

if (unlikely(ieee80211_is_data(hdr->frame_control) && (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC))))

Or wrap the above in a ieee80211_is_data_has_nullfunc() helper?

Cheers,

Harvey



2008-07-07 23:01:27

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Tue, Jul 8, 2008 at 1:44 AM, Harvey Harrison
<[email protected]> wrote:
> On Tue, 2008-07-08 at 01:16 +0300, Tomas Winkler wrote:
>> On Tue, Jul 8, 2008 at 12:43 AM, Harvey Harrison
>> > set.
>> >
>> I know but, what you missing is the check above. we bail out from the
>> function if the packet is not qos data before so it's not relevant for
>> checking
>> of any null data just qos null data. I didn't want to touch the PS
>> code that's why we've created a special handler for this particular
>> case.
>> I think you patch works as well and maybe can be used PS mode as
>> well but I'm not sure now.. I will look at this alter.
>> Tomas
>>
>
> Good point, a helper probably isn't right for this as it's already been
> checked that it's a data packet with qos, so opencoding the nullfunc
> test is probably the right thing (as it was before my change).
>
> if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
>
> Cheers,

Sound reasonable will drop this patch in favor of the open code.
Thanks
Tomas

2008-07-07 21:23:31

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

T24gTW9uLCBKdWwgNywgMjAwOCBhdCAxMDoyNiBQTSwgSGFydmV5IEhhcnJpc29uCjxoYXJ2ZXku
aGFycmlzb25AZ21haWwuY29tPiB3cm90ZToKPiBPbiBNb24sIDIwMDgtMDctMDcgYXQgMTk6MDIg
KzAyMDAsIE1pY2hhZWwgQnVlc2NoIHdyb3RlOgo+PiBPbiBNb25kYXkgMDcgSnVseSAyMDA4IDE4
OjIzOjQ0IEhhcnZleSBIYXJyaXNvbiB3cm90ZToKPj4gPiBPbiBNb24sIDIwMDgtMDctMDcgYXQg
MTU6NDggKzAzMDAsIFRvbWFzIFdpbmtsZXIgd3JvdGU6Cj4+ID4gPiBGcm9tOiBFbW1hbnVlbCBH
cnVtYmFjaCA8ZW1tYW51ZWwuZ3J1bWJhY2hAaW50ZWwuY29tPgo+PiA+ID4KPj4gPiA+IFRoaXMg
cGF0Y2ggZml4ZXMgdGhlIGNoZWNrIGF0IHRoZSBlbnRyYW5jZSB0byBpZWVlODAyMTFfcnhfcmVv
cmRlcl9hbXBkdS4KPj4gPiA+IFRoaXMgY2hlY2sgaGFzIGJlZW4gYnJva2VuIGJ5IDUxMWZlM2Yz
YzRiYTBiNWI3NzQyMTMzNmY2NGExOWI2Y2QwMGU2NWYKPj4gPiA+ICdtYWM4MDIxMTogcnguYyB1
c2UgbmV3IGhlbHBlcnMnCj4+ID4gPgo+PiA+IEFjdHVhbGx5LCBkbyB3ZSB3YW50IHNvbWV0aGlu
ZyBsaWtlIHRoaXMgaW5zdGVhZDoKPj4gPgo+Cj4gT3IsIGFmdGVyIGhhdmluZyBhIGNvZmZlZSBh
bmQgYWN0dWFsbHkgZW5nYWdpbmcgYnJhaW4gKHNpbWlsYXIgdG8KPiBpZWVlODAyMTFfaXNfZGF0
YV9xb3MpCj4KPiBpbnQgaWVlZTgwMjExX2lzX2RhdGFfbnVsbGZ1bmMoX19sZTE2IGZjKQo+IHsK
PiAgICAgICAgcmV0dXJuIChmYyAmIGNwdV90b19sZTE2KElFRUU4MDIxMV9GQ1RMX0ZUWVBFIHwg
77u/SUVFRTgwMjExX++7v1NUWVBFX05VTExGVU5DKSkgPT0KPiAgICAgICAgICAgICAgICBjcHVf
dG9fbGUxNihJRUVFODAyMTFfRlRZUEVfREFUQSB8IElFRUU4MDIxMV9TVFlQRV9OVUxMRlVOQyk7
Cj4gfQo+Cj4gVG9tYXMsIHlvdSB3YW50IGEgcGF0Y2g/Cj4KPiBIYXJ2ZXkKCk1heWJlIGl0J3Mg
dGhlIGxhdGUgaG91ciBidXQgSSdtIG5vdCBzdXJlIHdoYXQgaXMgd3Jvbmcgd2l0aCB0aGUKRW1t
YW51ZWwncyAgcGF0Y2guIE1vc3RseSBpdCBmaXhlcyBiaWcgc3RhbGwgaW4gUlguCkNoZWNrZWQg
YW5kIHZhbGlkYXRlZC4KVGhhbmtzClRvbWFzCg==

2008-07-07 19:26:48

by Harvey Harrison

[permalink] [raw]
Subject: Re: [PATCH 1/1 V2] mac80211: Fix ieee80211_rx_reorder_ampdu: ignore QoS null packets

On Mon, 2008-07-07 at 19:02 +0200, Michael Buesch wrote:
> On Monday 07 July 2008 18:23:44 Harvey Harrison wrote:
> > On Mon, 2008-07-07 at 15:48 +0300, Tomas Winkler wrote:
> > > From: Emmanuel Grumbach <[email protected]>
> > >=20
> > > This patch fixes the check at the entrance to ieee80211_rx_reorde=
r_ampdu.
> > > This check has been broken by 511fe3f3c4ba0b5b77421336f64a19b6cd0=
0e65f
> > > 'mac80211: rx.c use new helpers'
> > >=20
> > Actually, do we want something like this instead:
> >

Or, after having a coffee and actually engaging brain (similar to=20
ieee80211_is_data_qos)

int ieee80211_is_data_nullfunc(__le16 fc)
{
return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | =EF=BB=BFIEEE80211_=EF=
=BB=BFSTYPE_NULLFUNC)) =3D=3D
cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC);
}

Tomas, you want a patch?

Harvey