Received-SPF: pass (google.com: domain of linux-wireless+bounces-6328-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <647d23e5-0670-6d59-2887-5c77c311cb54@quicinc.com>
Date: Mon, 15 Apr 2024 21:27:38 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 06/13] wifi: nl80211: send iface combination to user space
 in multi-hardware wiphy
To: Johannes Berg <johannes@sipsolutions.net>,
        Vasanthakumar Thiagarajan
	<quic_vthiagar@quicinc.com>,
        <ath12k@lists.infradead.org>
CC: <linux-wireless@vger.kernel.org>
References: <20240328072916.1164195-1-quic_periyasa@quicinc.com>
 <20240328072916.1164195-7-quic_periyasa@quicinc.com>
 <29fc2719d6814a8e84f5db9ad81c76116a52828b.camel@sipsolutions.net>
 <14909268-53de-a686-eecd-443ce153c756@quicinc.com>
 <6147ac95-a572-62df-619d-33078d51ccc3@quicinc.com>
 <c58f318641ec2e836a23f8769eb32c67326643a5.camel@sipsolutions.net>
Content-Language: en-US
From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
In-Reply-To: <c58f318641ec2e836a23f8769eb32c67326643a5.camel@sipsolutions.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit


On 4/15/2024 7:57 PM, Johannes Berg wrote:
> On Fri, 2024-04-12 at 10:57 +0530, Karthikeyan Periyasamy wrote:
>>>>> + * @NL80211_IFACE_COMB_PER_HW_COMB_LIMITS: nested attribute
>>>>> containing the
>>>>> + *    limits for the given interface types, see
>>>>> + *    &enum nl80211_iface_limit_attrs.
>>>>> + * @NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM: u32 attribute giving the
>>>>> maximum
>>>>> + *    number of interfaces that can be created in this group. This
>>>>> number
>>>>> + *    does not apply to the interfaces purely managed in software.
>>>>> + * @NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS: u32 attribute
>>>>> specifying the
>>>>> + *    number of different channels that can be used in this group.
>>>>> + * @NUM_NL80211_IFACE_COMB_PER_HW_COMB: number of attributes
>>>>> + * @MAX_NL80211_IFACE_COMB_PER_HW_COMB: highest attribute number
>>>>> + */
>>>>> +enum nl80211_if_comb_per_hw_comb_attrs {
>>>>> +    NL80211_IFACE_COMB_PER_HW_COMB_UNSPEC,
>>>>> +    NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX,
>>>>> +    NL80211_IFACE_COMB_PER_HW_COMB_LIMITS,
>>>>> +    NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
>>>>> +    NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
>>>>
>>>> Almost all these attributes duplicate - including their docs -
>>>> attributes from enum nl80211_if_combination_attrs. Is it really worth
>>>> doing that, rather than adding NL80211_IFACE_COMB_HW_IDX and documenting
>>>> the different uses of the attribute set?
>>>>
>>>
>>> I agree, more duplication. So we have to have the per_hw_comb_attrs
>>> defines like below?
>>>
>>> enum nl80211_if_comb_per_hw_comb_attrs {
>>>       NL80211_IFACE_COMB_PER_HW_COMB_UNSPEC,
>>>       NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX =
>>>               NL80211_IFACE_COMB_NUM_CHANNELS + 1,
>>>       /* keep last */
>>>       NUM_NL80211_IFACE_COMB_PER_HW_COMB,
>>>       MAX_NL80211_IFACE_COMB_PER_HW_COMB =
>>>               NUM_NL80211_IFACE_COMB_PER_HW_COMB - 1
>>> };
>>>
>>
>> I agree this approach. Instead of NUM_NL80211_IFACE_COMB_PER_HW_COMB,
>> shall we have MAX_NL80211_IFACE_COMB like below so that hw_idx attribute
>> value will not get conflict to any IFACE combination attributes
>>
>>    enum nl80211_if_comb_per_hw_comb_attrs {
>>         NL80211_IFACE_COMB_PER_HW_COMB_UNSPEC,
>>         NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX =
>>                 MAX_NL80211_IFACE_COMB + 1,
>>         /* keep last */
>>         NUM_NL80211_IFACE_COMB_PER_HW_COMB,
>>         MAX_NL80211_IFACE_COMB_PER_HW_COMB =
>>                 NUM_NL80211_IFACE_COMB_PER_HW_COMB - 1
>> };
>>
> 
> You haven't thought this through - what happens here if something is
> added to enum nl80211_if_combination_attrs?
> 

Yeah, it break backward compatibility.

-- 
Karthikeyan Periyasamy
--
கார்த்திகேயன் பெரியசாமி