netdev - Re: [RFC v1 21/22] net: parametrise mp open with a queue config

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: <11caecf8-5b81-49c7-8b73-847033151d51@gmail.com>
Date: Mon, 4 Aug 2025 13:50:08 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: netdev@...r.kernel.org, io-uring@...r.kernel.org,
 Eric Dumazet <edumazet@...gle.com>, Willem de Bruijn <willemb@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, andrew+netdev@...n.ch, horms@...nel.org,
 davem@...emloft.net, sdf@...ichev.me, almasrymina@...gle.com,
 dw@...idwei.uk, michael.chan@...adcom.com, dtatulea@...dia.com,
 ap420073@...il.com
Subject: Re: [RFC v1 21/22] net: parametrise mp open with a queue config

On 8/2/25 01:10, Jakub Kicinski wrote:
> On Mon, 28 Jul 2025 12:04:25 +0100 Pavel Begunkov wrote:
>> This patch allows memory providers to pass a queue config when opening a
>> queue. It'll be used in the next patch to pass a custom rx buffer length
>> from zcrx. As there are many users of netdev_rx_queue_restart(), it's
>> allowed to pass a NULL qcfg, in which case the function will use the
>> default configuration.
> 
> This is not exactly what I anticipated, TBH, I was thinking of
> extending the config stuff with another layer.. Drivers will
> restart their queues for most random reasons, so we need to be able
> to reconstitute this config easily and serve it up via

Yeah, also noticed the gap that while replying to Stan.

> netdev_queue_config(). This was, IIUC, also Mina's first concern.
> 
> My thinking was that the config would be constructed like this:
> 
>    qcfg = init_to_defaults()
>    drv_def = get_driver_defaults()
>    for each setting:
>      if drv_def.X.set:
>         qcfg.X = drv_def.X.value
>      if dev.config.X.set:
>         qcfg.X = dev.config.X.value
>      if dev.config.qcfg[qid].X.set:
>         qcfg.X = dev.config.qcfg[qid].X.value
>      if dev.config.mp[qid].X.set:               << this was not in my
>         qcfg.X = dev.config.mp[qid].X.value     << RFC series
> 
> Since we don't allow MP to be replaced atomically today, we don't
> actually have to place the mp overrides in the config struct and
> involve the whole netdev_reconfig_start() _swap() _free() machinery.
> We can just stash the config in the queue state, and "logically"
> do what I described above.

I was thinking stashing it in struct pp_memory_provider_params and
applying in netdev_rx_queue_restart(). Let me try to move it
into __netdev_queue_config. Any preference between keeping just
the size vs a qcfg pointer in pp_memory_provider_params?

struct struct pp_memory_provider_params {
	const struct memory_provider_ops *mp_ops;
	u32 rx_buf_len;
};

vs

struct struct pp_memory_provider_params {
	const struct memory_provider_ops *mp_ops;
	// providers will need to allocate and keep the qcfg
	// until it's completely detached from the queues.
	struct netdev_queue_config *qcfg;
};

The former one would be simpler for now.

-- 
Pavel Begunkov