On Fri, Nov 14, 2014 at 1:11 AM, Luis R. Rodriguez <mcgrof@xxxxxxxx> wrote:
> On Thu, Nov 13, 2014 at 06:13:38PM +0200, Arik Nemtsov wrote:
>> From: Jonathan Doron <jond@xxxxxxxxxx>
>>
>> Add a new regulatory flag that allows a driver to manage regdomain
>> changes/updates for its own wiphy.
>> In this case the regdomain is local to the driver, and it does not use
>> the shared cfg80211 regdomain. It also implies that the driver does not
>> wish to get regulatory updates generated by other wiphys or by usermode.
>
> I'd like you to be clearer on this both on the commit log and also
> on the documentation:
>
> --
> Using this mean you are using your very own regulatory rule
> interpretations, vetting of your own regulatory rules then is
> completely on the driver / firmware using this. Any heuristics
> about regulatory that cfg80211 can help with is completely
> ignored.
> --
>
> There are gains by using cfg80211 and the work we have put into
> cfg80211's regulatory infrastructure, we want folks to evaluate
> not using it before discarding it, and ensure folks understand
> the risks and issues that might come up if they don't gain any
> help from cfg80211.
>
>> A new API lets the driver send a complete regdomain, to be applied on
>> its wiphy only.
>>
>> After a wiphy-specific regdomain change takes place, usermode will get
>> a new type of change notification. The regulatory core also takes care
>> enforce regulatory restrictions, in case some interfaces are on
>> forbidden channels.
>
> I want you to also address what happens when two different devices
> are used on a system that don't use the same mechanism. For instance,
> one device may use this feature, another set may use the cfg80211
> regulatory code. The implications are that the devices using this
> new feature are not learning or gaining any information from the
> core, its own its own. This is another reason that not using the
> cfg80211 regulatory information is a poor architectural choice,
> it doesn't scale well. I last considered simply disallowing such
> combinations because of this very reason -- but since you are
> separating this now I am OK with it -- I just want to ensure it is
> crystal clear of the downfall to this long term architecturally.
>
> In short: its fucking stupid.
>
> I want folks to think very fucking hard before being stupid.
>
> Please help with that.
I'll try to beef up the commit message.
>> Signed-off-by: Jonathan Doron <jonathanx.doron@xxxxxxxxx>
>> Signed-off-by: Arik Nemtsov <arikx.nemtsov@xxxxxxxxx>
>> ---
>> include/net/cfg80211.h | 14 ++++++++
>> include/net/regulatory.h | 9 +++++
>> include/uapi/linux/nl80211.h | 5 +++
>> net/wireless/core.c | 7 ++++
>> net/wireless/nl80211.c | 80 ++++++++++++++++++++++++++++++++++----------
>> net/wireless/nl80211.h | 1 +
>> net/wireless/reg.c | 59 ++++++++++++++++++++++++++++++++
>> 7 files changed, 157 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 220d5f5..dd4f811 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -3728,6 +3728,20 @@ const u8 *cfg80211_find_vendor_ie(unsigned int oui, u8 oui_type,
>> int regulatory_hint(struct wiphy *wiphy, const char *alpha2);
>>
>> /**
>> + * regulatory_set_wiphy_regd_rtnl - set regdom info for self managed drivers
>> + * @wiphy: the wireless device we want to process the regulatory domain on
>> + * @rd: the regulatory domain informatoin to use for this wiphy
>> + *
>> + * Set the regulatory domain information for self managed drivers, only they
>> + * can use this function.
>> + * This function requires the caller to hold the rtnl_lock.
>
> This documentation is lacking a lot of information. Please fell it with
> information as to reasoning for why folks may go down this path as well
> as well as caveats. The combination of devices is of particular concern
> and it should be made clear.
I'll clarify it.
>
>> + *
>> + * Return: 0 on success. -EINVAL, -EPERM
>> + */
>> +int regulatory_set_wiphy_regd_rtnl(struct wiphy *wiphy,
>> + struct ieee80211_regdomain *rd);
>> +
>> +/**
>> * wiphy_apply_custom_regulatory - apply a custom driver regulatory domain
>> * @wiphy: the wireless device we want to process the regulatory domain on
>> * @regd: the custom regulatory domain to use for this wiphy
>> diff --git a/include/net/regulatory.h b/include/net/regulatory.h
>> index 701177c..42345f4 100644
>> --- a/include/net/regulatory.h
>> +++ b/include/net/regulatory.h
>> @@ -141,6 +141,14 @@ struct regulatory_request {
>> * change, the interfaces are given a grace period to disconnect or move
>> * to an allowed channels. Interfaces on forbidden channels are forcibly
>> * disconnected.
>> + * @REGULATORY_WIPHY_SELF_MANAGED: for devices that employ wiphy-specific
>> + * regdom management. These devices will ignore all regdom changes not
>> + * originating from their own wiphy. This flag is incompatible with the
>> + * flags: %REGULATORY_CUSTOM_REG, %REGULATORY_STRICT_REG,
>> + * %REGULATORY_COUNTRY_IE_FOLLOW_POWER, %REGULATORY_COUNTRY_IE_IGNORE and
>> + * %REGULATORY_DISABLE_BEACON_HINTS. Mixing any of the above flags with
>> + * this flag will result in a failure to register the wiphy. This flag
>> + * implies %REGULATORY_DISABLE_BEACON_HINTS.
>
> Please discourage such use.
Will do.
>
>> */
>> enum ieee80211_regulatory_flags {
>> REGULATORY_CUSTOM_REG = BIT(0),
>> @@ -150,6 +158,7 @@ enum ieee80211_regulatory_flags {
>> REGULATORY_COUNTRY_IE_IGNORE = BIT(4),
>> REGULATORY_ENABLE_RELAX_NO_IR = BIT(5),
>> REGULATORY_ENFORCE_CHANNELS = BIT(6),
>> + REGULATORY_WIPHY_SELF_MANAGED = BIT(7),
>> };
>>
>> struct ieee80211_freq_range {
>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>> index 442369f..f050dcb 100644
>> --- a/include/uapi/linux/nl80211.h
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -762,6 +762,9 @@
>> * @NL80211_CMD_LEAVE_OCB: Leave the OCB network -- no special arguments, the
>> * network is determined by the network interface.
>> *
>> + * @NL80211_CMD_WIPHY_REG_CHANGE: Similar to %NL80211_CMD_REG_CHANGE, but used
>> + * for indicating changes for devices with wiphy-specific regdom management
>> + *
>> * @NL80211_CMD_MAX: highest used command number
>> * @__NL80211_CMD_AFTER_LAST: internal use
>> */
>> @@ -943,6 +946,8 @@ enum nl80211_commands {
>>
>> NL80211_CMD_CH_SWITCH_STARTED_NOTIFY,
>>
>> + NL80211_CMD_WIPHY_REG_CHANGE,
>> +
>> /* add new commands above here */
>>
>> /* used to define NL80211_CMD_MAX below */
>> diff --git a/net/wireless/core.c b/net/wireless/core.c
>> index a4d2792..656a1b1 100644
>> --- a/net/wireless/core.c
>> +++ b/net/wireless/core.c
>> @@ -541,6 +541,13 @@ int wiphy_register(struct wiphy *wiphy)
>> !wiphy->wowlan->tcp))
>> return -EINVAL;
>> #endif
>> + if (WARN_ON((wiphy->regulatory_flags & REGULATORY_WIPHY_SELF_MANAGED) &&
>> + (wiphy->regulatory_flags &
>> + (REGULATORY_CUSTOM_REG | REGULATORY_STRICT_REG |
>> + REGULATORY_COUNTRY_IE_FOLLOW_POWER |
>> + REGULATORY_COUNTRY_IE_IGNORE |
>> + REGULATORY_DISABLE_BEACON_HINTS))))
>> + return -EINVAL;
>
> Look at all those heuristics go away... That's alot. The documetnation should
> reflect all this not being used because of this decision. I also want you to
> think of the issues that may come up when combining devices that, one that
> uses this feature and one that does not.
Since this is a private regdomain, I guess this just means the
cfg80211 using device will be alone in the system for all regulatory
purposes.
I don't really see possible interoperability issues here. Am I missing
something?
Arik
--
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
