netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 1/3] d80211: use FCS_LEN instead of hardcoded number.
       [not found] <20061009170159.804032000@devicescape.com>
@ 2006-10-09 17:02 ` David Kimdon
  2006-10-09 17:03 ` [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag David Kimdon
  2006-10-09 17:03 ` [patch 3/3] d80211: silence sparse warning: bad constant expression David Kimdon
  2 siblings, 0 replies; 12+ messages in thread
From: David Kimdon @ 2006-10-09 17:02 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville, Jiri Benc, David Kimdon

[-- Attachment #1: FCS_LEN.patch --]
[-- Type: text/plain, Size: 1070 bytes --]

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: wireless-dev/net/d80211/ieee80211.c
===================================================================
--- wireless-dev.orig/net/d80211/ieee80211.c
+++ wireless-dev/net/d80211/ieee80211.c
@@ -451,7 +451,7 @@ ieee80211_tx_h_fragment(struct ieee80211
 
 	hdrlen = ieee80211_get_hdrlen(tx->fc);
 	payload_len = first->len - hdrlen;
-	per_fragm = frag_threshold - hdrlen - 4 /* FCS */;
+	per_fragm = frag_threshold - hdrlen - FCS_LEN;
 	num_fragm = (payload_len + per_fragm - 1) / per_fragm;
 
 	frags = kzalloc(num_fragm * sizeof(struct sk_buff *), GFP_ATOMIC);
@@ -1103,7 +1103,7 @@ __ieee80211_tx_prepare(struct ieee80211_
         control->no_ack = is_multicast_ether_addr(hdr->addr1);
 	tx->fragmented = local->fragmentation_threshold <
 		IEEE80211_MAX_FRAG_THRESHOLD && tx->u.tx.unicast &&
-		skb->len + 4 /* FCS */ > local->fragmentation_threshold &&
+		skb->len + FCS_LEN > local->fragmentation_threshold &&
 		(!local->hw->set_frag_threshold);
 	if (!tx->sta)
 		control->clear_dst_mask = 1;

--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
       [not found] <20061009170159.804032000@devicescape.com>
  2006-10-09 17:02 ` [patch 1/3] d80211: use FCS_LEN instead of hardcoded number David Kimdon
@ 2006-10-09 17:03 ` David Kimdon
  2006-10-10 10:00   ` Michael Buesch
  2006-10-09 17:03 ` [patch 3/3] d80211: silence sparse warning: bad constant expression David Kimdon
  2 siblings, 1 reply; 12+ messages in thread
From: David Kimdon @ 2006-10-09 17:03 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville, Jiri Benc, David Kimdon

[-- Attachment #1: remove-extra_hdr_room.patch --]
[-- Type: text/plain, Size: 4771 bytes --]

This flag is unused by all in tree drivers.  Furthermore, the way that
it is documented is not consistent with the way it is actually used by
ieee80211.c.  The original attempt appears to be something to do with
adding extra header room for low-level drivers which need to pad the
IEEE 802.11 header (example: Atheros).

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: wireless-dev/include/net/d80211.h
===================================================================
--- wireless-dev.orig/include/net/d80211.h
+++ wireless-dev/include/net/d80211.h
@@ -476,10 +476,6 @@ struct ieee80211_hw {
 	/* Force software encryption for TKIP packets if WMM is enabled. */
 	unsigned int no_tkip_wmm_hwaccel:1;
 
-	/* set if the payload needs to be padded at even boundaries after the
-	 * header */
-	unsigned int extra_hdr_room:1;
-
 	/* Some devices handle Michael MIC internally and do not include MIC in
 	 * the received packets passed up. device_strips_mic must be set
 	 * for such devices. The 'encryption' frame control bit is expected to
Index: wireless-dev/net/d80211/ieee80211.c
===================================================================
--- wireless-dev.orig/net/d80211/ieee80211.c
+++ wireless-dev/net/d80211/ieee80211.c
@@ -1551,7 +1551,7 @@ static int ieee80211_subif_start_xmit(st
 	 * build in headroom in __dev_alloc_skb() (linux/skbuff.h) and
 	 * alloc_skb() (net/core/skbuff.c)
 	 */
-	head_need = hdrlen + encaps_len + (local->hw->extra_hdr_room ? 2 : 0);
+	head_need = hdrlen + encaps_len;
 	head_need -= skb_headroom(skb);
 
 	/* We are going to modify skb data, so make a copy of it if happens to
Index: wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/adm8211/adm8211.c
+++ wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
@@ -2018,7 +2018,6 @@ static int __devinit adm8211_probe(struc
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 0;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 0;
 	hw->fraglist = 0;
Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
+++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
@@ -2578,7 +2578,6 @@ static int rt2400pci_init_hw(struct rt2x
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 1;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 1;
 	hw->fraglist = 0;
Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
+++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
@@ -2732,7 +2732,6 @@ static int rt2500pci_init_hw(struct rt2x
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 1;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 1;
 	hw->fraglist = 0;
Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
+++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
@@ -2419,7 +2419,6 @@ static int rt2500usb_init_hw(struct rt2x
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 1;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 1;
 	hw->fraglist = 0;
Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt61pci.c
+++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
@@ -3252,7 +3252,6 @@ static int rt61pci_init_hw(struct rt2x00
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 1;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 1;
 	hw->fraglist = 0;
Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt73usb.c
+++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
@@ -2792,7 +2792,6 @@ static int rt73usb_init_hw(struct rt2x00
 	hw->wep_include_iv = 1;
 	hw->data_nullfunc_ack = 1;
 	hw->no_tkip_wmm_hwaccel = 1;
-	hw->extra_hdr_room = 0;
 	hw->device_strips_mic = 0;
 	hw->monitor_during_oper = 1;
 	hw->fraglist = 0;

--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [patch 3/3] d80211: silence sparse warning: bad constant expression
       [not found] <20061009170159.804032000@devicescape.com>
  2006-10-09 17:02 ` [patch 1/3] d80211: use FCS_LEN instead of hardcoded number David Kimdon
  2006-10-09 17:03 ` [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag David Kimdon
@ 2006-10-09 17:03 ` David Kimdon
  2006-10-09 20:11   ` David Kimdon
  2 siblings, 1 reply; 12+ messages in thread
From: David Kimdon @ 2006-10-09 17:03 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville, Jiri Benc, David Kimdon

[-- Attachment #1: sparse.patch --]
[-- Type: text/plain, Size: 1081 bytes --]

Sparse does not figure out that algs[] isn't really a variable length array.
The message is:

net/d80211/ieee80211_sta.c:934:12: error: bad constant expression

This switches algs[] to be obviously a constant array, and derives the value of
num_algs algs[].  The code is correct and equivalent with or without this
change.

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: wireless-dev/net/d80211/ieee80211_sta.c
===================================================================
--- wireless-dev.orig/net/d80211/ieee80211_sta.c
+++ wireless-dev/net/d80211/ieee80211_sta.c
@@ -930,8 +930,8 @@ static void ieee80211_rx_mgmt_auth(struc
 		printk(KERN_DEBUG "%s: AP denied authentication (auth_alg=%d "
 		       "code=%d)\n", dev->name, ifsta->auth_alg, status_code);
 		if (status_code == WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG) {
-			const int num_algs = 3;
-			u8 algs[num_algs];
+			u8 algs[3];
+			const int num_algs = sizeof(algs) / sizeof(algs[0]);
 			int i, pos;
 			algs[0] = algs[1] = algs[2] = 0xff;
 			if (ifsta->auth_algs & IEEE80211_AUTH_ALG_OPEN)

--

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 3/3] d80211: silence sparse warning: bad constant expression
  2006-10-09 17:03 ` [patch 3/3] d80211: silence sparse warning: bad constant expression David Kimdon
@ 2006-10-09 20:11   ` David Kimdon
  2006-10-18 13:56     ` Jiri Benc
  0 siblings, 1 reply; 12+ messages in thread
From: David Kimdon @ 2006-10-09 20:11 UTC (permalink / raw)
  To: David Kimdon; +Cc: netdev, John W. Linville, Jiri Benc

Update to use ARRAY_SIZE, based on comment from Joe Perches.

d80211: silence sparse warning: 'bad constant expression'

Sparse does not figure out that algs[] isn't really a variable length array.
The message is:

net/d80211/ieee80211_sta.c:934:12: error: bad constant expression

This switches algs[] to be obviously a constant array, and derives the value of
num_algs algs[].  The code is correct and equivalent with or without this
change.

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: wireless-dev/net/d80211/ieee80211_sta.c
===================================================================
--- wireless-dev.orig/net/d80211/ieee80211_sta.c
+++ wireless-dev/net/d80211/ieee80211_sta.c
@@ -930,8 +930,8 @@ static void ieee80211_rx_mgmt_auth(struc
 		printk(KERN_DEBUG "%s: AP denied authentication (auth_alg=%d "
 		       "code=%d)\n", dev->name, ifsta->auth_alg, status_code);
 		if (status_code == WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG) {
-			const int num_algs = 3;
-			u8 algs[num_algs];
+			u8 algs[3];
+			const int num_algs = ARRAY_SIZE(algs);
 			int i, pos;
 			algs[0] = algs[1] = algs[2] = 0xff;
 			if (ifsta->auth_algs & IEEE80211_AUTH_ALG_OPEN)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
  2006-10-09 17:03 ` [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag David Kimdon
@ 2006-10-10 10:00   ` Michael Buesch
  2006-10-10 13:45     ` David Kimdon
  0 siblings, 1 reply; 12+ messages in thread
From: Michael Buesch @ 2006-10-10 10:00 UTC (permalink / raw)
  To: David Kimdon; +Cc: netdev, John W. Linville, Jiri Benc

On Monday 09 October 2006 19:03, David Kimdon wrote:
> This flag is unused by all in tree drivers.  Furthermore, the way that
> it is documented is not consistent with the way it is actually used by
> ieee80211.c.  The original attempt appears to be something to do with
> adding extra header room for low-level drivers which need to pad the
> IEEE 802.11 header (example: Atheros).

What about not removing it, but extending it.
In the current form it's rather useless. But we could make extra_hdr_room
a value instead of a flag. Bcm43xx needs somewhere around 30bytes extra
header room. So every driver which does not need header room, sets this
to 0. Every other driver to the specific value.

We currently workaround the problem by using an extra DMA descriptor
just for the header. That's not really a problem, though. But it means
a little bit of additional overhead in the TX path. Well.

> Signed-off-by: David Kimdon <david.kimdon@devicescape.com>
> 
> Index: wireless-dev/include/net/d80211.h
> ===================================================================
> --- wireless-dev.orig/include/net/d80211.h
> +++ wireless-dev/include/net/d80211.h
> @@ -476,10 +476,6 @@ struct ieee80211_hw {
>  	/* Force software encryption for TKIP packets if WMM is enabled. */
>  	unsigned int no_tkip_wmm_hwaccel:1;
>  
> -	/* set if the payload needs to be padded at even boundaries after the
> -	 * header */
> -	unsigned int extra_hdr_room:1;
> -
>  	/* Some devices handle Michael MIC internally and do not include MIC in
>  	 * the received packets passed up. device_strips_mic must be set
>  	 * for such devices. The 'encryption' frame control bit is expected to
> Index: wireless-dev/net/d80211/ieee80211.c
> ===================================================================
> --- wireless-dev.orig/net/d80211/ieee80211.c
> +++ wireless-dev/net/d80211/ieee80211.c
> @@ -1551,7 +1551,7 @@ static int ieee80211_subif_start_xmit(st
>  	 * build in headroom in __dev_alloc_skb() (linux/skbuff.h) and
>  	 * alloc_skb() (net/core/skbuff.c)
>  	 */
> -	head_need = hdrlen + encaps_len + (local->hw->extra_hdr_room ? 2 : 0);
> +	head_need = hdrlen + encaps_len;
>  	head_need -= skb_headroom(skb);
>  
>  	/* We are going to modify skb data, so make a copy of it if happens to
> Index: wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/adm8211/adm8211.c
> +++ wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
> @@ -2018,7 +2018,6 @@ static int __devinit adm8211_probe(struc
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 0;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 0;
>  	hw->fraglist = 0;
> Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> @@ -2578,7 +2578,6 @@ static int rt2400pci_init_hw(struct rt2x
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 1;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 1;
>  	hw->fraglist = 0;
> Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> @@ -2732,7 +2732,6 @@ static int rt2500pci_init_hw(struct rt2x
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 1;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 1;
>  	hw->fraglist = 0;
> Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> @@ -2419,7 +2419,6 @@ static int rt2500usb_init_hw(struct rt2x
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 1;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 1;
>  	hw->fraglist = 0;
> Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> @@ -3252,7 +3252,6 @@ static int rt61pci_init_hw(struct rt2x00
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 1;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 1;
>  	hw->fraglist = 0;
> Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> ===================================================================
> --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> @@ -2792,7 +2792,6 @@ static int rt73usb_init_hw(struct rt2x00
>  	hw->wep_include_iv = 1;
>  	hw->data_nullfunc_ack = 1;
>  	hw->no_tkip_wmm_hwaccel = 1;
> -	hw->extra_hdr_room = 0;
>  	hw->device_strips_mic = 0;
>  	hw->monitor_during_oper = 1;
>  	hw->fraglist = 0;
> 
> --
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
  2006-10-10 10:00   ` Michael Buesch
@ 2006-10-10 13:45     ` David Kimdon
  2006-10-10 14:30       ` Michael Wu
  2006-10-10 21:04       ` Michael Buesch
  0 siblings, 2 replies; 12+ messages in thread
From: David Kimdon @ 2006-10-10 13:45 UTC (permalink / raw)
  To: Michael Buesch; +Cc: David Kimdon, netdev, John W. Linville, Jiri Benc

On Tue, Oct 10, 2006 at 12:00:12PM +0200, Michael Buesch wrote:
> On Monday 09 October 2006 19:03, David Kimdon wrote:
> > This flag is unused by all in tree drivers.  Furthermore, the way that
> > it is documented is not consistent with the way it is actually used by
> > ieee80211.c.  The original attempt appears to be something to do with
> > adding extra header room for low-level drivers which need to pad the
> > IEEE 802.11 header (example: Atheros).
> 
> What about not removing it, but extending it.

Sure, I would be happy with that.  For Atheros it will be '2' to
ensure that we can properly align the packet in the tx path without
needing to realloc. 

> In the current form it's rather useless. But we could make extra_hdr_room
> a value instead of a flag. Bcm43xx needs somewhere around 30bytes extra
> header room. So every driver which does not need header room, sets this
> to 0. Every other driver to the specific value.
> 
> We currently workaround the problem by using an extra DMA descriptor
> just for the header. That's not really a problem, though. But it means
> a little bit of additional overhead in the TX path. Well.
> 
> > Signed-off-by: David Kimdon <david.kimdon@devicescape.com>
> > 
> > Index: wireless-dev/include/net/d80211.h
> > ===================================================================
> > --- wireless-dev.orig/include/net/d80211.h
> > +++ wireless-dev/include/net/d80211.h
> > @@ -476,10 +476,6 @@ struct ieee80211_hw {
> >  	/* Force software encryption for TKIP packets if WMM is enabled. */
> >  	unsigned int no_tkip_wmm_hwaccel:1;
> >  
> > -	/* set if the payload needs to be padded at even boundaries after the
> > -	 * header */
> > -	unsigned int extra_hdr_room:1;
> > -
> >  	/* Some devices handle Michael MIC internally and do not include MIC in
> >  	 * the received packets passed up. device_strips_mic must be set
> >  	 * for such devices. The 'encryption' frame control bit is expected to
> > Index: wireless-dev/net/d80211/ieee80211.c
> > ===================================================================
> > --- wireless-dev.orig/net/d80211/ieee80211.c
> > +++ wireless-dev/net/d80211/ieee80211.c
> > @@ -1551,7 +1551,7 @@ static int ieee80211_subif_start_xmit(st
> >  	 * build in headroom in __dev_alloc_skb() (linux/skbuff.h) and
> >  	 * alloc_skb() (net/core/skbuff.c)
> >  	 */
> > -	head_need = hdrlen + encaps_len + (local->hw->extra_hdr_room ? 2 : 0);
> > +	head_need = hdrlen + encaps_len;
> >  	head_need -= skb_headroom(skb);
> >  
> >  	/* We are going to modify skb data, so make a copy of it if happens to
> > Index: wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/adm8211/adm8211.c
> > +++ wireless-dev/drivers/net/wireless/d80211/adm8211/adm8211.c
> > @@ -2018,7 +2018,6 @@ static int __devinit adm8211_probe(struc
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 0;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 0;
> >  	hw->fraglist = 0;
> > Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> > +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2400pci.c
> > @@ -2578,7 +2578,6 @@ static int rt2400pci_init_hw(struct rt2x
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 1;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 1;
> >  	hw->fraglist = 0;
> > Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> > +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500pci.c
> > @@ -2732,7 +2732,6 @@ static int rt2500pci_init_hw(struct rt2x
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 1;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 1;
> >  	hw->fraglist = 0;
> > Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> > +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt2500usb.c
> > @@ -2419,7 +2419,6 @@ static int rt2500usb_init_hw(struct rt2x
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 1;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 1;
> >  	hw->fraglist = 0;
> > Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> > +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt61pci.c
> > @@ -3252,7 +3252,6 @@ static int rt61pci_init_hw(struct rt2x00
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 1;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 1;
> >  	hw->fraglist = 0;
> > Index: wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > ===================================================================
> > --- wireless-dev.orig/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > +++ wireless-dev/drivers/net/wireless/d80211/rt2x00/rt73usb.c
> > @@ -2792,7 +2792,6 @@ static int rt73usb_init_hw(struct rt2x00
> >  	hw->wep_include_iv = 1;
> >  	hw->data_nullfunc_ack = 1;
> >  	hw->no_tkip_wmm_hwaccel = 1;
> > -	hw->extra_hdr_room = 0;
> >  	hw->device_strips_mic = 0;
> >  	hw->monitor_during_oper = 1;
> >  	hw->fraglist = 0;
> > 
> > --
> > -
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
> -- 
> Greetings Michael.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
  2006-10-10 13:45     ` David Kimdon
@ 2006-10-10 14:30       ` Michael Wu
  2006-10-10 21:04       ` Michael Buesch
  1 sibling, 0 replies; 12+ messages in thread
From: Michael Wu @ 2006-10-10 14:30 UTC (permalink / raw)
  To: David Kimdon; +Cc: Michael Buesch, netdev, John W. Linville, Jiri Benc

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

On Tuesday 10 October 2006 09:45, David Kimdon wrote:
> On Tue, Oct 10, 2006 at 12:00:12PM +0200, Michael Buesch wrote:
> > On Monday 09 October 2006 19:03, David Kimdon wrote:
> > > This flag is unused by all in tree drivers.  Furthermore, the way that
> > > it is documented is not consistent with the way it is actually used by
> > > ieee80211.c.  The original attempt appears to be something to do with
> > > adding extra header room for low-level drivers which need to pad the
> > > IEEE 802.11 header (example: Atheros).
> >
> > What about not removing it, but extending it.
>
> Sure, I would be happy with that.  For Atheros it will be '2' to
> ensure that we can properly align the packet in the tx path without
> needing to realloc.
>
adm8211 needs this too to avoid realloc and possibly failing and dropping the 
packet. (except adm8211 needs closer to about 30 bytes of extra room to fit 
its hardware tx header)

Thanks,
-Michael Wu

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
  2006-10-10 13:45     ` David Kimdon
  2006-10-10 14:30       ` Michael Wu
@ 2006-10-10 21:04       ` Michael Buesch
  2006-10-11  0:37         ` David Kimdon
  1 sibling, 1 reply; 12+ messages in thread
From: Michael Buesch @ 2006-10-10 21:04 UTC (permalink / raw)
  To: David Kimdon; +Cc: netdev, John W. Linville, Jiri Benc

On Tuesday 10 October 2006 15:45, David Kimdon wrote:
> On Tue, Oct 10, 2006 at 12:00:12PM +0200, Michael Buesch wrote:
> > On Monday 09 October 2006 19:03, David Kimdon wrote:
> > > This flag is unused by all in tree drivers.  Furthermore, the way that
> > > it is documented is not consistent with the way it is actually used by
> > > ieee80211.c.  The original attempt appears to be something to do with
> > > adding extra header room for low-level drivers which need to pad the
> > > IEEE 802.11 header (example: Atheros).
> > 
> > What about not removing it, but extending it.
> 
> Sure, I would be happy with that.  For Atheros it will be '2' to
> ensure that we can properly align the packet in the tx path without
> needing to realloc. 

Ok, nice. Do you want to do a patch or should I do?

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag
  2006-10-10 21:04       ` Michael Buesch
@ 2006-10-11  0:37         ` David Kimdon
  0 siblings, 0 replies; 12+ messages in thread
From: David Kimdon @ 2006-10-11  0:37 UTC (permalink / raw)
  To: Michael Buesch; +Cc: David Kimdon, netdev, John W. Linville, Jiri Benc

On Tue, Oct 10, 2006 at 11:04:05PM +0200, Michael Buesch wrote:
> On Tuesday 10 October 2006 15:45, David Kimdon wrote:
> > On Tue, Oct 10, 2006 at 12:00:12PM +0200, Michael Buesch wrote:
> > > On Monday 09 October 2006 19:03, David Kimdon wrote:
> > > > This flag is unused by all in tree drivers.  Furthermore, the way that
> > > > it is documented is not consistent with the way it is actually used by
> > > > ieee80211.c.  The original attempt appears to be something to do with
> > > > adding extra header room for low-level drivers which need to pad the
> > > > IEEE 802.11 header (example: Atheros).
> > > 
> > > What about not removing it, but extending it.
> > 
> > Sure, I would be happy with that.  For Atheros it will be '2' to
> > ensure that we can properly align the packet in the tx path without
> > needing to realloc. 
> 
> Ok, nice. Do you want to do a patch or should I do?

Go ahead, thanks.

-David

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 3/3] d80211: silence sparse warning: bad constant expression
  2006-10-09 20:11   ` David Kimdon
@ 2006-10-18 13:56     ` Jiri Benc
  2006-10-18 15:12       ` David Kimdon
  0 siblings, 1 reply; 12+ messages in thread
From: Jiri Benc @ 2006-10-18 13:56 UTC (permalink / raw)
  To: David Kimdon; +Cc: netdev, John W. Linville

On Mon, 9 Oct 2006 13:11:02 -0700, David Kimdon wrote:
> --- wireless-dev.orig/net/d80211/ieee80211_sta.c
> +++ wireless-dev/net/d80211/ieee80211_sta.c
> @@ -930,8 +930,8 @@ static void ieee80211_rx_mgmt_auth(struc
>  		printk(KERN_DEBUG "%s: AP denied authentication (auth_alg=%d "
>  		       "code=%d)\n", dev->name, ifsta->auth_alg, status_code);
>  		if (status_code == WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG) {
> -			const int num_algs = 3;
> -			u8 algs[num_algs];
> +			u8 algs[3];
> +			const int num_algs = ARRAY_SIZE(algs);

Wouldn't it be better just to use ARRAY_SIZE(algs) and get rid of
num_algs completely?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 3/3] d80211: silence sparse warning: bad constant expression
  2006-10-18 13:56     ` Jiri Benc
@ 2006-10-18 15:12       ` David Kimdon
  2006-10-18 15:40         ` Jiri Benc
  0 siblings, 1 reply; 12+ messages in thread
From: David Kimdon @ 2006-10-18 15:12 UTC (permalink / raw)
  To: Jiri Benc; +Cc: David Kimdon, netdev, John W. Linville

On Wed, Oct 18, 2006 at 03:56:07PM +0200, Jiri Benc wrote:
> On Mon, 9 Oct 2006 13:11:02 -0700, David Kimdon wrote:
> > --- wireless-dev.orig/net/d80211/ieee80211_sta.c
> > +++ wireless-dev/net/d80211/ieee80211_sta.c
> > @@ -930,8 +930,8 @@ static void ieee80211_rx_mgmt_auth(struc
> >  		printk(KERN_DEBUG "%s: AP denied authentication (auth_alg=%d "
> >  		       "code=%d)\n", dev->name, ifsta->auth_alg, status_code);
> >  		if (status_code == WLAN_STATUS_NOT_SUPPORTED_AUTH_ALG) {
> > -			const int num_algs = 3;
> > -			u8 algs[num_algs];
> > +			u8 algs[3];
> > +			const int num_algs = ARRAY_SIZE(algs);
> 
> Wouldn't it be better just to use ARRAY_SIZE(algs) and get rid of
> num_algs completely?

I actually think the code reads slightly cleaner using num_algs, but
don't have a strong preference.  I'd be happy to make the change if
removing num_algs is preferred.

-David

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [patch 3/3] d80211: silence sparse warning: bad constant expression
  2006-10-18 15:12       ` David Kimdon
@ 2006-10-18 15:40         ` Jiri Benc
  0 siblings, 0 replies; 12+ messages in thread
From: Jiri Benc @ 2006-10-18 15:40 UTC (permalink / raw)
  To: David Kimdon; +Cc: netdev, John W. Linville

On Wed, 18 Oct 2006 08:12:27 -0700, David Kimdon wrote:
> I actually think the code reads slightly cleaner using num_algs, but
> don't have a strong preference.  I'd be happy to make the change if
> removing num_algs is preferred.

Don't know. But nobody except me objected for more than a week so I've
applied the patch to my tree.

I've also applied the following patches from you:
d80211: use FCS_LEN instead of hardcoded number.
d80211: remove unused Super AG definitions, purge comment (the second
version of the patch)

Thanks for the patches,

 Jiri

-- 
Jiri Benc
SUSE Labs

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2006-10-18 15:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20061009170159.804032000@devicescape.com>
2006-10-09 17:02 ` [patch 1/3] d80211: use FCS_LEN instead of hardcoded number David Kimdon
2006-10-09 17:03 ` [patch 2/3] d80211: remove poorly documented ieee80211_hw extra_hdr_room flag David Kimdon
2006-10-10 10:00   ` Michael Buesch
2006-10-10 13:45     ` David Kimdon
2006-10-10 14:30       ` Michael Wu
2006-10-10 21:04       ` Michael Buesch
2006-10-11  0:37         ` David Kimdon
2006-10-09 17:03 ` [patch 3/3] d80211: silence sparse warning: bad constant expression David Kimdon
2006-10-09 20:11   ` David Kimdon
2006-10-18 13:56     ` Jiri Benc
2006-10-18 15:12       ` David Kimdon
2006-10-18 15:40         ` Jiri Benc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).