Avoid always byteswapping the hdr->frame_control value and testing
against that. Move the byteswapping into the constants and test
directly against the __le16 value.
One function in wpa.c moved to use some of the helpers as an example
of what this transition will look like eliminating the local var fc.
Signed-off-by: Harvey Harrison <[email protected]>
---
More helpers could (should) be added to this, I just wanted to get some
feedback regarding the approach before going through the drivers and
mac80211 code.
drivers/net/wireless/iwl/iwl-helpers.h has a lot of similar helpers, but
takes the already byteswapped fc value rather than a struct ieee80211_hdr
as they do the byteswapping before testing. There are lots of places this
could be avoided with a set of helpers like this.
include/linux/ieee80211.h | 41 +++++++++++++++++++++++++++++++++++++++++
net/mac80211/wpa.c | 18 +++++++-----------
2 files changed, 48 insertions(+), 11 deletions(-)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 0b5e03e..2382cd0 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -81,6 +81,47 @@
#define IEEE80211_STYPE_QOS_CFPOLL 0x00E0
#define IEEE80211_STYPE_QOS_CFACKPOLL 0x00F0
+static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
+{
+ return hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_TODS);
+}
+
+static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
+{
+ return hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FROMDS);
+}
+
+static inline int ieee80211_fctl_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_ftype(struct ieee80211_hdr *hdr, u16 ftype)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
+ cpu_to_le16(ftype);
+}
+
+static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
+{
+ return (hdr->frame_control & cpu_to_le16(stype)) != 0;
+}
+
+static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_MGMT);
+}
+
+static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_CTL);
+}
+
+static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_DATA);
+}
/* miscellaneous IEEE 802.11 constants */
#define IEEE80211_MAX_FRAG_THRESHOLD 2352
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 45709ad..c37cc76 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -24,23 +24,21 @@ 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_fctl_has_a4(hdr)) {
hdrlen += ETH_ALEN;
+ a4_included = 1;
*sa = hdr->addr4;
*da = hdr->addr3;
- } else if (fc & IEEE80211_FCTL_FROMDS) {
+ } else if (ieee80211_fctl_fromds(hdr)) {
*sa = hdr->addr3;
*da = hdr->addr1;
- } else if (fc & IEEE80211_FCTL_TODS) {
+ } else if (ieee80211_fctl_tods(hdr)) {
*sa = hdr->addr2;
*da = hdr->addr3;
} else {
@@ -48,16 +46,14 @@ static int ieee80211_get_hdr_info(const struct sk_buff *skb, u8 **sa, u8 **da,
*da = hdr->addr1;
}
- if (fc & 0x80)
+ if (ieee80211_stype(hdr, IEEE_STYPE_QOS_DATA))
hdrlen += 2;
*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) {
+ if (ieee80211_ftype_data(hdr) &&
+ ieee80211_stype(hdr, IEEE_STYPE_QOS_DATA)) {
pos = (u8 *) &hdr->addr4;
if (a4_included)
pos += 6;
--
1.5.5.1.570.g26b5e
On Fri, 2008-05-16 at 23:12 +0200, Johannes Berg wrote:
> > > > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *=
hdr)
> > >=20
> > > That seems fine too.
> > >=20
> > > > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u=
16 ftype)
> > >=20
> > > That, I think, is misnamed, it should be ieee80211_is_ftype()
> >=20
> > Well, I didn't expect this to be used directly that much, hence the
> > following three helpers...ftype_mgmt/ctl/data
>=20
> Right. Still, I think it should be named with _is_ in there.
Yes, I agree with you, that's what I meant to write.
>=20
> > > > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u=
16 stype)
> > > > +{
> > > > + return (hdr->frame_control & cpu_to_le16(stype)) !=3D 0;
> > > > +}
> > >=20
> > > And that even seems implemented wrongly? stype is a 4-bit field, =
this
> > > doesn't make much sense to me.
> >=20
> > It was meant to be used to replace code like:
> >=20
> > =EF=BB=BFfc & IEEE80211_STYPE_QOS_DATA
> >=20
> > =EF=BB=BFieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)
> >=20
> > As most users of the stype bits test against a specific constant,
> > otherwise you could add a bunch of helpers, one for each stype
> > constant (similar to the ftype helpers I added. There were only
> > 3 in the ftype case, so I just added them all and dropped the secon=
d
> > parameter, in the stype case, there are more options, so I just
> > left one two-parameter helper.
>=20
> Well, yes, but that's semantically unclear because of the way these b=
its
> are used vs. how they are called. The subtype (stype) is a four-bit
> field in the frame control field which isn't actually defined to have
> "flags" or bits like that in it. Look at the table in IEEE 802.11-200=
7,
> it's at the beginning of clause 7 somewhere.
>=20
> Hence, hiding this simple AND behind a function that claims to check =
the
> subtype is confusing.
Ahhh, I see what you are getting at now, how about ieee80211_stype_mask=
?
That makes the & a little clearer and has better semantic meaning.
The other option is still to call it stype_mask, but change it to
return a __le16....so tests against !=3D 0 still work and really
is just a wrapper around the byteswap of the constant and the &.
What do you think of that?
Harvey
> > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
> > +{
> > + return (hdr->frame_control & cpu_to_le16(stype)) != 0;
> > +}
>
> And that even seems implemented wrongly? stype is a 4-bit field, this
> doesn't make much sense to me.
Now, before you get any ideas, this code
- 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) {
is still correct. You really want to read the frame type bit assignments
in 802.11-2007 :)
johannes
On Friday 16 May 2008 21:38:23 Harvey Harrison wrote:
> diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
> index 45709ad..f899a06 100644
> --- a/net/mac80211/wpa.c
> +++ b/net/mac80211/wpa.c
> @@ -24,23 +24,21 @@ 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_fctl_has_a4(hdr)) {
> hdrlen += ETH_ALEN;
> + a4_included = 1;
> *sa = hdr->addr4;
> *da = hdr->addr3;
> - } else if (fc & IEEE80211_FCTL_FROMDS) {
> + } else if (ieee80211_fctl_fromds(hdr)) {
> *sa = hdr->addr3;
> *da = hdr->addr1;
> - } else if (fc & IEEE80211_FCTL_TODS) {
> + } else if (ieee80211_fctl_tods(hdr)) {
> *sa = hdr->addr2;
> *da = hdr->addr3;
> } else {
> @@ -48,16 +46,14 @@ static int ieee80211_get_hdr_info(const struct sk_buff *skb, u8 **sa, u8 **da,
> *da = hdr->addr1;
> }
>
> - if (fc & 0x80)
> + if (ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA))
> hdrlen += 2;
>
> *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) {
> + if (ieee80211_ftype_data(hdr) &&
> + ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)) {
> pos = (u8 *) &hdr->addr4;
> if (a4_included)
Are we going to use a4_included uninitialized now?
> pos += 6;
--
Greetings Michael.
On Sat, 2008-05-17 at 00:03 +0200, Johannes Berg wrote:
> On Fri, 2008-05-16 at 14:57 -0700, Harvey Harrison wrote:
> > On Fri, 2008-05-16 at 23:12 +0200, Johannes Berg wrote:
> > >
> > > Right. Still, I think it should be named with _is_ in there.
> >
> > Yes, I agree with you, that's what I meant to write.
>
> Actually, maybe it should be "has" and not "is" since it's being passed
> a frame that can *have* a given frame type, and is a frame of a given
> type. Dunno.
I think _is_ works best, I'll send out a few trial patches, but this
seems to make sense in the places its used.
>
> > Ahhh, I see what you are getting at now, how about ieee80211_stype_mask?
> >
> > That makes the & a little clearer and has better semantic meaning.
> >
> > The other option is still to call it stype_mask, but change it to
> > return a __le16....so tests against != 0 still work and really
> > is just a wrapper around the byteswap of the constant and the &.
>
> Hmm. I'm not sure, how is all this really used? It seems you want
>
> ieee80211_has_qos_stype(hdr)
> and
>
> ieee80211_has_stype(hdr, stype)
>
> instead, no?
Just the latter I think. But I'll do a few patches to see how it looks
first. If there end up being a few common ones (like qos) a few
short-hand versions might be appropriate, I'll see how it goes.
Harvey
> > > Ahhh, I see what you are getting at now, how about ieee80211_stype_mask?
> > >
> > > That makes the & a little clearer and has better semantic meaning.
> > >
> > > The other option is still to call it stype_mask, but change it to
> > > return a __le16....so tests against != 0 still work and really
> > > is just a wrapper around the byteswap of the constant and the &.
> >
> > Hmm. I'm not sure, how is all this really used? It seems you want
> >
> > ieee80211_has_qos_stype(hdr)
> > and
> >
> > ieee80211_has_stype(hdr, stype)
> >
> > instead, no?
>
> Just the latter I think. But I'll do a few patches to see how it looks
> first. If there end up being a few common ones (like qos) a few
> short-hand versions might be appropriate, I'll see how it goes.
Yeah but I was thinking of ieee80211_stype_is() or whatever you want to
call it (I think I actually like it that way around better than
is_stype/ftype) to check the exact match of the four bits, i.e.
fc & stypemask == argument
and the QoS one check that the bit is set.
johannes
On Fri, 2008-05-16 at 14:57 -0700, Harvey Harrison wrote:
> On Fri, 2008-05-16 at 23:12 +0200, Johannes Berg wrote:
> > > > > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
> > > >
> > > > That seems fine too.
> > > >
> > > > > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 ftype)
> > > >
> > > > That, I think, is misnamed, it should be ieee80211_is_ftype()
> > >
> > > Well, I didn't expect this to be used directly that much, hence the
> > > following three helpers...ftype_mgmt/ctl/data
> >
> > Right. Still, I think it should be named with _is_ in there.
>
> Yes, I agree with you, that's what I meant to write.
Actually, maybe it should be "has" and not "is" since it's being passed
a frame that can *have* a given frame type, and is a frame of a given
type. Dunno.
> Ahhh, I see what you are getting at now, how about ieee80211_stype_mask?
>
> That makes the & a little clearer and has better semantic meaning.
>
> The other option is still to call it stype_mask, but change it to
> return a __le16....so tests against != 0 still work and really
> is just a wrapper around the byteswap of the constant and the &.
Hmm. I'm not sure, how is all this really used? It seems you want
ieee80211_has_qos_stype(hdr)
and
ieee80211_has_stype(hdr, stype)
instead, no?
johannes
On Fri, 2008-05-16 at 22:31 +0200, Johannes Berg wrote:
> > +static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
>=20
> These seem fine though I don't see why you implement them with a > 0
> rather than !=3D 0 comparison?
No reason, I'll move to all !=3D 0
>=20
> > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
>=20
> That seems fine too.
>=20
> > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 f=
type)
>=20
> That, I think, is misnamed, it should be ieee80211_is_ftype()
Well, I didn't expect this to be used directly that much, hence the
following three helpers...ftype_mgmt/ctl/data
>=20
> > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 s=
type)
> > +{
> > + return (hdr->frame_control & cpu_to_le16(stype)) !=3D 0;
> > +}
>=20
> And that even seems implemented wrongly? stype is a 4-bit field, this
> doesn't make much sense to me.
It was meant to be used to replace code like:
=EF=BB=BFfc & IEEE80211_STYPE_QOS_DATA
=EF=BB=BFieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)
As most users of the stype bits test against a specific constant,
otherwise you could add a bunch of helpers, one for each stype
constant (similar to the ftype helpers I added. There were only
3 in the ftype case, so I just added them all and dropped the second
parameter, in the stype case, there are more options, so I just
left one two-parameter helper.
>=20
> > +static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
>=20
> > +static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
>=20
> similarly, ieee80211_is_data() (remove the ftype, everybody hacking
> wireless should know that data/mgmt/ctl are frame types)
OK, I'll drop ftype from these three, but leave it in the two-param
version as it is really just a helper for these three.
What do you think if I just left the stype version as-is, subject
to the usage I showed above, which is the common case.
Harvey
Avoid always byteswapping the hdr->frame_control value and testing
against that. Move the byteswapping into the constants and test
directly against the __le16 value.
One function in wpa.c moved to use some of the helpers as an example
of what this transition will look like eliminating the local var fc.
Signed-off-by: Harvey Harrison <[email protected]>
---
Sorry, sent the wrong one in my previous e-mail.
Harvey
include/linux/ieee80211.h | 60 ++++++++++++++++++++++++++++++++++++++------
net/mac80211/wpa.c | 18 +++++--------
2 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 0b5e03e..5212cf9 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -81,6 +81,57 @@
#define IEEE80211_STYPE_QOS_CFPOLL 0x00E0
#define IEEE80211_STYPE_QOS_CFACKPOLL 0x00F0
+struct ieee80211_hdr {
+ __le16 frame_control;
+ __le16 duration_id;
+ u8 addr1[6];
+ u8 addr2[6];
+ u8 addr3[6];
+ __le16 seq_ctrl;
+ u8 addr4[6];
+} __attribute__ ((packed));
+
+static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_TODS)) > 0;
+}
+
+static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FROMDS)) > 0;
+}
+
+static inline int ieee80211_fctl_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_ftype(struct ieee80211_hdr *hdr, u16 ftype)
+{
+ return (hdr->frame_control & cpu_to_le16(IEEE80211_FCTL_FTYPE)) ==
+ cpu_to_le16(ftype);
+}
+
+static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
+{
+ return (hdr->frame_control & cpu_to_le16(stype)) != 0;
+}
+
+static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_MGMT);
+}
+
+static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_CTL);
+}
+
+static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
+{
+ return ieee80211_ftype(hdr, IEEE80211_FTYPE_DATA);
+}
/* miscellaneous IEEE 802.11 constants */
#define IEEE80211_MAX_FRAG_THRESHOLD 2352
@@ -99,15 +150,6 @@
#define IEEE80211_MAX_SSID_LEN 32
#define IEEE80211_MAX_MESH_ID_LEN 32
-struct ieee80211_hdr {
- __le16 frame_control;
- __le16 duration_id;
- u8 addr1[6];
- u8 addr2[6];
- u8 addr3[6];
- __le16 seq_ctrl;
- u8 addr4[6];
-} __attribute__ ((packed));
struct ieee80211s_hdr {
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 45709ad..f899a06 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -24,23 +24,21 @@ 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_fctl_has_a4(hdr)) {
hdrlen += ETH_ALEN;
+ a4_included = 1;
*sa = hdr->addr4;
*da = hdr->addr3;
- } else if (fc & IEEE80211_FCTL_FROMDS) {
+ } else if (ieee80211_fctl_fromds(hdr)) {
*sa = hdr->addr3;
*da = hdr->addr1;
- } else if (fc & IEEE80211_FCTL_TODS) {
+ } else if (ieee80211_fctl_tods(hdr)) {
*sa = hdr->addr2;
*da = hdr->addr3;
} else {
@@ -48,16 +46,14 @@ static int ieee80211_get_hdr_info(const struct sk_buff *skb, u8 **sa, u8 **da,
*da = hdr->addr1;
}
- if (fc & 0x80)
+ if (ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA))
hdrlen += 2;
*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) {
+ if (ieee80211_ftype_data(hdr) &&
+ ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)) {
pos = (u8 *) &hdr->addr4;
if (a4_included)
pos += 6;
--
1.5.5.1.570.g26b5e
> > > +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
> >
> > That seems fine too.
> >
> > > +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 ftype)
> >
> > That, I think, is misnamed, it should be ieee80211_is_ftype()
>
> Well, I didn't expect this to be used directly that much, hence the
> following three helpers...ftype_mgmt/ctl/data
Right. Still, I think it should be named with _is_ in there.
> > > +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
> > > +{
> > > + return (hdr->frame_control & cpu_to_le16(stype)) != 0;
> > > +}
> >
> > And that even seems implemented wrongly? stype is a 4-bit field, this
> > doesn't make much sense to me.
>
> It was meant to be used to replace code like:
>
> fc & IEEE80211_STYPE_QOS_DATA
>
> ieee80211_stype(hdr, IEEE80211_STYPE_QOS_DATA)
>
> As most users of the stype bits test against a specific constant,
> otherwise you could add a bunch of helpers, one for each stype
> constant (similar to the ftype helpers I added. There were only
> 3 in the ftype case, so I just added them all and dropped the second
> parameter, in the stype case, there are more options, so I just
> left one two-parameter helper.
Well, yes, but that's semantically unclear because of the way these bits
are used vs. how they are called. The subtype (stype) is a four-bit
field in the frame control field which isn't actually defined to have
"flags" or bits like that in it. Look at the table in IEEE 802.11-2007,
it's at the beginning of clause 7 somewhere.
Hence, hiding this simple AND behind a function that claims to check the
subtype is confusing.
johannes
> +static inline int ieee80211_fctl_tods(struct ieee80211_hdr *hdr)
> +static inline int ieee80211_fctl_fromds(struct ieee80211_hdr *hdr)
These seem fine though I don't see why you implement them with a > 0
rather than != 0 comparison?
> +static inline int ieee80211_fctl_has_a4(struct ieee80211_hdr *hdr)
That seems fine too.
> +static inline int ieee80211_ftype(struct ieee80211_hdr *hdr, u16 ftype)
That, I think, is misnamed, it should be ieee80211_is_ftype()
> +static inline int ieee80211_stype(struct ieee80211_hdr *hdr, u16 stype)
> +{
> + return (hdr->frame_control & cpu_to_le16(stype)) != 0;
> +}
And that even seems implemented wrongly? stype is a 4-bit field, this
doesn't make much sense to me.
> +static inline int ieee80211_ftype_mgmt(struct ieee80211_hdr *hdr)
> +static inline int ieee80211_ftype_ctl(struct ieee80211_hdr *hdr)
> +static inline int ieee80211_ftype_data(struct ieee80211_hdr *hdr)
similarly, ieee80211_is_data() (remove the ftype, everybody hacking
wireless should know that data/mgmt/ctl are frame types)
johannes