> + * Station specific retry configuration is valid only for STA's
> + * current connection. i.e. the configuration will be reset to default when
> + * the station connects back after disconnection/roaming.
> + * when user-space does not include %NL80211_ATTR_MAC, this configuration
> + * should be treated as per-netdev configuration. This configuration will
> + * be cleared when the interface goes down and on the disconnection from a
> + * BSS. When retry count has never been configured using this command, the
> + * other available radio level retry configuration
> + * (%NL80211_ATTR_WIPHY_RETRY_SHORT and %NL80211_ATTR_WIPHY_RETRY_LONG)
> + * should be used. Driver supporting this feature should advertise
> + * NL80211_EXT_FEATURE_PER_TID_RETRY_CONFIG and supporting per station
> + * retry count configuration should advertise
> + * NL80211_EXT_FEATURE_PER_STA_RETRY_CONFIG.
Here you pretty much copy-pasted all this text ... that's why I think it
should be in some other section. We want *everything* to be like that,
not have to check every single thing for different validity rules.
> + * @NL80211_TID_CONFIG_ATTR_RETRY_SHORT: Number of retries used with data frame
> + * transmission, user-space sets this configuration in
> + * &NL80211_CMD_SET_TID_CONFIG. It is u8 type, min value is 1 and
> + * the max value should be advertised by the driver through
> + * max_data_retry_count. when this attribute is not present, the driver
> + * would use the default configuration.
> + * @NL80211_TID_CONFIG_ATTR_RETRY_LONG: Number of retries used with data frame
> + * transmission, user-space sets this configuration in
> + * &NL80211_CMD_SET_TID_CONFIG. Its type is u8, min value is 1 and
> + * the max value should be advertised by the driver through
> + * max_data_retry_count. when this attribute is not present, the driver
> + * would use the default configuration.
I'm almost thinking that these should be a struct with two u8 values
instead of two separate attributes, and then renamed to
NL80211_TID_CONFIG_ATTR_RETRY, to carry both and really ensure thaty
they're always together as a single configuration.
This only really works right if we go for the reset= approach I outlined
in the previous patch though, since you otherwise need
NL80211_TID_CONFIG_ATTR_RETRY for the reset ... but that's a pretty
weird thing.
(there are also some typos here like "notfiy")
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -326,6 +326,9 @@ static int validate_ie_attr(const struct nlattr *attr,
> [NL80211_TID_CONFIG_ATTR_TID] = { .type = NLA_U8 },
> [NL80211_TID_CONFIG_ATTR_NOACK] =
> NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
> + [NL80211_TID_CONFIG_ATTR_RETRY] = { .type = NLA_FLAG },
> + [NL80211_TID_CONFIG_ATTR_RETRY_SHORT] = { .type = NLA_U8},
> + [NL80211_TID_CONFIG_ATTR_RETRY_LONG] = { .type = NLA_U8},
The min value of 1 should be reflected in the policy.
> + if (rdev->wiphy.max_data_retry_count) {
> + if (nla_put_u8(msg, NL80211_ATTR_MAX_RETRY_COUNT,
> + rdev->wiphy.max_data_retry_count))
bad indentation
> + goto nla_put_failure;
> + }
> +
> state->split_start++;
> if (state->split)
> break;
Also not sure which section you put this in, but it looks almost like
it's under "case 1:" where it really shouldn't be ... move it to the end
please.
johannes
