Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <59e2364e9085f725c03d05e3f720f8fae7b336ce.camel@sipsolutions.net>
Subject: Re: [PATCH v3 4/9] wifi: cfg80211: add NL command to set 6 GHz
 power mode
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Aditya Kumar Singh <quic_adisi@quicinc.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Tue, 29 Aug 2023 19:51:52 +0200
In-Reply-To: <20230315132904.31779-5-quic_adisi@quicinc.com>
References: <20230315132904.31779-1-quic_adisi@quicinc.com>
         <20230315132904.31779-5-quic_adisi@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2023-03-15 at 18:58 +0530, Aditya Kumar Singh wrote:
>=20
> +	{
> +		.cmd =3D NL80211_CMD_SET_6GHZ_POWER_MODE,
> +		.validate =3D GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> +		.doit =3D nl80211_set_6ghz_power_mode,
> +		.flags =3D GENL_UNS_ADMIN_PERM,
> +		.internal_flags =3D IFLAGS(NL80211_FLAG_NEED_NETDEV |
> +					 NL80211_FLAG_MLO_VALID_LINK_ID),
> +	},

Why is this even a new command, rather than a parameter to start AP or
similar?

Why do we even set it in client mode from userspace?

>  static struct genl_family nl80211_fam __ro_after_init =3D {
> @@ -17409,7 +17473,7 @@ static struct genl_family nl80211_fam __ro_after_=
init =3D {
>  	.n_ops =3D ARRAY_SIZE(nl80211_ops),
>  	.small_ops =3D nl80211_small_ops,
>  	.n_small_ops =3D ARRAY_SIZE(nl80211_small_ops),
> -	.resv_start_op =3D NL80211_CMD_REMOVE_LINK_STA + 1,
> +	.resv_start_op =3D NL80211_CMD_SET_6GHZ_POWER_MODE + 1,
>=20

Obviously, this should not be done.

But in any case, I don't think there's a lot of value in doing a
detailed review of the code if we haven't gotten a good grasp of the
semantics that you want.

johannes