[RFC 0/6] wifi: cfg80211/mac80211: improve support for multiple radios

[RFC 0/6] wifi: cfg80211/mac80211: improve support for multiple radios

[Felix Fietkau]

This series adds support for restricting vifs to a set of radios.
The allowed radios mask is used to restrict scanning, off-channel activity.
This is also used for per-radio start/stop ops, monitor state and filter
flags. It can also limit per-monitor-vif rx to specific radios.

Felix Fietkau (6):
  wifi: cfg80211/mac80211: add option for vif allowed radios
  wifi: mac80211: remove status->ampdu_delimiter_crc
  wifi: mac80211: notify driver about per-radio monitor enabled state
  wifi: mac80211: support per-radio driver start/stop calls
  wifi: mac80211: support per-radio filter flags
  wifi: mac80211: check vif radio_mask for monitor mode rx

 include/net/cfg80211.h       |  14 +++-
 include/net/mac80211.h       |  33 +++++--
 include/uapi/linux/nl80211.h |   5 +-
 net/mac80211/cfg.c           |   7 ++-
 net/mac80211/chan.c          |  13 ++-
 net/mac80211/driver-ops.h    |  50 +++++++++++-
 net/mac80211/ieee80211_i.h   |  29 +++++--
 net/mac80211/iface.c         | 159 +++++++++++++++++++++++++++---------
 net/mac80211/main.c          |  72 ++++++++++++----
 net/mac80211/mesh.c          |  33 +++++--
 net/mac80211/rx.c            |  65 +++++++--------
 net/mac80211/scan.c          |  10 +-
 net/mac80211/trace.h         |  38 +++++++++-
 net/wireless/nl80211.c       |  46 +++++++++-
 net/wireless/scan.c          |  10 +-
 net/wireless/util.c          |  29 +++++++-
 16 files changed, 494 insertions(+), 119 deletions(-)

base-commit: 30335bbc777aa14ccf5ea329296f59bb30d0180f
-- 
git-series 0.9.1

[RFC 1/6] wifi: cfg80211/mac80211: add option for vif allowed radios

[Felix Fietkau]

This allows users to prevent a vif from affecting radios other than the
configured ones. This can be useful in cases where e.g. an AP is running
on one radio, and triggering a scan on another radio should not disturb it.

It enforces the configured radio mask for configured frequencies, scan
frequency lists and remain-on-channel requests.

Changing the allowed radios list for a vif is supported, but only while
it is down.

While it is possible to achieve the same by always explicitly specifying
a frequency list for scan requests and ensuring that the wrong channel/band
is never accidentally set on an unrelated interface, this change makes
multi-radio wiphy setups a lot easier to deal with for CLI users.

Follow-up changes build on this to support per-radio monitor, filter and
started state.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/cfg80211.h       | 14 +++++++++++-
 include/uapi/linux/nl80211.h |  5 ++++-
 net/mac80211/cfg.c           |  7 ++++++-
 net/mac80211/chan.c          | 13 +++++++---
 net/mac80211/iface.c         |  1 +-
 net/mac80211/scan.c          | 10 +++++---
 net/wireless/nl80211.c       | 46 ++++++++++++++++++++++++++++++++++---
 net/wireless/scan.c          | 10 +++++---
 net/wireless/util.c          | 29 +++++++++++++++++++++++-
 9 files changed, 123 insertions(+), 12 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index c2a9af1e3c5e..3d7d07027f4f 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -769,6 +769,7 @@ static inline void wiphy_read_of_freq_limits(struct wiphy *wiphy)
  *	belonging to that MU-MIMO groupID; %NULL if not changed
  * @vht_mumimo_follow_addr: MU-MIMO follow address, used for monitoring
  *	MU-MIMO packets going to the specified station; %NULL if not changed
+ * @radio_mask: Bitmask of radios that this interface is allowed to operate on.
  */
 struct vif_params {
 	u32 flags;
@@ -776,6 +777,7 @@ struct vif_params {
 	u8 macaddr[ETH_ALEN];
 	const u8 *vht_mumimo_groups;
 	const u8 *vht_mumimo_follow_addr;
+	u32 radio_mask;
 };
 
 /**
@@ -6222,6 +6224,7 @@ enum ieee80211_ap_reg_power {
  * @links: array of %IEEE80211_MLD_MAX_NUM_LINKS elements containing @addr
  *	@ap and @client for each link
  * @valid_links: bitmap describing what elements of @links are valid
+ * @radio_mask: Bitmask of radios that this interface is allowed to operate on.
  */
 struct wireless_dev {
 	struct wiphy *wiphy;
@@ -6335,6 +6338,8 @@ struct wireless_dev {
 		};
 	} links[IEEE80211_MLD_MAX_NUM_LINKS];
 	u16 valid_links;
+
+	u32 radio_mask;
 };
 
 static inline const u8 *wdev_address(struct wireless_dev *wdev)
@@ -6521,6 +6526,15 @@ bool cfg80211_radio_chandef_valid(const struct wiphy_radio *radio,
 				  const struct cfg80211_chan_def *chandef);
 
 /**
+ * cfg80211_wdev_channel_allowed - Check if the wdev may use the channel
+ *
+ * @wdev: the wireless device
+ * @chan: channel to check
+ */
+bool cfg80211_wdev_channel_allowed(struct wireless_dev *wdev,
+				   struct ieee80211_channel *chan);
+
+/**
  * ieee80211_get_response_rate - get basic rate for a given rate
  *
  * @sband: the band to look for rates in
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index f97f5adc8d51..d31ccee99cc7 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2868,6 +2868,9 @@ enum nl80211_commands {
  *	nested item, it contains attributes defined in
  *	&enum nl80211_if_combination_attrs.
  *
+ * @NL80211_ATTR_VIF_RADIO_MASK: Bitmask of allowed radios (u32).
+ *	A value of 0 means all radios.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3416,6 +3419,8 @@ enum nl80211_attrs {
 	NL80211_ATTR_WIPHY_RADIOS,
 	NL80211_ATTR_WIPHY_INTERFACE_COMBINATIONS,
 
+	NL80211_ATTR_VIF_RADIO_MASK,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 85cb71de370f..32d9b9293ff3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -216,6 +216,13 @@ static int ieee80211_change_iface(struct wiphy *wiphy,
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
+	if (params->radio_mask && params->radio_mask != sdata->wdev.radio_mask) {
+		if (ieee80211_sdata_running(sdata))
+			return -EBUSY;
+
+		sdata->wdev.radio_mask = params->radio_mask;
+	}
+
 	ret = ieee80211_if_change_type(sdata, type);
 	if (ret)
 		return ret;
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index e8567723e94d..65451ffad1ce 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1166,7 +1166,7 @@ ieee80211_replace_chanctx(struct ieee80211_local *local,
 static bool
 ieee80211_find_available_radio(struct ieee80211_local *local,
 			       const struct ieee80211_chan_req *chanreq,
-			       int *radio_idx)
+			       u32 radio_mask, int *radio_idx)
 {
 	struct wiphy *wiphy = local->hw.wiphy;
 	const struct wiphy_radio *radio;
@@ -1177,6 +1177,9 @@ ieee80211_find_available_radio(struct ieee80211_local *local,
 		return true;
 
 	for (i = 0; i < wiphy->n_radio; i++) {
+		if (!(radio_mask & BIT(i)))
+			continue;
+
 		radio = &wiphy->radio[i];
 		if (!cfg80211_radio_chandef_valid(radio, &chanreq->oper))
 			continue;
@@ -1210,7 +1213,9 @@ int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
 	new_ctx = ieee80211_find_reservation_chanctx(local, chanreq, mode);
 	if (!new_ctx) {
 		if (ieee80211_can_create_new_chanctx(local, -1) &&
-		    ieee80211_find_available_radio(local, chanreq, &radio_idx))
+		    ieee80211_find_available_radio(local, chanreq,
+						   sdata->wdev.radio_mask,
+						   &radio_idx))
 			new_ctx = ieee80211_new_chanctx(local, chanreq, mode,
 							false, radio_idx);
 		else
@@ -1880,7 +1885,9 @@ int _ieee80211_link_use_channel(struct ieee80211_link_data *link,
 	/* Note: context is now reserved */
 	if (ctx)
 		reserved = true;
-	else if (!ieee80211_find_available_radio(local, chanreq, &radio_idx))
+	else if (!ieee80211_find_available_radio(local, chanreq,
+						 sdata->wdev.radio_mask,
+						 &radio_idx))
 		ctx = ERR_PTR(-EBUSY);
 	else
 		ctx = ieee80211_new_chanctx(local, chanreq, mode,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index d44920c937af..56fed4f69427 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -2172,6 +2172,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 
 	if (ndev) {
 		ndev->ieee80211_ptr->use_4addr = params->use_4addr;
+		ndev->ieee80211_ptr->radio_mask = params->radio_mask;
 		if (type == NL80211_IFTYPE_STATION)
 			sdata->u.mgd.use_4addr = params->use_4addr;
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index b5f2df61c7f6..86188d041263 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -1200,7 +1200,9 @@ int ieee80211_request_ibss_scan(struct ieee80211_sub_if_data *sdata,
 				    &local->hw.wiphy->bands[band]->channels[i];
 
 				if (tmp_ch->flags & (IEEE80211_CHAN_NO_IR |
-						     IEEE80211_CHAN_DISABLED))
+						     IEEE80211_CHAN_DISABLED) ||
+				    !cfg80211_wdev_channel_allowed(&sdata->wdev,
+								   tmp_ch))
 					continue;
 
 				local->int_scan_req->channels[n_ch] = tmp_ch;
@@ -1215,14 +1217,16 @@ int ieee80211_request_ibss_scan(struct ieee80211_sub_if_data *sdata,
 	} else {
 		for (i = 0; i < n_channels; i++) {
 			if (channels[i]->flags & (IEEE80211_CHAN_NO_IR |
-						  IEEE80211_CHAN_DISABLED))
+						  IEEE80211_CHAN_DISABLED) ||
+			    !cfg80211_wdev_channel_allowed(&sdata->wdev,
+							   channels[i]))
 				continue;
 
 			local->int_scan_req->channels[n_ch] = channels[i];
 			n_ch++;
 		}
 
-		if (WARN_ON_ONCE(n_ch == 0))
+		if (n_ch == 0)
 			goto unlock;
 
 		local->int_scan_req->n_channels = n_ch;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7397a372c78e..7096674c3610 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -829,6 +829,7 @@ 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 },
+	[NL80211_ATTR_VIF_RADIO_MASK] = { .type = NLA_U32 },
 };
 
 /* policy for the key attributes */
@@ -3996,7 +3997,8 @@ static int nl80211_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flag
 	    nla_put_u32(msg, NL80211_ATTR_GENERATION,
 			rdev->devlist_generation ^
 			(cfg80211_rdev_list_generation << 2)) ||
-	    nla_put_u8(msg, NL80211_ATTR_4ADDR, wdev->use_4addr))
+	    nla_put_u8(msg, NL80211_ATTR_4ADDR, wdev->use_4addr) ||
+	    nla_put_u32(msg, NL80211_ATTR_VIF_RADIO_MASK, wdev->radio_mask))
 		goto nla_put_failure;
 
 	if (rdev->ops->get_channel && !wdev->valid_links) {
@@ -4312,6 +4314,28 @@ static int nl80211_valid_4addr(struct cfg80211_registered_device *rdev,
 	return -EOPNOTSUPP;
 }
 
+static int nl80211_parse_vif_radio_mask(struct genl_info *info,
+					struct vif_params *params)
+{
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct nlattr *attr = info->attrs[NL80211_ATTR_VIF_RADIO_MASK];
+	u32 mask, allowed;
+
+	if (!attr)
+		return 0;
+
+	allowed = BIT(rdev->wiphy.n_radio) - 1;
+	mask = nla_get_u32(attr);
+	if (mask & ~allowed)
+		return -EINVAL;
+
+	if (!mask)
+		mask = allowed;
+	params->radio_mask = mask;
+
+	return 1;
+}
+
 static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -4364,6 +4388,12 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
 	if (err > 0)
 		change = true;
 
+	err = nl80211_parse_vif_radio_mask(info, &params);
+	if (err < 0)
+		return err;
+	if (err > 0)
+		change = true;
+
 	if (change)
 		err = cfg80211_change_iface(rdev, dev, ntype, &params);
 	else
@@ -4424,6 +4454,11 @@ static int _nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
 	if (err < 0)
 		return err;
 
+	params.radio_mask = BIT(rdev->wiphy.n_radio) - 1;
+	err = nl80211_parse_vif_radio_mask(info, &params);
+	if (err < 0)
+		return err;
+
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -9180,6 +9215,9 @@ static bool cfg80211_off_channel_oper_allowed(struct wireless_dev *wdev,
 
 	lockdep_assert_wiphy(wdev->wiphy);
 
+	if (!cfg80211_wdev_channel_allowed(wdev, chan))
+		return false;
+
 	if (!cfg80211_beaconing_iface_active(wdev))
 		return true;
 
@@ -9392,7 +9430,8 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 			}
 
 			/* ignore disabled channels */
-			if (chan->flags & IEEE80211_CHAN_DISABLED)
+			if (chan->flags & IEEE80211_CHAN_DISABLED ||
+			    !cfg80211_wdev_channel_allowed(wdev, chan))
 				continue;
 
 			request->channels[i] = chan;
@@ -9412,7 +9451,8 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 
 				chan = &wiphy->bands[band]->channels[j];
 
-				if (chan->flags & IEEE80211_CHAN_DISABLED)
+				if (chan->flags & IEEE80211_CHAN_DISABLED ||
+				    !cfg80211_wdev_channel_allowed(wdev, chan))
 					continue;
 
 				request->channels[i] = chan;
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index d99319d82205..fd49a2746675 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -956,7 +956,8 @@ static int cfg80211_scan_6ghz(struct cfg80211_registered_device *rdev)
 		struct ieee80211_channel *chan =
 			ieee80211_get_channel(&rdev->wiphy, ap->center_freq);
 
-		if (!chan || chan->flags & IEEE80211_CHAN_DISABLED)
+		if (!chan || chan->flags & IEEE80211_CHAN_DISABLED ||
+		    !cfg80211_wdev_channel_allowed(rdev_req->wdev, chan))
 			continue;
 
 		for (i = 0; i < rdev_req->n_channels; i++) {
@@ -3485,9 +3486,12 @@ int cfg80211_wext_siwscan(struct net_device *dev,
 			continue;
 
 		for (j = 0; j < wiphy->bands[band]->n_channels; j++) {
+			struct ieee80211_channel *chan;
+
 			/* ignore disabled channels */
-			if (wiphy->bands[band]->channels[j].flags &
-						IEEE80211_CHAN_DISABLED)
+			chan = &wiphy->bands[band]->channels[j];
+			if (chan->flags & IEEE80211_CHAN_DISABLED ||
+			    !cfg80211_wdev_channel_allowed(creq->wdev, chan))
 				continue;
 
 			/* If we have a wireless request structure and the
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 9a7c3adc8a3b..cef7ad288ad6 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -2921,3 +2921,32 @@ bool cfg80211_radio_chandef_valid(const struct wiphy_radio *radio,
 	return true;
 }
 EXPORT_SYMBOL(cfg80211_radio_chandef_valid);
+
+bool cfg80211_wdev_channel_allowed(struct wireless_dev *wdev,
+				   struct ieee80211_channel *chan)
+{
+	struct wiphy *wiphy = wdev->wiphy;
+	const struct wiphy_radio *radio;
+	struct cfg80211_chan_def chandef;
+	u32 radio_mask;
+	int i;
+
+	radio_mask = wdev->radio_mask;
+	if (!wiphy->n_radio || radio_mask == BIT(wiphy->n_radio) - 1)
+		return true;
+
+	cfg80211_chandef_create(&chandef, chan, NL80211_CHAN_HT20);
+	for (i = 0; i < wiphy->n_radio; i++) {
+		if (!(radio_mask & BIT(i)))
+			continue;
+
+		radio = &wiphy->radio[i];
+		if (!cfg80211_radio_chandef_valid(radio, &chandef))
+			continue;
+
+		return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(cfg80211_wdev_channel_allowed);
-- 
git-series 0.9.1

[RFC 2/6] wifi: mac80211: remove status->ampdu_delimiter_crc

[Felix Fietkau]

This was never used by any driver, so remove it to free up some space.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h | 6 +-----
 net/mac80211/rx.c      | 7 +------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9311041f5c8d..9618c82262e3 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1450,8 +1450,6 @@ ieee80211_tx_info_clear_status(struct ieee80211_tx_info *info)
  * @RX_FLAG_AMPDU_IS_LAST: this subframe is the last subframe of the A-MPDU
  * @RX_FLAG_AMPDU_DELIM_CRC_ERROR: A delimiter CRC error has been detected
  *	on this subframe
- * @RX_FLAG_AMPDU_DELIM_CRC_KNOWN: The delimiter CRC field is known (the CRC
- *	is stored in the @ampdu_delimiter_crc field)
  * @RX_FLAG_MIC_STRIPPED: The mic was stripped of this packet. Decryption was
  *	done by the hardware
  * @RX_FLAG_ONLY_MONITOR: Report frame only to monitor interfaces without
@@ -1523,7 +1521,7 @@ enum mac80211_rx_flags {
 	RX_FLAG_AMPDU_LAST_KNOWN	= BIT(12),
 	RX_FLAG_AMPDU_IS_LAST		= BIT(13),
 	RX_FLAG_AMPDU_DELIM_CRC_ERROR	= BIT(14),
-	RX_FLAG_AMPDU_DELIM_CRC_KNOWN	= BIT(15),
+	/* one free bit at 15 */
 	RX_FLAG_MACTIME			= BIT(16) | BIT(17),
 	RX_FLAG_MACTIME_PLCP_START	= 1 << 16,
 	RX_FLAG_MACTIME_START		= 2 << 16,
@@ -1620,7 +1618,6 @@ enum mac80211_rx_encoding {
  * @rx_flags: internal RX flags for mac80211
  * @ampdu_reference: A-MPDU reference number, must be a different value for
  *	each A-MPDU but the same for each subframe within one A-MPDU
- * @ampdu_delimiter_crc: A-MPDU delimiter CRC
  * @zero_length_psdu_type: radiotap type of the 0-length PSDU
  * @link_valid: if the link which is identified by @link_id is valid. This flag
  *	is set only when connection is MLO.
@@ -1658,7 +1655,6 @@ struct ieee80211_rx_status {
 	s8 signal;
 	u8 chains;
 	s8 chain_signal[IEEE80211_MAX_CHAINS];
-	u8 ampdu_delimiter_crc;
 	u8 zero_length_psdu_type;
 	u8 link_valid:1, link_id:4;
 };
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 59ad24a71141..718f02f0a181 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -508,18 +508,13 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
 			flags |= IEEE80211_RADIOTAP_AMPDU_IS_LAST;
 		if (status->flag & RX_FLAG_AMPDU_DELIM_CRC_ERROR)
 			flags |= IEEE80211_RADIOTAP_AMPDU_DELIM_CRC_ERR;
-		if (status->flag & RX_FLAG_AMPDU_DELIM_CRC_KNOWN)
-			flags |= IEEE80211_RADIOTAP_AMPDU_DELIM_CRC_KNOWN;
 		if (status->flag & RX_FLAG_AMPDU_EOF_BIT_KNOWN)
 			flags |= IEEE80211_RADIOTAP_AMPDU_EOF_KNOWN;
 		if (status->flag & RX_FLAG_AMPDU_EOF_BIT)
 			flags |= IEEE80211_RADIOTAP_AMPDU_EOF;
 		put_unaligned_le16(flags, pos);
 		pos += 2;
-		if (status->flag & RX_FLAG_AMPDU_DELIM_CRC_KNOWN)
-			*pos++ = status->ampdu_delimiter_crc;
-		else
-			*pos++ = 0;
+		*pos++ = 0;
 		*pos++ = 0;
 	}
 
-- 
git-series 0.9.1

[RFC 3/6] wifi: mac80211: notify driver about per-radio monitor enabled state

[Felix Fietkau]

This allows monitoring on one or more radios while minimizing performance
impact on the others.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h     |  3 +++
 net/mac80211/ieee80211_i.h |  6 ++++++
 net/mac80211/iface.c       | 26 +++++++++++++++++++++++++-
 net/mac80211/main.c        | 12 ++++++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9618c82262e3..7bee2d912efe 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1748,6 +1748,8 @@ enum ieee80211_smps_mode {
  *
  * @flags: configuration flags defined above
  *
+ * @monitor_radios: bitmask of radios with monitor mode enabled.
+ *
  * @listen_interval: listen interval in units of beacon interval
  * @ps_dtim_period: The DTIM period of the AP we're connected to, for use
  *	in power saving. Power saving will not be enabled until a beacon
@@ -1777,6 +1779,7 @@ enum ieee80211_smps_mode {
  */
 struct ieee80211_conf {
 	u32 flags;
+	u32 monitor_radios;
 	int power_level, dynamic_ps_timeout;
 
 	u16 listen_interval;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 304cce0b771d..3be9f8149117 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1330,6 +1330,10 @@ enum mac80211_scan_state {
 
 DECLARE_STATIC_KEY_FALSE(aql_disable);
 
+struct ieee80211_radio_data {
+	u32 monitors;
+};
+
 struct ieee80211_local {
 	/* embed the driver visible part.
 	 * don't cast (use the static inlines below), but we keep
@@ -1613,6 +1617,8 @@ struct ieee80211_local {
 	u8 ext_capa[8];
 
 	bool wbrf_supported;
+
+	struct ieee80211_radio_data *radio_data;
 };
 
 static inline struct ieee80211_sub_if_data *
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 56fed4f69427..4db867ae97f0 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -601,6 +601,18 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
 		}
 
+		for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+			if (!(sdata->wdev.radio_mask & BIT(i)))
+				continue;
+
+			local->radio_data[i].monitors--;
+			if (local->radio_data[i].monitors)
+				continue;
+
+			local->hw.conf.monitor_radios &= ~BIT(i);
+			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
+		}
+
 		ieee80211_adjust_monitor_flags(sdata, -1);
 		break;
 	case NL80211_IFTYPE_NAN:
@@ -1214,7 +1226,7 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 	struct net_device *dev = wdev->netdev;
 	struct ieee80211_local *local = sdata->local;
 	u64 changed = 0;
-	int res;
+	int i, res;
 	u32 hw_reconf_flags = 0;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
@@ -1330,6 +1342,18 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
 		}
 
+		for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+			if (!(sdata->wdev.radio_mask & BIT(i)))
+				continue;
+
+			local->radio_data[i].monitors++;
+			if (local->radio_data[i].monitors > 1)
+				continue;
+
+			local->hw.conf.monitor_radios |= BIT(i);
+			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
+		}
+
 		ieee80211_adjust_monitor_flags(sdata, 1);
 		ieee80211_configure_filter(local);
 		ieee80211_recalc_offload(local);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 6abf85a58133..2ce771744ea8 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1348,6 +1348,16 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	if (!local->int_scan_req)
 		return -ENOMEM;
 
+	if (hw->wiphy->n_radio) {
+		local->radio_data = kcalloc(hw->wiphy->n_radio,
+					    sizeof(*local->radio_data),
+					    GFP_KERNEL);
+		if (!local->radio_data) {
+			result = -ENOMEM;
+			goto fail_workqueue;
+		}
+	}
+
 	eth_broadcast_addr(local->int_scan_req->bssid);
 
 	for (band = 0; band < NUM_NL80211_BANDS; band++) {
@@ -1642,6 +1652,7 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		local->wiphy_ciphers_allocated = false;
 	}
 	kfree(local->int_scan_req);
+	kfree(local->radio_data);
 	return result;
 }
 EXPORT_SYMBOL(ieee80211_register_hw);
@@ -1694,6 +1705,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 	destroy_workqueue(local->workqueue);
 	ieee80211_led_exit(local);
 	kfree(local->int_scan_req);
+	kfree(local->radio_data);
 }
 EXPORT_SYMBOL(ieee80211_unregister_hw);
 
-- 
git-series 0.9.1

Re: [RFC 3/6] wifi: mac80211: notify driver about per-radio monitor enabled state

[Johannes Berg]

On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> This allows monitoring on one or more radios while minimizing performance
> impact on the others.

But why are you doing it this way? You could already solve this entirely
with the driver by setting WANT_MONITOR_VIF and dealing with that, I'd
think? At least after this series.

I generally don't like hw->conf, it just hasn't really matched reality
for years with all kinds of new concurrency capabilities. At the very
least you'd have to write more text here to convince me that we want to
add something to it ... :)

johannes

Re: [RFC 3/6] wifi: mac80211: notify driver about per-radio monitor enabled state

[Felix Fietkau]

> On 23. Aug 2024, at 12:16, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
>> This allows monitoring on one or more radios while minimizing performance
>> impact on the others.
> 
> But why are you doing it this way? You could already solve this entirely
> with the driver by setting WANT_MONITOR_VIF and dealing with that, I'd
> think? At least after this series.
> 
> I generally don't like hw->conf, it just hasn't really matched reality
> for years with all kinds of new concurrency capabilities. At the very
> least you'd have to write more text here to convince me that we want to
> add something to it ... :)

I really don’t see how WANT_MONITOR_VIF helps. It seems completely unrelated to me, since it only creates a single driver visible vif, if there are no non-monitor vifs on the phy.
I want to be able to control, which radios I want to capture on, regardless of which vifs are already active on the same phy.
A global monitor enable/disable status means that I can’t prevent monitor-incompatible offloads from being disabled on radios that I’m not monitoring on.

- Felix

Re: [RFC 3/6] wifi: mac80211: notify driver about per-radio monitor enabled state

[Johannes Berg]

On Fri, 2024-08-23 at 13:26 +0200, Felix Fietkau wrote:
> 
> > On 23. Aug 2024, at 12:16, Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> > > This allows monitoring on one or more radios while minimizing performance
> > > impact on the others.
> > 
> > But why are you doing it this way? You could already solve this entirely
> > with the driver by setting WANT_MONITOR_VIF and dealing with that, I'd
> > think? At least after this series.
> > 
> > I generally don't like hw->conf, it just hasn't really matched reality
> > for years with all kinds of new concurrency capabilities. At the very
> > least you'd have to write more text here to convince me that we want to
> > add something to it ... :)
> 
> I really don’t see how WANT_MONITOR_VIF helps. It seems completely unrelated to me, since it only creates a single driver visible vif, if there are no non-monitor vifs on the phy.

Well, it's true that it only creates one towards the driver, but that
one vif can also only be bound to a single channel context, and
therefore a single radio.

If we actually want(ed) to support monitoring on different radios
simultaneously we'd have to change mac80211 quite a bit, and probably
introduce multiple virtual monitor interfaces. Internally, we _always_
have it now, to be able to bind a channel context, so we'd actually need
multiple - one for each possible parallel channel.

So that's why I think having WANT_MONITOR_VIF helps - you can assume
today that only one chanctx can be used for monitoring, and once you
have the monitor vif in hand, you know which one it is. Therefore you
know which radio it is, and can adjust your offloads/etc. accordingly.

> I want to be able to control, which radios I want to capture on, regardless of which vifs are already active on the same phy.

Sure.

> A global monitor enable/disable status means that I can’t prevent monitor-incompatible offloads from being disabled on radios that I’m not monitoring on.
> 

Yeah I'd just say don't use that state, but the presence of the monitor
vif, and you can figure out which radio it's present on.

johannes

Re: [RFC 3/6] wifi: mac80211: notify driver about per-radio monitor enabled state

[Ben Greear]

On 9/17/24 01:13, Johannes Berg wrote:
> On Fri, 2024-08-23 at 13:26 +0200, Felix Fietkau wrote:
>>
>>> On 23. Aug 2024, at 12:16, Johannes Berg <johannes@sipsolutions.net> wrote:
>>>
>>> On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
>>>> This allows monitoring on one or more radios while minimizing performance
>>>> impact on the others.
>>>
>>> But why are you doing it this way? You could already solve this entirely
>>> with the driver by setting WANT_MONITOR_VIF and dealing with that, I'd
>>> think? At least after this series.
>>>
>>> I generally don't like hw->conf, it just hasn't really matched reality
>>> for years with all kinds of new concurrency capabilities. At the very
>>> least you'd have to write more text here to convince me that we want to
>>> add something to it ... :)
>>
>> I really don’t see how WANT_MONITOR_VIF helps. It seems completely unrelated to me, since it only creates a single driver visible vif, if there are no non-monitor vifs on the phy.
> 
> Well, it's true that it only creates one towards the driver, but that
> one vif can also only be bound to a single channel context, and
> therefore a single radio.
> 
> If we actually want(ed) to support monitoring on different radios
> simultaneously we'd have to change mac80211 quite a bit, and probably
> introduce multiple virtual monitor interfaces. Internally, we _always_
> have it now, to be able to bind a channel context, so we'd actually need
> multiple - one for each possible parallel channel.

I definitely want to be able to sniff on the individual underlying channels,
and not have the traffic from the different underlying radios mixed (unless
specifically requesting that feature somehow).

 From user-space perspective, having multiple monitor vifs seems the way
to do this, as that is how it works now.

Thanks,
Ben

> So that's why I think having WANT_MONITOR_VIF helps - you can assume
> today that only one chanctx can be used for monitoring, and once you
> have the monitor vif in hand, you know which one it is. Therefore you
> know which radio it is, and can adjust your offloads/etc. accordingly.
> 
>> I want to be able to control, which radios I want to capture on, regardless of which vifs are already active on the same phy.
> 
> Sure.
> 
>> A global monitor enable/disable status means that I can’t prevent monitor-incompatible offloads from being disabled on radios that I’m not monitoring on.
>>
> 
> Yeah I'd just say don't use that state, but the presence of the monitor
> vif, and you can figure out which radio it's present on.
> 
> johannes
> 

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

[RFC 4/6] wifi: mac80211: support per-radio driver start/stop calls

[Felix Fietkau]

Radios are started/stopped based on the vif allowed radios. This allows
drivers to keep unused radios inactive.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h     | 13 +++++++++++++
 net/mac80211/driver-ops.h  | 32 ++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/iface.c       | 37 +++++++++++++++++++++++++++++++++++--
 net/mac80211/trace.h       | 10 ++++++++++
 5 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7bee2d912efe..d0dcd66e565e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3827,6 +3827,17 @@ struct ieee80211_prep_tx_info {
  *	you should ensure to cancel it on this callback.
  *	Must be implemented and can sleep.
  *
+ * @start_radio: Called before the first netdevice attached to the radio
+ *	is enabled. This should turn on the per-radio hardware and must turn
+ *	on frame reception (for possibly enabled monitor interfaces.)
+ *	Returns negative error codes, these may be seen in userspace,
+ *	or zero. It can sleep.
+ *
+ * @stop_radio: Called after last netdevice attached to the radio
+ *	is disabled. This should turn off the per-radio hardware.
+ *	May be called right after add_interface if that rejects
+ *	an interface. It can sleep.
+ *
  * @suspend: Suspend the device; mac80211 itself will quiesce before and
  *	stop transmitting and doing any other configuration, and then
  *	ask the device to suspend. This is only invoked when WoWLAN is
@@ -4440,6 +4451,8 @@ struct ieee80211_ops {
 		   struct sk_buff *skb);
 	int (*start)(struct ieee80211_hw *hw);
 	void (*stop)(struct ieee80211_hw *hw, bool suspend);
+	int (*start_radio)(struct ieee80211_hw *hw, int idx);
+	void (*stop_radio)(struct ieee80211_hw *hw, int idx);
 #ifdef CONFIG_PM
 	int (*suspend)(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan);
 	int (*resume)(struct ieee80211_hw *hw);
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index d382d9729e85..cedc12b98bbb 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -90,6 +90,38 @@ static inline int drv_get_et_sset_count(struct ieee80211_sub_if_data *sdata,
 int drv_start(struct ieee80211_local *local);
 void drv_stop(struct ieee80211_local *local, bool suspend);
 
+static inline int drv_start_radio(struct ieee80211_local *local, u32 idx)
+{
+	int ret;
+
+	might_sleep();
+	lockdep_assert_wiphy(local->hw.wiphy);
+
+	if (!local->ops->start_radio || local->radio_data[idx].started)
+	    return 0;
+
+	trace_drv_start_radio(local, idx);
+	ret = local->ops->start_radio(&local->hw, idx);
+	if (!ret)
+		local->radio_data[idx].started = true;
+	trace_drv_return_int(local, ret);
+	return ret;
+}
+
+static inline void drv_stop_radio(struct ieee80211_local *local, u32 idx)
+{
+	might_sleep();
+	lockdep_assert_wiphy(local->hw.wiphy);
+
+	if (!local->ops->stop_radio || !local->radio_data[idx].started)
+	    return;
+
+	trace_drv_stop_radio(local, idx);
+	local->ops->stop_radio(&local->hw, idx);
+	local->radio_data[idx].started = false;
+	trace_drv_return_void(local);
+}
+
 #ifdef CONFIG_PM
 static inline int drv_suspend(struct ieee80211_local *local,
 			      struct cfg80211_wowlan *wowlan)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3be9f8149117..acc1a2d0f30f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1332,6 +1332,8 @@ DECLARE_STATIC_KEY_FALSE(aql_disable);
 
 struct ieee80211_radio_data {
 	u32 monitors;
+	u32 open_count;
+	bool started;
 };
 
 struct ieee80211_local {
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 4db867ae97f0..20a4a19c8ba1 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -578,8 +578,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 		spin_unlock_irqrestore(&ps->bc_buf.lock, flags);
 	}
 
-	if (going_down)
+	if (going_down) {
+		for (i = 0; i < local->hw.wiphy->n_radio; i++)
+			if (sdata->wdev.radio_mask & BIT(i))
+				local->radio_data[i].open_count--;
 		local->open_count--;
+	}
 
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP_VLAN:
@@ -714,6 +718,14 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 	if (cancel_scan)
 		wiphy_delayed_work_flush(local->hw.wiphy, &local->scan_work);
 
+	for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+		if (!(sdata->wdev.radio_mask & BIT(i)) ||
+		    local->radio_data[i].open_count)
+			continue;
+
+		drv_stop_radio(local, i);
+	}
+
 	if (local->open_count == 0) {
 		ieee80211_stop_device(local, false);
 
@@ -1294,6 +1306,16 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 					   IEEE80211_TPT_LEDTRIG_FL_RADIO, 0);
 	}
 
+	for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+		if (!(sdata->wdev.radio_mask & BIT(i)) ||
+		    local->radio_data[i].open_count)
+			continue;
+
+		res = drv_start_radio(local, i);
+		if (res)
+			goto err_stop;
+	}
+
 	/*
 	 * Copy the hopefully now-present MAC address to
 	 * this interface, if it has the special null one.
@@ -1444,8 +1466,15 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 	if (sdata->flags & IEEE80211_SDATA_ALLMULTI)
 		atomic_inc(&local->iff_allmultis);
 
-	if (coming_up)
+	if (coming_up) {
 		local->open_count++;
+		for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+			if (!(sdata->wdev.radio_mask & BIT(i)))
+				continue;
+
+			local->radio_data[i].open_count++;
+		}
+	}
 
 	if (local->open_count == 1)
 		ieee80211_hw_conf_init(local);
@@ -1462,6 +1491,10 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
  err_stop:
 	if (!local->open_count)
 		drv_stop(local, false);
+	for (i = 0; i < local->hw.wiphy->n_radio; i++)
+		if ((sdata->wdev.radio_mask & BIT(i)) &&
+		    !local->radio_data[i].open_count)
+			drv_stop_radio(local, i);
  err_del_bss:
 	sdata->bss = NULL;
 	if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index dc498cd8cd91..0b6b4ebc681a 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -342,6 +342,16 @@ TRACE_EVENT(drv_stop,
 	TP_printk(LOCAL_PR_FMT " suspend:%d", LOCAL_PR_ARG, __entry->suspend)
 );
 
+DEFINE_EVENT(local_u32_evt, drv_start_radio,
+	     TP_PROTO(struct ieee80211_local *local, u32 idx),
+	     TP_ARGS(local, idx)
+);
+
+DEFINE_EVENT(local_u32_evt, drv_stop_radio,
+	     TP_PROTO(struct ieee80211_local *local, u32 idx),
+	     TP_ARGS(local, idx)
+);
+
 DEFINE_EVENT(local_sdata_addr_evt, drv_add_interface,
 	TP_PROTO(struct ieee80211_local *local,
 		 struct ieee80211_sub_if_data *sdata),
-- 
git-series 0.9.1

Re: [RFC 4/6] wifi: mac80211: support per-radio driver start/stop calls

[Johannes Berg]

On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> Radios are started/stopped based on the vif allowed radios. This allows
> drivers to keep unused radios inactive.

Similar argument here, I'd think you don't need this with
WANT_MONITOR_VIF. Is this really something we want? Why?

johannes

Re: [RFC 4/6] wifi: mac80211: support per-radio driver start/stop calls

[Felix Fietkau]

> On 23. Aug 2024, at 12:17, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
>> Radios are started/stopped based on the vif allowed radios. This allows
>> drivers to keep unused radios inactive.
> 
> Similar argument here, I'd think you don't need this with
> WANT_MONITOR_VIF. Is this really something we want? Why?

I want to keep radios shut down when no scan, roc or normal operation can happen on them (based on allowed radios mask).
Since radio enable/disable can take some time, I don’t want to toggle it for every single offchannel operation or implement timer bases hacks to work around it.

- Felix

Re: [RFC 4/6] wifi: mac80211: support per-radio driver start/stop calls

[Johannes Berg]

On Fri, 2024-08-23 at 13:31 +0200, Felix Fietkau wrote:
> 
> > On 23. Aug 2024, at 12:17, Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> > > Radios are started/stopped based on the vif allowed radios. This allows
> > > drivers to keep unused radios inactive.
> > 
> > Similar argument here, I'd think you don't need this with
> > WANT_MONITOR_VIF. Is this really something we want? Why?
> 
> I want to keep radios shut down when no scan, roc or normal operation can happen on them (based on allowed radios mask).

Makes sense I guess, but do you really need mac80211 helping with that?

You already know which vifs are enabled and which aren't, and you can
also access their radio mask if you want to. Not sure I see the added
value of mac80211 keeping track of it. Worst case iterate the existing
vifs when removing one?

This is only needed/useful when there are multiple "real" devices and
you don't have anything else managing them, so I kind of see this as a
corner case, but perhaps I'm wrong? 

johannes

[RFC 5/6] wifi: mac80211: support per-radio filter flags

[Felix Fietkau]

This allows drivers to improve filtering of unwanted packets when using
different combinations of filter/monitor settings on radios.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h     |  8 +++-
 net/mac80211/driver-ops.h  | 18 +++++++-
 net/mac80211/ieee80211_i.h | 21 ++++++---
 net/mac80211/iface.c       | 95 ++++++++++++++++++++++++---------------
 net/mac80211/main.c        | 60 +++++++++++++++++--------
 net/mac80211/mesh.c        | 33 ++++++++++----
 net/mac80211/trace.h       | 28 +++++++++++-
 7 files changed, 196 insertions(+), 67 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d0dcd66e565e..7a5418713dfc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3921,6 +3921,10 @@ struct ieee80211_prep_tx_info {
  *	See the section "Frame filtering" for more information.
  *	This callback must be implemented and can sleep.
  *
+ * @config_radio_filter: Configure the radio's RX filter.
+ *	See the section "Frame filtering" for more information.
+ *	This callback is optional and can sleep.
+ *
  * @config_iface_filter: Configure the interface's RX filter.
  *	This callback is optional and is used to configure which frames
  *	should be passed to mac80211. The filter_flags is the combination
@@ -4489,6 +4493,10 @@ struct ieee80211_ops {
 				 unsigned int changed_flags,
 				 unsigned int *total_flags,
 				 u64 multicast);
+	void (*config_radio_filter)(struct ieee80211_hw *hw,
+				    unsigned int radio_idx,
+				    unsigned int changed_flags,
+				    unsigned int *total_flags);
 	void (*config_iface_filter)(struct ieee80211_hw *hw,
 				    struct ieee80211_vif *vif,
 				    unsigned int filter_flags,
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index cedc12b98bbb..4ee1d78b91c1 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -242,6 +242,24 @@ static inline void drv_configure_filter(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline void drv_config_radio_filter(struct ieee80211_local *local,
+					   unsigned int radio_idx,
+					   unsigned int changed_flags,
+					   unsigned int *total_flags)
+{
+	might_sleep();
+	lockdep_assert_wiphy(local->hw.wiphy);
+
+	if (!local->ops->config_radio_filter)
+	    return;
+
+	trace_drv_config_radio_filter(local, radio_idx, changed_flags,
+				      total_flags);
+	local->ops->config_radio_filter(&local->hw, radio_idx, changed_flags,
+					total_flags);
+	trace_drv_return_void(local);
+}
+
 static inline void drv_config_iface_filter(struct ieee80211_local *local,
 					   struct ieee80211_sub_if_data *sdata,
 					   unsigned int filter_flags,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index acc1a2d0f30f..55bb328b9c70 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1330,7 +1330,19 @@ enum mac80211_scan_state {
 
 DECLARE_STATIC_KEY_FALSE(aql_disable);
 
+struct ieee80211_radio_filter {
+	/* number of interfaces with corresponding FIF_ flags */
+	int fif_fcsfail, fif_plcpfail, fif_control, fif_other_bss, fif_pspoll,
+	    fif_probe_req;
+
+	/* number of interfaces with allmulti RX */
+	atomic_t iff_allmultis;
+
+	unsigned int filter_flags; /* FIF_* */
+};
+
 struct ieee80211_radio_data {
+	struct ieee80211_radio_filter filter;
 	u32 monitors;
 	u32 open_count;
 	bool started;
@@ -1378,12 +1390,10 @@ struct ieee80211_local {
 
 	int open_count;
 	int monitors, cooked_mntrs;
-	/* number of interfaces with corresponding FIF_ flags */
-	int fif_fcsfail, fif_plcpfail, fif_control, fif_other_bss, fif_pspoll,
-	    fif_probe_req;
+	struct ieee80211_radio_filter filter;
+
 	bool probe_req_reg;
 	bool rx_mcast_action_reg;
-	unsigned int filter_flags; /* FIF_* */
 
 	bool wiphy_ciphers_allocated;
 
@@ -1478,9 +1488,6 @@ struct ieee80211_local {
 
 	atomic_t agg_queue_stop[IEEE80211_MAX_QUEUES];
 
-	/* number of interfaces with allmulti RX */
-	atomic_t iff_allmultis;
-
 	struct rate_control_ref *rate_ctrl;
 
 	struct arc4_ctx wep_tx_ctx;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 20a4a19c8ba1..b7617f98cef6 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -458,6 +458,39 @@ static int ieee80211_open(struct net_device *dev)
 	return err;
 }
 
+static bool
+__ieee80211_sdata_set_filter(struct ieee80211_radio_filter *filter,
+			     struct ieee80211_sub_if_data *sdata, int ofs)
+{
+	if (sdata->flags & IEEE80211_SDATA_ALLMULTI)
+		atomic_add(ofs, &filter->iff_allmultis);
+
+	if (sdata->vif.type == NL80211_IFTYPE_AP) {
+		filter->fif_pspoll += ofs;
+		filter->fif_probe_req += ofs;
+	} else if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
+		filter->fif_probe_req += ofs;
+	} else {
+		return false;
+	}
+
+	return true;
+}
+
+static bool
+ieee80211_sdata_set_filter(struct ieee80211_sub_if_data *sdata, int ofs)
+{
+	struct ieee80211_local *local = sdata->local;
+	int i;
+
+	for (i = 0; i < local->hw.wiphy->n_radio; i++)
+		if (sdata->wdev.radio_mask & BIT(i))
+			__ieee80211_sdata_set_filter(&local->radio_data[i].filter,
+						     sdata, ofs);
+
+	return __ieee80211_sdata_set_filter(&local->filter, sdata, ofs);
+}
+
 static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_down)
 {
 	struct ieee80211_local *local = sdata->local;
@@ -514,16 +547,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 	flushed = sta_info_flush(sdata, -1);
 	WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_AP_VLAN && flushed > 0);
 
-	/* don't count this interface for allmulti while it is down */
-	if (sdata->flags & IEEE80211_SDATA_ALLMULTI)
-		atomic_dec(&local->iff_allmultis);
-
-	if (sdata->vif.type == NL80211_IFTYPE_AP) {
-		local->fif_pspoll--;
-		local->fif_probe_req--;
-	} else if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
-		local->fif_probe_req--;
-	}
+	ieee80211_sdata_set_filter(sdata, -1);
 
 	if (sdata->dev) {
 		netif_addr_lock_bh(sdata->dev);
@@ -796,16 +820,22 @@ static void ieee80211_set_multicast_list(struct net_device *dev)
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_radio_filter *filter = &local->filter;
 	int allmulti, sdata_allmulti;
+	int i, ofs;
 
 	allmulti = !!(dev->flags & IFF_ALLMULTI);
 	sdata_allmulti = !!(sdata->flags & IEEE80211_SDATA_ALLMULTI);
 
 	if (allmulti != sdata_allmulti) {
-		if (dev->flags & IFF_ALLMULTI)
-			atomic_inc(&local->iff_allmultis);
-		else
-			atomic_dec(&local->iff_allmultis);
+		ofs = (dev->flags & IFF_ALLMULTI) ? 1 : -1;
+		atomic_add(ofs, &filter->iff_allmultis);
+		for (i = 0; i < local->hw.wiphy->n_radio; i++) {
+			if (!(sdata->wdev.radio_mask & BIT(i)))
+				continue;
+			filter = &local->radio_data[i].filter;
+			atomic_add(ofs, &filter->iff_allmultis);
+		}
 		sdata->flags ^= IEEE80211_SDATA_ALLMULTI;
 	}
 
@@ -1080,15 +1110,16 @@ void ieee80211_recalc_offload(struct ieee80211_local *local)
 	}
 }
 
-void ieee80211_adjust_monitor_flags(struct ieee80211_sub_if_data *sdata,
-				    const int offset)
+static void
+__ieee80211_adjust_monitor_flags(struct ieee80211_radio_filter *filter,
+				 struct ieee80211_sub_if_data *sdata,
+				 const int offset)
 {
-	struct ieee80211_local *local = sdata->local;
 	u32 flags = sdata->u.mntr.flags;
 
 #define ADJUST(_f, _s)	do {					\
 	if (flags & MONITOR_FLAG_##_f)				\
-		local->fif_##_s += offset;			\
+		filter->fif_##_s += offset;			\
 	} while (0)
 
 	ADJUST(FCSFAIL, fcsfail);
@@ -1100,6 +1131,14 @@ void ieee80211_adjust_monitor_flags(struct ieee80211_sub_if_data *sdata,
 #undef ADJUST
 }
 
+void ieee80211_adjust_monitor_flags(struct ieee80211_sub_if_data *sdata,
+				    const int offset)
+{
+	struct ieee80211_local *local = sdata->local;
+
+	__ieee80211_adjust_monitor_flags(&local->filter, sdata, offset);
+}
+
 static void ieee80211_set_default_queues(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
@@ -1399,15 +1438,6 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 				goto err_del_interface;
 		}
 
-		if (sdata->vif.type == NL80211_IFTYPE_AP) {
-			local->fif_pspoll++;
-			local->fif_probe_req++;
-
-			ieee80211_configure_filter(local);
-		} else if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
-			local->fif_probe_req++;
-		}
-
 		if (sdata->vif.probe_req_reg)
 			drv_config_iface_filter(local, sdata,
 						FIF_PROBE_REQ,
@@ -1445,6 +1475,9 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 			sdata->vif.type != NL80211_IFTYPE_STATION);
 	}
 
+	if (ieee80211_sdata_set_filter(sdata, 1))
+		ieee80211_configure_filter(local);
+
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_P2P_DEVICE:
 		rcu_assign_pointer(local->p2p_sdata, sdata);
@@ -1458,14 +1491,6 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 		break;
 	}
 
-	/*
-	 * set_multicast_list will be invoked by the networking core
-	 * which will check whether any increments here were done in
-	 * error and sync them down to the hardware as filter flags.
-	 */
-	if (sdata->flags & IEEE80211_SDATA_ALLMULTI)
-		atomic_inc(&local->iff_allmultis);
-
 	if (coming_up) {
 		local->open_count++;
 		for (i = 0; i < local->hw.wiphy->n_radio; i++) {
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 2ce771744ea8..ce2ed4c06a8c 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -34,54 +34,80 @@
 #include "led.h"
 #include "debugfs.h"
 
-void ieee80211_configure_filter(struct ieee80211_local *local)
+static void
+__ieee80211_configure_filter(struct ieee80211_local *local,
+			   int radio_idx)
 {
-	u64 mc;
+	struct ieee80211_radio_filter *filter;
 	unsigned int changed_flags;
 	unsigned int new_flags = 0;
+	u32 monitors;
+	u64 mc;
 
-	if (atomic_read(&local->iff_allmultis))
+	if (radio_idx >= 0) {
+		filter = &local->radio_data[radio_idx].filter;
+		monitors = local->radio_data[radio_idx].monitors;
+	} else {
+		filter = &local->filter;
+		monitors = local->monitors;
+	}
+
+	if (atomic_read(&filter->iff_allmultis))
 		new_flags |= FIF_ALLMULTI;
 
-	if (local->monitors || test_bit(SCAN_SW_SCANNING, &local->scanning) ||
+	if (monitors || test_bit(SCAN_SW_SCANNING, &local->scanning) ||
 	    test_bit(SCAN_ONCHANNEL_SCANNING, &local->scanning))
 		new_flags |= FIF_BCN_PRBRESP_PROMISC;
 
-	if (local->fif_probe_req || local->probe_req_reg)
+	if (filter->fif_probe_req || local->probe_req_reg)
 		new_flags |= FIF_PROBE_REQ;
 
-	if (local->fif_fcsfail)
+	if (filter->fif_fcsfail)
 		new_flags |= FIF_FCSFAIL;
 
-	if (local->fif_plcpfail)
+	if (filter->fif_plcpfail)
 		new_flags |= FIF_PLCPFAIL;
 
-	if (local->fif_control)
+	if (filter->fif_control)
 		new_flags |= FIF_CONTROL;
 
-	if (local->fif_other_bss)
+	if (filter->fif_other_bss)
 		new_flags |= FIF_OTHER_BSS;
 
-	if (local->fif_pspoll)
+	if (filter->fif_pspoll)
 		new_flags |= FIF_PSPOLL;
 
 	if (local->rx_mcast_action_reg)
 		new_flags |= FIF_MCAST_ACTION;
 
 	spin_lock_bh(&local->filter_lock);
-	changed_flags = local->filter_flags ^ new_flags;
+	changed_flags = filter->filter_flags ^ new_flags;
 
-	mc = drv_prepare_multicast(local, &local->mc_list);
+	if (radio_idx < 0)
+		mc = drv_prepare_multicast(local, &local->mc_list);
 	spin_unlock_bh(&local->filter_lock);
 
-	/* be a bit nasty */
-	new_flags |= (1<<31);
 
-	drv_configure_filter(local, changed_flags, &new_flags, mc);
+	if (radio_idx < 0) {
+		/* be a bit nasty */
+		new_flags |= (1<<31);
+
+		drv_configure_filter(local, changed_flags, &new_flags, mc);
 
-	WARN_ON(new_flags & (1<<31));
+		WARN_ON(new_flags & (1<<31));
+	} else {
+		drv_config_radio_filter(local, radio_idx, changed_flags, &new_flags);
+	}
+
+	filter->filter_flags = new_flags & ~(1<<31);
+}
+
+void ieee80211_configure_filter(struct ieee80211_local *local)
+{
+	int i;
 
-	local->filter_flags = new_flags & ~(1<<31);
+	for (i = -1; i < local->hw.wiphy->n_radio; i++)
+		__ieee80211_configure_filter(local, i);
 }
 
 static void ieee80211_reconfig_filter(struct wiphy *wiphy,
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index f94e4be0be12..db4e6028074f 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1170,6 +1170,29 @@ void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
 	wiphy_work_queue(sdata->local->hw.wiphy, &sdata->work);
 }
 
+static void
+__ieee80211_mesh_set_filter(struct ieee80211_radio_filter *filter, int ofs)
+{
+	filter->fif_other_bss += ofs;
+
+	/* mesh ifaces must set allmulti to forward mcast traffic */
+	atomic_add(ofs, &filter->iff_allmultis);
+}
+
+static void
+ieee80211_mesh_set_filter(struct ieee80211_sub_if_data *sdata, int ofs)
+{
+	struct ieee80211_local *local = sdata->local;
+	int i;
+
+	__ieee80211_mesh_set_filter(&local->filter, ofs);
+	for (i = 0; i < local->hw.wiphy->n_radio; i++)
+		if (sdata->wdev.radio_mask & BIT(i))
+			__ieee80211_mesh_set_filter(&local->radio_data[i].filter,
+						    ofs);
+	ieee80211_configure_filter(local);
+}
+
 int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
@@ -1181,11 +1204,7 @@ int ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
 		      BSS_CHANGED_BEACON_INT |
 		      BSS_CHANGED_MCAST_RATE;
 
-	local->fif_other_bss++;
-	/* mesh ifaces must set allmulti to forward mcast traffic */
-	atomic_inc(&local->iff_allmultis);
-	ieee80211_configure_filter(local);
-
+	ieee80211_mesh_set_filter(sdata, 1);
 	ifmsh->mesh_cc_id = 0;	/* Disabled */
 	/* register sync ops from extensible synchronization framework */
 	ifmsh->sync_ops = ieee80211_mesh_sync_ops_get(ifmsh->mesh_sp_id);
@@ -1249,9 +1268,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
 	ifmsh->wrkq_flags = 0;
 	memset(ifmsh->mbss_changed, 0, sizeof(ifmsh->mbss_changed));
 
-	local->fif_other_bss--;
-	atomic_dec(&local->iff_allmultis);
-	ieee80211_configure_filter(local);
+	ieee80211_mesh_set_filter(sdata, -1);
 }
 
 static void ieee80211_mesh_csa_mark_radar(struct ieee80211_sub_if_data *sdata)
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 0b6b4ebc681a..6504935b11e3 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -610,6 +610,34 @@ TRACE_EVENT(drv_configure_filter,
 	)
 );
 
+TRACE_EVENT(drv_config_radio_filter,
+	TP_PROTO(struct ieee80211_local *local,
+		 unsigned int radio,
+		 unsigned int changed_flags,
+		 unsigned int *total_flags),
+
+	TP_ARGS(local, radio, changed_flags, total_flags),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(unsigned int, radio)
+		__field(unsigned int, changed)
+		__field(unsigned int, total)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->radio = radio;
+		__entry->changed = changed_flags;
+		__entry->total = *total_flags;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " radio:%d, changed:%#x total:%#x",
+		LOCAL_PR_ARG, __entry->radio, __entry->changed, __entry->total
+	)
+);
+
 TRACE_EVENT(drv_config_iface_filter,
 	TP_PROTO(struct ieee80211_local *local,
 		 struct ieee80211_sub_if_data *sdata,
-- 
git-series 0.9.1

Re: [RFC 5/6] wifi: mac80211: support per-radio filter flags

[Johannes Berg]

On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> This allows drivers to improve filtering of unwanted packets when using
> different combinations of filter/monitor settings on radios.
> 

Here, it would seem to make more sense to simply give the necessary per-
interface information to the driver? Drivers likely have more
restrictions and will need less complex data structures (or just
iteration) to figure out what's eventually needed for each hw etc.

Some drivers might anyway even have different filters per interface,
even if they're operating on the same channel.

johannes

Re: [RFC 5/6] wifi: mac80211: support per-radio filter flags

[Felix Fietkau]

On 23.08.24 12:20, Johannes Berg wrote:
> On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
>> This allows drivers to improve filtering of unwanted packets when using
>> different combinations of filter/monitor settings on radios.
>> 
> 
> Here, it would seem to make more sense to simply give the necessary per-
> interface information to the driver? Drivers likely have more
> restrictions and will need less complex data structures (or just
> iteration) to figure out what's eventually needed for each hw etc.
> 
> Some drivers might anyway even have different filters per interface,
> even if they're operating on the same channel.

I can do that as well. However, I do think it makes sense to have 
per-radio tracking in mac80211 as well, because I'm pretty sure that not 
just mt76, but ath12k would also be able to make use of this.

- Felix

Re: [RFC 5/6] wifi: mac80211: support per-radio filter flags

[Johannes Berg]

On Fri, 2024-08-23 at 18:26 +0200, Felix Fietkau wrote:
> On 23.08.24 12:20, Johannes Berg wrote:
> > On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> > > This allows drivers to improve filtering of unwanted packets when using
> > > different combinations of filter/monitor settings on radios.
> > > 
> > 
> > Here, it would seem to make more sense to simply give the necessary per-
> > interface information to the driver? Drivers likely have more
> > restrictions and will need less complex data structures (or just
> > iteration) to figure out what's eventually needed for each hw etc.
> > 
> > Some drivers might anyway even have different filters per interface,
> > even if they're operating on the same channel.
> 
> I can do that as well. However, I do think it makes sense to have 
> per-radio tracking in mac80211 as well, because I'm pretty sure that not 
> just mt76, but ath12k would also be able to make use of this.

But if you have per-link, deriving per-radio is effectively trivial, so
I don't see why we should do the extra work to track it?

johannes

[RFC 6/6] wifi: mac80211: check vif radio_mask for monitor mode rx

[Felix Fietkau]

When restricting a monitor vif to only operate on a specific set of radios,
filter out rx packets belonging to other radios. This only works if drivers
fill in radio_valid and radio_idx in the rx status.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 include/net/mac80211.h |  3 ++-
 net/mac80211/rx.c      | 58 +++++++++++++++++++++++--------------------
 2 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7a5418713dfc..6222f4f44ac2 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1619,6 +1619,8 @@ enum mac80211_rx_encoding {
  * @ampdu_reference: A-MPDU reference number, must be a different value for
  *	each A-MPDU but the same for each subframe within one A-MPDU
  * @zero_length_psdu_type: radiotap type of the 0-length PSDU
+ * @radio_valid: if the index of the radio in radio_idx is valid.
+ * @radio_idx: index of the wiphy radio that the frame was received on.
  * @link_valid: if the link which is identified by @link_id is valid. This flag
  *	is set only when connection is MLO.
  * @link_id: id of the link used to receive the packet. This is used along with
@@ -1656,6 +1658,7 @@ struct ieee80211_rx_status {
 	u8 chains;
 	s8 chain_signal[IEEE80211_MAX_CHAINS];
 	u8 zero_length_psdu_type;
+	u8 radio_valid:1, radio_idx:5;
 	u8 link_valid:1, link_id:4;
 };
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 718f02f0a181..de4196fa3eb5 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -762,8 +762,8 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 		     struct ieee80211_rate *rate)
 {
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(origskb);
-	struct ieee80211_sub_if_data *sdata;
-	struct sk_buff *monskb = NULL;
+	struct ieee80211_sub_if_data *sdata, *prev_sdata = NULL;
+	struct sk_buff *skb, *monskb = NULL;
 	int present_fcs_len = 0;
 	unsigned int rtap_space = 0;
 	struct ieee80211_sub_if_data *monitor_sdata =
@@ -837,40 +837,46 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 	ieee80211_handle_mu_mimo_mon(monitor_sdata, origskb, rtap_space);
 
 	list_for_each_entry_rcu(sdata, &local->mon_list, u.mntr.list) {
-		bool last_monitor = list_is_last(&sdata->u.mntr.list,
-						 &local->mon_list);
+		if (status->radio_valid &&
+		    !(sdata->wdev.radio_mask & BIT(status->radio_idx)))
+			continue;
+
+		if (!prev_sdata) {
+			prev_sdata = sdata;
+			continue;
+		}
 
 		if (!monskb)
 			monskb = ieee80211_make_monitor_skb(local, &origskb,
 							    rate, rtap_space,
-							    only_monitor &&
-							    last_monitor);
+							    false);
+		if (!monskb)
+			continue;
 
-		if (monskb) {
-			struct sk_buff *skb;
+		skb = skb_clone(monskb, GFP_ATOMIC);
+		if (!skb)
+			continue;
 
-			if (last_monitor) {
-				skb = monskb;
-				monskb = NULL;
-			} else {
-				skb = skb_clone(monskb, GFP_ATOMIC);
-			}
+		skb->dev = prev_sdata->dev;
+		dev_sw_netstats_rx_add(skb->dev, skb->len);
+		netif_receive_skb(skb);
+		prev_sdata = sdata;
+	}
 
-			if (skb) {
-				skb->dev = sdata->dev;
-				dev_sw_netstats_rx_add(skb->dev, skb->len);
-				netif_receive_skb(skb);
-			}
+	if (prev_sdata) {
+		if (monskb)
+			skb = monskb;
+		else
+			skb = ieee80211_make_monitor_skb(local, &origskb,
+							 rate, rtap_space,
+							 only_monitor);
+		if (skb) {
+			skb->dev = prev_sdata->dev;
+			dev_sw_netstats_rx_add(skb->dev, skb->len);
+			netif_receive_skb(skb);
 		}
-
-		if (last_monitor)
-			break;
 	}
 
-	/* this happens if last_monitor was erroneously false */
-	dev_kfree_skb(monskb);
-
-	/* ditto */
 	if (!origskb)
 		return NULL;
 
-- 
git-series 0.9.1

Re: [RFC 6/6] wifi: mac80211: check vif radio_mask for monitor mode rx

[Johannes Berg]

On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> When restricting a monitor vif to only operate on a specific set of radios,
> filter out rx packets belonging to other radios. This only works if drivers
> fill in radio_valid and radio_idx in the rx status.

Why does the driver need to provide the radio, it already provides the
frequency?

But then I wonder if this doesn't go a step too far? This is pretty much
pretending that monitor only exists on a specific sub-radio, but ...
what for? Even userspace could filter on the frequency.

I mean ... I get that you're trying to preserve a notion that you had
that an interface exists on a given PHY and they're all separate, but
they're not really separate any more, get used to it?

johannes

Re: [RFC 6/6] wifi: mac80211: check vif radio_mask for monitor mode rx

[Felix Fietkau]

On 23.08.24 12:23, Johannes Berg wrote:
> On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
>> When restricting a monitor vif to only operate on a specific set of radios,
>> filter out rx packets belonging to other radios. This only works if drivers
>> fill in radio_valid and radio_idx in the rx status.
> 
> Why does the driver need to provide the radio, it already provides the
> frequency?
> 
> But then I wonder if this doesn't go a step too far? This is pretty much
> pretending that monitor only exists on a specific sub-radio, but ...
> what for? Even userspace could filter on the frequency.
> 
> I mean ... I get that you're trying to preserve a notion that you had
> that an interface exists on a given PHY and they're all separate, but
> they're not really separate any more, get used to it?

Well, there's a performance aspect as well. When only monitoring 
specific radios (while operating normally on others), relying on 
filtering in user space or even BPF comes at a cost, since mac80211 
still has to prepare radiotap headers and potentially clone data packets 
received on other radios.

- Felix

Re: [RFC 6/6] wifi: mac80211: check vif radio_mask for monitor mode rx

[Johannes Berg]

On Fri, 2024-08-23 at 19:33 +0200, Felix Fietkau wrote:
> On 23.08.24 12:23, Johannes Berg wrote:
> > On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> > > When restricting a monitor vif to only operate on a specific set of radios,
> > > filter out rx packets belonging to other radios. This only works if drivers
> > > fill in radio_valid and radio_idx in the rx status.
> > 
> > Why does the driver need to provide the radio, it already provides the
> > frequency?
> > 
> > But then I wonder if this doesn't go a step too far? This is pretty much
> > pretending that monitor only exists on a specific sub-radio, but ...
> > what for? Even userspace could filter on the frequency.
> > 
> > I mean ... I get that you're trying to preserve a notion that you had
> > that an interface exists on a given PHY and they're all separate, but
> > they're not really separate any more, get used to it?
> 
> Well, there's a performance aspect as well.

Which I'd firmly put into the "premature optimisation" basket at this
stage though.

johannes

Re: [RFC 0/6] wifi: cfg80211/mac80211: improve support for multiple radios

[Johannes Berg]

On Mon, 2024-08-05 at 21:23 +0200, Felix Fietkau wrote:
> This series adds support for restricting vifs to a set of radios.
> The allowed radios mask is used to restrict scanning, off-channel activity.
> This is also used for per-radio start/stop ops, monitor state and filter
> flags. It can also limit per-monitor-vif rx to specific radios.
> 

I see the idea, I guess, but but honestly I don't like the way this is
all mixed up between cfg80211 and mac80211.

I think more of this should be in cfg80211 (e.g. not sure why the
changes to ieee80211_change_iface, nothing there that cfg80211 couldn't
do; and generally would prefer to have the patch split), but since it
can never be perfect there (internal scans like in mac80211, etc.), I
guess it really also should come with a feature flag. That'd also make
the series more robust, with enabling the feature only in the last
commit.

johannes