On Tue, Aug 12, 2025 at 11:55:39AM -0700, Jesse Brandeburg wrote: > On 8/12/25 8:44 AM, 'Dragos Tatulea' via kernel-team wrote: > > > diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c > > index 482d284a1553..484216c7454d 100644 > > --- a/kernel/bpf/devmap.c > > +++ b/kernel/bpf/devmap.c > > @@ -408,8 +408,10 @@ static void bq_xmit_all(struct xdp_dev_bulk_queue *bq, u32 flags) > > /* If not all frames have been transmitted, it is our > > * responsibility to free them > > */ > > + xdp_set_return_frame_no_direct(); > > for (i = sent; unlikely(i < to_send); i++) > > xdp_return_frame_rx_napi(bq->q[i]); > > + xdp_clear_return_frame_no_direct(); > > Why can't this instead just be xdp_return_frame(bq->q[i]); with no > "no_direct" fussing? > > Wouldn't this be the safest way for this function to call frame completion? > It seems like presuming the calling context is napi is wrong? > It would be better indeed. Thanks for removing my horse glasses! Once Chris verifies that this works for him I can prepare a fix patch. > The other option here seems to be using the xdp_return_frame_bulk() but > you'd need to be careful to make sure the rcu lock was taken or already > held, but it should already be, since it's taken inside xdp_do_flush. > That would be even better, but bq_xmit_all() is also called by bq_enqueue() which doesn't seem to have the rcu lock taken. Thannks, Dragos
