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: <ba5b4313-6e42-4340-8301-239dac046424@redhat.com>
Date: Thu, 17 Apr 2025 10:59:37 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Lorenzo Bianconi <lorenzo@...nel.org>, Simon Horman <horms@...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>, 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 4/16/25 5:55 PM, Lorenzo Bianconi wrote:
>> 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.
> 
> you are right Simon, this change is not backwards compatible but the fw is
> not publicly available yet and the official fw version will use this new layout
> (the previous one was just a private version I used to develop the driver).
> Can we use this simple approach or do you think we should differentiate the two
> firmware version in some way? (even if the previous one will never be used).

I think it's better if you clarify the commit message. I read the above
as the current (unpatched) code will not work with the official
firmware, so bug addressed here does not look theoretical.

Thanks,

Paolo