2024-06-05 10:58:22

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 0/7] cfg80211/mac80211 patches from our internal tree 2024-06-05

Hi,

A bunch of patches from our internal tree with mac80211 and
cfg80211 changes. It's the usual developement, adding a few small
features.

Thanks,
Miri

Avraham Stern (2):
wifi: nl80211: remove the FTMs per burst limit for NDP ranging
wifi: mac80211_hwsim: add 320 MHz to hwsim channel widths

Emmanuel Grumbach (1):
wifi: cfg80211: honor WIPHY_FLAG_SPLIT_SCAN_6GHZ in cfg80211_conn_scan

Ilan Peer (2):
wifi: cfg80211: Add support for interface usage notification
wifi: mac80211: Add support for interface usage notification

Johannes Berg (2):
wifi: mac80211: fix erroneous errors for STA changes
wifi: mac80211: clean up 'ret' in sta_link_apply_parameters()

drivers/net/wireless/virtual/mac80211_hwsim.c | 1 +
include/net/cfg80211.h | 12 ++++
include/net/mac80211.h | 4 ++
include/uapi/linux/nl80211.h | 16 +++++
net/mac80211/cfg.c | 72 +++++++++++++------
net/mac80211/driver-ops.h | 14 ++++
net/mac80211/trace.h | 23 ++++++
net/wireless/nl80211.c | 39 +++++++++-
net/wireless/pmsr.c | 10 ++-
net/wireless/rdev-ops.h | 13 ++++
net/wireless/sme.c | 4 +-
net/wireless/trace.h | 18 +++++
12 files changed, 200 insertions(+), 26 deletions(-)

--
2.34.1



2024-06-05 10:58:26

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 2/7] wifi: mac80211_hwsim: add 320 MHz to hwsim channel widths

From: Avraham Stern <[email protected]>

Setting a channel with 320 MHz channel width over hwsim results in an
array-index-out-of-bounds error. Fix it by adding 320 MHz to hwsim
supported channel widths.

Signed-off-by: Avraham Stern <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
drivers/net/wireless/virtual/mac80211_hwsim.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index b5afaec61827..d0dac0db979e 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -2361,6 +2361,7 @@ static const char * const hwsim_chanwidths[] = {
[NL80211_CHAN_WIDTH_4] = "4MHz",
[NL80211_CHAN_WIDTH_8] = "8MHz",
[NL80211_CHAN_WIDTH_16] = "16MHz",
+ [NL80211_CHAN_WIDTH_320] = "eht320",
};

static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed)
--
2.34.1


2024-06-05 10:58:29

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 3/7] wifi: mac80211: fix erroneous errors for STA changes

From: Johannes Berg <[email protected]>

When e.g. wpa_supplicant sets only the MLD "sta" authorized
state, the code actually applies that change, but then returns
an error to userspace anyway because there were no changes to
the link station, and no link ID was given. However, it's not
incorrect to not have a link ID when wanting to change only
the MLD peer ("sta") state, so the code shouldn't require it.

To fix this, separate the "new_link" argument out into a new
three-state enum, because if modify is called on a link STA
only, it should return an error if no link is given or if it
doesn't exist. For modify on the MLD "sta", not having a link
ID is OK, but if there is one it should be validated.

This seems to not have mattered much as wpa_supplicant just
prints a message and continues, and the authorized state was
already set before this error return. However, in the later
code powersave recalculation etc. will be skipped, so that it
may result in never allowing powersave on MLO connections.

Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
net/mac80211/cfg.c | 55 ++++++++++++++++++++++++++++++----------------
1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 62119e957cd8..54b03a86f71e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1814,8 +1814,15 @@ static void sta_apply_mesh_params(struct ieee80211_local *local,
#endif
}

+enum sta_link_apply_mode {
+ STA_LINK_MODE_NEW,
+ STA_LINK_MODE_STA_MODIFY,
+ STA_LINK_MODE_LINK_MODIFY,
+};
+
static int sta_link_apply_parameters(struct ieee80211_local *local,
- struct sta_info *sta, bool new_link,
+ struct sta_info *sta,
+ enum sta_link_apply_mode mode,
struct link_station_parameters *params)
{
int ret = 0;
@@ -1827,18 +1834,29 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
struct link_sta_info *link_sta =
rcu_dereference_protected(sta->link[link_id],
lockdep_is_held(&local->hw.wiphy->mtx));
-
- /*
- * If there are no changes, then accept a link that exist,
- * unless it's a new link.
- */
- if (params->link_id >= 0 && !new_link &&
- !params->link_mac && !params->txpwr_set &&
- !params->supported_rates_len &&
- !params->ht_capa && !params->vht_capa &&
- !params->he_capa && !params->eht_capa &&
- !params->opmode_notif_used)
- return 0;
+ bool changes = params->link_mac ||
+ params->txpwr_set ||
+ params->supported_rates_len ||
+ params->ht_capa ||
+ params->vht_capa ||
+ params->he_capa ||
+ params->eht_capa ||
+ params->opmode_notif_used;
+
+ switch (mode) {
+ case STA_LINK_MODE_NEW:
+ if (!params->link_mac)
+ return -EINVAL;
+ break;
+ case STA_LINK_MODE_LINK_MODIFY:
+ break;
+ case STA_LINK_MODE_STA_MODIFY:
+ if (params->link_id >= 0)
+ break;
+ if (!changes)
+ return 0;
+ break;
+ }

if (!link || !link_sta)
return -EINVAL;
@@ -1848,15 +1866,13 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
return -EINVAL;

if (params->link_mac) {
- if (new_link) {
+ if (mode == STA_LINK_MODE_NEW) {
memcpy(link_sta->addr, params->link_mac, ETH_ALEN);
memcpy(link_sta->pub->addr, params->link_mac, ETH_ALEN);
} else if (!ether_addr_equal(link_sta->addr,
params->link_mac)) {
return -EINVAL;
}
- } else if (new_link) {
- return -EINVAL;
}

if (params->txpwr_set) {
@@ -2028,7 +2044,7 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->listen_interval >= 0)
sta->listen_interval = params->listen_interval;

- ret = sta_link_apply_parameters(local, sta, false,
+ ret = sta_link_apply_parameters(local, sta, STA_LINK_MODE_STA_MODIFY,
&params->link_sta_params);
if (ret)
return ret;
@@ -5004,7 +5020,7 @@ ieee80211_add_link_station(struct wiphy *wiphy, struct net_device *dev,
if (ret)
return ret;

- ret = sta_link_apply_parameters(local, sta, true, params);
+ ret = sta_link_apply_parameters(local, sta, STA_LINK_MODE_NEW, params);
if (ret) {
ieee80211_sta_free_link(sta, params->link_id);
return ret;
@@ -5031,7 +5047,8 @@ ieee80211_mod_link_station(struct wiphy *wiphy, struct net_device *dev,
if (!(sta->sta.valid_links & BIT(params->link_id)))
return -EINVAL;

- return sta_link_apply_parameters(local, sta, false, params);
+ return sta_link_apply_parameters(local, sta, STA_LINK_MODE_LINK_MODIFY,
+ params);
}

static int
--
2.34.1


2024-06-05 10:58:30

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 6/7] wifi: cfg80211: Add support for interface usage notification

From: Ilan Peer <[email protected]>

In some cases, when an interface is added by user space, user space
might not know yet what is the intended type of the interface, e.g.,
before a P2P Group Ownership Negotiation (GON) an interface is added
but only at the end of the GON flow the final interface type is
determined. This doesn't allow the kernel drivers to prepare for the
actual interface type, e.g., make resources available for the
interface type etc.

Generally, adding an interface doesn't necessarily imply that it will
actually be used soon, and as described the interface might not be used
with the type it's added as.

This new API allows user space to indicate that it does indeed intend to
use the interface soon, along with the types (of which the interface
must be one) that may be selected for that usage. This will allow the
underlying driver to adjust resources accordingly.

Signed-off-by: Ilan Peer <[email protected]>
Reviewed-by: Johannes Berg <[email protected]>
Tested-by: iil_jenkins iil_jenkins <[email protected]>
---
include/net/cfg80211.h | 12 ++++++++++++
include/uapi/linux/nl80211.h | 16 ++++++++++++++++
net/wireless/nl80211.c | 36 ++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 13 +++++++++++++
net/wireless/trace.h | 18 ++++++++++++++++++
5 files changed, 95 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5da9bb0ac6a4..f2defbfd758e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4176,6 +4176,15 @@ struct mgmt_frame_regs {
u32 global_mcast_stypes, interface_mcast_stypes;
};

+/**
+ * struct cfg80211_iface_usage - Notify about intended interface usage
+ *
+ * @types_mask: mask of interface types that are going to be used.
+ */
+struct cfg80211_iface_usage {
+ u32 types_mask;
+};
+
/**
* struct cfg80211_ops - backend description for wireless configuration
*
@@ -4577,6 +4586,7 @@ struct mgmt_frame_regs {
*
* @set_hw_timestamp: Enable/disable HW timestamping of TM/FTM frames.
* @set_ttlm: set the TID to link mapping.
+ * @iface_usage: notify about intended usage of added interfaces.
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -4938,6 +4948,8 @@ struct cfg80211_ops {
struct cfg80211_set_hw_timestamp *hwts);
int (*set_ttlm)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_ttlm_params *params);
+ void (*iface_usage)(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_iface_usage *usage);
};

/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f917bc6c9b6f..92dbbb0589f0 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1329,6 +1329,15 @@
* %NL80211_ATTR_MLO_TTLM_ULINK attributes are used to specify the
* TID to Link mapping for downlink/uplink traffic.
*
+ * @NL80211_CMD_IFACE_USAGE_NOTIF: Notify kernel about the expected interface
+ * usage. Allows user space to indicate to the kernel that it intends to
+ * use the interface soon and what is the expected usage of the interface
+ * so resources could be prepared etc. This is useful in case an added
+ * interface is not going to be used immediately but soon, and its type
+ * might change. The %NL80211_ATTR_IFACE_USAGE_IFTYPES attribute is used to
+ * provide the mask of intended interface types (the current type must be
+ * included in the mask).
+ *
* @NL80211_CMD_MAX: highest used command number
* @__NL80211_CMD_AFTER_LAST: internal use
*/
@@ -1586,6 +1595,8 @@ enum nl80211_commands {

NL80211_CMD_SET_TID_TO_LINK_MAPPING,

+ NL80211_CMD_IFACE_USAGE_NOTIF,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -2856,6 +2867,9 @@ enum nl80211_commands {
* %NL80211_CMD_ASSOCIATE indicating the SPP A-MSDUs
* are used on this connection
*
+ * @NL80211_ATTR_IFACE_USAGE_IFTYPES: a bitmask of interface types that might be
+ * used with the interface.
+ *
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3401,6 +3415,8 @@ enum nl80211_attrs {

NL80211_ATTR_ASSOC_SPP_AMSDU,

+ NL80211_ATTR_IFACE_USAGE_IFTYPES,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 296acd2a2a1b..04110b74547d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -825,6 +825,11 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MLO_TTLM_DLINK] = NLA_POLICY_EXACT_LEN(sizeof(u16) * 8),
[NL80211_ATTR_MLO_TTLM_ULINK] = NLA_POLICY_EXACT_LEN(sizeof(u16) * 8),
[NL80211_ATTR_ASSOC_SPP_AMSDU] = { .type = NLA_FLAG },
+
+ /* Set the limits to not allow NL80211_IFTYPE_UNSPECIFIED */
+ [NL80211_ATTR_IFACE_USAGE_IFTYPES] =
+ NLA_POLICY_RANGE(NLA_U32, BIT(NL80211_IFTYPE_UNSPECIFIED + 1),
+ BIT(NUM_NL80211_IFTYPES) - 2),
};

/* policy for the key attributes */
@@ -16319,6 +16324,31 @@ nl80211_set_ttlm(struct sk_buff *skb, struct genl_info *info)
return rdev_set_ttlm(rdev, dev, &params);
}

+static int
+nl80211_iface_usage(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_iface_usage iface_usage = {};
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+
+ /* once the interface is up and running its type can no longer change */
+ if (wdev_running(wdev))
+ return -EINVAL;
+
+ if (!info->attrs[NL80211_ATTR_IFACE_USAGE_IFTYPES])
+ return -EINVAL;
+
+ iface_usage.types_mask =
+ nla_get_u32(info->attrs[NL80211_ATTR_IFACE_USAGE_IFTYPES]);
+
+ if (!(BIT(wdev->iftype) & iface_usage.types_mask))
+ return -EINVAL;
+
+ rdev_iface_usage(rdev, dev, &iface_usage);
+ return 0;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -17511,6 +17541,12 @@ static const struct genl_small_ops nl80211_small_ops[] = {
.flags = GENL_UNS_ADMIN_PERM,
.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
},
+ {
+ .cmd = NL80211_CMD_IFACE_USAGE_NOTIF,
+ .doit = nl80211_iface_usage,
+ .flags = GENL_UNS_ADMIN_PERM,
+ .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV),
+ },
};

static struct genl_family nl80211_fam __ro_after_init = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 43897a5269b6..0a13212d9c8d 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -1542,4 +1542,17 @@ rdev_set_ttlm(struct cfg80211_registered_device *rdev,

return ret;
}
+
+static inline void
+rdev_iface_usage(struct cfg80211_registered_device *rdev,
+ struct net_device *dev,
+ struct cfg80211_iface_usage *iface_usage)
+{
+ struct wiphy *wiphy = &rdev->wiphy;
+
+ trace_rdev_iface_usage(wiphy, dev, iface_usage);
+ if (rdev->ops->iface_usage)
+ rdev->ops->iface_usage(wiphy, dev, iface_usage);
+ trace_rdev_return_void(wiphy);
+}
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 14cfa0aba93a..2f5df52918cf 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3032,6 +3032,24 @@ TRACE_EVENT(rdev_set_ttlm,
WIPHY_PR_ARG, NETDEV_PR_ARG)
);

+TRACE_EVENT(rdev_iface_usage,
+ TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+ struct cfg80211_iface_usage *iface_usage),
+ TP_ARGS(wiphy, netdev, iface_usage),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ NETDEV_ENTRY
+ __field(u32, types_mask)
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ NETDEV_ASSIGN;
+ __entry->types_mask = iface_usage->types_mask;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", types_mask=0x%x",
+ WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->types_mask)
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
2.34.1


2024-06-05 10:58:33

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 1/7] wifi: nl80211: remove the FTMs per burst limit for NDP ranging

From: Avraham Stern <[email protected]>

In NDP ranging, the number of NDP exchanges is not negotiated
and thus is not limited by the protocol. Remove the limit on
FTMs per burst for trigger based and non trigger based ranging.

Signed-off-by: Avraham Stern <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
net/wireless/nl80211.c | 3 +--
net/wireless/pmsr.c | 10 +++++++++-
2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8ff5f79d446a..296acd2a2a1b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -315,8 +315,7 @@ nl80211_pmsr_ftm_req_attr_policy[NL80211_PMSR_FTM_REQ_ATTR_MAX + 1] = {
[NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD] = { .type = NLA_U16 },
[NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION] =
NLA_POLICY_MAX(NLA_U8, 15),
- [NL80211_PMSR_FTM_REQ_ATTR_FTMS_PER_BURST] =
- NLA_POLICY_MAX(NLA_U8, 31),
+ [NL80211_PMSR_FTM_REQ_ATTR_FTMS_PER_BURST] = { .type = NLA_U8 },
[NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES] = { .type = NLA_U8 },
[NL80211_PMSR_FTM_REQ_ATTR_REQUEST_LCI] = { .type = NLA_FLAG },
[NL80211_PMSR_FTM_REQ_ATTR_REQUEST_CIVICLOC] = { .type = NLA_FLAG },
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index e106dcea3977..4af6bd354ad1 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
- * Copyright (C) 2018 - 2021, 2023 Intel Corporation
+ * Copyright (C) 2018 - 2021, 2023 - 2024 Intel Corporation
*/
#include <net/cfg80211.h>
#include "core.h"
@@ -148,6 +148,14 @@ static int pmsr_parse_ftm(struct cfg80211_registered_device *rdev,
return -EINVAL;
}

+ if (out->ftm.ftms_per_burst > 31 && !out->ftm.non_trigger_based &&
+ !out->ftm.trigger_based) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[NL80211_PMSR_FTM_REQ_ATTR_FTMS_PER_BURST],
+ "FTM: FTMs per burst must be set lower than 31");
+ return -ERANGE;
+ }
+
if ((out->ftm.trigger_based || out->ftm.non_trigger_based) &&
out->ftm.preamble != NL80211_PREAMBLE_HE) {
NL_SET_ERR_MSG_ATTR(info->extack,
--
2.34.1


2024-06-05 10:58:42

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 4/7] wifi: mac80211: clean up 'ret' in sta_link_apply_parameters()

From: Johannes Berg <[email protected]>

There's no need to have the always-zero ret variable in
the function scope, move it into the inner scope only.

Signed-off-by: Johannes Berg <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
net/mac80211/cfg.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 54b03a86f71e..890590146fa4 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1825,7 +1825,6 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
enum sta_link_apply_mode mode,
struct link_station_parameters *params)
{
- int ret = 0;
struct ieee80211_supported_band *sband;
struct ieee80211_sub_if_data *sdata = sta->sdata;
u32 link_id = params->link_id < 0 ? 0 : params->link_id;
@@ -1876,6 +1875,8 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
}

if (params->txpwr_set) {
+ int ret;
+
link_sta->pub->txpwr.type = params->txpwr.type;
if (params->txpwr.type == NL80211_TX_POWER_LIMITED)
link_sta->pub->txpwr.power = params->txpwr.power;
@@ -1928,7 +1929,7 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,

ieee80211_sta_init_nss(link_sta);

- return ret;
+ return 0;
}

static int sta_apply_parameters(struct ieee80211_local *local,
--
2.34.1


2024-06-05 10:58:44

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 5/7] wifi: cfg80211: honor WIPHY_FLAG_SPLIT_SCAN_6GHZ in cfg80211_conn_scan

From: Emmanuel Grumbach <[email protected]>

If a user uses iw to connect to a network and we don't have any
information about the existing networks, cfg80211 will trigger a scan
internally even if the user didn't ask for a scan. This scan is
implemented by cfg80211_conn_scan(). This function called rdev_scan()
directly without honoring the WIPHY_FLAG_SPLIT_SCAN_6GHZ flag.
Use cfg80211_scan instead, this will split the scan if the low level
driver asked to.

Signed-off-by: Emmanuel Grumbach <[email protected]>
Reviewed-by: Johannes Berg <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
net/wireless/sme.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index a8ad55f11133..e419aa8c4a5a 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -5,7 +5,7 @@
* (for nl80211's connect() and wext)
*
* Copyright 2009 Johannes Berg <[email protected]>
- * Copyright (C) 2009, 2020, 2022-2023 Intel Corporation. All rights reserved.
+ * Copyright (C) 2009, 2020, 2022-2024 Intel Corporation. All rights reserved.
* Copyright 2017 Intel Deutschland GmbH
*/

@@ -130,7 +130,7 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)

rdev->scan_req = request;

- err = rdev_scan(rdev, request);
+ err = cfg80211_scan(rdev);
if (!err) {
wdev->conn->state = CFG80211_CONN_SCANNING;
nl80211_send_scan_start(rdev, wdev);
--
2.34.1


2024-06-05 10:58:48

by Miri Korenblit

[permalink] [raw]
Subject: [PATCH 7/7] wifi: mac80211: Add support for interface usage notification

From: Ilan Peer <[email protected]>

When user space indicates the intended usage of a network interface,
propagate the information to the driver.

Signed-off-by: Ilan Peer <[email protected]>
Signed-off-by: Miri Korenblit <[email protected]>
---
include/net/mac80211.h | 4 ++++
net/mac80211/cfg.c | 12 ++++++++++++
net/mac80211/driver-ops.h | 14 ++++++++++++++
net/mac80211/trace.h | 23 +++++++++++++++++++++++
4 files changed, 53 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ecfa65ade226..047dce69e79b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4438,6 +4438,7 @@ struct ieee80211_prep_tx_info {
* if the requested TID-To-Link mapping can be accepted or not.
* If it's not accepted the driver may suggest a preferred mapping and
* modify @ttlm parameter with the suggested TID-to-Link mapping.
+ * @iface_usage: notify about intended usage of added interfaces.
*/
struct ieee80211_ops {
void (*tx)(struct ieee80211_hw *hw,
@@ -4822,6 +4823,9 @@ struct ieee80211_ops {
enum ieee80211_neg_ttlm_res
(*can_neg_ttlm)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct ieee80211_neg_ttlm *ttlm);
+
+ void (*iface_usage)(struct ieee80211_hw *hw,
+ struct cfg80211_iface_usage *iface_usage);
};

/**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 890590146fa4..b121fc65eb09 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -5104,6 +5104,17 @@ ieee80211_set_ttlm(struct wiphy *wiphy, struct net_device *dev,
return ieee80211_req_neg_ttlm(sdata, params);
}

+static void
+ieee80211_iface_usage(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_iface_usage *iface_usage)
+{
+ struct ieee80211_local *local = wiphy_priv(wiphy);
+
+ lockdep_assert_wiphy(wiphy);
+
+ drv_iface_usage(local, iface_usage);
+}
+
const struct cfg80211_ops mac80211_config_ops = {
.add_virtual_intf = ieee80211_add_iface,
.del_virtual_intf = ieee80211_del_iface,
@@ -5217,4 +5228,5 @@ const struct cfg80211_ops mac80211_config_ops = {
.del_link_station = ieee80211_del_link_station,
.set_hw_timestamp = ieee80211_set_hw_timestamp,
.set_ttlm = ieee80211_set_ttlm,
+ .iface_usage = ieee80211_iface_usage,
};
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index d4e73d3630e0..ccc697bd98ff 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1728,4 +1728,18 @@ drv_can_neg_ttlm(struct ieee80211_local *local,

return res;
}
+
+static inline void drv_iface_usage(struct ieee80211_local *local,
+ struct cfg80211_iface_usage *iface_usage)
+{
+ might_sleep();
+
+ lockdep_assert_wiphy(local->hw.wiphy);
+
+ trace_drv_iface_usage(local, iface_usage);
+ if (local->ops->iface_usage)
+ local->ops->iface_usage(&local->hw, iface_usage);
+
+ trace_drv_return_void(local);
+}
#endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 8e758b5074bd..b2f2588c7f82 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -3145,6 +3145,29 @@ TRACE_EVENT(drv_neg_ttlm_res,
LOCAL_PR_ARG, VIF_PR_ARG, __entry->res
)
);
+
+TRACE_EVENT(drv_iface_usage,
+ TP_PROTO(struct ieee80211_local *local,
+ struct cfg80211_iface_usage *iface_usage),
+
+ TP_ARGS(local, iface_usage),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ __field(u32, types_mask)
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ __entry->types_mask = iface_usage->types_mask;
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT " types_mask=0x%x",
+ LOCAL_PR_ARG, __entry->types_mask
+ )
+);
+
#endif /* !__MAC80211_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */

#undef TRACE_INCLUDE_PATH
--
2.34.1


2024-06-06 09:57:05

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 6/7] wifi: cfg80211: Add support for interface usage notification

Miri Korenblit <[email protected]> writes:

> From: Ilan Peer <[email protected]>
>
> In some cases, when an interface is added by user space, user space
> might not know yet what is the intended type of the interface, e.g.,
> before a P2P Group Ownership Negotiation (GON) an interface is added
> but only at the end of the GON flow the final interface type is
> determined. This doesn't allow the kernel drivers to prepare for the
> actual interface type, e.g., make resources available for the
> interface type etc.
>
> Generally, adding an interface doesn't necessarily imply that it will
> actually be used soon, and as described the interface might not be used
> with the type it's added as.
>
> This new API allows user space to indicate that it does indeed intend to
> use the interface soon, along with the types (of which the interface
> must be one) that may be selected for that usage. This will allow the
> underlying driver to adjust resources accordingly.
>
> Signed-off-by: Ilan Peer <[email protected]>
> Reviewed-by: Johannes Berg <[email protected]>
> Tested-by: iil_jenkins iil_jenkins <[email protected]>

This new command just looks weird to me, do we really need it? I would
expect to see a workaround like this in out-of-tree drivers but not in
upstream.

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2024-06-09 07:35:16

by Peer, Ilan

[permalink] [raw]
Subject: RE: [PATCH 6/7] wifi: cfg80211: Add support for interface usage notification

Hi,

> -----Original Message-----
> From: Kalle Valo <[email protected]>
> Sent: Thursday, 6 June 2024 12:28
> To: Korenblit, Miriam Rachel <[email protected]>
> Cc: [email protected]; [email protected]; Peer, Ilan
> <[email protected]>; Berg, Johannes <[email protected]>;
> iil_jenkins iil_jenkins <[email protected]>
> Subject: Re: [PATCH 6/7] wifi: cfg80211: Add support for interface usage
> notification
>
> Miri Korenblit <[email protected]> writes:
>
> > From: Ilan Peer <[email protected]>
> >
> > In some cases, when an interface is added by user space, user space
> > might not know yet what is the intended type of the interface, e.g.,
> > before a P2P Group Ownership Negotiation (GON) an interface is added
> > but only at the end of the GON flow the final interface type is
> > determined. This doesn't allow the kernel drivers to prepare for the
> > actual interface type, e.g., make resources available for the
> > interface type etc.
> >
> > Generally, adding an interface doesn't necessarily imply that it will
> > actually be used soon, and as described the interface might not be
> > used with the type it's added as.
> >
> > This new API allows user space to indicate that it does indeed intend
> > to use the interface soon, along with the types (of which the
> > interface must be one) that may be selected for that usage. This will
> > allow the underlying driver to adjust resources accordingly.
> >
> > Signed-off-by: Ilan Peer <[email protected]>
> > Reviewed-by: Johannes Berg <[email protected]>
> > Tested-by: iil_jenkins iil_jenkins <[email protected]>
>
> This new command just looks weird to me, do we really need it? I would
> expect to see a workaround like this in out-of-tree drivers but not in upstream.
>

As depicted above, the need to inform the driver about the intended usage of the interface is real. We encountered
several P2P cases in which an interface was added and P2P Group Ownership Negotiation and P2P Invitation signalling
were completed successfully, but the P2P Group Session establishment failed since the interface type changed from P2P Client
to P2P GO and the local device was no longer able to accommodate the P2P GO operation due to resource constraints.

With this new API, user space can now inform the driver about the intended usage of the interface so the driver will
make the resources available for all possible interface types. With this the information exchanged during the P2P signalling
would correctly reflect state and the P2P group session would be able to be established.

I think that this could be useful in other use cases where the interface type is changed, e.g., when starting AP operation on the
interface, but before the AP is setup, the interface type is set to station mode to allow scanning operation etc.

Regards,

Ilan.

2024-06-12 16:27:51

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 6/7] wifi: cfg80211: Add support for interface usage notification

"Peer, Ilan" <[email protected]> writes:

> Hi,
>
>> -----Original Message-----
>> From: Kalle Valo <[email protected]>
>> Sent: Thursday, 6 June 2024 12:28
>> To: Korenblit, Miriam Rachel <[email protected]>
>> Cc: [email protected]; [email protected]; Peer, Ilan
>> <[email protected]>; Berg, Johannes <[email protected]>;
>> iil_jenkins iil_jenkins <[email protected]>
>> Subject: Re: [PATCH 6/7] wifi: cfg80211: Add support for interface usage
>> notification
>>
>> Miri Korenblit <[email protected]> writes:
>>
>> > From: Ilan Peer <[email protected]>
>> >
>> > In some cases, when an interface is added by user space, user space
>> > might not know yet what is the intended type of the interface, e.g.,
>> > before a P2P Group Ownership Negotiation (GON) an interface is added
>> > but only at the end of the GON flow the final interface type is
>> > determined. This doesn't allow the kernel drivers to prepare for the
>> > actual interface type, e.g., make resources available for the
>> > interface type etc.
>> >
>> > Generally, adding an interface doesn't necessarily imply that it will
>> > actually be used soon, and as described the interface might not be
>> > used with the type it's added as.
>> >
>> > This new API allows user space to indicate that it does indeed intend
>> > to use the interface soon, along with the types (of which the
>> > interface must be one) that may be selected for that usage. This will
>> > allow the underlying driver to adjust resources accordingly.
>> >
>> > Signed-off-by: Ilan Peer <[email protected]>
>> > Reviewed-by: Johannes Berg <[email protected]>
>> > Tested-by: iil_jenkins iil_jenkins <[email protected]>
>>
>> This new command just looks weird to me, do we really need it? I would
>> expect to see a workaround like this in out-of-tree drivers but not in upstream.
>>
>
> As depicted above, the need to inform the driver about the intended
> usage of the interface is real.

Sure, I can understand the need is real. This just feels like an ugly
workaround, not a proper solution.

And the documentation for this is quite vague, I'm worried how do we get
similarly working drivers? Let's say if I were to implement a user space
application for this, or a driver implementation for that matter, it
would be a guessing game for me. For example, what's "soon" in this
context? 5 mins, 50 secs or 5 secs? Can the mac80211 operation sleep?

So user space is now always supposed to always call this nl80211 command
and at what stage exactly? Or is it optional? But if it's optional
what's the point of adding it?

> We encountered several P2P cases in which an interface was added and
> P2P Group Ownership Negotiation and P2P Invitation signalling were
> completed successfully, but the P2P Group Session establishment failed
> since the interface type changed from P2P Client to P2P GO and the
> local device was no longer able to accommodate the P2P GO operation
> due to resource constraints.
>
> With this new API, user space can now inform the driver about the
> intended usage of the interface so the driver will
> make the resources available for all possible interface types. With
> this the information exchanged during the P2P signalling
> would correctly reflect state and the P2P group session would be able
> to be established.

Why not allocate the resources during driver initialisation? Or when
changing the interface? Why need this weird interface?

For easier reading below are all the patches, including the iwlwifi
patch. Honestly, this just looks like something like a workaround for a
problem in your firmware or something like that.

https://patchwork.kernel.org/project/linux-wireless/patch/20240605135233.23d15e758640.I7a62740a6868416acaed01e41157b3c0a7a41b4d@changeid/

https://patchwork.kernel.org/project/linux-wireless/patch/20240605135233.4d602acf0e65.I01fecab3b41961038f37ca6e0e3039c5fe9bb6bf@changeid/

https://patchwork.kernel.org/project/linux-wireless/patch/20240605140556.21582e74a0e0.I7c423d03b4412d77509bd31bd41e4573f76c0e84@changeid/

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches