On Tue, 2014-03-11 at 12:45 +0200, Eliad Peller wrote:
> On Mon, Mar 10, 2014 at 11:31 PM, Luciano Coelho
> <luciano.coelho@xxxxxxxxx> wrote:
> > Some interface types don't require DFS (such as STATION, P2P_CLIENT
> > etc). In order to centralize these decisions, make
> > cfg80211_chandef_dfs_required() take the iftype into consideration.
> >
> > Signed-off-by: Luciano Coelho <luciano.coelho@xxxxxxxxx>
> > ---
> [...]
>
> > int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > - const struct cfg80211_chan_def *chandef)
> > + const struct cfg80211_chan_def *chandef,
> > + enum nl80211_iftype iftype)
> > {
> > int width;
> > - int r;
> > + int ret;
> >
> > if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> > return -EINVAL;
> >
> > - width = cfg80211_chandef_get_width(chandef);
> > - if (width < 0)
> > - return -EINVAL;
> > + switch (iftype) {
> > + case NL80211_IFTYPE_ADHOC:
> > + case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_P2P_GO:
> > + case NL80211_IFTYPE_MESH_POINT:
> > + width = cfg80211_chandef_get_width(chandef);
> > + if (width < 0)
> > + return -EINVAL;
> >
> it's a matter of style, but i think bailing out in non-relevant cases
> and taking the code out of the switch looks better (less indentation,
> etc.).
Yeah, it's a style issue and I tend to agree with you. But I also think
it's good to have a switch where we can easily see what happens with
each interface type.
I won't change this now, but we can refactor this at a later point if it
starts getting annoying. ;)
> [...]
>
> > + case NL80211_IFTYPE_STATION:
> > + case NL80211_IFTYPE_P2P_CLIENT:
> > + case NL80211_IFTYPE_MONITOR:
> > + case NL80211_IFTYPE_AP_VLAN:
> > + case NL80211_IFTYPE_WDS:
> > + case NL80211_IFTYPE_P2P_DEVICE:
> > + break;
> > + case NL80211_IFTYPE_UNSPECIFIED:
> > + case NUM_NL80211_IFTYPES:
> > + WARN_ON(1);
> > + }
>
> [...]
>
> > @@ -671,7 +700,8 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
> > - if (cfg80211_chandef_dfs_required(wiphy, chandef) > 0 &&
> > + if (cfg80211_chandef_dfs_required(wiphy, chandef,
> > + NL80211_IFTYPE_UNSPECIFIED) > 0 &&
> wouldn't this WARN_ON()?
Yep, you're right. Previously I was calling this and I didn't really
care what was the interface type. But Johannes asked me to change the
cfg80211_dfs_required() code to warn if unspecified was passed and I
forgot to change it here. I'll either have to add the iftype to the
cfg80211_reg_can_beacon() function arguments (which is a bit ugly,
because it's not really needed) or I'll have to move the UNSPECIFIED to
some sort of "don't care" in the cfg80211_chandef_dfs_required()
function.
> > @@ -5796,7 +5799,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
> > if (wdev->cac_started)
> > return -EBUSY;
> >
> > - err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > + err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef,
> > + NL80211_IFTYPE_UNSPECIFIED);
> ditto.
Ditto. :)
--
Cheers,
Luca.
--
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
