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

[Nicolai Buchwitz <nb@tipi-net.de>]

On 18.3.2026 10:53, Théo Lebrun wrote:
> [...]

>> 
>> 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.

The unnamed struct idea is nice for keeping the diff small, but I'm not
sure it works cleanly for the per-queue case. The macb_queue_ring fields
would be embedded anonymously in macb_queue, but the alloc helpers need
to write into a separate clone. The swap then becomes memcpy/cast tricks
between the anonymous portion of each queue and the clone's qring[q].

Embedding the full queues[] array in the swappable struct would be
simpler for the swap, but then NAPI, IRQs, spinlocks etc. travel
along, which we don't want.

For struct macb itself (rx_buffer_size, ring sizes, tieoff) the unnamed
struct works fine and saves a lot of churn. It's really the per-queue
ring fields where it gets awkward.

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

Cheers
Nicolai