Re: [PATCH bpf-next v6 3/6] bpf: add bpf_icmp_send kfunc

[Mahe Tardy <mahe.tardy@gmail.com>]

On Mon, May 18, 2026 at 01:34:48PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 9590877b0714..843fa775596b 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> 
> [ ... ]
> 
> > @@ -12464,6 +12466,110 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
> >  	return 0;
> >  }
> >
> > +static DEFINE_PER_CPU(bool, bpf_icmp_send_in_progress);
> > +
> > +/**
> > + * bpf_icmp_send - Send an ICMP control message
> > + * @skb_ctx: Packet that triggered the control message
> > + * @type: ICMP type (only ICMP_DEST_UNREACH/ICMPV6_DEST_UNREACH supported)
> > + * @code: ICMP code (0-15 for IPv4, 0-6 for IPv6)
> > + *
> > + * Sends an ICMP control message in response to the packet. The original packet
> > + * is cloned before sending the ICMP message, so the BPF program can still let
> > + * the packet pass if desired.
> > + *
> > + * Currently only ICMP_DEST_UNREACH (IPv4) and ICMPV6_DEST_UNREACH (IPv6) are
> > + * supported.
> > + *
> > + * Recursion protection: If called from a context that would trigger recursion
> > + * (e.g., root cgroup processing its own ICMP packets), returns -EBUSY on
> > + * re-entry.
> > + *
> > + * Return: 0 on success, negative error code on failure:
> > + *         -EINVAL: Invalid code parameter
> > + *         -EBADMSG: Packet too short or malformed
> > + *         -ENOMEM: Memory allocation failed
> > + *         -EBUSY: Recursion detected
> > + *         -EHOSTUNREACH: Routing failed
> > + *         -EPROTONOSUPPORT: Non-IP protocol
> > + *         -EOPNOTSUPP: Unsupported ICMP type
> > + */
> > +__bpf_kfunc int bpf_icmp_send(struct __sk_buff *skb_ctx, int type, int code)
> > +{
> > +	struct sk_buff *skb = (struct sk_buff *)skb_ctx;
> > +	struct sk_buff *nskb;
> > +	bool *in_progress;
> > +
> > +	in_progress = this_cpu_ptr(&bpf_icmp_send_in_progress);
> > +	if (*in_progress)
> > +		return -EBUSY;
> 
> Is the per-CPU recursion protection flag safe without disabling
> preemption or bottom-halves?
> 
> Since cgroup_skb egress programs execute in process context, they can be
> interrupted by network RX softirqs on the same CPU. If a task sets
> *in_progress to true and is immediately preempted by a softirq that runs a
> TC ingress program calling bpf_icmp_send(), it will read the same per-CPU
> flag and spuriously fail with -EBUSY.
> 
> This was raised in v5 by the Sashiko AI bot:
> 
>     Since cgroup_skb egress programs execute in process context, they can be
>     interrupted by network RX softirqs on the same CPU.
> 
>     If a task sets *in_progress to true and is immediately preempted by a
>     softirq that runs a TC ingress program calling bpf_icmp_send(), will it
>     read the same per-CPU flag and spuriously fail with -EBUSY?
> 
>     Does this critical section need to run with bottom-halves disabled?
> 
> https://lore.kernel.org/bpf/20260515202358.20252C2BCB0@smtp.kernel.org/
> 
> Would using two separate per-CPU flags (one for softirq context, one for
> process context) or disabling bottom-halves around the critical section
> address this race condition?
> 
> [ ... ]

I've replied on the v5: the race should be acceptable as it will fail
close with EBUSY, if we want to avoid this we might want two PER_CPU
in_progress flags, one for softirq and one for the process context. In
any case, what we want to prevent is recursion (the kfunc triggering
itself), so softirq interruption from process should be okay as it
should concern another packet.

> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26034287312