[PATCH v3 01/29] bpf: replace deprecated SMP barriers with C11 fences

[Stephen Hemminger]

The rx/tx callback use counter relied on rte_smp_mb() and
rte_smp_rmb(), which are deprecated. Convert it to C11 atomics.

The counter is odd while the datapath is inside the callback and
even otherwise. Only one thread at a time does rx/tx burst on a
queue, so the counter has a single writer and is updated with a
plain load and store.

 - bpf_eth_cbi_inuse(): keep a full barrier after the store.
   The datapath stores the counter then loads cb; unload stores
   cb then loads the counter. A seq_cst fence on each side makes
   sure at least one of them sees the other's store.

 - bpf_eth_cbi_unuse(): use a release store instead of a read
   barrier before the increment.

 - bpf_eth_cbi_wait(): use acquire loads to pair with that store,
   so rte_bpf_destroy() is ordered after the datapath's last use
   of the program.

 - bpf_eth_cbi_unload(): drop the barrier; bpf_eth_cbi_wait()
   starts with one.

With enable_stdatomic the old cbi->use++ was a locked add. On x86
that build now does one locked instruction per burst instead of
three.

Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
---
 lib/bpf/bpf_pkt.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/lib/bpf/bpf_pkt.c b/lib/bpf/bpf_pkt.c
index f072fdaaed..95f76e47c7 100644
--- a/lib/bpf/bpf_pkt.c
+++ b/lib/bpf/bpf_pkt.c
@@ -36,10 +36,6 @@ struct __rte_cache_aligned bpf_eth_cbi {
 	uint16_t queue;
 };
 
-/*
- * Odd number means that callback is used by datapath.
- * Even number means that callback is not used by datapath.
- */
 #define BPF_ETH_CBI_INUSE  1
 
 /*
@@ -74,15 +70,34 @@ static struct bpf_eth_cbh tx_cbh = {
 	.type = BPF_ETH_TX,
 };
 
+/*
+ * Removing an rx/tx callback involves two steps (similar to RCU).
+ * The callback is first removed from the ethdev queue so that
+ * it will not be used by later burst.
+ * But the callback may still be in process or the core may have
+ * raced and seen the old callback.
+ * The use counter is used to indicate that it is not safe to
+ * free the BPF program yet.
+ * The datapath makes the counter odd on entry and even on exit.
+ * During unload, if the counter is odd then it indicates
+ * we must wait.
+ * This assumes that only one thread at a time may do rx/tx burst
+ * on a queue. Therefore the counter has a single writer and the
+ * increment need not be atomic.
+ */
+
 /*
  * Marks given callback as used by datapath.
  */
 static __rte_always_inline void
 bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
 {
-	cbi->use++;
-	/* make sure no store/load reordering could happen */
-	rte_smp_mb();
+	rte_atomic_store_explicit(&cbi->use,
+		rte_atomic_load_explicit(&cbi->use, rte_memory_order_relaxed) + 1,
+		rte_memory_order_relaxed);
+
+	/* full barrier: count must be visible before cb is read */
+	rte_atomic_thread_fence(rte_memory_order_seq_cst);
 }
 
 /*
@@ -91,9 +106,10 @@ bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
 static __rte_always_inline void
 bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi)
 {
-	/* make sure all previous loads are completed */
-	rte_smp_rmb();
-	cbi->use++;
+	/* release: pairs with the acquire bpf_eth_cbi_wait() */
+	rte_atomic_store_explicit(&cbi->use,
+		rte_atomic_load_explicit(&cbi->use, rte_memory_order_relaxed) + 1,
+		rte_memory_order_release);
 }
 
 /*
@@ -104,15 +120,15 @@ bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
 {
 	uint32_t puse;
 
-	/* make sure all previous loads and stores are completed */
-	rte_smp_mb();
+	/* full barrier: cleared cb must be visible before counter is read */
+	rte_atomic_thread_fence(rte_memory_order_seq_cst);
 
-	puse = cbi->use;
+	puse = rte_atomic_load_explicit(&cbi->use, rte_memory_order_acquire);
 
 	/* in use, busy wait till current RX/TX iteration is finished */
 	if ((puse & BPF_ETH_CBI_INUSE) != 0) {
 		RTE_WAIT_UNTIL_MASKED((__rte_atomic uint32_t *)(uintptr_t)&cbi->use,
-			UINT32_MAX, !=, puse, rte_memory_order_relaxed);
+			UINT32_MAX, !=, puse, rte_memory_order_acquire);
 	}
 }
 
@@ -439,7 +455,6 @@ bpf_eth_cbi_unload(struct bpf_eth_cbi *bc)
 {
 	/* mark this cbi as empty */
 	bc->cb = NULL;
-	rte_smp_mb();
 
 	/* make sure datapath doesn't use bpf anymore, then destroy bpf */
 	bpf_eth_cbi_wait(bc);
-- 
2.53.0