Re: [PATCH] Fix access point rate for trunk and stable

[Dan Williams <dcbw redhat com>]

On Mon, 2007-10-08 at 15:46 +0200, Helmut Schaa wrote:
> Am Mo 08 Okt 2007 15:10:11 CEST schrieb Dan Williams <dcbw redhat com>:
> 
> > On Mon, 2007-10-08 at 12:41 +0200, Helmut Schaa wrote:
> >> Hi,
> >>
> >> NetworkManager (stable and trunk) uses a guint16 for holding the
> >> access-point's rate property. Unfortunately wpa_supplicant, iwlist and the
> >> wext-interface use an int32 for the same. The attached patches fix   
> >> this issue
> >> by using a gint32 instead of a guint16 for trunk and stable.
> >
> > I'm not really opposed to the change; but there's no point in even
> > allowing values that are < 0 for rate since that just cannot happen...
> > A change from uint16 -> uint32 would be fine, including the move to Mb
> > rates instead of MB rates.  Is there a particular need to match the rate
> > type with wpa_supplicant and such, a specific issue?
> 
> uint32 would be fine for me too.
> 
> The only objection is that the wext-interface uses a signed integer  
> and therefore negative values could happen (what is very unlikely of  
> course). I don't know if there is a special case where it could  
> contain -1 to indicate something?
> 
> I can of course adapt the patch to use uint32 instead of int32 :)

Yeah, I think I'd prefer that.  If we ever get a rate out of
wpa_supplicant that is < 0, NM should just clamp to 0.  I think this
would only happen in error conditions, and with the D-Bus interface
errors are reported through exceptions anyway.

So making it UINT32 would be great.

As for the divisor, the reason wext and wpa_supplicant (which just
passes back what wext tells it) use large numbers is because Jean was
trying to write wext for a wide range of hardware, which is fine.  But
it turns out that it's only used for 802.11 hardware really, which means
a certain number of rates starting at 1Mb/s.  There does need to be at
least .1Mb/s resolution.  So I propose the following:

1) Make it a uint32
2) clamp the int to (0, INT_MAX) and cast to uint32 (ie, leave the value
as-is with out dividing by 1000000)

There certainly are bitrates of 5.5 Mb/s for example, that we need to
make available, and the way it was done before with an integer and
dividing by 1000000 meant that could not be done.

Dan

> 
> Helmut
> 
> > Dan
> >
> >> I hope I didn't forget anything :)
> >>
> >> Dan, Tambet, please have a look at the patches and commit if everything is
> >> fine.
> >>
> >> Thanks,
> >> Helmut
> >
> >
> 
>