Re: [PATCH net-next 00/11] net: macb: implement context swapping

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

Hello Nicolai,

On Thu Apr 2, 2026 at 1:35 PM CEST, Nicolai Buchwitz wrote:
> On 1.4.2026 18:39, Théo Lebrun wrote:
>> MACB has a pretty primitive approach to buffer management. They are all
>> stored in `struct macb *bp`. On operations that require buffer realloc
>> (set_ringparam & change_mtu ATM), the only option is to close the
>> interface, change our global state and re-open the interface.
>> 
>> Two issues:
>> - It doesn't fly on memory pressured systems; we free our precious
>>   buffers and don't manage to reallocate fully, meaning our machine
>>   just lost its network access.
>> - Anecdotally, it is pretty slow because it implies a full PHY reinit.
>> 
>> Instead, we shall:
>>  - allocate a new context (including buffers) first
>>  - if it fails, early return without any impact to the interface
>>  - stop interface
>>  - update global state (bp, netdev, etc)
>>  - pass newly allocated buffer pointers to the hardware
>>  - start interface
>>  - free old context
>> 
>> This is what we implement here. Both .set_ringparam() and
>> .ndo_change_mtu() are covered by this series. In the future,
>> at least .set_channels() [0], XDP [1] and XSK [2] would benefit.
>
> Thanks for your work, the context swapping approach probably
> makes a lot of sense and will finally bring proper MTU change
> support that I tried to patch earlier.

Thanks for the review!

>> The change is super intrusive so conflicts will be major. Sorry!
>> 
>> Thanks,
>> Have a nice day,
>> Théo
>> 
>> [0]: 
>> https://lore.kernel.org/netdev/20260317-macb-set-channels-v4-0-1bd4f4ffcfca@bootlin.com/
>> [1]: 
>> https://lore.kernel.org/netdev/20260323221047.2749577-1-pvalerio@redhat.com/
>> [2]: 
>> https://lore.kernel.org/netdev/20260304-macb-xsk-v1-0-ba2ebe2bdaa3@bootlin.com/
>> 
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> Théo Lebrun (11):
>>       net: macb: unify device pointer naming convention
>>       net: macb: unify `struct macb *` naming convention
>>       net: macb: unify queue index variable naming convention and types
>>       net: macb: enforce reverse christmas tree (RCT) convention
>>       net: macb: allocate tieoff descriptor once across device lifetime
>>       net: macb: introduce macb_context struct for buffer management
>>       net: macb: avoid macb_init_rx_buffer_size() modifying state
>>       net: macb: make `struct macb` subset reachable from macb_context 
>> struct
>>       net: macb: introduce macb_context_alloc() helper
>>       net: macb: use context swapping in .set_ringparam()
>>       net: macb: use context swapping in .ndo_change_mtu()
>> 
>>  drivers/net/ethernet/cadence/macb.h      |  119 +-
>>  drivers/net/ethernet/cadence/macb_main.c | 1731 
>> +++++++++++++++++-------------
>>  drivers/net/ethernet/cadence/macb_pci.c  |   46 +-
>>  drivers/net/ethernet/cadence/macb_ptp.c  |   26 +-
>>  4 files changed, 1090 insertions(+), 832 deletions(-)
>> ---
>> base-commit: 321d1ee521de1362c22adadbc0ce066050a17783
>
> The series didn't apply cleanly on current net-next. The
> base commit 321d1ee521de doesn't seem to be upstream yet, is
> this based on your set_channels v4 series?

Surprising. I fetched net-next/main yesterday morning. My branch is:
 - net-next/main @ f1359c240191
 - my three series needed for working networking on MACB [0][1][2]
 - some dev defconfigs
 - finally the series sent upstream (b4 cover letter then series)

I confirm it applies on f1359c240191 but not on today's
net-next/main @ 269389ba5398.

I should experiment with b4 series dependency management. IIUC it would
have exposed through public metadata that my parent commit was
f1359c240191 even if it isn't strictly true locally.

We happen to add macb_{alloc,free}_tieoff() just above
at91_default_usrio which got edited by Conor in cee10a01e286 ("net:
macb: fix use of at91_default_usrio without CONFIG_OF"). Will fix in V2.

Thanks,

[0]: https://lore.kernel.org/all/20260225-macb-phy-v7-0-665bd8619d51@bootlin.com/
[1]: https://lore.kernel.org/all/20260225-macb-phy-v7-0-d3c9842ec931@bootlin.com/
[2]: https://lore.kernel.org/linux-phy/20260309-macb-phy-v9-0-5afd87d9db43@bootlin.com/

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