[PATCH 06/13] wifi: nl80211: send iface combination to user space in multi-hardware wiphy

[Johannes Berg]

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