On Fri, 2014-03-14 at 09:40 +0100, Michal Kazior wrote:
> On 11 March 2014 15:16, Luciano Coelho <luciano.coelho@xxxxxxxxx> wrote:
>
> [...]
>
> > +int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
> > + const struct cfg80211_chan_def *chandef,
> > + enum ieee80211_chanctx_mode chanmode,
> > + u8 radar_detect)
> > +{
> > + struct ieee80211_local *local = sdata->local;
> > + struct ieee80211_sub_if_data *sdata_iter;
> > + enum nl80211_iftype iftype = sdata->wdev.iftype;
> > + int num[NUM_NL80211_IFTYPES];
> > + struct ieee80211_chanctx *ctx;
> > + int num_different_channels = 1;
> > + int total = 1;
> > +
> > + lockdep_assert_held(&local->chanctx_mtx);
> > +
> > + if (WARN_ON(hweight32(radar_detect) > 1))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(chanmode == IEEE80211_CHANCTX_SHARED && !chandef->chan))
> > + return -EINVAL;
> > +
> > + if (WARN_ON(iftype >= NUM_NL80211_IFTYPES))
> > + return -EINVAL;
> > +
> > + /* Always allow software iftypes */
> > + if (local->hw.wiphy->software_iftypes & BIT(iftype)) {
> > + if (radar_detect)
> > + return -EINVAL;
> > + return 0;
> > + }
> > +
> > + memset(num, 0, sizeof(num));
> > +
> > + if (iftype != NL80211_IFTYPE_UNSPECIFIED)
> > + num[iftype] = 1;
> > +
>
> > + list_for_each_entry(ctx, &local->chanctx_list, list) {
> > + if (ctx->conf.radar_enabled)
> > + radar_detect |= BIT(ctx->conf.def.width);
>
> Is this really correct?
I'm not sure anymore. :)
> The original behaviour was to:
>
> for each wdev:
> get_chan_state(..., &radar_width)
>
> where get_chan_state() ORs BIT(width) accordingly to iftype, etc.
>
> For 2 APs with different operational widths (e.g. 20 and 40) this means:
>
> a) old code: width_detect = BIT(WIDTH_20) | BIT(WIDTH_40)
> b) new code: width_detect = BIT(chanctx->width) which is BIT(WIDTH_40)
>
> I think radar_detect should be computed:
>
> radar_detect = 0 // **
> for each vif:
> if vif.ctx == ctx && vif.radar_required:
> radar_detect = BIT(vif.bss.width)
>
> ** you could argue to initialize with BIT(ctx->width), but all things
> seem to use ieee80211_vif_use_channel() (including CAC/radar
> detection) so bss_conf.chandef.width should be set correctly and
> iteration alone should suffice.
But does this really make sense? This was more or less the idea I had
when I changed the cfg80211_chandef_dfs_required() to return a bit mask
(which I later reverted).
I think the current behavior may be wrong. In the old
cfg80211_can_use_iftype_chan() we match *any* of the bits in
radar_detect with the bits supported in the interface combinations. So,
in your example, we would accept the combination even if the driver did
not support WIDTH_40 (because it would match WIDTH_20). If the context
is set to WIDTH_40, don't we have to make sure the driver can detect
radars on that width?
--
Cheers,
Luca.
��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
