Message-ID: <65670cf42abc593502e9f7bafc56c1d0360d5631.camel@sipsolutions.net>
Subject: Re: [PATCH v3 1/3] wifi: cfg80211: add support to enable/disable
 bss color collision detection
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Rameshkumar Sundaram <quic_ramess@quicinc.com>,
        ath11k@lists.infradead.org
Cc:     linux-wireless@vger.kernel.org, Jouni Malinen <j@w1.fi>
Date:   Wed, 15 Feb 2023 18:34:58 +0100
In-Reply-To: <20230201065638.25990-2-quic_ramess@quicinc.com>
References: <20230201065638.25990-1-quic_ramess@quicinc.com>
         <20230201065638.25990-2-quic_ramess@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.3 (3.46.3-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-02-01 at 12:26 +0530, Rameshkumar Sundaram wrote:
> As per 802.11ax-2021, STAs shall process BSS Color Change Announcement
> (BCCA) from AP and switch to new color, but some STAs aren't processing
> BCCA from AP and not doing color switch, causing them to drop data
> frames from AP post color change.

Hmm. We had this bug in EHT. Should we really allow this? It's a station
bug, why bother?

> Provide an option to disable color collision detection and therefore
> not to do BCCA to mitigate the same from AP. If it's required in case
> where STA supports BCCA handling, then it can enabled in AP using this
> option.

This could be rewritten a bit, not entirely clear to me. "enabled" is
more like "disabled"? Or no? Confusing.

> + * @NL80211_HE_BSS_COLOR_ATTR_COLLISION_DETECTION_DISABLED: is BSS
> + * color collision detection disabled.

please use a tab to indent the second line like elsewhere

> +	he_bss_color->collision_detection_enabled =3D
> +		!nla_get_flag(tb[NL80211_HE_BSS_COLOR_ATTR_COLLISION_DETECTION_DISABLE=
D]);

Is it really worth to have one side say "enabled" and the other
"disabled"?

johannes