On Wed, 2014-01-22 at 13:43 +0100, Janusz Dziedzic wrote:
> Introduce regulatory flags field, NL80211_REG_WIDE_BW
> country flag and new attribute NL80211_ATTR_REG_FLAGS.
> If NL80211_REG_WIDE_BW is set, check rules and calculate
> maximum bandwidth base on all contiguous regulatory rules.
> If unset get maximum badwitdh from regulatory rule.
> This patch is backward compatible with current CRDA/db.txt
> implementation.
This seems reasonable, thanks. Maybe we should require the bandwidth to
not be set at all or something? At least maybe in the regdb parser - it
makes very little sense to have @20 and then ignore it completely?
Or maybe the userspace code could just not expose the flag, but rather
set the new "wide_bw" flag when all the rules are marked as @N/A (and
treat a combination of @number and @N/A as a bug)?
> + * @NL80211_ATTR_REG_FLAGS: Regulatory flags, see &enum nl80211_reg_flags
should probably say it's a u32 attribute
> /**
> + * enum nl80211_reg_flags - regulatory flags
> + * @NL80211_REG_FLAG_WIDE_BW: Calculate maximum available bandwidth
> + * base on contiguous regulatory rules instead of using
> + * maximum bandwidth from regulatory database.
And that should probably not refer to the database but rather the
nl80211 attribute that contains the number.
> @@ -5203,6 +5208,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
> if (reg_supported_dfs_region(dfs_region))
> rd->dfs_region = dfs_region;
>
> + rd->flags = flags;
Why bother with the temporary flags variable?
> nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
> rem_reg_rules) {
> nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,
As per above, maybe check that no bandwidth is actually in the rules?
> +static unsigned int reg_get_max_bandwidth(const struct ieee80211_regdomain *rd,
> + const struct ieee80211_reg_rule *rule)
> +{
> + const struct ieee80211_freq_range *freq_range = &rule->freq_range;
> + const struct ieee80211_freq_range *freq_range_tmp;
> + const struct ieee80211_reg_rule *tmp;
> + u32 start_freq, end_freq, idx, no;
> +
> + /* First check if strict regulatory database bandwidth calculation */
> + if (!(rd->flags & NL80211_REG_FLAG_WIDE_BW))
> + return freq_range->end_freq_khz - freq_range->start_freq_khz;
shouldn't that check the range's @bandwidth (as well)?
or this is for sanity check only I guess.
> + freq_diff = reg_get_max_bandwidth(rd, rule);
>
> - if (freq_range->end_freq_khz <= freq_range->start_freq_khz ||
> - freq_range->max_bandwidth_khz > freq_diff)
> + if (freq_range->max_bandwidth_khz > freq_diff)
> return false;
Err, wait, this won't work as advertised, would it?
> @@ -645,11 +702,17 @@ reg_intersect_dfs_region(const enum nl80211_dfs_regions dfs_region1,
> return dfs_region1;
> }
>
> +static u32 reg_intersect_flags(const u32 flags1, const u32 flags2)
> +{
> + return flags1 | flags2;
> +}
Wow, ok, this is getting tricky. But I think it should be the other way
around? I mean, it seems WIDE_BW flag should be unset if it's unset in
either of them?
But we might also have to set max_bandwidth_khz to a proper calculated
wide_bw value before intersecting...
johannes
--
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
