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 <[email protected]> --- 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
