[DPDK/ethdev Bug 2051] BPF inuse counter is not safe when using lockfree transmit

[DPDK/ethdev Bug 2051] BPF inuse counter is not safe when using lockfree transmit

[bugzilla]

https://bugs.dpdk.org/show_bug.cgi?id=2051

            Bug ID: 2051
           Summary: BPF inuse counter is not safe when using lockfree
                    transmit
           Product: DPDK
           Version: 26.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---

The BPF ethdev callbacks (lib/bpf/bpf_pkt.c) guard cbi->bpf and cbi->jit
with a seqcount: bpf_eth_cbi_inuse() makes cbi->use odd,
bpf_eth_cbi_unuse() makes it even, and bpf_eth_cbi_wait() treats even as
"no thread is inside the callback". This is only valid with one thread
at a time in the callback for a given port/queue.

With RTE_ETH_TX_OFFLOAD_MT_LOCKFREE, multiple threads may call
rte_eth_tx_burst() on the same queue concurrently, so the tx callbacks
(bpf_tx_callback_vm/jit/mb_vm/mb_jit) run concurrently on the same cbi.

1. Parity. Two threads pass bpf_eth_cbi_inuse() and see cb != NULL; the
   count is even. rte_bpf_eth_tx_unload() clears cb, bpf_eth_cbi_wait()
   samples an even count and returns, and rte_bpf_destroy() frees the
   program and JIT code while both threads are executing it.
   Use after free.

2. Lost update. In the default build (enable_stdatomic=false) cbi->use++
   is a non-atomic read-modify-write. Concurrent increments can lose
   one, which inverts the parity permanently. An idle queue then reads
   odd, so bpf_eth_cbi_wait() spins until the next burst (forever if
   there is none), and afterwards even means in use, which leads back
   to case 1 with a single thread.

Rx is not affected; rx burst on a queue is never MT safe.

Nothing in rte_bpf_eth_tx_install() or rte_bpf_eth_tx_elf_load() checks
for the offload, and the restriction is not documented.

lib/pdump has the same pattern (pdump_cb_hold/pdump_cb_release/
pdump_cb_wait on use_count, used by pdump_tx) and the same problem.

Found by inspection while reviewing the conversion of rte_smp_mb() to
C11 fences. Not reproduced.

Possible fixes:
 - Fail tx install with -ENOTSUP when MT_LOCKFREE is enabled on the
   port or queue, and document it.
 - Replace the seqcount with a real reference count (atomic add/sub,
   wait for zero, keep the cb check). Costs two locked operations per
   burst.

-- 
You are receiving this mail because:
You are the assignee for the bug.