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: <aAjNC7zUrhc2Ma0z@lore-desk>
Date: Wed, 23 Apr 2025 13:20:43 +0200
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: 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>,
	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.

correct

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

correct

> 
> 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?

yes, right

> 
> > 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.

yes, you are right. Let's hold on for a while with this patch and let me ask
Airoha folks if we can "pack" the struct in the NPU firmware binary so we can use
__packed attribute here. In any case I will use "u32" instead of "int" in the next
version.

Regards,
Lorenzo

> 
> 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,
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)