RE: [PATCH] mac80211: disable BSS color collision detection in case of no free colors

["Rameshkumar Sundaram (QUIC)" <quic_ramess@quicinc.com>]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Friday, December 3, 2021 1:39 PM
> To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Lavanya Suresh
> <lavaks@codeaurora.org>
> Subject: Re: [PATCH] mac80211: disable BSS color collision detection in case
> of no free colors
> 
> On Fri, 2021-12-03 at 10:11 +0530, Rameshkumar Sundaram wrote:
> >
> > +++ b/net/mac80211/cfg.c
> > @@ -995,6 +995,8 @@ static int ieee80211_assign_beacon(struct
> ieee80211_sub_if_data *sdata,
> >  	struct beacon_data *new, *old;
> >  	int new_head_len, new_tail_len;
> >  	int size, err;
> > +	const u8 *cap;
> > +	struct ieee80211_he_operation *he_oper = NULL;
> >  	u32 changed = BSS_CHANGED_BEACON;
> >
> >  	old = sdata_dereference(sdata->u.ap.beacon, sdata); @@ -1082,6
> > +1084,27 @@ static int ieee80211_assign_beacon(struct
> ieee80211_sub_if_data *sdata,
> >  		changed |= BSS_CHANGED_FTM_RESPONDER;
> >  	}
> >
> > +	if (sdata->vif.bss_conf.he_support) {
> > +		cap = cfg80211_find_ext_ie(WLAN_EID_EXT_HE_OPERATION,
> > +					   params->tail, params->tail_len);
> > +		if (cap && cap[1] >= sizeof(*he_oper) + 1)
> > +			he_oper = (void *)(cap + 3);
> >
> 
> I'm not sure I like this mechanism - in ieee80211_start_ap() we explicitly take
> it from the parameters given via nl80211, so it feels the same should be true
> here. Why isn't it done that way?

This is because in this case only beacon will change and in nl80211_set_beacon()
we don’t parse NL80211_ATTR_HE_BSS_COLOR attribute or do nl80211_calculate_ap_params()
> 
> (And if we decide it should be this way then you should be using a new
> "const struct element *cap" instead of "const u8 *cap", with the better
> helpers functions etc.)
> 
Sure cfg80211_find_ext_ie() returns const u8 *, you want me to use cfg80211_find_ext_elem instead (i.e. like how nl80211_calculate_ap_params() does ?

> johannes