Re: [RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[Johannes Berg]

On Wed, 2018-03-28 at 14:20 +0530, vthia...@codeaurora.org wrote:
> 
> > Btw, we should probably also have a way to *delete* the per-station
> > configuration, so it uses the default again?
> 
> Sure. How about setting it to default when the command is received with 
> no NL80211_ATTR_NOACK_MAP attribute for a station?

Sounds good to me.

johannes

Re: [RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[vthiagar]

On 2018-03-28 13:36, Johannes Berg wrote:

On Wed, 2018-03-28 at 10:24 +0530, vthia...@codeaurora.org wrote:


> The question is how that interacts with having enough space - are you
> sure this is a concern?

This will not be an issue at lest for ath10k. This is mainly for a
(new)driver
which implements the offload but has limitation in supporting more 
than

certain
number of peers. Perhaps we can remove it now and add it when such
driver is
available?


Ok, that's good. Yes, I think that sounds better - I have a hard time
imagining a firmware/driver that has space for the station, but doesn't
automatically allocate a u16 bitmap as part of the station :-)


> >   * @NL80211_CMD_SET_NOACK_MAP: sets a bitmap for the individual TIDs
> > whether
> > - *  No Acknowledgement Policy should be applied.
> > + *   No Acknowledgement Policy should be applied. %NL80211_ATTR_MAC is
> > used
> > + *   to apply No Acknowledgement policy for a particular connected
> > station.
> > + *   Station specific NoAck policy configuration is valid only for
> > STA's
> > + *   current connection, i.e. the configuration will not be used when
> > the
> > + *   station connects back after disconnection/roaming.
> > + *   When user-space does not include %NL80211_ATTR_MAC, the No
> > + *   Acknowledgement Policy setting should be treated as per-netdev
> > + *   configuration.
>
> Here you describe different semantics - i.e. you didn't describe the
> "previous per-station settings are kept" part. I'm not sure that part
> makes much sense anyhow?

Not sure I got this comment right. As mentioned in the doc, the 
previous

settings
would be reset upon reconnection of the station and any ndev wide
configuration
will be used. As mentioned above, additionally default value will be 
set

to the
station to mark no per-station configuration is given so far.


I just thought that there was a difference in how this applies to a
certain station.


May be the doc needs more update



Btw, we should probably also have a way to *delete* the per-station
configuration, so it uses the default again?


Sure. How about setting it to default when the command is received with 
no

NL80211_ATTR_NOACK_MAP attribute for a station?

Vasanth

Re: [RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[Johannes Berg]

On Wed, 2018-03-28 at 10:24 +0530, vthia...@codeaurora.org wrote:

> > The question is how that interacts with having enough space - are you
> > sure this is a concern?
> 
> This will not be an issue at lest for ath10k. This is mainly for a 
> (new)driver
> which implements the offload but has limitation in supporting more than 
> certain
> number of peers. Perhaps we can remove it now and add it when such 
> driver is
> available?

Ok, that's good. Yes, I think that sounds better - I have a hard time
imagining a firmware/driver that has space for the station, but doesn't
automatically allocate a u16 bitmap as part of the station :-)

> > >   * @NL80211_CMD_SET_NOACK_MAP: sets a bitmap for the individual TIDs 
> > > whether
> > > - *  No Acknowledgement Policy should be applied.
> > > + *   No Acknowledgement Policy should be applied. %NL80211_ATTR_MAC 
> > > is 
> > > used
> > > + *   to apply No Acknowledgement policy for a particular connected 
> > > station.
> > > + *   Station specific NoAck policy configuration is valid only for 
> > > STA's
> > > + *   current connection, i.e. the configuration will not be used 
> > > when 
> > > the
> > > + *   station connects back after disconnection/roaming.
> > > + *   When user-space does not include %NL80211_ATTR_MAC, the No
> > > + *   Acknowledgement Policy setting should be treated as per-netdev
> > > + *   configuration.
> > 
> > Here you describe different semantics - i.e. you didn't describe the
> > "previous per-station settings are kept" part. I'm not sure that part
> > makes much sense anyhow?
> 
> Not sure I got this comment right. As mentioned in the doc, the previous 
> settings
> would be reset upon reconnection of the station and any ndev wide 
> configuration
> will be used. As mentioned above, additionally default value will be set 
> to the
> station to mark no per-station configuration is given so far.

I just thought that there was a difference in how this applies to a
certain station.

Btw, we should probably also have a way to *delete* the per-station
configuration, so it uses the default again?

johannes

Re: [RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[vthiagar]

On 2018-03-27 18:17, Johannes Berg wrote:

On Tue, 2018-03-27 at 14:12 +0530, Vasanthakumar Thiagarajan wrote:


- * @set_noack_map: Set the NoAck Map for the TIDs.
+ * @set_noack_map: Set the NoAck Map for the TIDs. When peer is not 
%NULL NoAck
+ *	map will be applied for that particular peer. When peer is %NULL 
NoAck
+ *	map will be applied for all the connected stations (except the 
ones

+ * which already have per-peer TID map configured) on the netdev.
+ * Driver should return -ENOSPC when the it does not have room for
+ * additional entries for per-peer NoAck map.


I guess it should also set the default for new stations when the peer 
is

not given? At least that's how mac80211 would behave now, afaict.


Sure. May be setting -1 as the default value when the per-peer NoAck 
policy is not

set would work.



The question is how that interacts with having enough space - are you
sure this is a concern?


This will not be an issue at lest for ath10k. This is mainly for a 
(new)driver
which implements the offload but has limitation in supporting more than 
certain
number of peers. Perhaps we can remove it now and add it when such 
driver is

available?



  * @NL80211_CMD_SET_NOACK_MAP: sets a bitmap for the individual TIDs 
whether

- *  No Acknowledgement Policy should be applied.
+ *	No Acknowledgement Policy should be applied. %NL80211_ATTR_MAC is 
used
+ *	to apply No Acknowledgement policy for a particular connected 
station.
+ *	Station specific NoAck policy configuration is valid only for 
STA's
+ *	current connection, i.e. the configuration will not be used when 
the

+ * station connects back after disconnection/roaming.
+ * When user-space does not include %NL80211_ATTR_MAC, the No
+ * Acknowledgement Policy setting should be treated as per-netdev
+ * configuration.


Here you describe different semantics - i.e. you didn't describe the
"previous per-station settings are kept" part. I'm not sure that part
makes much sense anyhow?


Not sure I got this comment right. As mentioned in the doc, the previous 
settings
would be reset upon reconnection of the station and any ndev wide 
configuration
will be used. As mentioned above, additionally default value will be set 
to the

station to mark no per-station configuration is given so far.

Vasanth

Re: [RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[Johannes Berg]

On Tue, 2018-03-27 at 14:12 +0530, Vasanthakumar Thiagarajan wrote:
> 
> - * @set_noack_map: Set the NoAck Map for the TIDs.
> + * @set_noack_map: Set the NoAck Map for the TIDs. When peer is not %NULL 
> NoAck
> + *   map will be applied for that particular peer. When peer is %NULL NoAck
> + *   map will be applied for all the connected stations (except the ones
> + *   which already have per-peer TID map configured) on the netdev.
> + *   Driver should return -ENOSPC when the it does not have room for
> + *   additional entries for per-peer NoAck map.

I guess it should also set the default for new stations when the peer is
not given? At least that's how mac80211 would behave now, afaict.

The question is how that interacts with having enough space - are you
sure this is a concern?

>   * @NL80211_CMD_SET_NOACK_MAP: sets a bitmap for the individual TIDs whether
> - *  No Acknowledgement Policy should be applied.
> + *   No Acknowledgement Policy should be applied. %NL80211_ATTR_MAC is used
> + *   to apply No Acknowledgement policy for a particular connected station.
> + *   Station specific NoAck policy configuration is valid only for STA's
> + *   current connection, i.e. the configuration will not be used when the
> + *   station connects back after disconnection/roaming.
> + *   When user-space does not include %NL80211_ATTR_MAC, the No
> + *   Acknowledgement Policy setting should be treated as per-netdev
> + *   configuration.

Here you describe different semantics - i.e. you didn't describe the
"previous per-station settings are kept" part. I'm not sure that part
makes much sense anyhow?

johannes

[RFC 2/4] nl80211/mac80211: Extend NoAck policy command with peer MAC address

[Vasanthakumar Thiagarajan]

Provides peer level NoAck policy configuration by extending
NL80211_CMD_SET_NOACK_MAP command with peer MAC address.
If user space does not give any peer mac address, the driver
should retain the existing functionality of applying the NoAck
policy for all the staions connected to the netdev. Peer specific
configuration takes precedence over netdev level configuration when
both are set by the user. Drivers supporting per-sta NoAck policy
must advertise the support through the extended flag index
NL80211_EXT_FEATURE_PER_STA_NOACK_MAP.

Signed-off-by: Vasanthakumar Thiagarajan 
---
 include/net/cfg80211.h   |  9 +++--
 include/uapi/linux/nl80211.h | 12 +++-
 net/mac80211/cfg.c   |  1 +
 net/wireless/nl80211.c   | 12 +++-
 net/wireless/rdev-ops.h  |  7 ---
 net/wireless/trace.h | 11 +++
 6 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fc40843..b974d32 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2872,7 +2872,12 @@ struct cfg80211_external_auth_params {
  * @probe_client: probe an associated client, must return a cookie that it
  * later passes to cfg80211_probe_status().
  *
- * @set_noack_map: Set the NoAck Map for the TIDs.
+ * @set_noack_map: Set the NoAck Map for the TIDs. When peer is not %NULL NoAck
+ * map will be applied for that particular peer. When peer is %NULL NoAck
+ * map will be applied for all the connected stations (except the ones
+ * which already have per-peer TID map configured) on the netdev.
+ * Driver should return -ENOSPC when the it does not have room for
+ * additional entries for per-peer NoAck map.
  *
  * @get_channel: Get the current operating channel for the virtual interface.
  * For monitor interfaces, it should return %NULL unless there's a single
@@ -3180,7 +3185,7 @@ struct cfg80211_ops {
 
int (*set_noack_map)(struct wiphy *wiphy,
  struct net_device *dev,
- u16 noack_map);
+ const u8 *peer, u16 noack_map);
 
int (*get_channel)(struct wiphy *wiphy,
   struct wireless_dev *wdev,
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 60fefc5..7425ea6 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -784,7 +784,14 @@
  * messages. Note that per PHY only one application may register.
  *
  * @NL80211_CMD_SET_NOACK_MAP: sets a bitmap for the individual TIDs whether
- *  No Acknowledgement Policy should be applied.
+ * No Acknowledgement Policy should be applied. %NL80211_ATTR_MAC is used
+ * to apply No Acknowledgement policy for a particular connected station.
+ * Station specific NoAck policy configuration is valid only for STA's
+ * current connection, i.e. the configuration will not be used when the
+ * station connects back after disconnection/roaming.
+ * When user-space does not include %NL80211_ATTR_MAC, the No
+ * Acknowledgement Policy setting should be treated as per-netdev
+ * configuration.
  *
  * @NL80211_CMD_CH_SWITCH_NOTIFY: An AP or GO may decide to switch channels
  * independently of the userspace SME, send this event indicating
@@ -5005,6 +5012,8 @@ enum nl80211_feature_flags {
  * channel change triggered by radar detection event.
  * No need to start CAC from user-space, no need to react to
  * "radar detected" event.
+ * @NL80211_EXT_FEATURE_PER_STA_NOACK_MAP: Driver supports STA specific NoAck
+ * policy functionality.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -5036,6 +5045,7 @@ enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_LOW_POWER_SCAN,
NL80211_EXT_FEATURE_HIGH_ACCURACY_SCAN,
NL80211_EXT_FEATURE_DFS_OFFLOAD,
+   NL80211_EXT_FEATURE_PER_STA_NOACK_MAP,
 
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a2f0eae..621ef38 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -341,6 +341,7 @@ static void ieee80211_del_nan_func(struct wiphy *wiphy,
 
 static int ieee80211_set_noack_map(struct wiphy *wiphy,
  struct net_device *dev,
+ const u8 *peer,
  u16 noack_map)
 {
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fe27ab4..8d7f055a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3095,16 +3095,26 @@ static int nl80211_set_noack_map(struct sk_buff *skb, 
struct genl_info *info)
struct cfg80211_registered_device *rdev = info->user_ptr[0];