[PATCH net-next v8 06/15] ibmveth: Refactor TX resource allocation in open/close paths

[Mingming Cao <mmc@linux.ibm.com>]

As with the RX refactor, pull TX LTB alloc/free out of open/close
into helpers and wire them in this patch.

  ibmveth_alloc_tx_resources()
  ibmveth_free_tx_resources()

They wrap the existing per-queue allocate_tx_ltb() / free_tx_ltb()
primitives. alloc_tx_resources() allocates every TX queue and unwinds
partial failure itself; free_tx_resources() walks real_num_tx_queues.
The helpers remove dependence on shared open/close loop indices and
match the RX helper structure. TX was already multi-queue capable via
ethtool -L.

free_tx_ltb() already returns early on a NULL slot and clears
tx_ltb_ptr[] before unmapping. Clear tx_ltb_dma[] there too, and on
the allocate_tx_ltb() DMA-map failure path, so a freed slot carries no
stale DMA address.

Stop TX at the start of open() with netif_tx_disable() so TX cannot
run while TX LTBs are still being allocated. That covers first-open
generic XDP_TX, whose queues are not DRV_XOFF.
netif_tx_start_all_queues() stays after alloc_tx_resources(), under
netif_tx_lock_bh(): RX NAPI is already live there, and generic
XDP_TX and netpoll test DRV_XOFF under the queue's xmit lock before
they read the LTB, so the lock orders the LTB stores before any
sender that sees the queues started.
Move TX LTB allocation to the end of open(), after LAN registration,
RX pools, RX interrupt setup, and the initial replenish kick. A late
alloc_tx_resources() failure jumps to out_cleanup_rx_interrupts and
must not call free_tx_resources() again: alloc already freed any
partial TX LTBs. RX is live by then; cleanup_rx_interrupts() waits
for the poll tail before the RX ring and pools are freed, as it
does for close(). start_xmit() bails if
tx_ltb_ptr[] is gone, counting the drop in tx_dropped and falling
into the existing out: label like the function's other drop paths.
That guard catches a sender that picked its queue before an
ethtool -L shrink and arrives after the free; a sender already in
xmit during the free is not covered, as before this series. The
close path is quiesced by netif_tx_disable().

Every failure after LAN registration still issues
h_free_logical_lan() before the RX pools are freed. As in close(),
the unwind does not check its result, and the late TX failure now
reaches it with RX buffers already posted; a later patch makes the
multi-queue paths keep that memory mapped on a hard failure.

close() keeps netif_tx_disable() and frees the LTBs through
free_tx_resources() after h_free_logical_lan().

Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>
Tested-by: Shaik Abdulla <shaik.abdulla1@ibm.com>
---
Changes in v8:
- netif_tx_disable() at the start of open(); the TX queues start under
  netif_tx_lock_bh(), since NAPI (and generic XDP_TX) is already live
- a late TX allocation failure unwinds through cleanup_rx_interrupts()
  (patch 5), which already waits for the poll
- start_xmit LTB guard comment is about a sender that picked its queue
  before a shrink
- commit message: the late TX failure unwind frees posted RX after an
  unchecked H_FREE, as close() does (MQ keeps it mapped from patch 10)

Dropped from this series (merged fixes own them):
- commit c807573ea58e: close() netif_tx_stop_all_queues() ->
  netif_tx_disable() (in base; this patch adds open()-start disable)
- commit 84bec0bf0352: free_tx_ltb() NULL check and NULL-first clear
  (in base; MQ refactor keeps the v6 behavior, not a duplicate fix hunk)
- commit 95811c873d1b: set_channels IFF_UP vs opened commit-message claim
  (base gates set_channels() on opened)

Changes in v7:
- late alloc_tx_resources() failure: synchronize_net()
  after cleanup_rx_interrupts(), as close() does, before
  the RX ring and pools are freed
- noted: no Fixes: peel of the pool-fail h_free; stay
  at 15; one standalone [PATCH net] after this series
  (Fixes: d43732ce021f)

Changes in v6:
- free_tx_ltb(): clear tx_ltb_ptr[idx] then tx_ltb_dma[idx] before
  unmap (NULL-first narrows the race; close still uses netif_tx_disable())
- start_xmit(): tx_dropped++ and goto out: when the LTB is missing
- allocate_tx_ltb(): comment that caller must leave tx_ltb_ptr[idx] NULL
- alloc/free_tx_resources kdoc uses real_num_tx_queues
- noted: ethtool -L TX shrink still stop-then-free; cover leftovers

Changes in v5:
- Quiesce TX with netif_tx_disable before free (stop_all_queues does not
  wait for in-flight xmit); free LTBs after h_free_logical_lan - direct
  close() callers bypass synchronize_net()
- Guard start_xmit if tx_ltb_ptr gone so open can leave RX live while TX
  LTB alloc still runs (also covers close/failed-reopen with IFF_UP set)
- Drop fake mid-open TX-leak / Fixes: motivation; reword as helper
  extraction matching RX (shared loop-index independence)
- Free TX LTB by pointer presence (drop dma==0 sentinel; dma_mapping_error
  already cleared the slot on map failure)
- Document intentional open-fail LAN-first unwind (free_lan before RX
  pool/DMA teardown) rather than leaving it silent in a TX-only refactor
- Drop drive-by blank-line cosmetics (header / start_xmit)

Changes in v4:
- Introduce the TX resource helpers in the same patch that wires their
  first open/close callers.
- Do not free TX LTBs again after a failed alloc_tx_resources();
  harden free_tx_ltb() against unset slots.
- Move TX allocation after RX IRQ setup / replenish kick so open()
  failure unwind no longer depends on a shared loop index (also fixes
  a mid-open TX LTB leak).

 drivers/net/ethernet/ibm/ibmveth.c | 111 +++++++++++++++++++++++------
 1 file changed, 89 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 62e6c9a9f5e9..cab895cc970c 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1248,17 +1248,27 @@ static int ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter,
 
 static void ibmveth_free_tx_ltb(struct ibmveth_adapter *adapter, int idx)
 {
-	void *ptr = adapter->tx_ltb_ptr[idx];
+	void *ltb = adapter->tx_ltb_ptr[idx];
+	dma_addr_t dma = adapter->tx_ltb_dma[idx];
 
-	if (!ptr)
+	if (!ltb)
 		return;
 
+	/*
+	 * Clear the slot before releasing it. start_xmit() tests
+	 * tx_ltb_ptr[idx] to decide whether the LTB is usable.
+	 */
 	adapter->tx_ltb_ptr[idx] = NULL;
-	dma_unmap_single(&adapter->vdev->dev, adapter->tx_ltb_dma[idx],
-			 adapter->tx_ltb_size, DMA_TO_DEVICE);
-	kfree(ptr);
+	adapter->tx_ltb_dma[idx] = 0;
+
+	dma_unmap_single(&adapter->vdev->dev, dma, adapter->tx_ltb_size,
+			 DMA_TO_DEVICE);
+	kfree(ltb);
 }
 
+/* Caller must ensure tx_ltb_ptr[idx] is NULL. open() runs on
+ * probe-zeroed slots; set_channels() skips populated indices.
+ */
 static int ibmveth_allocate_tx_ltb(struct ibmveth_adapter *adapter, int idx)
 {
 	adapter->tx_ltb_ptr[idx] = kzalloc(adapter->tx_ltb_size,
@@ -1277,12 +1287,54 @@ static int ibmveth_allocate_tx_ltb(struct ibmveth_adapter *adapter, int idx)
 			   "unable to DMA map tx long term buffer\n");
 		kfree(adapter->tx_ltb_ptr[idx]);
 		adapter->tx_ltb_ptr[idx] = NULL;
+		adapter->tx_ltb_dma[idx] = 0;
 		return -ENOMEM;
 	}
 
 	return 0;
 }
 
+/**
+ * ibmveth_alloc_tx_resources - Allocate TX LTBs for real_num_tx_queues
+ * @adapter: ibmveth adapter structure
+ *
+ * Allocates TX Long Term Buffers (LTBs) for real_num_tx_queues.
+ *
+ * Return: 0 on success, -ENOMEM on failure
+ */
+static int ibmveth_alloc_tx_resources(struct ibmveth_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+	int i;
+
+	for (i = 0; i < netdev->real_num_tx_queues; i++) {
+		if (ibmveth_allocate_tx_ltb(adapter, i))
+			goto err_free_ltbs;
+	}
+
+	return 0;
+
+err_free_ltbs:
+	while (--i >= 0)
+		ibmveth_free_tx_ltb(adapter, i);
+	return -ENOMEM;
+}
+
+/**
+ * ibmveth_free_tx_resources - Free TX LTBs for real_num_tx_queues
+ * @adapter: ibmveth adapter structure
+ *
+ * Frees TX Long Term Buffers (LTBs) for real_num_tx_queues.
+ */
+static void ibmveth_free_tx_resources(struct ibmveth_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+	int i;
+
+	for (i = 0; i < netdev->real_num_tx_queues; i++)
+		ibmveth_free_tx_ltb(adapter, i);
+}
+
 static int ibmveth_register_logical_lan(struct ibmveth_adapter *adapter,
         union ibmveth_buf_desc rxq_desc, u64 mac_address)
 {
@@ -1322,6 +1374,11 @@ static int ibmveth_open(struct net_device *netdev)
 
 	netdev_dbg(netdev, "open starting\n");
 
+	/* Stop TX before RX is live. First-open queues are not DRV_XOFF,
+	 * so generic XDP_TX can run during TX LTB alloc.
+	 */
+	netif_tx_disable(netdev);
+
 	for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
 		rxq_entries += adapter->rx_buff_pool[0][i].size;
 
@@ -1333,12 +1390,6 @@ static int ibmveth_open(struct net_device *netdev)
 	if (rc)
 		goto out_free_filter_list;
 
-	rc = -ENOMEM;
-	for (i = 0; i < netdev->real_num_tx_queues; i++) {
-		if (ibmveth_allocate_tx_ltb(adapter, i))
-			goto out_free_tx_ltb;
-	}
-
 	mac_address = ether_addr_to_u64(netdev->dev_addr);
 
 	rxq_desc.fields.flags_len = IBMVETH_BUF_VALID |
@@ -1360,12 +1411,12 @@ static int ibmveth_open(struct net_device *netdev)
 				     rxq_desc.desc,
 				     mac_address);
 		rc = -ENONET;
-		goto out_free_tx_ltb;
+		goto out_free_queue_mem;
 	}
 
 	rc = ibmveth_alloc_buffer_pools(adapter);
 	if (rc)
-		goto out_free_buffer_pools;
+		goto out_unregister_lan;
 
 	/* Enables NAPI, so everything ibmveth_poll() touches must exist:
 	 * netpoll during the direct close()/open() pairs, or a direct
@@ -1373,26 +1424,36 @@ static int ibmveth_open(struct net_device *netdev)
 	 */
 	rc = ibmveth_setup_rx_interrupts(adapter);
 	if (rc)
-		goto out_free_buffer_pools;
+		goto out_unregister_lan;
 
 	netdev_dbg(netdev, "initial replenish cycle\n");
 	ibmveth_schedule_rx_queue(adapter, 0);
 
+	rc = ibmveth_alloc_tx_resources(adapter);
+	if (rc)
+		goto out_cleanup_rx_interrupts;
+
+	/* NAPI is live, so generic XDP_TX can transmit as soon as it sees
+	 * the queues started. Senders test DRV_XOFF under the xmit lock;
+	 * starting the queues under it orders the LTB stores before them.
+	 */
+	netif_tx_lock_bh(netdev);
 	netif_tx_start_all_queues(netdev);
+	netif_tx_unlock_bh(netdev);
 
 	adapter->opened = true;
 	netdev_dbg(netdev, "open complete\n");
 
 	return 0;
 
-out_free_buffer_pools:
+out_cleanup_rx_interrupts:
+	ibmveth_cleanup_rx_interrupts(adapter);
+out_unregister_lan:
 	do {
 		lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
 	} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
 	ibmveth_free_buffer_pools(adapter);
-out_free_tx_ltb:
-	for (i = netdev->real_num_tx_queues - 1; i >= 0; i--)
-		ibmveth_free_tx_ltb(adapter, i);
+out_free_queue_mem:
 	ibmveth_cleanup_rx_resources(adapter);
 out_free_filter_list:
 	ibmveth_free_filter_list(adapter);
@@ -1404,7 +1465,6 @@ static int ibmveth_close(struct net_device *netdev)
 {
 	struct ibmveth_adapter *adapter = netdev_priv(netdev);
 	long lpar_rc;
-	int i;
 
 	/* change_mtu, pool sysfs, set_csum and set_tso call close() and
 	 * open() directly. If that open() fails, IFF_UP stays set and
@@ -1430,13 +1490,12 @@ static int ibmveth_close(struct net_device *netdev)
 			   lpar_rc);
 	}
 	ibmveth_update_rx_no_buffer(adapter);
+	/* netif_tx_disable() above keeps xmit off the TX LTBs. */
+	ibmveth_free_tx_resources(adapter);
 	ibmveth_free_buffer_pools(adapter);
 	ibmveth_cleanup_rx_resources(adapter);
 	ibmveth_free_filter_list(adapter);
 
-	for (i = 0; i < netdev->real_num_tx_queues; i++)
-		ibmveth_free_tx_ltb(adapter, i);
-
 	netdev_dbg(netdev, "close complete\n");
 
 	return 0;
@@ -1864,6 +1923,14 @@ static netdev_tx_t ibmveth_start_xmit(struct sk_buff *skb,
 	int i, queue_num = skb_get_queue_mapping(skb);
 	unsigned long mss = 0;
 
+	/* TX LTBs are freed on shrink; a sender that picked its queue
+	 * before the shrink can get here after the free.
+	 */
+	if (unlikely(!adapter->tx_ltb_ptr[queue_num])) {
+		netdev->stats.tx_dropped++;
+		goto out;
+	}
+
 	if (ibmveth_is_packet_unsupported(skb, netdev))
 		goto out;
 	/* veth can't checksum offload UDP */
-- 
2.50.1 (Apple Git-155)