On Tue, 13 Oct 2026 17:04:33 +0100 Marat Khalili <[email protected]> wrote:
> On 13/10/2026 08:54, David Marchand wrote: > > This blocks the SMP barrier removal for rc1. > > > > What is your proposal? > > > > I think it makes sense to take > > https://patchwork.dpdk.org/project/dpdk/list/?series=39599 first, then > > resolve the conflict for the patch from this thread. > > Is this BPF series ready for merge? > > > > If it is not ready yet, what should be done on the current patch? > > Is a s/rte_smp_rmb()/rte_atomic_thread_fence(rte_memory_order_seq_cst)/ > > enough? > > The commitlog would also need some rewritting. > > I think we should be able to unblock this patchset with the diff below. > Last version contains some additional out-of-scope changes that might > not be helpful in the long run, but I tend to believe Stephen and > Wathsala that they are not breaking anything (I have not checked it > myself). If the version below is ok with Stephen I would prefer it > (consider it acked), otherwise I can withdraw my NAK for the sake of the > release, at the cost of some potentially confusing noise in the commit > history in the future. > > diff --git a/lib/bpf/bpf_pkt.c b/lib/bpf/bpf_pkt.c > index f072fdaaed..0d0c37cdc5 100644 > --- a/lib/bpf/bpf_pkt.c > +++ b/lib/bpf/bpf_pkt.c > @@ -82,7 +82,7 @@ bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi) > { > cbi->use++; > /* make sure no store/load reordering could happen */ > - rte_smp_mb(); > + rte_atomic_thread_fence(rte_memory_order_seq_cst); > } > > /* > @@ -92,7 +92,7 @@ static __rte_always_inline void > bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi) > { > /* make sure all previous loads are completed */ > - rte_smp_rmb(); > + rte_atomic_thread_fence(rte_memory_order_release); > cbi->use++; > } > > @@ -105,7 +105,7 @@ 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(); > + rte_atomic_thread_fence(rte_memory_order_seq_cst); > > puse = cbi->use; > > @@ -439,7 +439,7 @@ bpf_eth_cbi_unload(struct bpf_eth_cbi *bc) > { > /* mark this cbi as empty */ > bc->cb = NULL; > - rte_smp_mb(); > + rte_atomic_thread_fence(rte_memory_order_seq_cst); > > /* make sure datapath doesn't use bpf anymore, then destroy bpf */ > bpf_eth_cbi_wait(bc); > I am not a language lawyer, but what I read is that C11 atomic fence is defined to only guarantee ordering of atomic objects. Since the use counter is not atomic in the original code, the replacement with rte_atomic_thread_fence() is not the same as original asm. My patch made these atomic and that is why. Should have no impact in datapath. I will respin with better human oriented comments, I have developed a distaste for AI prose. Reads like a lawyer wrote it.
