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

[Mina Almasry <almasrymina@google.com>]

On Mon, Aug 4, 2025 at 5:48 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> 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;
> };
>

Is this suggesting one more place where we put rx_buf_len, so in
addition to netdev_config?

Honestly I'm in favor of de-duplicating the info as much as possible,
to reduce the headache of keeping all the copies in sync.
pp_memory_provider_params is part of netdev_rx_queue. How about we add
either all of netdev_config or just rx_buf_len there? And set the
precedent that queue configs should be in netdev_rx_queue and all
pieces that need it should grab it from there? Unless the driver needs
a copy of the param I guess.

iouring zcrx and devmem can configure netdev_rx_queue->rx_buf_len in
addition to netdev_rx_queue->mp_params in this scenario.

-- 
Thanks,
Mina