Re: [PATCH net-next] net: macb: allow MTU changes while the interface is running

["Théo Lebrun" <theo.lebrun@bootlin.com>]

On Wed Mar 18, 2026 at 12:23 AM CET, Jakub Kicinski wrote:
> On Tue, 17 Mar 2026 23:58:21 +0100 Nicolai Buchwitz wrote:
>> On 17.3.2026 23:23, Jakub Kicinski wrote:
>> > On Tue, 17 Mar 2026 18:00:02 +0100 Théo Lebrun wrote:  
>> >> Reviewed-by: Théo Lebrun <theo.lebrun@bootlin.com>  
>> > 
>> > I told you recently that open / close is not allowed.
>> > So why are you putting a rb tag on a patch which does exactly that?
>> > 
>> > https://lore.kernel.org/all/20260306190948.44d23f8f@kernel.org/  

Ah, I might have misunderstood at the time.

   > allocate all necessary resources upfront then just swap them in
   > and reconfigure HW

In .set_channels() context, does "upfront" mean
(1) alloc all queue buffers at probe (and never dealloc) or
(2) alloc new buffer at operation start, swap in, then dealloc?

I had understood #1. I didn't see how that applied to this series (an
MTU change) which of course must realloc. But now it seems it was #2 as
you pointed to fbnic as reference and that's what fbnic_set_channels()
does.

>> Just checking if I got the idea before submitting another approach.
>> Something like:
>
> Hm, not really. Take a look at fbnic_set_ringparam()
> You need some struct that's config + pointers to all the resources.
> And make all allocation helpers operate on that without touching the HW.
> Then you can just allocate a new struct, give it whatever config you
> need, call all the alloc helpers with it. Now you have a fully
> populated struct and haven't touched the HW yet at all. Stop HW, 
> swap the resources, start HW.
>
> I did something similar for the nfp driver but that code has been
> slightly adulterated since I left Netronome so fbnic is clearer :)

Do you feel we should (1) clone the full `struct macb` as done by fbnic
or, (2) just partially, with the few interesting fields. Something like
`struct stmmac_dma_conf`.

stmmac is not the greatest example. They have this struct that carries
their buffers but they still "close -> update -> open" on operations
versus the optimal "alloc -> reconfigure_hw -> free".

With #2 we could use an unnamed structure field.
https://gcc.gnu.org/onlinedocs/gcc/Unnamed-Fields.html
See commit c4781dc3d1cf ("Kbuild: enable -fms-extensions").

struct macb_buffers {
	struct macb_queue queues[N];
	...
};

struct macb {
	struct macb_buffers;
	...
};

Goal is to keep `bp->queues` & co as before, to minimise the diff.

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com