Re: [RFC 4/4] wifi: nl80211: send iface combination to user space in multi-hardware wiphy

[Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>]

On 10/21/2022 5:55 PM, Johannes Berg wrote:
> On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote:
>> A new nested nl attribute is added to the same existing NL command to
>> advertise the iface combination capability for each underlying hardware
>> when driver groups more than one physical hardware under one wiphy to
>> enable MLO among them.
> 
> That's a good point - are we assuming this implies you can do MLO across
> the groups? Maybe somebody would want to use this advertisement without
> allowing MLO? (Not sure that's useful vs. multiple wiphys though, but
> maybe to simplify driver if there are some devices w/o MLO?)

Good point. Yes, this can be used even without MLO (i.e. not quite tied 
to MLO feature). Ill update the doc accordingly. Thanks.

> 
>> +		for (l = 0; l < c->iface_hw_list->n_limits; l++) {
>> +			struct nlattr *limit;
>> +
>> +			limit = nla_nest_start(msg, l + 1);
>> +			if (!limit)
>> +				return -ENOBUFS;
>> +
>> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_MAX,
>> +					c->iface_hw_list[i].limits[l].max))
>> +				return -ENOBUFS;
>> +
>> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_TYPES,
>> +					c->iface_hw_list[i].limits[l].types))
>> +				return -ENOBUFS;
>> +
>> +			nla_nest_end(msg, limit);
>> +		}
>> +		nla_nest_end(msg, limits);
> 
> 
> Feels like this part is kind of pre-existing code, or should be, could
> it be refactored?
> 

let me check, thanks.

>> +		if (nla_put_u32(msg,
>> +				NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
>> +				c->iface_hw_list[i].num_different_channels))
>> +			return -ENOBUFS;
>> +
>> +		if (nla_put_u16(msg,
>> +				NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
>> +				c->iface_hw_list[i].max_interfaces))
>> +			return -ENOBUFS;
>> +
>> +		nla_nest_end(msg, hw_combi);
> 
> And even this feels like it must already exist in some way? Wouldn't it
> even be easier to parse for userspace if it wasn't a separate set of
> attributes?
>

Yes, reusing existing attribute will be simpler. let me check that.

Thanks for the review!


Vasanth