On Tue, 2019-06-18 at 08:19 +0200, John Crispin wrote:
> Store the OBSS PD parameters inside bss_conf when bringing up an AP and/or
> when a station connects to an AP. This allows the driver to configure the
> HW accordingly.
>
> Signed-off-by: Shashidhar Lakkavalli <slakkavalli@xxxxxxxxx>
> Signed-off-by: John Crispin <john@xxxxxxxxxxx>
> ---
> include/net/cfg80211.h | 15 +++++++++++++
> include/net/mac80211.h | 4 ++++
> include/uapi/linux/nl80211.h | 27 ++++++++++++++++++++++
> net/mac80211/cfg.c | 5 ++++-
> net/mac80211/he.c | 24 ++++++++++++++++++++
> net/mac80211/ieee80211_i.h | 3 +++
> net/mac80211/mlme.c | 1 +
> net/wireless/nl80211.c | 43 ++++++++++++++++++++++++++++++++++++
> 8 files changed, 121 insertions(+), 1 deletion(-)
Not sure if I missed this before, but in any case please split between
cfg80211 and mac80211 for all but the most trivial patches.
> +/**
> + * enum nl80211_he_spr - spatial reuse attributes
bad copy/paste? :)
> + * @__NL80211_HE_OBSS_PD_ATTR_INVALID: Invalid
> + *
> + * @NL80211_ATTR_HE_OBSS_PD_MIN_OFFSET: the OBSS PD minimum tx power offset.
> + * @NL80211_ATTR_HE_OBSS_PD_MAX_OFFSET: the OBSS PD maximum tx power offset.
> + *
> + * @__NL80211_HE_OBSS_PD_ATTR_LAST: Internal
> + * @NL80211_HE_OBSS_PD_ATTR_MAX: highest spiatl reuse attribute.
typo & wrong anyway, OBSS PD not SPR
> + */
Those prefixes are a bit confusing - IMHO they should all be
NL80211_HE_OBSS_PD_ATTR_*, NL80211_ATTR_* is mostly (except for a few
historical bugs) the top-level attributes.
> +enum nl80211_he_spr_attributes {
here also
> + memcpy(&sdata->vif.bss_conf.he_obss_pd, ¶ms->he_obss_pd,
> + sizeof(struct ieee80211_he_obss_pd));
just use struct assignment
blabla.he_obss_pd = params->he_obss_pd;
> + [NL80211_ATTR_HE_OBSS_PD] = { .type = NLA_NESTED },
please use NLA_POLICY_NESTED() (requires putting the below policy above
this point)
> +static const struct nla_policy
> + he_spr_policy[NL80211_HE_OBSS_PD_ATTR_MAX + 1] = {
I guess I'd lose all the tabs here but don't really care that much.
> + [NL80211_ATTR_HE_OBSS_PD_MIN_OFFSET] = { .type = NLA_U32 },
> + [NL80211_ATTR_HE_OBSS_PD_MAX_OFFSET] = { .type = NLA_U32 },
That can't be right, they go into u8 eventually, no? Use NLA_U8 or maybe
even NLA_POLICY_RANGE().
Also in the struct ieee80211_he_obss_pd you have u32 for no real reason?
In the element in the frame you only used a single u8.
> + if (!tb[NL80211_ATTR_HE_OBSS_PD_MIN_OFFSET] ||
> + !tb[NL80211_ATTR_HE_OBSS_PD_MAX_OFFSET])
> + return -EINVAL;
> + if (he_obss_pd->min_offset >= he_obss_pd->max_offset)
> + return -EINVAL;
Maybe add some extack error messages for this.
johannes
