Received-SPF: pass (google.com: domain of linux-wireless+bounces-8059-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Kalle Valo <kvalo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
Cc: linux-wireless@vger.kernel.org,  linux-kernel@vger.kernel.org,  Brian
 Norris <briannorris@chromium.org>,  Francesco Dolcini
 <francesco@dolcini.it>,  David Lin <yu-hao.lin@nxp.com>
Subject: Re: [PATCH] wifi: mwifiex: fix parsing of more than two AKM suites
References: <20240523081428.2852276-1-s.hauer@pengutronix.de>
Date: Sat, 25 May 2024 12:15:22 +0300
In-Reply-To: <20240523081428.2852276-1-s.hauer@pengutronix.de> (Sascha Hauer's
	message of "Thu, 23 May 2024 10:14:28 +0200")
Message-ID: <878qzyntg5.fsf@kernel.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Sascha Hauer <s.hauer@pengutronix.de> writes:

> params->crypto.n_akm_suites seems to be limited to two AKM suites. Once
> there are more they will be passed as extra elements of type WLAN_EID_RSN
> or WLAN_EID_VENDOR_SPECIFIC.
>
> This takes some snippets from the downstream vendor driver to parse
> these elements and to set the correct protocol and key_mgmt bits to
> enable the desired key managements algorithms in the hardware.
>
> This patch is not a request for inclusion, more a heads up that there's
> something missing and the question if the approach taken is the right
> one or if there are other preferred ways to fix this issue.

Please mark patches like this as "[PATCH RFC]", that way we maintainers
know to drop them automatically.

> --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> @@ -9,6 +9,112 @@
>  #include "11ac.h"
>  #include "11n.h"
>  
> +struct wpa_suite_ucast {
> +        /* count */
> +        u16 count;
> +        /** wpa_suite list */
> +	__be32 suite[1];
> +} __packed;

The comments here are not adding any extra information, please remove.

In general having a some kind of prefix in the struct name would be
nice. I don't know what mwifiex uses (if any) but, for example, in
ath12k we use 'ath12k_'.

> +struct IEEEtypes_Rsn_t {

Lower case, no '_t' and also improve the naming.

> +        /** Rsn : version */
> +        u16 version;
> +        /** Rsn : group cipher */
> +        __be32 group_cipher;
> +        /** Rsn : pairwise cipher */
> +        struct wpa_suite_ucast pairwise_cipher;
> +} __packed;

Useless comments.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches