On Sun, Dec 09, 2007 at 01:35:00PM -0500, Dan Williams wrote: > > > > On Sun, 2007-12-09 at 18:34 +0000, Dave wrote: > > > > Dan Williams wrote: > > > On Fri, 2007-12-07 at 18:12 -0800, David Miller wrote: > > >> From: Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@xxxxxxxxxxxxxxxx> > > >> Date: Fri, 07 Dec 2007 19:22:46 -0500 > > >> > > >>> @@ -1040,6 +1049,16 @@ struct iw_range > > >>> * because each entry contain its channel index */ > > >>> > > >>> __u32 enc_capa; /* IW_ENC_CAPA_* bit field */ > > >>> + > > >>> + /* Do *NOT* use those fields, they are just used as padding to get > > >>> + * proper alignement with user space */ > > >>> + __s32 reserved1; > > >>> + __s32 reserved2; > > >>> + __u16 reserved3; > > >>> + __s32 reserved4; > > >>> + __u32 reserved5; > > >>> + > > >>> + __u32 scan_capa; /* IW_SCAN_CAPA_* bit field */ > > >>> }; > > >>> > > >>> /* > > >> Major NACK. These datastructure usages are complete wrong, and > > >> we have to stop spreading this problem instead of continuing on > > >> with it as if it's OK. > > > > > > There's not too much we can do here. We need a better way to support > > > driver/card capabilities in WEXT right _now_, in parallel with > > > cfg80211/nl80211. The other alternative here is to have a 64-bit > > > generic capabilities field-to-end-all-fields and add more bitfield > > > position constants to that without extending the structure any more. > > > > > > Is there a better way you'd propose to do this _in_WEXT_? > > > > Since iw_range is not packed, there are a few locations where there is some padding. You could quite easily shoehorn an 8 bit bitmask into the existing structure without impacting backward compatibility (unless userspace is using the padding for something). For example: > > > > --- a/include/linux/wireless.h > > +++ b/include/linux/wireless.h > > @@ -1035,6 +1035,7 @@ struct iw_range > > /* Frequency */ > > __u16 num_channels; /* Number of channels [0; num - 1] */ > > __u8 num_frequency; /* Number of entry in the list */ > > + __u8 scan_capa; /* scan capabilities */ > > struct iw_freq freq[IW_MAX_FREQUENCIES]; /* list */ > > /* Note : this frequency list doesn't need to fit channel numbers, > > * because each entry contain its channel index */ > > > > Other candidate blocks are Old Frequency, Rates, Encoder stuff, Transmit power. > > Hmm; this could work as long as that part of the structure is guaranteed > to be 0 if it wasn't touched by the driver. If it could be filled with > garbage bits at any point, then it's not going to work. Interesting > thought. You can count on zero being there in almost every case, for this precise reason. The first thing a driver is supposed to do with iwrange is : ---------------------------------------- memset(range, 0, sizeof(struct iw_range)); ---------------------------------------- From what I remember, all drivers are doing it. If a driver does not do it, it should be fixed ASAP as other things would break (most driver only fill a few field of the struct and don't touch others). > Dan Regards, Jean P.S. : What's up with all the bogus e-mail addresses in cc ? - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
