Re: [PATCH v3 01/29] bpf: replace deprecated SMP barriers with C11 fences

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 13 Oct 2026 17:04:33 +0100
Marat Khalili <qm2k21@gmail.com> 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.