On Tue, Mar 27, 2007 at 06:01:45PM +0200, Johannes Berg wrote:
> This patch optimises the ieee80211_get_hdrlen function by exploiting the
> bit masks directly.
> --- wireless-dev.orig/net/mac80211/ieee80211.c 2007-03-27 16:33:47.163155480 +0200
> +++ wireless-dev/net/mac80211/ieee80211.c 2007-03-27 16:33:48.473155480 +0200
> @@ -258,19 +258,24 @@ int ieee80211_get_hdrlen(u16 fc)
> case IEEE80211_FTYPE_DATA:
> if ((fc & IEEE80211_FCTL_FROMDS) && (fc & IEEE80211_FCTL_TODS))
> hdrlen = 30; /* Addr4 */
> - if (fc & IEEE80211_STYPE_QOS_DATA)
> - hdrlen += 2; /* QoS Control Field */
> + /* QoS Control Field */
> + hdrlen += (fc & IEEE80211_STYPE_QOS_DATA)
> + >> (ilog2(IEEE80211_STYPE_QOS_DATA)-1);
> break;
Could you please add a comment explaining what exactly happens here and
more importantly, include an easy way of understanding that this adds 2
bytes.
> case IEEE80211_FTYPE_CTL:
> - switch (fc & IEEE80211_FCTL_STYPE) {
> - case IEEE80211_STYPE_CTS:
> - case IEEE80211_STYPE_ACK:
> + /*
> + * ACK and CTS are 10 bytes, all others 16. To see how
> + * to get this condition consider
> + * subtype mask: 0b0000000011110000 (0x00F0)
> + * ACK subtype: 0b0000000011010000 (0x00D0)
> + * CTS subtype: 0b0000000011000000 (0x00C0)
> + * bits that matter: ^^^ (0x00E0)
> + * value of those: 0b0000000011000000 (0x00C0)
> + */
> + if ((fc & 0xE0) == 0xC0)
> hdrlen = 10;
> - break;
> - default:
> + else
> hdrlen = 16;
> - break;
> - }
Is this case even used anywhere? Looks like unnecessary optimization at
the cost of making the source code more difficult to understand and
modify.
--
Jouni Malinen PGP id EFC895FA
-
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
