[PATCH 3/7] mac80211: add utility function to get header length

[PATCH 3/7] mac80211: add utility function to get header length

[Harvey Harrison]

Take a __le16 directly rather than a host-endian value.

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
---
 include/net/mac80211.h |    6 ++++++
 net/mac80211/util.c    |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1196de8..53c3b5e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1548,6 +1548,12 @@ int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb);
 int ieee80211_get_hdrlen(u16 fc);
 
 /**
+ * ieee80211_hdrlen - get header length in bytes from frame control
+ * @fc: frame control field in little-endian format
+ */
+unsigned int ieee80211_hdrlen(__le16 fc);
+
+/**
  * ieee80211_get_tkip_key - get a TKIP rc4 for skb
  *
  * This function computes a TKIP rc4 key for an skb. It computes
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 6513bc2..fade001 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -133,6 +133,38 @@ int ieee80211_get_hdrlen(u16 fc)
 }
 EXPORT_SYMBOL(ieee80211_get_hdrlen);
 
+unsigned int ieee80211_hdrlen(__le16 fc)
+{
+	unsigned int hdrlen = 24;
+
+	if (ieee80211_is_data(fc)) {
+		if (ieee80211_has_a4(fc))
+			hdrlen = 30;
+		if (ieee80211_data_has_qos(fc))
+			hdrlen += IEEE80211_QOS_CTL_LEN;
+		goto out;
+	}
+
+	if (ieee80211_is_ctl(fc)) {
+		/*
+		 * 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 & cpu_to_le16(0x00E0)) == cpu_to_le16(0x00C0))
+			hdrlen = 10;
+		else
+			hdrlen = 16;
+	}
+out:
+	return hdrlen;
+}
+EXPORT_SYMBOL(ieee80211_hdrlen);
+
 int ieee80211_get_hdrlen_from_skb(const struct sk_buff *skb)
 {
 	const struct ieee80211_hdr *hdr = (const struct ieee80211_hdr *) skb->data;
-- 
1.5.6.rc1.257.gba91d

Re: [PATCH 3/7] mac80211: add utility function to get header length

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]


>  /**
> + * ieee80211_hdrlen - get header length in bytes from frame control
> + * @fc: frame control field in little-endian format
> + */
> +unsigned int ieee80211_hdrlen(__le16 fc);
> +

> +EXPORT_SYMBOL(ieee80211_hdrlen);

Do we really need to export that?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/7] mac80211: add utility function to get header length

[Tomas Winkler]

On Mon, Jun 9, 2008 at 12:16 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
>>  /**
>> + * ieee80211_hdrlen - get header length in bytes from frame control
>> + * @fc: frame control field in little-endian format
>> + */
>> +unsigned int ieee80211_hdrlen(__le16 fc);
>> +
>
>> +EXPORT_SYMBOL(ieee80211_hdrlen);
>
> Do we really need to export that?

so now we have hdrlen(u16) and hdrlen(__le16)  it will be fun.

So I guess will we converting idioms
u16 fc = le16_to_cpu(hdr->frame_control);
int hdr_len = ieee80211_get_hdrlen(fc);
to
int hdr_len = ieee80211_hdrlen(hdr->frame_control)

This is how it used in driver code so it make sense to export this
function and remove ieee80211_get_hdrlen(fc)

Since all fc operations are bitwise 'and' and 'or'
u16 rx->fc can be dropped in future as well

Tomas

Re: [PATCH 3/7] mac80211: add utility function to get header length

[Harvey Harrison]

On Mon, 2008-06-09 at 13:01 +0300, Tomas Winkler wrote:
> On Mon, Jun 9, 2008 at 12:16 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> >
> >>  /**
> >> + * ieee80211_hdrlen - get header length in bytes from frame control
> >> + * @fc: frame control field in little-endian format
> >> + */
> >> +unsigned int ieee80211_hdrlen(__le16 fc);
> >> +
> >
> >> +EXPORT_SYMBOL(ieee80211_hdrlen);
> >
> > Do we really need to export that?
> 
> so now we have hdrlen(u16) and hdrlen(__le16)  it will be fun.
> 
> So I guess will we converting idioms
> u16 fc = le16_to_cpu(hdr->frame_control);
> int hdr_len = ieee80211_get_hdrlen(fc);
> to
> int hdr_len = ieee80211_hdrlen(hdr->frame_control)
> 
> This is how it used in driver code so it make sense to export this
> function and remove ieee80211_get_hdrlen(fc)

Yes, that was my thinking, I just did it this way to avoid the flag day
change, I'll trickle the changes in and then remove _get_hdrlen.

> 
> Since all fc operations are bitwise 'and' and 'or'
> u16 rx->fc can be dropped in future as well

I was going to convert it to a __le16, but if it is just a copy of the
->frame_control in the header, I'll look at removing it instead.

Cheers,

Harvey

Re: [PATCH 3/7] mac80211: add utility function to get header length

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]


> > So I guess will we converting idioms
> > u16 fc = le16_to_cpu(hdr->frame_control);
> > int hdr_len = ieee80211_get_hdrlen(fc);
> > to
> > int hdr_len = ieee80211_hdrlen(hdr->frame_control)
> > 
> > This is how it used in driver code so it make sense to export this
> > function and remove ieee80211_get_hdrlen(fc)
> 
> Yes, that was my thinking, I just did it this way to avoid the flag day
> change, I'll trickle the changes in and then remove _get_hdrlen.

Sounds good to me.

> > Since all fc operations are bitwise 'and' and 'or'
> > u16 rx->fc can be dropped in future as well
> 
> I was going to convert it to a __le16, but if it is just a copy of the
> ->frame_control in the header, I'll look at removing it instead.

Yeah, I think rx->fc was meant to be a cpu-byteorder copy of
hdr->frame_control to avoid repeated byteswapping. If we do all
operations on the constants instead as you're doing with this series, we
ought to be able to remove it. Bonus point: that gets rid of the
possible "rx->fc is out of sync" issue we had to fix once already.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/7] mac80211: add utility function to get header length

[Harvey Harrison]

On Mon, 2008-06-09 at 19:24 +0200, Johannes Berg wrote:
> > > So I guess will we converting idioms
> > > u16 fc = le16_to_cpu(hdr->frame_control);
> > > int hdr_len = ieee80211_get_hdrlen(fc);
> > > to
> > > int hdr_len = ieee80211_hdrlen(hdr->frame_control)
> > > 
> > > This is how it used in driver code so it make sense to export this
> > > function and remove ieee80211_get_hdrlen(fc)
> > 
> > Yes, that was my thinking, I just did it this way to avoid the flag day
> > change, I'll trickle the changes in and then remove _get_hdrlen.
> 
> Sounds good to me.
> 

OK, will keep going on this then.

> > > Since all fc operations are bitwise 'and' and 'or'
> > > u16 rx->fc can be dropped in future as well
> > 
> > I was going to convert it to a __le16, but if it is just a copy of the
> > ->frame_control in the header, I'll look at removing it instead.
> 
> Yeah, I think rx->fc was meant to be a cpu-byteorder copy of
> hdr->frame_control to avoid repeated byteswapping. If we do all
> operations on the constants instead as you're doing with this series, we
> ought to be able to remove it. Bonus point: that gets rid of the
> possible "rx->fc is out of sync" issue we had to fix once already.

That and be arches eliminate a bunch of byteswaps.

Cheers,

Harvey