Hello Johannes, Tamizh, and all
This patch series contains several minor fixes and enhancements for TID
specific configuration functionality. The first three patches include
minor fixes and TID specific AMSDU configuration.
The 4th patch is somewhat controversial, so the series is marked as RFC.
This patch simplifies current override logic. It suggests to make no
difference between 'specific peer' and 'all peers' cases and to apply
new TID configuration immediately after resetting the previous one.
The 5th patch enables access to new kernel functionality from iw tool.
Note that nl80211.h changes are not included into iw. So to make this
change work in iw , header should be updated upfront, including new
AMSDU attribute from this patch series.
Regards,
Sergey
v1 -> v2
- add policy for new AMSDU attribute
- add patch for iw with support for per-tid per-node configuration
Sergey Matyukevich (5):
cfg80211: fix mask type in cfg80211_tid_cfg structure
mac80211: fix variable names in TID config methods
cfg80211: add support for TID specific AMSDU configuration
nl80211: simplify peer specific TID configuration
iw: add TID specific configuration command
include/net/cfg80211.h | 6 ++++--
include/uapi/linux/nl80211.h | 20 +++++++++++---------
net/mac80211/cfg.c | 6 +++---
net/mac80211/driver-ops.h | 4 ++--
net/wireless/nl80211.c | 13 +++++++++----
5 files changed, 29 insertions(+), 20 deletions(-)
--
2.11.0
Current rule for applying TID configuration for specific peer looks overly
complicated. No need to reject new TID configuration when override flag is
specified. Another call with the same TID configuration, but without
override flag, allows to apply new configuration anyway.
Use the same approach as for the 'all peers' case: if override flag is
specified, then reset existing TID configuration and immediately
apply a new one.
Signed-off-by: Sergey Matyukevich <[email protected]>
---
include/uapi/linux/nl80211.h | 10 ++++------
net/wireless/nl80211.c | 5 +----
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1ccb0bf657ec..d1b1d9e49887 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4823,12 +4823,10 @@ enum nl80211_tid_config {
* (%NL80211_TID_CONFIG_ATTR_TIDS, %NL80211_TID_CONFIG_ATTR_OVERRIDE).
* @NL80211_TID_CONFIG_ATTR_PEER_SUPP: same as the previous per-vif one, but
* per peer instead.
- * @NL80211_TID_CONFIG_ATTR_OVERRIDE: flag attribue, if no peer
- * is selected, if set indicates that the new configuration overrides
- * all previous peer configurations, otherwise previous peer specific
- * configurations should be left untouched. If peer is selected then
- * it will reset particular TID configuration of that peer and it will
- * not accept other TID config attributes along with peer.
+ * @NL80211_TID_CONFIG_ATTR_OVERRIDE: flag attribue, if set indicates
+ * that the new configuration overrides all previous peer
+ * configurations, otherwise previous peer specific configurations
+ * should be left untouched.
* @NL80211_TID_CONFIG_ATTR_TIDS: a bitmask value of TIDs (bit 0 to 7)
* Its type is u16.
* @NL80211_TID_CONFIG_ATTR_NOACK: Configure ack policy for the TID.
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index aef0dd59dd4f..31c61e8739d9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -14083,10 +14083,7 @@ static int parse_tid_conf(struct cfg80211_registered_device *rdev,
if (rdev->ops->reset_tid_config) {
err = rdev_reset_tid_config(rdev, dev, peer,
tid_conf->tids);
- /* If peer is there no other configuration will be
- * allowed
- */
- if (err || peer)
+ if (err)
return err;
} else {
return -EINVAL;
--
2.11.0
Use command NL80211_CMD_SET_TID_CONFIG to perform per-node TID specific
configuration. If peer is not specified, then configuration is applied
to all the peers. Currently kernel supports configuration of the
following parameters:
- short/long retry
- mpdu/msdu aggregation on/off
- rts/cts on/off
- noack on/off
Examples:
Apply configuration for specific peer and TIDs:
$ iw dev wlan0 set tidconf peer 1:2:3:4:5:6 tids 0x3 ampdu off tids 0x2 sretry 10 lretry 100
$ iw dev wlan0 set tidconf peer 1:2:3:4:5:6 tids 0x1 override ampdu off amsdu off
Apply configuration for all peers and all TIDs:
$ iw dev wlan0 set tidconf tids 0xff ampdu off amsdu off sretry 10 lretry 100 noack off
Apply configuration for all peers and specific TIDs:
$ iw dev wlan0 set tidconf peer 0xff:0xff:0xff:0xff:0xff:0xff tids 0x1 ampdu off amsdu off
Signed-off-by: Sergey Matyukevich <[email protected]>
---
interface.c | 229 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 229 insertions(+)
diff --git a/interface.c b/interface.c
index 6a44304..df96bed 100644
--- a/interface.c
+++ b/interface.c
@@ -727,3 +727,232 @@ COMMAND(switch, freq,
"Switch the operating channel by sending a channel switch announcement (CSA).");
COMMAND(switch, channel, "<channel> [NOHT|HT20|HT40+|HT40-|5MHz|10MHz|80MHz] [beacons <count>] [block-tx]",
NL80211_CMD_CHANNEL_SWITCH, 0, CIB_NETDEV, handle_chan, NULL);
+
+
+static int toggle_tid_param(const char *argv0, const char *argv1,
+ struct nl_msg *msg, uint32_t attr)
+{
+ uint8_t val;
+
+ if (strcmp(argv1, "on") == 0) {
+ val = NL80211_TID_CONFIG_ENABLE;
+ } else if (strcmp(argv1, "off") == 0) {
+ val = NL80211_TID_CONFIG_DISABLE;
+ } else {
+ fprintf(stderr, "Invalid %s parameter: %s\n", argv0, argv1);
+ return 2;
+ }
+
+ NLA_PUT_U8(msg, attr, val);
+ return 0;
+
+ nla_put_failure:
+ return -ENOBUFS;
+}
+
+static int handle_tid_config(struct nl80211_state *state,
+ struct nl_msg *msg,
+ int argc, char **argv,
+ enum id_input id)
+{
+ struct nlattr *tids_array = NULL;
+ struct nlattr *tids_entry = NULL;
+ unsigned char peer[ETH_ALEN];
+ int tids_num = 0;
+ char *end;
+ int ret;
+ enum {
+ PS_ADDR,
+ PS_TIDS,
+ PS_CONF,
+ } parse_state = PS_ADDR;
+
+ while (argc) {
+ switch (parse_state) {
+ case PS_ADDR:
+ if (strcmp(argv[0], "peer") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "Not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ if (mac_addr_a2n(peer, argv[1])) {
+ fprintf(stderr, "Invalid MAC address\n");
+ return 2;
+ }
+
+ NLA_PUT(msg, NL80211_ATTR_MAC, ETH_ALEN, peer);
+
+ argc -= 2;
+ argv += 2;
+ parse_state = PS_TIDS;
+
+ } else if (strcmp(argv[0], "tids") == 0) {
+ parse_state = PS_TIDS;
+ } else {
+ fprintf(stderr, "Peer MAC address expected\n");
+ return HANDLER_RET_USAGE;
+ }
+
+ break;
+ case PS_TIDS:
+ if (strcmp(argv[0], "tids") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ if (!tids_array) {
+ tids_array = nla_nest_start(msg, NL80211_ATTR_TID_CONFIG);
+ if (!tids_array)
+ return -ENOBUFS;
+ }
+
+ if (tids_entry) {
+ nla_nest_end(msg, tids_entry);
+ tids_num++;
+ }
+
+ tids_entry = nla_nest_start(msg, tids_num);
+ if (!tids_entry)
+ return -ENOBUFS;
+
+ NLA_PUT_U16(msg, NL80211_TID_CONFIG_ATTR_TIDS, strtol(argv[1], &end, 0));
+ if (*end) {
+ fprintf(stderr, "Invalid TID mask value: %s\n", argv[1]);
+ return 2;
+ }
+
+ argc -= 2;
+ argv += 2;
+ parse_state = PS_CONF;
+ } else {
+ fprintf(stderr, "TID mask expected\n");
+ return HANDLER_RET_USAGE;
+ }
+
+ break;
+ case PS_CONF:
+ if (strcmp(argv[0], "tids") == 0) {
+ parse_state = PS_TIDS;
+ } else if (strcmp(argv[0], "override") == 0) {
+ NLA_PUT_FLAG(msg, NL80211_TID_CONFIG_ATTR_OVERRIDE);
+
+ argc -= 1;
+ argv += 1;
+ } else if (strcmp(argv[0], "ampdu") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ ret = toggle_tid_param(argv[0], argv[1], msg,
+ NL80211_TID_CONFIG_ATTR_AMPDU_CTRL);
+ if (ret)
+ return ret;
+
+ argc -= 2;
+ argv += 2;
+ } else if (strcmp(argv[0], "amsdu") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ ret = toggle_tid_param(argv[0], argv[1], msg,
+ NL80211_TID_CONFIG_ATTR_AMSDU_CTRL);
+ if (ret)
+ return ret;
+
+ argc -= 2;
+ argv += 2;
+ } else if (strcmp(argv[0], "noack") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ ret = toggle_tid_param(argv[0], argv[1], msg,
+ NL80211_TID_CONFIG_ATTR_NOACK);
+ if (ret)
+ return ret;
+
+ argc -= 2;
+ argv += 2;
+ } else if (strcmp(argv[0], "rtscts") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ ret = toggle_tid_param(argv[0], argv[1], msg,
+ NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL);
+ if (ret)
+ return ret;
+
+ argc -= 2;
+ argv += 2;
+ } else if (strcmp(argv[0], "sretry") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ NLA_PUT_U8(msg, NL80211_TID_CONFIG_ATTR_RETRY_SHORT, strtol(argv[1], &end, 0));
+ if (*end) {
+ fprintf(stderr, "Invalid short_retry value: %s\n", argv[1]);
+ return 2;
+ }
+
+ argc -= 2;
+ argv += 2;
+ } else if (strcmp(argv[0], "lretry") == 0) {
+ if (argc < 2) {
+ fprintf(stderr, "not enough args for %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ NLA_PUT_U8(msg, NL80211_TID_CONFIG_ATTR_RETRY_LONG, strtol(argv[1], &end, 0));
+ if (*end) {
+ fprintf(stderr, "Invalid long_retry value: %s\n", argv[1]);
+ return 2;
+ }
+
+ argc -= 2;
+ argv += 2;
+ } else {
+ fprintf(stderr, "Unknown parameter: %s\n", argv[0]);
+ return HANDLER_RET_USAGE;
+ }
+
+ break;
+ default:
+ fprintf(stderr, "Failed to parse: internal failure\n");
+ return HANDLER_RET_USAGE;
+ }
+ }
+
+ if (tids_entry)
+ nla_nest_end(msg, tids_entry);
+
+ if (tids_array)
+ nla_nest_end(msg, tids_array);
+
+ return 0;
+
+nla_put_failure:
+ return -ENOBUFS;
+}
+
+COMMAND(set, tidconf, "[peer <MAC address>] tids <mask> [override] [sretry <num>] [lretry <num>] "
+ "[ampdu [on|off]] [amsdu [on|off]] [noack [on|off]] [rtscts [on|off]]",
+ NL80211_CMD_SET_TID_CONFIG, 0, CIB_NETDEV, handle_tid_config,
+ "Setup per-node TID specific configuration for TIDs selected by bitmask.\n"
+ "If MAC address is not specified, then supplied TID configuration\n"
+ "applied to all the peers.\n"
+ "Examples:\n"
+ " $ iw dev wlan0 tids 0x1 ampdu off\n"
+ " $ iw dev wlan0 tids 0x5 ampdu off amsdu off rtscts on\n"
+ " $ iw dev wlan0 tids 0x3 override ampdu on noack on rtscts on\n"
+ " $ iw dev wlan0 peer xx:xx:xx:xx:xx:xx tids 0x1 ampdu off tids 0x3 amsdu off rtscts on\n"
+ );
--
2.11.0
TIDs mask type is u64 in wiphy settings and nl80211 processing, see:
- wiphy TIDs mask sizes in tid_config_support structure
- prepare driver command in parse_tid_conf
Use the same type for TIDs mask in cfg80211_tid_cfg.
Signed-off-by: Sergey Matyukevich <[email protected]>
---
include/net/cfg80211.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a82fc59a1d82..a2b2d31530a9 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -646,7 +646,7 @@ struct cfg80211_chan_def {
struct cfg80211_tid_cfg {
bool config_override;
u8 tids;
- u32 mask;
+ u64 mask;
enum nl80211_tid_config noack;
u8 retry_long, retry_short;
enum nl80211_tid_config ampdu;
--
2.11.0
Fix all variable names from 'tid' to 'tids' to avoid confusion.
Now this is not TID number, but TID mask.
Signed-off-by: Sergey Matyukevich <[email protected]>
---
net/mac80211/cfg.c | 6 +++---
net/mac80211/driver-ops.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index ae3e06375a28..608c2b034804 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3933,7 +3933,7 @@ static int ieee80211_set_tid_config(struct wiphy *wiphy,
static int ieee80211_reset_tid_config(struct wiphy *wiphy,
struct net_device *dev,
- const u8 *peer, u8 tid)
+ const u8 *peer, u8 tids)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct sta_info *sta;
@@ -3943,7 +3943,7 @@ static int ieee80211_reset_tid_config(struct wiphy *wiphy,
return -EOPNOTSUPP;
if (!peer)
- return drv_reset_tid_config(sdata->local, sdata, NULL, tid);
+ return drv_reset_tid_config(sdata->local, sdata, NULL, tids);
mutex_lock(&sdata->local->sta_mtx);
sta = sta_info_get_bss(sdata, peer);
@@ -3952,7 +3952,7 @@ static int ieee80211_reset_tid_config(struct wiphy *wiphy,
return -ENOENT;
}
- ret = drv_reset_tid_config(sdata->local, sdata, &sta->sta, tid);
+ ret = drv_reset_tid_config(sdata->local, sdata, &sta->sta, tids);
mutex_unlock(&sdata->local->sta_mtx);
return ret;
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 3877710e3b48..de69fc9c4f07 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1375,12 +1375,12 @@ static inline int drv_set_tid_config(struct ieee80211_local *local,
static inline int drv_reset_tid_config(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
- struct ieee80211_sta *sta, u8 tid)
+ struct ieee80211_sta *sta, u8 tids)
{
int ret;
might_sleep();
- ret = local->ops->reset_tid_config(&local->hw, &sdata->vif, sta, tid);
+ ret = local->ops->reset_tid_config(&local->hw, &sdata->vif, sta, tids);
trace_drv_return_int(local, ret);
return ret;
--
2.11.0
This patch adds support to control per TID MSDU aggregation
using the NL80211_TID_CONFIG_ATTR_AMSDU_CTRL attribute.
Signed-off-by: Sergey Matyukevich <[email protected]>
---
include/net/cfg80211.h | 4 +++-
include/uapi/linux/nl80211.h | 10 +++++++---
net/wireless/nl80211.c | 8 ++++++++
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index a2b2d31530a9..83148b2d4907 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -640,8 +640,9 @@ struct cfg80211_chan_def {
* @noack: noack configuration value for the TID
* @retry_long: retry count value
* @retry_short: retry count value
- * @ampdu: Enable/Disable aggregation
+ * @ampdu: Enable/Disable MPDU aggregation
* @rtscts: Enable/Disable RTS/CTS
+ * @amsdu: Enable/Disable MSDU aggregation
*/
struct cfg80211_tid_cfg {
bool config_override;
@@ -651,6 +652,7 @@ struct cfg80211_tid_cfg {
u8 retry_long, retry_short;
enum nl80211_tid_config ampdu;
enum nl80211_tid_config rtscts;
+ enum nl80211_tid_config amsdu;
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9679d561f7d0..1ccb0bf657ec 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4844,12 +4844,15 @@ enum nl80211_tid_config {
* &NL80211_CMD_SET_TID_CONFIG. Its type is u8, min value is 1 and
* the max value is advertised by the driver in this attribute on
* output in wiphy capabilities.
- * @NL80211_TID_CONFIG_ATTR_AMPDU_CTRL: Enable/Disable aggregation for the TIDs
- * specified in %NL80211_TID_CONFIG_ATTR_TIDS. Its type is u8, using
- * the values from &nl80211_tid_config.
+ * @NL80211_TID_CONFIG_ATTR_AMPDU_CTRL: Enable/Disable MPDU aggregation
+ * for the TIDs specified in %NL80211_TID_CONFIG_ATTR_TIDS.
+ * Its type is u8, using the values from &nl80211_tid_config.
* @NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL: Enable/Disable RTS_CTS for the TIDs
* specified in %NL80211_TID_CONFIG_ATTR_TIDS. It is u8 type, using
* the values from &nl80211_tid_config.
+ * @NL80211_TID_CONFIG_ATTR_AMSDU_CTRL: Enable/Disable MSDU aggregation
+ * for the TIDs specified in %NL80211_TID_CONFIG_ATTR_TIDS.
+ * Its type is u8, using the values from &nl80211_tid_config.
*/
enum nl80211_tid_config_attr {
__NL80211_TID_CONFIG_ATTR_INVALID,
@@ -4863,6 +4866,7 @@ enum nl80211_tid_config_attr {
NL80211_TID_CONFIG_ATTR_RETRY_LONG,
NL80211_TID_CONFIG_ATTR_AMPDU_CTRL,
NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL,
+ NL80211_TID_CONFIG_ATTR_AMSDU_CTRL,
/* keep last */
__NL80211_TID_CONFIG_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 3d27b24c68b2..aef0dd59dd4f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -345,6 +345,8 @@ nl80211_tid_config_attr_policy[NL80211_TID_CONFIG_ATTR_MAX + 1] = {
NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL] =
NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
+ [NL80211_TID_CONFIG_ATTR_AMSDU_CTRL] =
+ NLA_POLICY_MAX(NLA_U8, NL80211_TID_CONFIG_DISABLE),
};
const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
@@ -14127,6 +14129,12 @@ static int parse_tid_conf(struct cfg80211_registered_device *rdev,
nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_RTSCTS_CTRL]);
}
+ if (attrs[NL80211_TID_CONFIG_ATTR_AMSDU_CTRL]) {
+ tid_conf->mask |= BIT(NL80211_TID_CONFIG_ATTR_AMSDU_CTRL);
+ tid_conf->amsdu =
+ nla_get_u8(attrs[NL80211_TID_CONFIG_ATTR_AMSDU_CTRL]);
+ }
+
if (peer)
mask = rdev->wiphy.tid_config_support.peer;
else
--
2.11.0
On 2020-04-24 16:59, Sergey Matyukevich wrote:
> Hello Johannes, Tamizh, and all
>
> This patch series contains several minor fixes and enhancements for TID
> specific configuration functionality. The first three patches include
> minor fixes and TID specific AMSDU configuration.
>
> The 4th patch is somewhat controversial, so the series is marked as
> RFC.
> This patch simplifies current override logic. It suggests to make no
> difference between 'specific peer' and 'all peers' cases and to apply
> new TID configuration immediately after resetting the previous one.
>
> The 5th patch enables access to new kernel functionality from iw tool.
> Note that nl80211.h changes are not included into iw. So to make this
> change work in iw , header should be updated upfront, including new
> AMSDU attribute from this patch series.
>
> Regards,
> Sergey
>
> v1 -> v2
>
> - add policy for new AMSDU attribute
> - add patch for iw with support for per-tid per-node configuration
>
>
> Sergey Matyukevich (5):
> cfg80211: fix mask type in cfg80211_tid_cfg structure
> mac80211: fix variable names in TID config methods
> cfg80211: add support for TID specific AMSDU configuration
> nl80211: simplify peer specific TID configuration
Patches looks good to me
Reviewed-by: Tamizh Chelvam <[email protected]>
> iw: add TID specific configuration command
Looks neat to me :)
Reviewed-by: Tamizh Chelvam <[email protected]>
>
> include/net/cfg80211.h | 6 ++++--
> include/uapi/linux/nl80211.h | 20 +++++++++++---------
> net/mac80211/cfg.c | 6 +++---
> net/mac80211/driver-ops.h | 4 ++--
> net/wireless/nl80211.c | 13 +++++++++----
> 5 files changed, 29 insertions(+), 20 deletions(-)