| Message ID | 20201103091743.1924854-1-john@phrozen.org |
|---|---|
| Headers | show |
| Series | mac80211: add multiple bssid support | expand |
On Tue, 2020-11-03 at 10:17 +0100, John Crispin wrote: > +/** > + * struct ieee80211_multiple_bssid - AP settings for multi bssid > + * > + * @index: the index of this AP in the multi bssid group. > + * @count: the total number of multi bssid peer APs. > + * @parent: non-transmitted BSSs transmitted parents index > + * @ema: Shall the beacons be sent out in EMA mode. > + */ > +struct ieee80211_multiple_bssid { > + u8 index; > + u8 count; > + u32 parent; > + bool ema; > +}; > + Why is that ieee80211_ rather than cfg80211_? We've often (mostly?) reserved that for spec structs. > +/** > + * struct cfg80211_multiple_bssid_data - multiple_bssid data > + * @ies: array of extra information element(s) to add into Beacon frames for multiple > + * bssid or %NULL > + * @len: array of lengths of multiple_bssid.ies in octets > + * @cnt: number of entries in multiple_bssid.ies > + */ > +struct cfg80211_multiple_bssid_data { > + u8 *ies[NL80211_MULTIPLE_BSSID_IES_MAX]; > + size_t len[NL80211_MULTIPLE_BSSID_IES_MAX]; > + int cnt; > +}; Not sure if this is dynamically allocated but if now we could, and then we can make that struct ... { unsigned int cnt; struct { const u8 *data; size_t len; } elems[]; }; and get rid of NL80211_MULTIPLE_BSSID_IES_MAX. > @@ -1072,6 +1101,8 @@ struct cfg80211_beacon_data { > size_t probe_resp_len; > size_t lci_len; > size_t civicloc_len; > + > + struct cfg80211_multiple_bssid_data multiple_bssid; OK, so it's not (right now), but could even be embedded as the dynamically sized struct here at the end ... Or maybe keep some for the common case? But I don't think there's any general limit of 8, so I'm not convinced it makes sense to have such a (somewhat artificial) limit in nl80211. > + * @NL80211_ATTR_MULTIPLE_BSSID_PARENT: If this is a Non-Transmitted BSSID, define > + * the parent (transmitting) interface. by what, interface index? wdev id? > + * @NL80211_ATTR_MULTIPLE_BSSID_INDEX: The index of this BSS inside the multi bssid > + * element. > + * > + * @NL80211_ATTR_MULTIPLE_BSSID_COUNT: The number of BSSs inside the multi bssid element. > + * > + * @NL80211_ATTR_MULTIPLE_BSSID_IES: The Elements that describe our multiple BSS group. Might be better called "ELEMS" or something now, since "IE" is no longer used in the spec. > + * these get passed separately as the kernel might need to split them up for EMA VAP. > + * > + * @NL80211_ATTR_MULTIPLE_BSSID_EMA: Shall the multiple BSS beacons be sent out in EMA mode. Probably should describe the formats a bit - U32, nested with..., flag, etc. > +++ b/net/wireless/nl80211.c > @@ -715,6 +715,11 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = { > NLA_POLICY_EXACT_LEN(IEEE80211_S1G_CAPABILITY_LEN), > [NL80211_ATTR_S1G_CAPABILITY_MASK] = > NLA_POLICY_EXACT_LEN(IEEE80211_S1G_CAPABILITY_LEN), > + [NL80211_ATTR_MULTIPLE_BSSID_PARENT] = { .type = NLA_U32 }, > + [NL80211_ATTR_MULTIPLE_BSSID_INDEX] = { .type = NLA_U8 }, > + [NL80211_ATTR_MULTIPLE_BSSID_COUNT] = NLA_POLICY_RANGE(NLA_U8, 1, 16), Where does the 16 come from? johannes
On Fri, 2020-11-06 at 10:24 +0100, Johannes Berg wrote: > > and get rid of NL80211_MULTIPLE_BSSID_IES_MAX. Oh, and the fact that this today has a value of 8 _probably_ means you hardcoded the limits of your specific device that you were thinking of now, and we really should have the device advertising the maximum possible value or so? And then I guess we can still hard-code the maximum possible value in cfg80211.h somewhere, use it for array sizes etc., but limit the userspace API to the driver-advertised value (and check in wiphy registration that the cfg80211 limit is >= the driver value). Or is this some fundamental (spec) limit? johannes