linux-kernel - Re: [PATCH net-next v29 1/3] mailbox: add callback function for rx buffer allocation

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 for Android: free password hash cracker in your pocket

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

Message-ID: <aNa09S7h8MgWajKe@bogus>
Date: Fri, 26 Sep 2025 16:44:53 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Adam Young <admiyo@...amperecomputing.com>
Cc: Jassi Brar <jassisinghbrar@...il.com>, <netdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Sudeep Holla <sudeep.holla@....com>,
	Jeremy Kerr <jk@...econstruct.com.au>,
	Matt Johnston <matt@...econstruct.com.au>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Huisong Li <lihuisong@...wei.com>
Subject: Re: [PATCH net-next v29 1/3] mailbox: add callback function for rx
 buffer allocation

Hi,

On Thu, Sep 25, 2025 at 03:00:24PM -0400, Adam Young wrote:
> Allows the mailbox client to specify how to allocate the memory
> that the mailbox controller uses to send the message to the client.
> 
> In the case of a network driver, the message should be allocated as
> a struct sk_buff allocated and managed by the network subsystem.  The
> two parameters passed back from the callback represent the sk_buff
> itself and the data section inside the skbuff where the message gets
> written.
> 
> For simpler cases where the client kmallocs a buffer or returns
> static memory, both pointers should point to the same value.
> 

I have posted a revert[1] of a patch that you got merged recently and
this change builds on top of that. Please start fresh with that patch/revert
applied and explain why you can't use tx_prepare and rx_callback as
suggested and then we can take it from there. It makes no sense to add
these optional callbacks the way it is done at the least. If you are
worried about duplication of shmem handling, IIUC, it is done intentionally
to give that flexibility to the clients. If you think it can be consolidated,
I would rather have them as a standard helper which can be assigned to
tx_prepare and rx_callback and convert few users to demonstrate the same.

So I definitely don't like this addition and NACK until all those details
are looked at and you have convinced it can't be achieved via those and need
more changes.

On the other hand, the consolidation is a good thought, but it can't be
addition without any other drivers except you new yet to be merged driver
as the sole user of that. If there are no users needing copying to/from
the shared memory like you driver, then I am happy to consider this but it
needs more thinking and reviewing for sure. I will also have a look.

Sorry for missing the review of the original patch that got merged, my bad.

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20250926153311.2202648-1-sudeep.holla@arm.com