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