* [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
* 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
* [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 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).