Search Linux Wireless

Re: [PATCH v10 3/5] cfg80211/mac80211: move interface counting for combination check to mac80211

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux