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

[Harvey Harrison <harvey.harrison@gmail.com>]

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 <harvey.harrison@gmail.com>
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 <harvey.harrison@gmail.com>
---
 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



--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html