2008-06-05 02:33:22

by Harvey Harrison

[permalink] [raw]
Subject: [RFC-PATCH] mac80211: add helpers for common frame_control testing

Example users added in wpa.c.

The byteshifting is done on the constants at compile-time making
each one of these helpers a mask+test only.

Signed-off-by: Harvey Harrison <[email protected]>
---
include/linux/ieee80211.h | 63 +++++++++++++++++++++++++++++++++++----------
net/mac80211/wpa.c | 40 ++++++----------------------
2 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 9300f37..5880b9f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -109,6 +109,50 @@ struct ieee80211_hdr {
u8 addr4[6];
} __attribute__ ((packed));

+static inline int ieee80211_has_tods(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_TODS)) != 0;
+}
+
+static inline int ieee80211_has_fromds(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FROMDS)) != 0;
+}
+
+static inline int ieee80211_has_a4(struct ieee80211_hdr *hdr)
+{
+ __le16 tmp = cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_TODS);
+ return (hdr->frame_control & tmp) == tmp;
+}
+
+static inline int ieee80211_is_mgmt(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
+ cpu_to_le16(IEEE80211_FTYPE_MGMT);
+}
+
+static inline int ieee80211_is_ctl(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
+ cpu_to_le16(IEEE80211_FTYPE_CTL);
+}
+
+static inline int ieee80211_is_data(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
+ cpu_to_le16(IEEE80211_FTYPE_DATA);
+}
+
+static inline int ieee80211_data_has_qos(struct ieee80211_hdr *hdr)
+{
+ /*
+ * mask with QOS_DATA rather than IEEE80211_FCTL_STYPE as we just need
+ * to check the one bit
+ */
+ return (hdr->frame_control &
+ cpu_to_le16(IEEE80211_FCTL_FTYPE | IEEE80211_STYPE_QOS_DATA)) ==
+ cpu_to_le16(IEEE80211_FTYPE_DATA | IEEE80211_STYPE_QOS_DATA);
+}

struct ieee80211s_hdr {
u8 flags;
@@ -564,17 +608,11 @@ enum ieee80211_back_parties {
*/
static inline u8 *ieee80211_get_SA(struct ieee80211_hdr *hdr)
{
- __le16 fc = hdr->frame_control;
- fc &= cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
-
- switch (fc) {
- case __constant_cpu_to_le16(IEEE80211_FCTL_FROMDS):
- return hdr->addr3;
- case __constant_cpu_to_le16(IEEE80211_FCTL_TODS|IEEE80211_FCTL_FROMDS):
+ if (ieee80211_has_a4(hdr))
return hdr->addr4;
- default:
- return hdr->addr2;
- }
+ if (ieee80211_has_fromds(hdr))
+ return hdr->addr3;
+ return hdr->addr2;
}

/**
@@ -590,10 +628,7 @@ static inline u8 *ieee80211_get_SA(struct ieee80211_hdr *hdr)
*/
static inline u8 *ieee80211_get_DA(struct ieee80211_hdr *hdr)
{
- __le16 fc = hdr->frame_control;
- fc &= cpu_to_le16(IEEE80211_FCTL_TODS);
-
- if (fc)
+ if (ieee80211_has_tods(hdr))
return hdr->addr3;
else
return hdr->addr1;
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 9f6fd20..0c728e0 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -24,48 +24,26 @@ static int ieee80211_get_hdr_info(const struct sk_buff *skb, u8 **sa, u8 **da,
{
struct ieee80211_hdr *hdr;
size_t hdrlen;
- u16 fc;
- int a4_included;
- u8 *pos;

hdr = (struct ieee80211_hdr *) skb->data;
- fc = le16_to_cpu(hdr->frame_control);

hdrlen = 24;
- if ((fc & (IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS)) ==
- (IEEE80211_FCTL_FROMDS | IEEE80211_FCTL_TODS)) {
+ if (ieee80211_has_a4(hdr))
hdrlen += ETH_ALEN;
- *sa = hdr->addr4;
- *da = hdr->addr3;
- } else if (fc & IEEE80211_FCTL_FROMDS) {
- *sa = hdr->addr3;
- *da = hdr->addr1;
- } else if (fc & IEEE80211_FCTL_TODS) {
- *sa = hdr->addr2;
- *da = hdr->addr3;
- } else {
- *sa = hdr->addr2;
- *da = hdr->addr1;
- }

- if (fc & 0x80)
+ *sa = ieee80211_get_SA(hdr);
+ *da = ieee80211_get_DA(hdr);
+
+ if (ieee80211_data_has_qos(hdr)) {
hdrlen += 2;
+ *qos_tid = (*((u8 *)hdr + hdrlen - 2) & 0x0f) | 0x80;
+ } else {
+ *qos_tid = 0;
+ }

*data = skb->data + hdrlen;
*data_len = skb->len - hdrlen;

- a4_included = (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) ==
- (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
- if ((fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_DATA &&
- fc & IEEE80211_STYPE_QOS_DATA) {
- pos = (u8 *) &hdr->addr4;
- if (a4_included)
- pos += 6;
- *qos_tid = pos[0] & 0x0f;
- *qos_tid |= 0x80; /* qos_included flag */
- } else
- *qos_tid = 0;
-
return skb->len < hdrlen ? -1 : 0;
}

--
1.5.6.rc1.257.gba91d





2008-06-05 18:16:24

by Harvey Harrison

[permalink] [raw]
Subject: Re: [RFC-PATCH] mac80211: add helpers for common frame_control testing

From: Harvey Harrison <[email protected]>
Subject: [PATCH] mac80211: const fixes and sign change in ieee80211_get_hdrlen_from_skb

Signed-off-by: Harvey Harrison <[email protected]>
---
Incremental patch to my last, just looking for comments on the approach/helpers.
If the approach looks good to people I'll split it into a proper series:

drivers/net/wireless/ath5k/base.c | 4 +-
include/linux/ieee80211.h | 16 +++++++-------
include/net/mac80211.h | 2 +-
net/mac80211/main.c | 17 ++++++---------
net/mac80211/rx.c | 3 +-
net/mac80211/util.c | 41 +++++++++++++++++++++++++++++++-----
net/mac80211/wme.c | 2 +-
7 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 85045af..1107ac5 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1775,7 +1775,7 @@ ath5k_tasklet_rx(unsigned long data)
struct ath5k_buf *bf;
struct ath5k_desc *ds;
int ret;
- int hdrlen;
+ unsigned int hdrlen;
int pad;

spin_lock(&sc->rxbuflock);
@@ -2627,7 +2627,7 @@ ath5k_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
struct ath5k_buf *bf;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
unsigned long flags;
- int hdrlen;
+ unsigned int hdrlen;
int pad;

ath5k_debug_dump_skb(sc, skb, "TX ", 1);
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 5880b9f..dd379f9 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -109,41 +109,41 @@ struct ieee80211_hdr {
u8 addr4[6];
} __attribute__ ((packed));

-static inline int ieee80211_has_tods(struct ieee80211_hdr *hdr)
+static inline int ieee80211_has_tods(const struct ieee80211_hdr *hdr)
{
return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_TODS)) != 0;
}

-static inline int ieee80211_has_fromds(struct ieee80211_hdr *hdr)
+static inline int ieee80211_has_fromds(const struct ieee80211_hdr *hdr)
{
return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FROMDS)) != 0;
}

-static inline int ieee80211_has_a4(struct ieee80211_hdr *hdr)
+static inline int ieee80211_has_a4(const struct ieee80211_hdr *hdr)
{
- __le16 tmp = cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_TODS);
+ __le16 tmp = cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS);
return (hdr->frame_control & tmp) == tmp;
}

-static inline int ieee80211_is_mgmt(struct ieee80211_hdr *hdr)
+static inline int ieee80211_is_mgmt(const struct ieee80211_hdr *hdr)
{
return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
cpu_to_le16(IEEE80211_FTYPE_MGMT);
}

-static inline int ieee80211_is_ctl(struct ieee80211_hdr *hdr)
+static inline int ieee80211_is_ctl(const struct ieee80211_hdr *hdr)
{
return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
cpu_to_le16(IEEE80211_FTYPE_CTL);
}

-static inline int ieee80211_is_data(struct ieee80211_hdr *hdr)
+static inline int ieee80211_is_data(const struct ieee80211_hdr *hdr)
{
return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
cpu_to_le16(IEEE80211_FTYPE_DATA);
}

-static inline int ieee80211_data_has_qos(struct ieee80211_hdr *hdr)
+static inline int ieee80211_data_has_qos(const struct ieee80211_hdr *hdr)
{
/*
* mask with QOS_DATA rather than IEEE80211_FCTL_STYPE as we just need
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1196de8..f9b391f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1535,7 +1535,7 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
*
* @skb: the frame
*/
-int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb);
+unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb);

/**
* ieee80211_get_hdrlen - get header length from frame control
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 4b57de4..26b97c8 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1252,8 +1252,9 @@ static void ieee80211_remove_tx_extra(struct ieee80211_local *local,
struct ieee80211_key *key,
struct sk_buff *skb)
{
- int hdrlen, iv_len, mic_len;
+ unsigned int hdrlen, iv_len, mic_len;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_hdr *hdr;

info->flags &= IEEE80211_TX_CTL_REQ_TX_STATUS |
IEEE80211_TX_CTL_DO_NOT_ENCRYPT |
@@ -1291,15 +1292,11 @@ static void ieee80211_remove_tx_extra(struct ieee80211_local *local,
}

no_key:
- {
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- u16 fc = le16_to_cpu(hdr->frame_control);
- if ((fc & 0x8C) == 0x88) /* QoS Control Field */ {
- fc &= ~IEEE80211_STYPE_QOS_DATA;
- hdr->frame_control = cpu_to_le16(fc);
- memmove(skb->data + 2, skb->data, hdrlen - 2);
- skb_pull(skb, 2);
- }
+ hdr = (struct ieee80211_hdr *)skb->data;
+ if (ieee80211_data_has_qos(hdr)) {
+ hdr->frame_control &= ~cpu_to_le16(IEEE80211_STYPE_QOS_DATA);
+ memmove(skb->data + 2, skb->data, hdrlen - 2);
+ skb_pull(skb, 2);
}
}

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a3643fd..63f19cb 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1586,7 +1586,8 @@ static void ieee80211_rx_michael_mic_report(struct net_device *dev,
struct ieee80211_hdr *hdr,
struct ieee80211_rx_data *rx)
{
- int keyidx, hdrlen;
+ int keyidx;
+ unsigned int hdrlen;
DECLARE_MAC_BUF(mac);
DECLARE_MAC_BUF(mac2);

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 6513bc2..c883d59 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -94,6 +94,36 @@ u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len,
return NULL;
}

+static unsigned int ieee80211_hdrlen(const struct ieee80211_hdr *hdr)
+{
+ unsigned int hdrlen = 24;
+
+ if (ieee80211_is_data(hdr)) {
+ if (ieee80211_has_a4(hdr))
+ hdrlen = 30;
+
+ if (ieee80211_data_has_qos(hdr))
+ hdrlen += 2;
+ } else if (ieee80211_is_ctl(hdr)) {
+ /*
+ * 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 ((hdr->frame_control & cpu_to_le16(0x00E0)) ==
+ cpu_to_le16(0x00C0))
+ hdrlen = 10;
+ else
+ hdrlen = 16;
+ }
+
+ return hdrlen;
+}
+
int ieee80211_get_hdrlen(u16 fc)
{
int hdrlen = 24;
@@ -133,15 +163,14 @@ int ieee80211_get_hdrlen(u16 fc)
}
EXPORT_SYMBOL(ieee80211_get_hdrlen);

-int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb)
+unsigned int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb)
{
- const struct ieee80211_hdr *hdr = (const struct ieee80211_hdr *) skb->data;
- int hdrlen;
+ unsigned int hdrlen;

- if (unlikely(skb->len < 10))
+ if (skb->len < 10)
return 0;
- hdrlen = ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control));
- if (unlikely(hdrlen > skb->len))
+ hdrlen = ieee80211_hdrlen((const struct ieee80211_hdr *)skb->data);
+ if (hdrlen > skb->len)
return 0;
return hdrlen;
}
diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 14a9ff1..bfd7cbf 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -44,7 +44,7 @@ static inline unsigned classify_1d(struct sk_buff *skb, struct Qdisc *qd)
{
struct iphdr *ip;
int dscp;
- int offset;
+ unsigned int offset;

struct ieee80211_sched_data *q = qdisc_priv(qd);
struct tcf_result res = { -1, 0 };
--
1.5.6.rc1.257.gba91d




2008-06-05 13:55:34

by Bob Copeland

[permalink] [raw]
Subject: Re: [RFC-PATCH] mac80211: add helpers for common frame_control testing

On Wed, Jun 4, 2008 at 10:33 PM, Harvey Harrison
<[email protected]> wrote:
> +static inline int ieee80211_has_a4(struct ieee80211_hdr *hdr)
> +{
> + __le16 tmp = cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_TODS);
> + return (hdr->frame_control & tmp) == tmp;
> +}

One should be FROMDS?

--
Bob Copeland %% http://www.bobcopeland.com