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?
johannes
