Re: [PATCH] cfg80211: send regulatory beacon hint events to userspace

[Johannes Berg <johannes@sipsolutions.net>]

[-- Attachment #1: Type: text/plain, Size: 2899 bytes --]

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.

johanes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]