netdev - Re: [PATCH net-next] 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: <20250416154144.GT395307@horms.kernel.org>
Date: Wed, 16 Apr 2025 16:41:44 +0100
From: Simon Horman <horms@...nel.org>
To: Lorenzo Bianconi <lorenzo@...nel.org>
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
Subject: Re: [PATCH net-next] net: airoha: Add missing filed to ppe_mbox_data
 struct

On Tue, Apr 15, 2025 at 09:27:21AM +0200, 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 offload does not require this field. This patch fixes
> just a theoretical bug since the Airoha EN7581 firmware is not posted to
> linux-firware or other repositories (e.g. OpenWrt) yet.
> 
> Fixes: 23290c7bc190d ("net: airoha: Introduce Airoha NPU support")
> Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.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;

Hi Lorenzo,

I'm a little confused by this.

As I understand it ppe_mbox_data is sent as the data of a mailbox message
send to the device.  But by adding the max_packet field the layout is
changed. The size of the structure changes. And perhaps more importantly
the offset of fields after max_packet, e.g.  wan_mode, change.

Looking at how this is used, f.e. in the following code, I'm unclear on
how this change is backwards compatible.

static int airoha_npu_ppe_init(struct airoha_npu *npu)
{
        struct ppe_mbox_data ppe_data = {
                .func_type = NPU_OP_SET,
                .func_id = PPE_FUNC_SET_WAIT_HWNAT_INIT,
                .init_info = {
                        .ppe_type = PPE_TYPE_L2B_IPV4_IPV6,
                        .wan_mode = QDMA_WAN_ETHER,
                },
        };

        return airoha_npu_send_msg(npu, NPU_FUNC_PPE, &ppe_data,
                                   sizeof(struct ppe_mbox_data));
}