mbox series

[00/10] cfg80211/mac80211: support defining multiple radios per wiphy

Message ID cover.a7730420cfba0f204a60e4c30e6b0e07b441ff6b.1718881762.git-series.nbd@nbd.name
Headers show
Series cfg80211/mac80211: support defining multiple radios per wiphy | expand

Message

Felix Fietkau June 20, 2024, 11:11 a.m. UTC
The prerequisite for MLO support in cfg80211/mac80211 is that all the links
participating in MLO must be from the same wiphy/ieee80211_hw. To meet this
expectation, some drivers may need to group multiple discrete hardware each
acting as a link in MLO under single wiphy.

With this change, supported frequencies and interface combinations of each
individual radio are reported to user space. This allows user space to figure
out the limitations of what combination of channels can be used concurrently.

Each mac80211 channel context is assigned to a radio based on radio specific
frequency ranges and interface combinations.

Even for non-MLO devices, this improves support for devices capable of
running on multiple channels at the same time.

This is loosely based on Karthikeyan Periyasamy's series
"[PATCH 00/13] wifi: Add multi physical hardware iface combination support"
with some differences:

 - a struct wiphy_radio is defined, which holds the frequency ranges and
   a full struct ieee80211_iface_combination array
 - a channel context is explicitly assigned to a radio when created
 - both global and per-radio interface combination limits are checked
   and enforced on channel context assignment
 - improve comments/docs and attributes
 - add cfg80211 helper for checking radio freq range

Changes since RFC v4:
 - report the first radio's ifcomb as main ifcomb for legacy compatibility
 - report the global wiphy ifcomb separately
 - add mac80211_hwsim support

Changes since RFC v3:
 - fix __ieee80211_get_radio_mask to return per-vif radio mask
 - fix params->radio in ieee80211_check_combinations()
 - fix indentation
 - pass radio_idx in struct iface_combination_params
 - improve get_radio_mask callback

Changes since RFC v2:
 - fix uninitialized variables
 - fix multiple radios with DFS
 - add support for per-radio beacon interval checking

Changes since RFC:
 - replace static per-radio number of channels limit with full ifcomb
   checks
 - remove band bitmask in favor of only using freq ranges

Felix Fietkau (10):
  wifi: nl80211: split helper function from nl80211_put_iface_combinations
  wifi: cfg80211: add support for advertising multiple radios belonging to a wiphy
  wifi: cfg80211: extend interface combination check for multi-radio
  wifi: cfg80211: add helper for checking if a chandef is valid on a radio
  wifi: mac80211: add support for DFS with multiple radios
  wifi: mac80211: add radio index to ieee80211_chanctx_conf
  wifi: mac80211: extend ifcomb check functions for multi-radio
  wifi: mac80211: move code in ieee80211_link_reserve_chanctx to a helper
  wifi: mac80211: add wiphy radio assignment and validation
  wifi: mac80211_hwsim: add support for multi-radio wiphy

 drivers/net/wireless/virtual/mac80211_hwsim.c |  77 ++++++-
 drivers/net/wireless/virtual/mac80211_hwsim.h |   3 +-
 include/net/cfg80211.h                        |  51 +++++-
 include/net/mac80211.h                        |   2 +-
 include/uapi/linux/nl80211.h                  |  61 ++++++-
 net/mac80211/cfg.c                            |   7 +-
 net/mac80211/chan.c                           | 210 +++++++++++--------
 net/mac80211/ibss.c                           |   2 +-
 net/mac80211/ieee80211_i.h                    |   5 +-
 net/mac80211/iface.c                          |   2 +-
 net/mac80211/main.c                           |  32 ++-
 net/mac80211/util.c                           | 127 +++++++----
 net/wireless/nl80211.c                        | 197 +++++++++++++-----
 net/wireless/rdev-ops.h                       |  17 ++-
 net/wireless/trace.h                          |   5 +-
 net/wireless/util.c                           |  68 +++++-
 16 files changed, 666 insertions(+), 200 deletions(-)

base-commit: 5bcd9a0a59953b5ff55ac226f903397319bf7f40

Comments

Johannes Berg June 28, 2024, 8:01 a.m. UTC | #1
On Thu, 2024-06-20 at 13:11 +0200, Felix Fietkau wrote:
> 
> +static int nl80211_put_ifcomb_data(struct sk_buff *msg, bool large, int idx,
> +				   const struct ieee80211_iface_combination *c)
>  {
> -	struct nlattr *nl_combis;
> -	int i, j;
> +	struct nlattr *nl_combi, *nl_limits;
> +	int i;
>  
> -	nl_combis = nla_nest_start_noflag(msg,
> -					  NL80211_ATTR_INTERFACE_COMBINATIONS);
> -	if (!nl_combis)
> +	nl_combi = nla_nest_start_noflag(msg, idx);
> +	if (!nl_combi)
>  		goto nla_put_failure;

Not sure why the diff looks so much different here between my git and
yours, but anyway ...

Even if it's ugly here, I do think you should add an argument to the
function

	..., u16 nested

pass 0 for the existing user, and NLA_F_NESTED for the later new users,
and use

	nla_nest_start_noflag(msg, idx | nested);

etc. for all the nla_nest_start_noflag() calls in this function. I
really don't want to propagate the _noflag() API further.

(The above was the simplest I could come up with, maybe you can find a
better solution, but callikng nla_nest_start() conditionally seemed
worse)

johannes
Johannes Berg June 28, 2024, 8:16 a.m. UTC | #2
On Thu, 2024-06-20 at 13:11 +0200, Felix Fietkau wrote:
> 
> +static inline u32
> +rdev_get_radio_mask(struct cfg80211_registered_device *rdev,
> +		    struct net_device *dev)
> +{
> +	struct wiphy *wiphy = &rdev->wiphy;
> +	u32 ret;
> +
> +	if (!rdev->ops->get_radio_mask)
> +		return 0;
> +
> +	trace_rdev_get_radio_mask(wiphy, dev);
> +	ret = rdev->ops->get_radio_mask(wiphy, dev);
> +	trace_rdev_return_int(wiphy, ret);

I'd tend to prefer tracing even if it's not implemented, so we see
what's going on? Though personally I guess in this case I don't even
care much since mac80211 will unconditionally implement it ...

> @@ -2366,14 +2374,19 @@ int cfg80211_iter_combinations(struct wiphy *wiphy,
>  					    void *data),
>  			       void *data)
>  {
> +	const struct wiphy_radio *radio = NULL;
> +	const struct ieee80211_iface_combination *c, *cs;
>  	const struct ieee80211_regdomain *regdom;
>  	enum nl80211_dfs_regions region = 0;
> -	int i, j, iftype;
> +	int i, j, n, iftype;
>  	int num_interfaces = 0;
>  	u32 used_iftypes = 0;
>  	u32 beacon_int_gcd;
>  	bool beacon_int_different;
>  
> +	if (params->radio_idx >= 0)
> +		radio = &wiphy->radio[params->radio_idx];

Maybe we should have a sanity bounds check?

Or even really just __counted_by() annotations in struct wiphy_radio, so
we can run with UBSAN in testing (which would be a comment for patch 2,
but I'm not going to send it there again ;-) ).

johannes