Skip to contentSkip to conversation

BPF inuse counter is not safe when using lockfree transmit

DPDK/ethdev Bug 2051 [email protected] 1 participant 1 message Oct 8

lore
Share & tools
lore
#0 lore raw

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

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.

Signature · 2 lines
You are receiving this mail because:
You are the assignee for the bug.