Re: [RFC 1/2] cfg80211: add support for advertising multiple radios belonging to a wiphy

[Felix Fietkau <nbd@xxxxxxxx>]

On 22.05.24 16:23, Ben Greear wrote:
> On 5/22/24 05:30, Felix Fietkau wrote:
> > 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, the bands and supported frequencies 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.
> >
> > Signed-off-by: Felix Fietkau <nbd@xxxxxxxx>
> > ---
> >   include/net/cfg80211.h       | 34 ++++++++++++++++
> >   include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++
> >   net/wireless/nl80211.c       | 75 ++++++++++++++++++++++++++++++++++++
> >   3 files changed, 157 insertions(+)
> >
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> > index f2ca495ac9f8..58d8375ffa11 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -5407,6 +5407,34 @@ struct wiphy_iftype_akm_suites {
> >   	int n_akm_suites;
> >   };
> >   
> > +/**
> > + * struct wiphy_radio_freq_range - wiphy frequency range
> > + * @start_freq:  start range edge frequency (kHz)
> > + * @end_freq:    end range edge frequency (kHz)
> > + */
> > +struct wiphy_radio_freq_range {
> > +	u32 start_freq;
> > +	u32 end_freq;
> > +};
> > +
> > +
> > +/**
> > + * struct wiphy_radio - This structure describes a physical radio belonging
> > + * to a wiphy. It is used to describe concurrent-channel capabilities of the
> > + * phy. Only one channel can be active on the radio described by struct
> > + * wiphy_radio.
> > + *
> > + * @band_mask: Mask of enum nl80211_band describing the bands that the radio
> > + *	can operate on.
> > + * @num_freq_range: number of elements in @freq_range
> > + * @freq_range: frequency range that the radio can operate on (optional)
> > + */
> > +struct wiphy_radio {
> > +	u16 band_mask;
> > +	u16 n_freq_range;
> > +	const struct wiphy_radio_freq_range *freq_range;
> > +};
>
> Do you think we might should add the radio_idx in here so that we don't
> depend on position in the array in case we need to add/remove radios
> for some reason?
>
> Or radio MAC addr or some other more persistent way to detect exactly
> what this is in user-space?

Why would radios be added/removed at run time?

> >   #define CFG80211_HW_TIMESTAMP_ALL_PEERS	0xffff
> >   
> >   /**
> > @@ -5625,6 +5653,9 @@ struct wiphy_iftype_akm_suites {
> >    *	A value of %CFG80211_HW_TIMESTAMP_ALL_PEERS indicates the driver
> >    *	supports enabling HW timestamping for all peers (i.e. no need to
> >    *	specify a mac address).
> > + *
> > + * @radio: radios belonging to this wiphy
> > + * @n_radio: number of radios
> >    */
> >   struct wiphy {
> >   	struct mutex mtx;
> > @@ -5775,6 +5806,9 @@ struct wiphy {
> >   
> >   	u16 hw_timestamp_max_peers;
> >   
> > +	const struct wiphy_radio *radio;
> > +	int n_radio;
> > +
> >   	char priv[] __aligned(NETDEV_ALIGN);
> >   };
> >   
> > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> > index f917bc6c9b6f..dfb01d673094 100644
> > --- a/include/uapi/linux/nl80211.h
> > +++ b/include/uapi/linux/nl80211.h
> > @@ -3401,6 +3401,8 @@ enum nl80211_attrs {
> >   
> >   	NL80211_ATTR_ASSOC_SPP_AMSDU,
> >   
> > +	NL80211_ATTR_RADIOS,
> > +
> >   	/* add attributes here, update the policy in nl80211.c */
> >   
> >   	__NL80211_ATTR_AFTER_LAST,
> > @@ -7999,4 +8001,50 @@ enum nl80211_ap_settings_flags {
> >   	NL80211_AP_SETTINGS_SA_QUERY_OFFLOAD_SUPPORT	= 1 << 1,
> >   };
> >   
> > +/**
> > + * enum nl80211_wiphy_radio_attrs - wiphy radio attributes
> > + *
> > + * @__NL80211_WIPHY_RADIO_ATTR_INVALID: Invalid
> > + *
> > + * @NL80211_WIPHY_RADIO_ATTR_BAND_MASK: Bitmask of bands supported by this
> > + *	radio.
> > + *
> > + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges
> > + *	supported by this radio.
> > + *
> > + * @__NL80211_WIPHY_RADIO_ATTR_LAST: Internal
> > + * @NL80211_WIPHY_RADIO_ATTR_MAX: Highest attribute
> > + */
> > +enum nl80211_wiphy_radio_attrs {
> > +	__NL80211_WIPHY_RADIO_ATTR_INVALID,
> > +
> > +	NL80211_WIPHY_RADIO_ATTR_BAND_MASK,
> > +	NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES,
> > +
> > +	/* keep last */
> > +	__NL80211_WIPHY_RADIO_ATTR_LAST,
> > +	NL80211_WIPHY_RADIO_ATTR_MAX = __NL80211_WIPHY_RADIO_ATTR_LAST - 1,
> > +};
> > +
> > +/**
> > + * enum nl80211_wiphy_radio_freq_range - wiphy radio frequency range
> > + *
> > + * @__NL80211_WIPHY_RADIO_FREQ_ATTR_INVALID: Invalid
> > + *
> > + * @NL80211_WIPHY_RADIO_FREQ_ATTR_START: Frequency range start
> > + * @NL80211_WIPHY_RADIO_FREQ_ATTR_END: Frequency range end
> > + *
> > + * @__NL80211_WIPHY_RADIO_FREQ_ATTR_LAST: Internal
> > + * @NL80211_WIPHY_RADIO_FREQ_ATTR_MAX: Highest attribute
> > + */
> > +enum nl80211_wiphy_radio_freq_range {
> > +	__NL80211_WIPHY_RADIO_FREQ_ATTR_INVALID,
> > +
> > +	NL80211_WIPHY_RADIO_FREQ_ATTR_START,
> > +	NL80211_WIPHY_RADIO_FREQ_ATTR_END,
> > +
> > +	__NL80211_WIPHY_RADIO_FREQ_ATTR_LAST,
> > +	NL80211_WIPHY_RADIO_FREQ_ATTR_MAX = __NL80211_WIPHY_RADIO_FREQ_ATTR_LAST - 1,
> > +};
> > +
> >   #endif /* __LINUX_NL80211_H */
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 3c0bca4238d3..c07abdf104ec 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -2392,6 +2392,78 @@ static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg)
> >   	return -ENOBUFS;
> >   }
> >   
> > +static int nl80211_put_radio(struct wiphy *wiphy, struct sk_buff *msg, int idx)
> > +{
> > +	const struct wiphy_radio *r = &wiphy->radio[idx];
>
> Maybe allow radio[] array to be sparse (ie, idx 0 is there, idx 2 is there, idx 1 is NULL
> because user wants to remove that radio from MLO consideration for whatever reason?

Why should the user want to do this?

- Felix