svn commit: r206358 - in head/sys: conf dev/bwi dev/bwn dev/iwn dev/ral dev/usb/wlan dev/wpi modules/wlan net80211

[Rui Paulo]

On 7 Apr 2010, at 18:42, Gavin Atkinson wrote:

> On Wed, 2010-04-07 at 15:29 +0000, Rui Paulo wrote:
>> Author: rpaulo
>> Date: Wed Apr  7 15:29:13 2010
>> New Revision: 206358
>> URL: http://svn.freebsd.org/changeset/base/206358
>> 
>> Log:
>>  net80211 rate control framework (net80211 ratectl).
> 
> This looks to be great work!  Thanks for doing this.
> 
> [...]
> 
>>  MFC after:    1 months
> 
> The changes change KBI - I don't think it can be MFC'd as-is?
> (especially the sys/net80211/ieee80211_var.h changes)

It can be MFC'ed but not as-is.

> 
> Also...
> 
>> @@ -3393,8 +3368,8 @@ _bwi_txeof(struct bwi_softc *sc, uint16_
>> 	bus_dmamap_unload(sc->sc_buf_dtag, tb->tb_dmap);
>> 
>> 	ni = tb->tb_ni;
>> +	vap = ni->ni_vap;
>> 	if (tb->tb_ni != NULL) {
>> -		struct bwi_node *bn = (struct bwi_node *) tb->tb_ni;
> 
> If (tb->tb_ni) can indeed be NULL here, we'll now panic. If not, there's
> no need for the conditional.
> 
>> @@ -921,13 +900,12 @@ rt2661_tx_intr(struct rt2661_softc *sc)
>> 		data->m = NULL;
>> 		ni = data->ni;
>> 		data->ni = NULL;
>> +		vap = ni->ni_vap;
>> 
>> 		/* if no frame has been sent, ignore */
>> 		if (ni == NULL)
>> 			continue;
>> 
>> -		rn = RT2661_NODE(ni);
>> -
> 
> Same here.  Either we'll panic, or the test for (ni == NULL) isn't
> needed.

Right, this needs to be moved down.

> 
>> @@ -1022,6 +1008,7 @@ rt2560_tx_intr(struct rt2560_softc *sc)
>>                data->m = NULL;
>>                ieee80211_free_node(data->ni);
>>                data->ni = NULL;
>> +               ni = NULL;
>> 
>>                /* descriptor is no longer valid */
>>                desc->flags &= ~htole32(RT2560_TX_VALID);
> 
> This seems unnecessary - we never read "ni" again.  Either we fall out
> of the loop and exit the subroutine, or we go round the loop again and
> overwrite it.

I'll remove it.

> 
>> Index: head/sys/dev/usb/wlan/if_urtw.c
>> ===================================================================
>> --- head/sys/dev/usb/wlan/if_urtw.c     (revision 206357)
>> +++ head/sys/dev/usb/wlan/if_urtw.c     (revision 206358)
>> @@ -62,6 +62,7 @@ __FBSDID("$FreeBSD$");
>> #include <dev/usb/wlan/if_urtwvar.h>
>> 
>> SYSCTL_NODE(_hw_usb, OID_AUTO, urtw, CTLFLAG_RW, 0, "USB Realtek 8187L");
>> +#define URTW_DEBUG
>> #ifdef URTW_DEBUG
>> int urtw_debug = 0;
>> SYSCTL_INT(_hw_usb_urtw, OID_AUTO, debug, CTLFLAG_RW, &urtw_debug, 0,
> 
> Is this change intentional?

No.

Regards,
--
Rui Paulo