On Tue, Jul 8, 2008 at 12:43 AM, Harvey Harrison<harvey.harrison@xxxxxxxxx> wrote:> 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 | IEEE80211_STYPE_NULLFUNC)) ==>> > cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC);>> > }>> >>> > Tomas, you want a patch?>> >>> > Harvey>>>> 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 being> set.>I know but, what you missing is the check above. we bail out from thefunction if the packet is not qos data before so it's not relevant forcheckingof any null data just qos null data. I didn't want to touch the PScode that's why we've created a special handler for this particularcase.I think you patch works as well and maybe can be used PS mode aswell but I'm not sure now.. I will look at this alter.Tomas
> How about this:>> From: Harvey Harrison <harvey.harrison@xxxxxxxxx>> 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 exact> match.>> Signed-off-by: Harvey Harrison <harvey.harrison@xxxxxxxxx>> ---> 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)> }>> /**> + * 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 just need> + * to check the one bit> + */> + return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | IEEE80211_STYPE_NULLFUNC)) ==> + cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_NULLFUNC);> +}> +> +/**> * ieee80211_is_data_present - check if type is IEEE80211_FTYPE_DATA and 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 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_data_nullfunc(hdr->frame_control)))> goto end_reorder;>> /* new un-ordered ampdu frame - process it */> --> 1.5.6.1.322.ge904b>>>>��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
