On Tue, Mar 31, 2009 at 1:16 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
> On Tue, 2009-03-31 at 01:10 -0700, Luis R. Rodriguez wrote:
>
>> >> + /* Unforunately this is needed */
>> >> + nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
>> >> + if (!nl_bands)
>> >> + goto nla_put_failure;
>> >> + nl_band = nla_nest_start(msg, band);
>> >> + if (!nl_band)
>> >> + goto nla_put_failure;
>> >> +
>> >> + /*
>> >> + * Our hack is to piggy back the channel prior to beacon hint
>> >> + * and after the beacon hint so userspace can analyze the
>> >> + * differences. Right now only no-ibss and passive-scan flags
>> >> + * can change as that's the only thing we expect to learn out
>> >> + * of a beacon for now. By re-using these attributes we can
>> >> + * avoid introducing new structs.
>> >> + */
>> >> + nl_freqs = nla_nest_start(msg, NL80211_BAND_ATTR_FREQS);
>> >> + if (!nl_freqs)
>> >> + goto nla_put_failure;
>> >> +
>> >> + for (i = 0; i <= 1; i++) {
>> >> + nl_freq = nla_nest_start(msg, i);
>> >> + if (!nl_freq)
>> >> + goto nla_put_failure;
>> >> +
>> >> + chan = (i == 0) ? channel_before : channel_after;
>> >> +
>> >> + NLA_PUT_U32(mNo, that's not exclusive...sg, NL80211_FREQUENCY_ATTR_FREQ,
>> >> + chan->center_freq);
>> >> +
>> >> + if (chan->flags & IEEE80211_CHAN_PASSIVE_SCAN)
>> >> + NLA_PUT_FLAG(msg, NL80211_FREQUENCY_ATTR_PASSIVE_SCAN);
>> >> + if (chan->flags & IEEE80211_CHAN_NO_IBSS)
>> >> + NLA_PUT_FLAG(msg, NL80211_FREQUENCY_ATTR_NO_IBSS);
>> >> +
>> >> + nla_nest_end(msg, nl_freq);
>> >> + }
>> >
>> > I don't think I like this -- it's confusing to userspace code that wants
>> > to use a unified message parser.
>>
>> I know the feeling, more on this below.
>>
>> > Do we really need that before/after thing anyway? I think if we really
>> > need this then we should add new attributes.
>>
>> Well we can definitely add new attributes, but remember we still have
>> to pass the center of freq. The cleanest solution is to define an
>> attribute with a center-freq, if-passive-lifted, if-beaconing-enabled
>> flags. But that ends up adding all that for something we already have
>> attributes for. I chose to use what we have.
>
> No, we don't have to do that. All we'd need to do is add a new attribute
> NL80211_ATTR_FREQ_CHANGE, which we document to
> 1) contain an array
> 2) in that array, contain nesting
> 3) in that nesting contain NL80211_FREQUENCY_ATTR
>
> That means you'd only need to remove the bands nesting and replace the
> BAND_ATTR_FREQS nesting by ATTR_FREQ_CHANGE nesting. Only one new
> attribute needed in total, and you can keep almost all the code too. The
> array nesting is a little ugly, but that's not a big concern.
Sounds good, thanks.
Luis
--
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
