On Fri, Jul 6, 2012 at 10:34 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> wrote:
> On Fri, 2012-07-06 at 10:30 +0300, Arik Nemtsov wrote:
>
>> >> Also putting ht_cap_invalid inside the struct creates strange corner
>> >> cases. One could mark the vif ht_cap as invalid as well, but the code
>> >> would just ignore it.
>> >
>> > Good point. But then let's just move it out and change drivers ...
>> > that's much cleaner I think.
>>
>> You mean move out ht_supported?
>
> Yeah, I think that'd make more sense? Have it in the sband directly.
It does make more sense. But I was thinking there might be code
setting this dynamically in some drivers, so the changes would be
non-trivial. The union was sort of a compromise.
Maybe move it out for vht_cap for now? :)
>
>
>> >> >> @@ -30,13 +31,16 @@ rdev_freq_to_chan(struct cfg80211_registered_device *rdev,
>> >> >> chan->flags & IEEE80211_CHAN_NO_HT40PLUS)
>> >> >> return NULL;
>> >> >>
>> >> >> - ht_cap = &rdev->wiphy.bands[chan->band]->ht_cap;
>> >> >> + sband = rdev->wiphy.bands[chan->band];
>> >> >> + ht_cap = &sband->ht_cap;
>> >> >>
>> >> >> if (channel_type != NL80211_CHAN_NO_HT) {
>> >> >> - if (!ht_cap->ht_supported)
>> >> >> + if (!sband->ht_supported)
>> >> >> return NULL;
>> >> >>
>> >> >> - if (channel_type != NL80211_CHAN_HT20 &&
>> >> >> + /* this check is ignored when per-band HT caps are not used */
>> >> >> + if (!sband->ht_cap_invalid &&
>> >> >> + channel_type != NL80211_CHAN_HT20 &&
>> >> >
>> >> > This is also major confusion, it seems you should roll 2/3 into one
>> >> > patch?
>> >>
>> >> I'm not sure that would help. rdev_freq_to_chan doesn't get the
>> >> net_device, so it can't get the HT caps. I guess I can add the
>> >> net_device as an optional argument where it is used and check the
>> >> caps. OTOH it seem like a sanity check we can do without?
>> >
>> > I don't think we should do without that check? If you request say AP
>> > operation on 40 MHz then you absolutely rely on that happening or
>> > returning an error, not randomly operating in legacy mode!
>>
>> Sure. I'll send the netdev to the function where possible (I think
>> everywhere except the virtual monitor).
>
> I think the function should restrict to NO_HT in case it doesn't have
> caps, otherwise it can't properly restrict. I suppose that means you
> won't be able to do HT monitoring with your device, but I can live with
> that.
Actually the ht_supported flag is still global, so HT monitoring is
possible, just not HT40 monitoring I think. We'll fix that if the need
arises (or just boot the driver with fixed caps via a module param).
--
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
