Search Linux Wireless

Re: [PATCH 05/14] mac80211: adding 802.11n essential A-MPDU addBA capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> As this series of patches only adds HT handling with no aggregations,
> (A-MPDU aggregations acceptance is not obligatory according to 802.11n draft)
> we are currently sending back a refusal upon this request.

Heh. In the Kconfig you just wrote that it handles AMPDU and AMSDU
aggregation which I'd expect to imply deaggregation too. Nothing
serious, just seemed a bit weird! :)

And isn't one of the two (I always forget which one) rather a feature of
the hardware?

> +#ifdef CONFIG_MAC80211_HT
> +/* mgmt header + 1 byte action code */
> +#define IEEE80211_MIN_ACTION_SIZE (24 + 1)
> +
> +#define IEEE80211_ADDBA_PARAM_POLICY_MASK 0x0002
> +#define IEEE80211_ADDBA_PARAM_TID_MASK 0x003C
> +#define IEEE80211_ADDBA_PARAM_BUF_SIZE_MASK 0xFFA0
> +#endif /* CONFIG_MAC80211_HT */

Leave out the ifdefs please when they only protect definitions or such,
i.e. nothing that will generate code/allocate structure space.


> +	capab |= (u16)(buf_size << 6);	/* bit 15:6 max size of aggergation */

typo: aggregation. Interesting what an email program with a spell
checker makes you notice :) (No, I'm not suggesting you run a spell
checker, I'm rather saying that I probably wouldn't have noticed
otherwise)

> +static void ieee80211_sta_process_addba_request(struct net_device *dev,


> +	/* TODO - add here aggregation support */

I think that comment is misleading. We don't add aggregation support
here but rather when adding it we must change this code, but anyway.
Don't bother.

> +			printk(KERN_DEBUG "%s: received unsupported BACK\n",
> +					dev->name);

net_ratelimit() please, otherwise people can flood our logs by just
sending us strange frames.

Btw. I'd like to remove code from ieee80211_sta.c rather than add this
much. Do you see any way to partition your code differently? The
ieee80211_sta.c file is a huge file and barely understandable when you
just read it... I was at one time working on extracting the scan code
out from it and keep only MLME stuff in there. My goal was to be able to
not compile ieee80211_sta.c unless the user wanted the in-kernel MLME,
but maybe I should rather put the MLME into a new file then. I'd
appreciate if you could give it (partitioning the code otherwise) a
thought, but I promise I won't be disappointed if you don't want to do
it.

johannes

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux