[PATCH 6/9] bpf: simplify callback inuse tracking to boolean flag

[Marat Khalili]

The even/odd usage counter semantics in struct bpf_eth_cbi was
originally used to detect when in-flight packet callbacks complete.
However, with at most one thread running the callback per port/queue
pair and no risk of the ABA problem, a binary flag (1 for in use,
0 for idle) suffices.

Replace the counter increment in the datapath with stores of 1 and 0,
and simplify bpf_eth_cbi_wait() to wait directly until the flag is 0.
Update the documentation and synchronization sequence diagram
accordingly.

Signed-off-by: Marat Khalili <qm2k21 at gmail.com>
---
 lib/bpf/bpf_pkt.c | 46 ++++++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/lib/bpf/bpf_pkt.c b/lib/bpf/bpf_pkt.c
index 68671848e9..7ae64ed7d0 100644
--- a/lib/bpf/bpf_pkt.c
+++ b/lib/bpf/bpf_pkt.c
@@ -36,12 +36,12 @@
  * 2. Member `cb` of struct `bpf_eth_cbi`;
  * 3. Member `use` of struct `bpf_eth_cbi`;
  *
- * BPF program is only executed if the first two are set, and increments `use` at the start and
- * end of the callback execution so that in the safe state its value is even.
+ * BPF program is only executed if the first two are set, and sets `use` to 1 at the start and
+ * to 0 at the end of the callback execution.
  * This relies on only one thread calling the per-port/queue callback at a time.
  *
  * Unload processing starts with clearing the first two entities,
- * then waits for `use` to become even before destroying the BPF program.
+ * then waits for `use` to become 0 before destroying the BPF program.
  * The whole operation is protected by a control-plane-only spinlock (not shown on the diagram).
  *
  * Typical sequence of synchronization actions between CP and DP on unload is shown below:
@@ -52,7 +52,7 @@
  *     |                      |               |              |              |                |
  *     |                      |<---------------------- fetch callback -----------------------|
  *     |                      |               |              |              |                |
- *     |                      |               |<------------- increment to odd --------------|
+ *     |                      |               |<------------- store 1 -----------------------|
  *     |                      |               |              |              |                |
  *     |-- remove callback -->|               |              |              |       rte_smp_mb()
  *     |                      |               |              |              |                |
@@ -62,17 +62,17 @@
  *     |                      |               |              |              |                |
  *   rte_smp_mb()             |               |              |              |                |
  *     |                      |               |              |              |                |
- *     +-if use is odd        |               |              |              | if cb not NULL-+
+ *     +-while use is 1       |               |              |              | if cb not NULL-+
  *     | |                    |               |              |              |              | |
- *     | +------------ fetch odd ------------>|              |              |<-- execute --+ |
+ *     | +------------ fetch 1 -------------->|              |              |<-- execute --+ |
  *     | |                    |               |              |              |  BPF program | |
- *     | +-while use is same  |               |              |              |              +-+
- *     | | |                  |               |              |              |                |
- *     | | +---------- fetch odd ------------>|              |              |      rte_smp_rmb()
- *     | | |                  |               |              |              |                |
- *     +-+-+                  |               |<------------- increment to even -------------|
+ *     | |                    |               |              |              |              +-+
+ *     | |                    |               |              |              |                |
+ *     | |                    |               |              |              |      rte_smp_rmb()
+ *     | |                    |               |              |              |                |
+ *     +-+                    |               |<------------- store 0 -----------------------|
  *     |                      |               |              |              |                |
- *     |------------- fetch even ------------>|              |              |                |
+ *     |------------- fetch 0 --------------->|              |              |                |
  *     |                      |               |              |              |                |
  *     |----------------- destroy BPF program and cleanup ----------------->|                |
  *     |                      |               |              |              |                |
@@ -87,7 +87,7 @@
  */
 struct __rte_cache_aligned bpf_eth_cbi {
 	/* used by both data & control path */
-	RTE_ATOMIC(uint32_t) use;    /*usage counter */
+	RTE_ATOMIC(uint32_t) use;    /* usage flag: 1 when in use, 0 otherwise */
 	const struct rte_eth_rxtx_callback *cb;  /* callback handle */
 	struct rte_bpf *bpf;
 	struct rte_bpf_jit jit;
@@ -98,8 +98,8 @@ struct __rte_cache_aligned bpf_eth_cbi {
 };
 
 /*
- * Odd number means that callback is used by datapath.
- * Even number means that callback is not used by datapath.
+ * 1 means that callback is used by datapath.
+ * 0 means that callback is not used by datapath.
  */
 #define BPF_ETH_CBI_INUSE  1
 
@@ -141,20 +141,20 @@ static struct bpf_eth_cbh tx_cbh = {
 static __rte_always_inline void
 bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
 {
-	cbi->use++;
+	cbi->use = BPF_ETH_CBI_INUSE;
 	/* make sure no store/load reordering could happen */
 	rte_smp_mb();
 }
 
 /*
- * Marks given callback list as not used by datapath.
+ * Marks given callback as not used by datapath.
  */
 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++;
+	cbi->use = 0;
 }
 
 /*
@@ -163,18 +163,12 @@ bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi)
 static void
 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();
 
-	puse = cbi->use;
-
 	/* 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);
-	}
+	RTE_WAIT_UNTIL_MASKED((__rte_atomic uint32_t *)(uintptr_t)&cbi->use,
+		UINT32_MAX, ==, 0, rte_memory_order_relaxed);
 }
 
 static void
-- 
2.43.0