[PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Larry Finger]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

All of the rtlwifi drivers have an error in the routine that tests if
the data is "special". If it is, the subsequant transmission will be
at the lowest rate to enhance reliability. The 16-bit quantity is
big-endian, but was being extracted in native CPU mode. One of the
effects of this bug is to inhibit association under some conditions
as the TX rate is too high.

A statement that would have made the code correct had been changed to
a comment. Rather than just reinstating that code, the fix here passes
sparse tests. A side effect of fixing this problem would have been to force
all IPv6 frames to run at the lowest rate. The test for that frame type
is removed.

The original code only checked the lower-order byte of UDP ports for BOOTP
protocol. That is extended to the full 16-bit source and destination ports.

One of the local headers contained duplicates of some of the ETH_P_XXX
definitions. These are deleted.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Stable <stable@vger.kernel.org> [2.6.38+]
---

V2 - Addresses comments by Ben Hutchings and Bjorn Mork.

 drivers/net/wireless/rtlwifi/base.c |   15 ++++++---------
 drivers/net/wireless/rtlwifi/wifi.h |    6 +-----
 2 files changed, 7 insertions(+), 14 deletions(-)
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
@@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
 
 	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
 			      SNAP_SIZE + PROTOC_TYPE_SIZE);
-	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
-	/*	ether_type = ntohs(ether_type); */
+	ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
+					     SNAP_SIZE));
 
 	if (ETH_P_IP == ether_type) {
 		if (IPPROTO_UDP == ip->protocol) {
 			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
 							       (ip->ihl << 2));
-			if (((((u8 *) udp)[1] == 68) &&
-			     (((u8 *) udp)[3] == 67)) ||
-			    ((((u8 *) udp)[1] == 67) &&
-			     (((u8 *) udp)[3] == 68))) {
+			if (((((u16 *) udp)[0] == 68) &&
+			     (((u16 *) udp)[2] == 67)) ||
+			    ((((u16 *) udp)[0] == 67) &&
+			     (((u16 *) udp)[2] == 68))) {
 				/*
 				 * 68 : UDP BOOTP client
 				 * 67 : UDP BOOTP server
@@ -1126,9 +1126,6 @@ u8 rtl_is_special_data(struct ieee80211_
 		}
 
 		return true;
-	} else if (ETH_P_IPV6 == ether_type) {
-		/* IPv6 */
-		return true;
 	}
 
 	return false;
Index: wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
===================================================================
--- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/wifi.h
+++ wireless-testing-save/drivers/net/wireless/rtlwifi/wifi.h
@@ -77,11 +77,7 @@
 #define RTL_SLOT_TIME_9				9
 #define RTL_SLOT_TIME_20			20
 
-/*related with tcp/ip. */
-/*if_ehther.h*/
-#define ETH_P_PAE		0x888E	/*Port Access Entity (IEEE 802.1X) */
-#define ETH_P_IP		0x0800	/*Internet Protocol packet */
-#define ETH_P_ARP		0x0806	/*Address Resolution packet */
+/*related to tcp/ip. */
 #define SNAP_SIZE		6
 #define PROTOC_TYPE_SIZE	2
 

Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Ben Hutchings]

On Thu, 2013-10-31 at 19:43 -0500, Larry Finger wrote:
> From: Mark Cave-Ayland <mark.cave-ayland-r2/yp8inhXLQXOPxS62xeg@public.gmane.org>
> 
> All of the rtlwifi drivers have an error in the routine that tests if
> the data is "special". If it is, the subsequant transmission will be
> at the lowest rate to enhance reliability. The 16-bit quantity is
> big-endian, but was being extracted in native CPU mode. One of the
> effects of this bug is to inhibit association under some conditions
> as the TX rate is too high.
> 
> A statement that would have made the code correct had been changed to
> a comment. Rather than just reinstating that code, the fix here passes
> sparse tests. A side effect of fixing this problem would have been to force
> all IPv6 frames to run at the lowest rate. The test for that frame type
> is removed.
> 
> The original code only checked the lower-order byte of UDP ports for BOOTP
> protocol. That is extended to the full 16-bit source and destination ports.
> 
> One of the local headers contained duplicates of some of the ETH_P_XXX
> definitions. These are deleted.
> 
> Signed-off-by: Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>
> Cc: Mark Cave-Ayland <mark.cave-ayland-r2/yp8inhXLQXOPxS62xeg@public.gmane.org>
> Cc: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> [2.6.38+]
> ---
> 
> V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
> 
>  drivers/net/wireless/rtlwifi/base.c |   15 ++++++---------
>  drivers/net/wireless/rtlwifi/wifi.h |    6 +-----
>  2 files changed, 7 insertions(+), 14 deletions(-)
> Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> ===================================================================
> --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
> +++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> @@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
>  
>  	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
>  			      SNAP_SIZE + PROTOC_TYPE_SIZE);
> -	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
> -	/*	ether_type = ntohs(ether_type); */
> +	ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
> +					     SNAP_SIZE));
>  
>  	if (ETH_P_IP == ether_type) {
>  		if (IPPROTO_UDP == ip->protocol) {
>  			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
>  							       (ip->ihl << 2));
> -			if (((((u8 *) udp)[1] == 68) &&
> -			     (((u8 *) udp)[3] == 67)) ||
> -			    ((((u8 *) udp)[1] == 67) &&
> -			     (((u8 *) udp)[3] == 68))) {
> +			if (((((u16 *) udp)[0] == 68) &&
> +			     (((u16 *) udp)[2] == 67)) ||
> +			    ((((u16 *) udp)[0] == 67) &&
> +			     (((u16 *) udp)[2] == 68))) {
[...]

Now you're missing byte-swapping here, and using the wrong offset for
the dest port (4 bytes rather than 2).

If you really think this is necessary then use something like:
			if ((udp->source == htons(68) &&
			     udp->dest == htons(67)) ||
			     ...

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Joe Perches]

On Fri, 2013-11-01 at 01:02 +0000, Ben Hutchings wrote:
> On Thu, 2013-10-31 at 19:43 -0500, Larry Finger wrote:
> > From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > 
> > All of the rtlwifi drivers have an error in the routine that tests if
> > the data is "special". If it is, the subsequant transmission will be
> > at the lowest rate to enhance reliability. The 16-bit quantity is
> > big-endian, but was being extracted in native CPU mode. One of the
> > effects of this bug is to inhibit association under some conditions
> > as the TX rate is too high.
> > 
> > A statement that would have made the code correct had been changed to
> > a comment. Rather than just reinstating that code, the fix here passes
> > sparse tests. A side effect of fixing this problem would have been to force
> > all IPv6 frames to run at the lowest rate. The test for that frame type
> > is removed.
> > 
> > The original code only checked the lower-order byte of UDP ports for BOOTP
> > protocol. That is extended to the full 16-bit source and destination ports.
> > 
> > One of the local headers contained duplicates of some of the ETH_P_XXX
> > definitions. These are deleted.
> > 
> > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > Cc: Stable <stable@vger.kernel.org> [2.6.38+]
> > ---
> > 
> > V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
> > 
> >  drivers/net/wireless/rtlwifi/base.c |   15 ++++++---------
> >  drivers/net/wireless/rtlwifi/wifi.h |    6 +-----
> >  2 files changed, 7 insertions(+), 14 deletions(-)
> > Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> > ===================================================================
> > --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
> > +++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
> > @@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
> >  
> >  	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
> >  			      SNAP_SIZE + PROTOC_TYPE_SIZE);
> > -	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
> > -	/*	ether_type = ntohs(ether_type); */
> > +	ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
> > +					     SNAP_SIZE));
> >  
> >  	if (ETH_P_IP == ether_type) {
> >  		if (IPPROTO_UDP == ip->protocol) {
> >  			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
> >  							       (ip->ihl << 2));
> > -			if (((((u8 *) udp)[1] == 68) &&
> > -			     (((u8 *) udp)[3] == 67)) ||
> > -			    ((((u8 *) udp)[1] == 67) &&
> > -			     (((u8 *) udp)[3] == 68))) {
> > +			if (((((u16 *) udp)[0] == 68) &&
> > +			     (((u16 *) udp)[2] == 67)) ||
> > +			    ((((u16 *) udp)[0] == 67) &&
> > +			     (((u16 *) udp)[2] == 68))) {
> [...]
> 
> Now you're missing byte-swapping here, and using the wrong offset for
> the dest port (4 bytes rather than 2).
> 
> If you really think this is necessary then use something like:
> 			if ((udp->source == htons(68) &&
> 			     udp->dest == htons(67)) ||
> 			     ...

Or maybe something like this?
---
 drivers/net/wireless/rtlwifi/base.c | 91 +++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
index 9a78e3d..7e9df65 100644
--- a/drivers/net/wireless/rtlwifi/base.c
+++ b/drivers/net/wireless/rtlwifi/base.c
@@ -37,6 +37,7 @@
 
 #include <linux/ip.h>
 #include <linux/module.h>
+#include <linux/udp.h>
 
 /*
  *NOTICE!!!: This file will be very big, we should
@@ -1074,64 +1075,54 @@ u8 rtl_is_special_data(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
 	if (!ieee80211_is_data(fc))
 		return false;
 
+	ip = (const struct iphdr *)(skb->data + mac_hdr_len +
+				    SNAP_SIZE + PROTOC_TYPE_SIZE);
+	ether_type = be16_to_cpup((__be16 *)
+				  (skb->data + mac_hdr_len + SNAP_SIZE));
 
-	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
-			      SNAP_SIZE + PROTOC_TYPE_SIZE);
-	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
-	/*	ether_type = ntohs(ether_type); */
-
-	if (ETH_P_IP == ether_type) {
-		if (IPPROTO_UDP == ip->protocol) {
-			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
-							       (ip->ihl << 2));
-			if (((((u8 *) udp)[1] == 68) &&
-			     (((u8 *) udp)[3] == 67)) ||
-			    ((((u8 *) udp)[1] == 67) &&
-			     (((u8 *) udp)[3] == 68))) {
-				/*
-				 * 68 : UDP BOOTP client
-				 * 67 : UDP BOOTP server
-				 */
-				RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV),
-					 DBG_DMESG, "dhcp %s !!\n",
-					 is_tx ? "Tx" : "Rx");
-
-				if (is_tx) {
-					rtlpriv->enter_ps = false;
-					schedule_work(&rtlpriv->
-						      works.lps_change_work);
-					ppsc->last_delaylps_stamp_jiffies =
-					    jiffies;
-				}
+	switch (ether_type) {
+	case ETH_P_IP: {
+		struct udphdr *udp;
+		u16 src;
+		u16 dst;
 
-				return true;
-			}
-		}
-	} else if (ETH_P_ARP == ether_type) {
-		if (is_tx) {
-			rtlpriv->enter_ps = false;
-			schedule_work(&rtlpriv->works.lps_change_work);
-			ppsc->last_delaylps_stamp_jiffies = jiffies;
-		}
+		if (ip->protocol != IPPROTO_UDP)
+			return false;
 
-		return true;
-	} else if (ETH_P_PAE == ether_type) {
-		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
-			 "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
+		udp = (struct udphdr *)((u8 *)ip + (ip->ihl << 2));
+		src = be16_to_cpu(udp->source) >> 8;
+		dst = be16_to_cpu(udp->dest) >> 8;
 
-		if (is_tx) {
-			rtlpriv->enter_ps = false;
-			schedule_work(&rtlpriv->works.lps_change_work);
-			ppsc->last_delaylps_stamp_jiffies = jiffies;
-		}
+		/*
+		 * 68 : UDP BOOTP client
+		 * 67 : UDP BOOTP server
+		 */
+		if (!((src == 68 && dst == 67) || (src == 67 && dst == 68)))
+			return false;
 
+		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			 "dhcp %s !!\n", is_tx ? "Tx" : "Rx");
+		break;
+	}
+	case ETH_P_ARP:
+		break;
+	case ETH_P_PAE:
+		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			 "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
+		break;
+	case ETH_P_IPV6:
 		return true;
-	} else if (ETH_P_IPV6 == ether_type) {
-		/* IPv6 */
-		return true;
+	default:
+		return false;
 	}
 
-	return false;
+	if (is_tx) {
+		rtlpriv->enter_ps = false;
+		schedule_work(&rtlpriv->works.lps_change_work);
+		ppsc->last_delaylps_stamp_jiffies = jiffies;
+	}
+
+	return true;
 }
 EXPORT_SYMBOL_GPL(rtl_is_special_data);
 

Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Joe Perches]

On Thu, 2013-10-31 at 18:48 -0700, Joe Perches wrote:
> Or maybe something like this?
[]
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
[]
> +		src = be16_to_cpu(udp->source) >> 8;
> +		dst = be16_to_cpu(udp->dest) >> 8;

Of course this doesn't need the >> 8;

Re: [PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Larry Finger]

On 10/31/2013 08:48 PM, Joe Perches wrote:
> On Fri, 2013-11-01 at 01:02 +0000, Ben Hutchings wrote:
>> On Thu, 2013-10-31 at 19:43 -0500, Larry Finger wrote:
>>> From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>
>>> All of the rtlwifi drivers have an error in the routine that tests if
>>> the data is "special". If it is, the subsequant transmission will be
>>> at the lowest rate to enhance reliability. The 16-bit quantity is
>>> big-endian, but was being extracted in native CPU mode. One of the
>>> effects of this bug is to inhibit association under some conditions
>>> as the TX rate is too high.
>>>
>>> A statement that would have made the code correct had been changed to
>>> a comment. Rather than just reinstating that code, the fix here passes
>>> sparse tests. A side effect of fixing this problem would have been to force
>>> all IPv6 frames to run at the lowest rate. The test for that frame type
>>> is removed.
>>>
>>> The original code only checked the lower-order byte of UDP ports for BOOTP
>>> protocol. That is extended to the full 16-bit source and destination ports.
>>>
>>> One of the local headers contained duplicates of some of the ETH_P_XXX
>>> definitions. These are deleted.
>>>
>>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>>> Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Cc: Stable <stable@vger.kernel.org> [2.6.38+]
>>> ---
>>>
>>> V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
>>>
>>>   drivers/net/wireless/rtlwifi/base.c |   15 ++++++---------
>>>   drivers/net/wireless/rtlwifi/wifi.h |    6 +-----
>>>   2 files changed, 7 insertions(+), 14 deletions(-)
>>> Index: wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
>>> ===================================================================
>>> --- wireless-testing-save.orig/drivers/net/wireless/rtlwifi/base.c
>>> +++ wireless-testing-save/drivers/net/wireless/rtlwifi/base.c
>>> @@ -1077,17 +1077,17 @@ u8 rtl_is_special_data(struct ieee80211_
>>>
>>>   	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
>>>   			      SNAP_SIZE + PROTOC_TYPE_SIZE);
>>> -	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
>>> -	/*	ether_type = ntohs(ether_type); */
>>> +	ether_type = be16_to_cpu(*(__be16 *)((u8 *)skb->data + mac_hdr_len +
>>> +					     SNAP_SIZE));
>>>
>>>   	if (ETH_P_IP == ether_type) {
>>>   		if (IPPROTO_UDP == ip->protocol) {
>>>   			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
>>>   							       (ip->ihl << 2));
>>> -			if (((((u8 *) udp)[1] == 68) &&
>>> -			     (((u8 *) udp)[3] == 67)) ||
>>> -			    ((((u8 *) udp)[1] == 67) &&
>>> -			     (((u8 *) udp)[3] == 68))) {
>>> +			if (((((u16 *) udp)[0] == 68) &&
>>> +			     (((u16 *) udp)[2] == 67)) ||
>>> +			    ((((u16 *) udp)[0] == 67) &&
>>> +			     (((u16 *) udp)[2] == 68))) {
>> [...]
>>
>> Now you're missing byte-swapping here, and using the wrong offset for
>> the dest port (4 bytes rather than 2).
>>
>> If you really think this is necessary then use something like:
>> 			if ((udp->source == htons(68) &&
>> 			     udp->dest == htons(67)) ||
>> 			     ...
>
> Or maybe something like this?
> ---
>   drivers/net/wireless/rtlwifi/base.c | 91 +++++++++++++++++--------------------
>   1 file changed, 41 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
> index 9a78e3d..7e9df65 100644
> --- a/drivers/net/wireless/rtlwifi/base.c
> +++ b/drivers/net/wireless/rtlwifi/base.c
> @@ -37,6 +37,7 @@
>
>   #include <linux/ip.h>
>   #include <linux/module.h>
> +#include <linux/udp.h>
>
>   /*
>    *NOTICE!!!: This file will be very big, we should
> @@ -1074,64 +1075,54 @@ u8 rtl_is_special_data(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
>   	if (!ieee80211_is_data(fc))
>   		return false;
>
> +	ip = (const struct iphdr *)(skb->data + mac_hdr_len +
> +				    SNAP_SIZE + PROTOC_TYPE_SIZE);
> +	ether_type = be16_to_cpup((__be16 *)
> +				  (skb->data + mac_hdr_len + SNAP_SIZE));
>
> -	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
> -			      SNAP_SIZE + PROTOC_TYPE_SIZE);
> -	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
> -	/*	ether_type = ntohs(ether_type); */
> -
> -	if (ETH_P_IP == ether_type) {
> -		if (IPPROTO_UDP == ip->protocol) {
> -			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
> -							       (ip->ihl << 2));
> -			if (((((u8 *) udp)[1] == 68) &&
> -			     (((u8 *) udp)[3] == 67)) ||
> -			    ((((u8 *) udp)[1] == 67) &&
> -			     (((u8 *) udp)[3] == 68))) {
> -				/*
> -				 * 68 : UDP BOOTP client
> -				 * 67 : UDP BOOTP server
> -				 */
> -				RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV),
> -					 DBG_DMESG, "dhcp %s !!\n",
> -					 is_tx ? "Tx" : "Rx");
> -
> -				if (is_tx) {
> -					rtlpriv->enter_ps = false;
> -					schedule_work(&rtlpriv->
> -						      works.lps_change_work);
> -					ppsc->last_delaylps_stamp_jiffies =
> -					    jiffies;
> -				}
> +	switch (ether_type) {
> +	case ETH_P_IP: {
> +		struct udphdr *udp;
> +		u16 src;
> +		u16 dst;
>
> -				return true;
> -			}
> -		}
> -	} else if (ETH_P_ARP == ether_type) {
> -		if (is_tx) {
> -			rtlpriv->enter_ps = false;
> -			schedule_work(&rtlpriv->works.lps_change_work);
> -			ppsc->last_delaylps_stamp_jiffies = jiffies;
> -		}
> +		if (ip->protocol != IPPROTO_UDP)
> +			return false;
>
> -		return true;
> -	} else if (ETH_P_PAE == ether_type) {
> -		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
> -			 "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
> +		udp = (struct udphdr *)((u8 *)ip + (ip->ihl << 2));
> +		src = be16_to_cpu(udp->source) >> 8;
> +		dst = be16_to_cpu(udp->dest) >> 8;
>
> -		if (is_tx) {
> -			rtlpriv->enter_ps = false;
> -			schedule_work(&rtlpriv->works.lps_change_work);
> -			ppsc->last_delaylps_stamp_jiffies = jiffies;
> -		}
> +		/*
> +		 * 68 : UDP BOOTP client
> +		 * 67 : UDP BOOTP server
> +		 */
> +		if (!((src == 68 && dst == 67) || (src == 67 && dst == 68)))
> +			return false;
>
> +		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
> +			 "dhcp %s !!\n", is_tx ? "Tx" : "Rx");
> +		break;
> +	}
> +	case ETH_P_ARP:
> +		break;
> +	case ETH_P_PAE:
> +		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
> +			 "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
> +		break;
> +	case ETH_P_IPV6:
>   		return true;
> -	} else if (ETH_P_IPV6 == ether_type) {
> -		/* IPv6 */
> -		return true;
> +	default:
> +		return false;
>   	}
>
> -	return false;
> +	if (is_tx) {
> +		rtlpriv->enter_ps = false;
> +		schedule_work(&rtlpriv->works.lps_change_work);
> +		ppsc->last_delaylps_stamp_jiffies = jiffies;
> +	}
> +
> +	return true;
>   }
>   EXPORT_SYMBOL_GPL(rtl_is_special_data);

Joe,

Thanks for this. I have rewritten the function somewhat along these lines. It is 
much cleaner this way.

Larry

[PATCH NEXT V2] rtlwifi: Fix endian error in extracting packet type

[Larry Finger]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

All of the rtlwifi drivers have an error in the routine that tests if
the data is "special". If it is, the subsequant transmission will be
at the lowest rate to enhance reliability. The 16-bit quantity is
big-endian, but was being extracted in native CPU mode. One of the
effects of this bug is to inhibit association under some conditions
as the TX rate is too high.

Based on suggestions by Joe Perches, the entire routine is rewritten.

One of the local headers contained duplicates of some of the ETH_P_XXX
definitions. These are deleted.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Stable <stable@vger.kernel.org> [2.6.38+]
---

V2 - Addresses comments by Ben Hutchings and Bjorn Mork.
V3 - Rewrite of rtl_is_special_data()

 drivers/net/wireless/rtlwifi/base.c |   15 ++++++---------
 drivers/net/wireless/rtlwifi/wifi.h |    6 +-----
 2 files changed, 7 insertions(+), 14 deletions(-)
Index: wireless-testing/drivers/net/wireless/rtlwifi/base.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtlwifi/base.c
+++ wireless-testing/drivers/net/wireless/rtlwifi/base.c
@@ -37,6 +37,7 @@
 
 #include <linux/ip.h>
 #include <linux/module.h>
+#include <linux/udp.h>
 
 /*
  *NOTICE!!!: This file will be very big, we should
@@ -1074,64 +1075,52 @@ u8 rtl_is_special_data(struct ieee80211_
 	if (!ieee80211_is_data(fc))
 		return false;
 
+	ip = (const struct iphdr *)(skb->data + mac_hdr_len +
+				    SNAP_SIZE + PROTOC_TYPE_SIZE);
+	ether_type = be16_to_cpup((__be16 *)
+				  (skb->data + mac_hdr_len + SNAP_SIZE));
+
+	switch (ether_type) {
+	case ETH_P_IP: {
+		struct udphdr *udp;
+		u16 src;
+		u16 dst;
+
+		if (ip->protocol != IPPROTO_UDP)
+			return false;
+		udp = (struct udphdr *)((u8 *)ip + (ip->ihl << 2));
+		src = be16_to_cpu(udp->source);
+		dst = be16_to_cpu(udp->dest);
+
+		/* If this case involves port 68 (UDP BOOTP client) connecting
+		 * with port 67 (UDP BOOTP server), then return true so that
+		 * the lowest speed is used.
+		 */
+		if (!((src == 68 && dst == 67) || (src == 67 && dst == 68)))
+			return false;
 
-	ip = (struct iphdr *)((u8 *) skb->data + mac_hdr_len +
-			      SNAP_SIZE + PROTOC_TYPE_SIZE);
-	ether_type = *(u16 *) ((u8 *) skb->data + mac_hdr_len + SNAP_SIZE);
-	/*	ether_type = ntohs(ether_type); */
-
-	if (ETH_P_IP == ether_type) {
-		if (IPPROTO_UDP == ip->protocol) {
-			struct udphdr *udp = (struct udphdr *)((u8 *) ip +
-							       (ip->ihl << 2));
-			if (((((u8 *) udp)[1] == 68) &&
-			     (((u8 *) udp)[3] == 67)) ||
-			    ((((u8 *) udp)[1] == 67) &&
-			     (((u8 *) udp)[3] == 68))) {
-				/*
-				 * 68 : UDP BOOTP client
-				 * 67 : UDP BOOTP server
-				 */
-				RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV),
-					 DBG_DMESG, "dhcp %s !!\n",
-					 is_tx ? "Tx" : "Rx");
-
-				if (is_tx) {
-					rtlpriv->enter_ps = false;
-					schedule_work(&rtlpriv->
-						      works.lps_change_work);
-					ppsc->last_delaylps_stamp_jiffies =
-					    jiffies;
-				}
-
-				return true;
-			}
-		}
-	} else if (ETH_P_ARP == ether_type) {
-		if (is_tx) {
-			rtlpriv->enter_ps = false;
-			schedule_work(&rtlpriv->works.lps_change_work);
-			ppsc->last_delaylps_stamp_jiffies = jiffies;
-		}
-
-		return true;
-	} else if (ETH_P_PAE == ether_type) {
+		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
+			 "dhcp %s !!\n", is_tx ? "Tx" : "Rx");
+		break;
+	}
+	case ETH_P_ARP:
+		break;
+	case ETH_P_PAE:
 		RT_TRACE(rtlpriv, (COMP_SEND | COMP_RECV), DBG_DMESG,
 			 "802.1X %s EAPOL pkt!!\n", is_tx ? "Tx" : "Rx");
-
-		if (is_tx) {
-			rtlpriv->enter_ps = false;
-			schedule_work(&rtlpriv->works.lps_change_work);
-			ppsc->last_delaylps_stamp_jiffies = jiffies;
-		}
-
-		return true;
-	} else if (ETH_P_IPV6 == ether_type) {
-		/* IPv6 */
-		return true;
+		break;
+	case ETH_P_IPV6:
+		/* TODO: Is this right? */
+		return false;
+	default:
+		return false;
 	}
-
-	return false;
+	if (is_tx) {
+		rtlpriv->enter_ps = false;
+		schedule_work(&rtlpriv->works.lps_change_work);
+		ppsc->last_delaylps_stamp_jiffies = jiffies;
+	}
+	return true;
 }
 EXPORT_SYMBOL_GPL(rtl_is_special_data);
 
Index: wireless-testing/drivers/net/wireless/rtlwifi/wifi.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/rtlwifi/wifi.h
+++ wireless-testing/drivers/net/wireless/rtlwifi/wifi.h
@@ -77,11 +77,7 @@
 #define RTL_SLOT_TIME_9				9
 #define RTL_SLOT_TIME_20			20
 
-/*related with tcp/ip. */
-/*if_ehther.h*/
-#define ETH_P_PAE		0x888E	/*Port Access Entity (IEEE 802.1X) */
-#define ETH_P_IP		0x0800	/*Internet Protocol packet */
-#define ETH_P_ARP		0x0806	/*Address Resolution packet */
+/*related to tcp/ip. */
 #define SNAP_SIZE		6
 #define PROTOC_TYPE_SIZE	2