All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Théo Lebrun" <theo.lebrun@bootlin.com>
To: "Théo Lebrun" <theo.lebrun@bootlin.com>,
	"Conor Dooley" <conor.dooley@microchip.com>,
	"Andrew Lunn" <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Richard Cochran" <richardcochran@gmail.com>,
	"Russell King" <linux@armlinux.org.uk>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
	"Paolo Valerio" <pvalerio@redhat.com>,
	"Nicolai Buchwitz" <nb@tipi-net.de>,
	"Vladimir Kondratiev" <vladimir.kondratiev@mobileye.com>,
	"Gregory CLEMENT" <gregory.clement@bootlin.com>,
	"Benoît Monin" <benoit.monin@bootlin.com>,
	"Tawfik Bayouk" <tawfik.bayouk@mobileye.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Maxime Chevallier" <maxime.chevallier@bootlin.com>
Subject: [PATCH net-next v3 00/15] net: macb: implement context swapping
Date: Wed, 01 Jul 2026 17:59:03 +0200	[thread overview]
Message-ID: <20260701-macb-context-v3-0-00268d5b1502@bootlin.com> (raw)

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 at the moment), 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.

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>
---
Changes in v3:
- Use `const struct macb_info *info` instead of bare `u32 caps` as
  helper arguments, for type safety.
- macb_interrupt(): the pre-lock readl(ISR) to detect spurious
  interrupts is only done if CLEAR_ON_WRITE.
- Don't forget allocating context in at91ether_open().
- swap:
  - Refuse swap for EMAC HW; it would crash because codepaths are so
    different.
  - Grab new bp->mac_cfg_lock to serialise with phylink MAC callbacks.
    We cannot rely on phydev->lock because it isn't present in the SFP
    or fixed-link cases. We also want to avoid phylink_stop() which
    triggers a slow PHY retrain.
- swap start:
  - We used to do disable-irqs-and-hw then drain-all-bh-features, but
    then HW might be raced against. Instead we disable-irqs then
    drain-all-bh then disable-hw which means at disable-hw step no BH
    context can be active.
  - Use macb_halt_tx() helper to properly stop HW.
  - Disable BH features before netif_tx_disable() call to avoid queue
    wakeup races.
  - Use macb_queue_isr_clear() helper instead of manual if-then-writel.
- swap end:
  - Grab bp->lock for the hardware reinit sequence composed of DMACFG
    and NCR writes.
  - Drop now useless EMAC check (we refuse EMAC HW before swapping).
- nits:
  - New patch to rename macb_{alloc,free}_consistent() which don't only
    allocate consistent buffers since a long time ago.
  - Fix the start_xmit verbose netdev_vdbg() format string from %hu to %u
    because the queue index type changed.
  - Strong commit reword from "unify `struct macb *` naming convention"
    to "unify variable naming convention in at91ether functions" which
    was underselling the changes.
- Rebase upon latest net-next/main (1c664ec4b9ea).
- Link to v2: https://patch.msgid.link/20260410-macb-context-v2-0-af39f71d40b6@bootlin.com

Changes in v2:
- Patch "add subset of `struct macb` to `struct macb_context`" was
  messed up. It contained much more than what the name implied. Split
  into three commits (I caused trouble by rebase reordering).
- Fix tieoff; V1 allocated it without initialisation.
- Fix NULL pointer dereference on context in mab_get_regs() and
  macb_get_ringparam() when interface is offline.
- Patch "unify device pointer naming convention":
  - Fix build issue when CONFIG_NETCONSOLE=y.
  - Rename `struct net_device *dev` to `netdev` in macb.h.
  - Rename `struct phy_device *phy` to `phydev` in macb_main.c.
- On swap, call netdev_tx_reset_queue() to reset all DQL counters.
- At end of swap, add missing kfree(old_ctx).
- During HW disabling in swap, grab bp->lock to protect against IRQ
  handler.
- On swap, cancel the three BH features MACB has:
  bp->hresp_err_bh_work, bp->tx_lpi_work and queue->tx_error_task.
- On swap, call macb_configure_dma() which writes buffer size to
  hardware registers. This is important because the change_mtu codepath
  changes the buffer size.
- Rebase onto latest net-next/main (58dd34dbd5b0) & resolve conflicts.
- Link to v1: https://patch.msgid.link/20260401-macb-context-v1-0-9590c5ab7272@bootlin.com

---
Théo Lebrun (15):
      net: macb: drop "consistent" from alloc/free function names
      net: macb: unify device pointer naming convention
      net: macb: unify variable naming convention in at91ether functions
      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: change caps helpers signatures
      net: macb: change function signatures to take contexts
      net: macb: introduce macb_context_alloc() helper
      net: macb: re-read ISR inside IRQ handler locked section
      net: macb: use context swapping in .set_ringparam()
      net: macb: use context swapping in .ndo_change_mtu()

 drivers/net/ethernet/cadence/macb.h      |  127 +-
 drivers/net/ethernet/cadence/macb_main.c | 1849 +++++++++++++++++-------------
 drivers/net/ethernet/cadence/macb_pci.c  |   46 +-
 drivers/net/ethernet/cadence/macb_ptp.c  |   26 +-
 4 files changed, 1199 insertions(+), 849 deletions(-)
---
base-commit: 64e6f7613459a9b344fb09a129584435b281088b
change-id: 20260401-macb-context-bd0caf20414d

Best regards,
--  
Théo Lebrun <theo.lebrun@bootlin.com>


             reply	other threads:[~2026-07-01 15:59 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 15:59 Théo Lebrun [this message]
2026-07-01 15:59 ` [PATCH net-next v3 01/15] net: macb: drop "consistent" from alloc/free function names Théo Lebrun
2026-07-02 10:41   ` Nicolai Buchwitz
2026-07-03 11:28   ` Conor Dooley
2026-07-03 16:32     ` Théo Lebrun
2026-07-03 16:34       ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 02/15] net: macb: unify device pointer naming convention Théo Lebrun
2026-07-03 11:29   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 03/15] net: macb: unify variable naming convention in at91ether functions Théo Lebrun
2026-07-02 10:42   ` Nicolai Buchwitz
2026-07-03 11:30   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 04/15] net: macb: unify queue index variable naming convention and types Théo Lebrun
2026-07-02 10:43   ` Nicolai Buchwitz
2026-07-03 11:34   ` Conor Dooley
2026-07-03 17:10     ` Théo Lebrun
2026-07-01 15:59 ` [PATCH net-next v3 05/15] net: macb: enforce reverse christmas tree (RCT) convention Théo Lebrun
2026-07-02 10:48   ` Nicolai Buchwitz
2026-07-03 11:35   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 06/15] net: macb: allocate tieoff descriptor once across device lifetime Théo Lebrun
2026-07-02 10:54   ` Nicolai Buchwitz
2026-07-01 15:59 ` [PATCH net-next v3 07/15] net: macb: introduce macb_context struct for buffer management Théo Lebrun
2026-07-03 11:39   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 08/15] net: macb: avoid macb_init_rx_buffer_size() modifying state Théo Lebrun
2026-07-01 15:59 ` [PATCH net-next v3 09/15] net: macb: make `struct macb` subset reachable from macb_context struct Théo Lebrun
2026-07-01 15:59 ` [PATCH net-next v3 10/15] net: macb: change caps helpers signatures Théo Lebrun
2026-07-03 11:43   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 11/15] net: macb: change function signatures to take contexts Théo Lebrun
2026-07-03 11:45   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 12/15] net: macb: introduce macb_context_alloc() helper Théo Lebrun
2026-07-01 15:59 ` [PATCH net-next v3 13/15] net: macb: re-read ISR inside IRQ handler locked section Théo Lebrun
2026-07-03 12:09   ` Conor Dooley
2026-07-01 15:59 ` [PATCH net-next v3 14/15] net: macb: use context swapping in .set_ringparam() Théo Lebrun
2026-07-02 10:37   ` Nicolai Buchwitz
2026-07-01 15:59 ` [PATCH net-next v3 15/15] net: macb: use context swapping in .ndo_change_mtu() Théo Lebrun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260701-macb-context-v3-0-00268d5b1502@bootlin.com \
    --to=theo.lebrun@bootlin.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=benoit.monin@bootlin.com \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=conor.dooley@microchip.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gregory.clement@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=maxime.chevallier@bootlin.com \
    --cc=nb@tipi-net.de \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=pabeni@redhat.com \
    --cc=pvalerio@redhat.com \
    --cc=richardcochran@gmail.com \
    --cc=tawfik.bayouk@mobileye.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vladimir.kondratiev@mobileye.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.