Re: [PATCH v9 1/4] nl80211: add basic multiple bssid support

[Aloka Dixit <alokad@codeaurora.org>]

On 2021-04-08 05:05, Johannes Berg wrote:
> On Wed, 2021-03-10 at 10:26 -0800, Aloka Dixit wrote:
>> 
>> +/**
>> + * struct cfg80211_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 cfg80211_multiple_bssid {
>> +	u8 index;
>> +	u8 count;
>> +	u32 parent;
>> +	bool ema;
>> +};
> 
> Can reorder this to make it smaller: bool before u32), or even u32
> element first.
> 

Will do.

>> +/**
>> + * struct cfg80211_multiple_bssid_data - Multiple BSSID elements
>> + *
>> + * @cnt: Number of elements in array %elems.
>> + *
>> + * @elems: Array of multiple BSSID element(s) to be added into Beacon 
>> frames.
>> + * @elems.data: Data for multiple BSSID elements.
>> + * @elems.len: Length of data.
>> + */
>> +struct cfg80211_multiple_bssid_data {
>> +	u8 cnt;
>> +	struct {
>> +		u8 *data;
>> +		size_t len;
>> +	} elems[];
>> +};
> 
> Why are the elements separate, rather than a single big buffer with all
> elements?
> 
> 

This is for EMA AP where each element will hold different
non-transmitted profiles. While copying elements in
ieee80211_assign_beacon(), all are copied one by one.
However during beacon generation in __ieee80211_beacon_get(),
only the element at a given index is actually added.
Hence separate elements.

>> + * @multiple_bssid.max_num_vaps: Maximum number of VAPS supported by 
>> the driver
> 
> Please use upstream terminology, we don't use the term "VAP" (and 
> "VAPS"
> would be wrong anyway, "VAPs").
> 
> I'd probably call this "max_interfaces", maybe we'll extend this to
> other things (mesh?) later?
>> 

Will change.

>> +/**
>> + * enum nl80211_multiple_bssid_elems_attributes - Attributes used to 
>> pass
>> + *	multiple BSSID elements data.
>> + *
>> + * @__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID: Invalid
>> + *
>> + * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT: Number of multiple BSSID
>> + *	elements (u8).
>> + *
>> + * @NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA: Array of multiple BSSID 
>> elements.
>> + */
>> +enum nl80211_multiple_bssid_elems_attributes {
>> +	__NL80211_MULTIPLE_BSSID_ELEMS_ATTR_INVALID,
>> +
>> +	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_COUNT,
>> +	NL80211_MULTIPLE_BSSID_ELEMS_ATTR_DATA,
>> 
> 
> Can you clarify why this is an array? See also above.
> 
> 
> johannes

Array to pass separate MBSSID elements for EMA which will
hold different profiles in the same MBSSID set.