Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 1 Sep 2020 20:50:17 +0000 (UTC)
From:      Vincenzo Maffione <vmaffione@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r365065 - stable/12/sys/net
Message-ID:  <202009012050.081KoHWF098770@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: vmaffione
Date: Tue Sep  1 20:50:16 2020
New Revision: 365065
URL: https://svnweb.freebsd.org/changeset/base/365065

Log:
  MFC r364655
  
  iflib: fix isc_rxd_flush call in netmap_fl_refill()
  
  The semantic of the pidx argument of isc_rxd_flush() is the
  last valid index of in the free list, rather than the next
  index to be published. However, netmap was still using the
  old convention. While there, also refactor the netmap_fl_refill()
  to simplify a little bit and add an assertion.

Modified:
  stable/12/sys/net/iflib.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/net/iflib.c
==============================================================================
--- stable/12/sys/net/iflib.c	Tue Sep  1 20:47:59 2020	(r365064)
+++ stable/12/sys/net/iflib.c	Tue Sep  1 20:50:16 2020	(r365065)
@@ -762,7 +762,7 @@ iflib_num_tx_descs(if_ctx_t ctx)
 
 MODULE_DEPEND(iflib, netmap, 1, 1, 1);
 
-static int netmap_fl_refill(iflib_rxq_t rxq, struct netmap_kring *kring, uint32_t nm_i, bool init);
+static int netmap_fl_refill(iflib_rxq_t rxq, struct netmap_kring *kring, bool init);
 
 /*
  * device-specific sysctl variables:
@@ -833,33 +833,43 @@ iflib_netmap_register(struct netmap_adapter *na, int o
 }
 
 static int
-netmap_fl_refill(iflib_rxq_t rxq, struct netmap_kring *kring, uint32_t nm_i, bool init)
+netmap_fl_refill(iflib_rxq_t rxq, struct netmap_kring *kring, bool init)
 {
 	struct netmap_adapter *na = kring->na;
 	u_int const lim = kring->nkr_num_slots - 1;
 	u_int head = kring->rhead;
+	u_int nm_i = kring->nr_hwcur;
 	struct netmap_ring *ring = kring->ring;
 	bus_dmamap_t *map;
 	struct if_rxd_update iru;
 	if_ctx_t ctx = rxq->ifr_ctx;
 	iflib_fl_t fl = &rxq->ifr_fl[0];
-	uint32_t nic_i_first, nic_i;
+	u_int nic_i_first, nic_i;
 	int i;
 #if IFLIB_DEBUG_COUNTERS
 	int rf_count = 0;
 #endif
 
-	if (nm_i == head && __predict_true(!init))
+	/*
+	 * Netmap requires that we leave (at least) one free slot
+	 * in the ring, so that it can distinguish between an empty
+	 * ring (nr_hwcur == nr_hwtail, i.e. all the buffers owned by the
+	 * user) and a full ring (nr_hwtail == (nr_hwcur - 1) mod N, i.e.
+	 * all the buffers owned by the kernel).
+	 * We thus set head (the refill limit) to nr_hwcur - 1
+	 * at initialization. The rest of the code will then make sure
+	 * than nr_hwtail never overcomes nr_hwcur.
+	 */
+	if (__predict_false(init)) {
+		head = nm_prev(nm_i, lim);
+	} else if (nm_i == head) {
+		/* Nothing to do. We can leave early. */
 		return (0);
+	}
 
 	iru_init(&iru, rxq, 0 /* flid */);
 	map = fl->ifl_sds.ifsd_map;
 	nic_i = netmap_idx_k2n(kring, nm_i);
-	/*
-	 * IMPORTANT: we must leave one free slot in the ring,
-	 * so move head back by one unit
-	 */
-	head = nm_prev(head, lim);
 	DBG_COUNTER_INC(fl_refills);
 	while (nm_i != head) {
 #if IFLIB_DEBUG_COUNTERS
@@ -900,9 +910,13 @@ netmap_fl_refill(iflib_rxq_t rxq, struct netmap_kring 
 	}
 	kring->nr_hwcur = head;
 
+	/* The pidx argument of isc_rxd_flush() is the index of the last valid
+	 * slot in the free list ring. We need therefore to decrement nic_i,
+	 * similarly to what happens in iflib_fl_refill() for ifl_pidx. */
 	bus_dmamap_sync(fl->ifl_ifdi->idi_tag, fl->ifl_ifdi->idi_map,
 	    BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE);
-	ctx->isc_rxd_flush(ctx->ifc_softc, rxq->ifr_id, fl->ifl_id, nic_i);
+	ctx->isc_rxd_flush(ctx->ifc_softc, rxq->ifr_id, fl->ifl_id,
+	    nm_prev(nic_i, lim));
 	DBG_COUNTER_INC(rxd_flush);
 
 	return (0);
@@ -1132,6 +1146,7 @@ iflib_netmap_rxsync(struct netmap_kring *kring, int fl
 
 		nic_i = fl->ifl_cidx;
 		nm_i = netmap_idx_n2k(kring, nic_i);
+		MPASS(nm_i == kring->nr_hwtail);
 		for (n = 0; avail > 0 && nm_i != hwtail_lim; n++, avail--) {
 			rxd_info_zero(&ri);
 			ri.iri_frags = rxq->ifr_frags;
@@ -1170,9 +1185,9 @@ iflib_netmap_rxsync(struct netmap_kring *kring, int fl
 	 * nic_i is the index in the NIC ring, and
 	 * nm_i == (nic_i + kring->nkr_hwofs) % ring_size
 	 */
-	nm_i = kring->nr_hwcur;
+	netmap_fl_refill(rxq, kring, false);
 
-	return (netmap_fl_refill(rxq, kring, nm_i, false));
+	return (0);
 }
 
 static void
@@ -1244,14 +1259,12 @@ iflib_netmap_rxq_init(if_ctx_t ctx, iflib_rxq_t rxq)
 	struct netmap_adapter *na = NA(ctx->ifc_ifp);
 	struct netmap_kring *kring;
 	struct netmap_slot *slot;
-	uint32_t nm_i;
 
 	slot = netmap_reset(na, NR_RX, rxq->ifr_id, 0);
 	if (slot == NULL)
 		return (0);
 	kring = na->rx_rings[rxq->ifr_id];
-	nm_i = netmap_idx_n2k(kring, 0);
-	netmap_fl_refill(rxq, kring, nm_i, true);
+	netmap_fl_refill(rxq, kring, true);
 	return (1);
 }
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202009012050.081KoHWF098770>