netdev - Re: [PATCH net v3] net: airoha: Add missing filed to ppe_mbox

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c34ef8a0-20fd-4d0b-84cc-8f829f4be675@intel.com>
Date: Tue, 22 Apr 2025 15:29:07 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Lorenzo Bianconi <lorenzo@...nel.org>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>
CC: <linux-arm-kernel@...ts.infradead.org>,
	<linux-mediatek@...ts.infradead.org>, <netdev@...r.kernel.org>, Simon Horman
	<horms@...nel.org>
Subject: Re: [PATCH net v3] net: airoha: Add missing filed to ppe_mbox_data
 struct



On 4/22/2025 8:59 AM, Lorenzo Bianconi wrote:
> The official Airoha EN7581 firmware requires adding max_packet filed in
> ppe_mbox_data struct while the unofficial one used to develop the Airoha
> EN7581 flowtable support does not require this field.
> This patch does not introduce any real backwards compatible issue since
> EN7581 fw is not publicly available in linux-firmware or other
> repositories (e.g. OpenWrt) yet and the official fw version will use this
> new layout. For this reason this change needs to be backported.
> 

To clarify if I understand correctly:

The original data structure without max_packet is for an unreleased
version of firmware which is unofficial and which is not released publicly.

Then, the official public release will include this additional field,
and thus won't work with the current kernel code.

Of course anyone who happens to have the unofficial firmware will need
to work around this, but that should only include a small handful of
folks with development images?

> Fixes: 23290c7bc190d ("net: airoha: Introduce Airoha NPU support")
> Reviewed-by: Simon Horman <horms@...nel.org>
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> ---
> Changes in v3:
> - resend targeting net tree
> - Link to v2: https://lore.kernel.org/r/20250417-airoha-en7581-fix-ppe_mbox_data-v2-1-43433cfbe874@kernel.org
> 
> Changes in v2:
> - Add more details to commit log
> - Link to v1: https://lore.kernel.org/r/20250415-airoha-en7581-fix-ppe_mbox_data-v1-1-4408c60ba964@kernel.org
> ---
>  drivers/net/ethernet/airoha/airoha_npu.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
> index 7a5710f9ccf6a4a4f555ab63d67cb6b318de9b52..16201b5ce9f27866896226c3611b4a154d19bc2c 100644
> --- a/drivers/net/ethernet/airoha/airoha_npu.c
> +++ b/drivers/net/ethernet/airoha/airoha_npu.c
> @@ -104,6 +104,7 @@ struct ppe_mbox_data {
>  			u8 xpon_hal_api;
>  			u8 wan_xsi;
>  			u8 ct_joyme4;
> +			u8 max_packet;
>  			int ppe_type;
>  			int wan_mode;
>  			int wan_sel;
> 

One oddity here is that the structure is not marked __packed. This
addition of a u8 means there will be a 3-byte gap on platforms which
have a 4-byte integer... It feels very weird these are ints and not s32
or something to fully clarify the sizes.

Regardless, assuming the correctness that the unofficial firmware is
only available to developers and isn't widely available:

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

> ---
> base-commit: c03a49f3093a4903c8a93c8b5c9a297b5343b169
> change-id: 20250422-airoha-en7581-fix-ppe_mbox_data-56df12d4df72
> 
> Best regards,