On Wed, 2007-10-24 at 12:22 +0200, Ron Rindjunsky wrote:
> +#define BSS_VERIFY_STATE_ASSOC (1<<0)
Why "verify"? Shouldn't it be "CHANGED" instead?
> +struct ieee80211_bss_data {
> + u8 verify_map;
Seeing that you already used 5 of 8 bits and we're likely to increase
this in the future, I'd go with a u32 here. Also, have you attempted to
make all fields 'valid' all the time and if so, why is there no
'changed' bitmap passed to the callback?
> + /* association related data */
> + u8 assoc;
> + u16 aid;
And then reorder these two for less padding.
> + * @bss_info_changed: Handler for configuration requests related to BSS
> + * parameters that may vary during BSS's lifespan, and may affect low
> + * level driver (e.g. assoc/disassoc status, erp parameters).
> + * This function should not be used if no BSS has been set, unless
> + * for association indication.
I don't understand the last sentence here. Can you elaborate? Is it
meant as a note for mac80211 hackers? If so, wouldn't it be more
appropriate to add it to the function that calls this? Also, you didn't
add the if_id which will even break Intel's driver once multi-MAC
support is added...
> @@ -276,6 +276,7 @@ struct ieee80211_if_sta {
> u32 supp_rates_bits;
>
> int wmm_last_param_set;
> + struct ieee80211_bss_data bss_data;
Are you sure this will work properly? The same stuff must also be used
for AP mode, when hostapd decides to change things, no?
> + if (bss->has_erp_value)
> + ieee80211_handle_erp_ie(dev,
> + bss->erp_value);
Seeing that you're shuffling around this code anyway, I'd put patch 2
into this patch.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
