On Thu, 15 Apr 2021 17:05:36 +0200
Daniel Borkmann <dan...@iogearbox.net> wrote:
> On 4/13/21 6:22 PM, Lorenzo Bianconi wrote:
> > Rely on netif_receive_skb_list routine to send skbs converted from
> > xdp_frames in cpu_map_kthread_run in order to improve i-cache usage.
> > The proposed patch has been tested running xdp_redirect_cpu bpf sample
> > available in the kernel tree that is used to redirect UDP frames from
> > ixgbe driver to a cpumap entry and then to the networking stack.
> > UDP frames are generated using pkt_gen.
> >
> > $xdp_redirect_cpu --cpu <cpu> --progname xdp_cpu_map0 --dev <eth>
> >
> > bpf-next: ~2.2Mpps
> > bpf-next + cpumap skb-list: ~3.15Mpps
> >
> > Signed-off-by: Lorenzo Bianconi <lore...@kernel.org>
> > ---
> > Changes since v1:
> > - fixed comment
> > - rebased on top of bpf-next tree
> > ---
> > kernel/bpf/cpumap.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> > index 0cf2791d5099..d89551a508b2 100644
> > --- a/kernel/bpf/cpumap.c
> > +++ b/kernel/bpf/cpumap.c
> > @@ -27,7 +27,7 @@
> > #include <linux/capability.h>
> > #include <trace/events/xdp.h>
> >
> > -#include <linux/netdevice.h> /* netif_receive_skb_core */
> > +#include <linux/netdevice.h> /* netif_receive_skb_list */
> > #include <linux/etherdevice.h> /* eth_type_trans */
> >
> > /* General idea: XDP packets getting XDP redirected to another CPU,
> > @@ -257,6 +257,7 @@ static int cpu_map_kthread_run(void *data)
> > void *frames[CPUMAP_BATCH];
> > void *skbs[CPUMAP_BATCH];
> > int i, n, m, nframes;
> > + LIST_HEAD(list);
> >
> > /* Release CPU reschedule checks */
> > if (__ptr_ring_empty(rcpu->queue)) {
> > @@ -305,7 +306,6 @@ static int cpu_map_kthread_run(void *data)
> > for (i = 0; i < nframes; i++) {
> > struct xdp_frame *xdpf = frames[i];
> > struct sk_buff *skb = skbs[i];
> > - int ret;
> >
> > skb = __xdp_build_skb_from_frame(xdpf, skb,
> > xdpf->dev_rx);
> > @@ -314,11 +314,10 @@ static int cpu_map_kthread_run(void *data)
> > continue;
> > }
> >
> > - /* Inject into network stack */
> > - ret = netif_receive_skb_core(skb);
> > - if (ret == NET_RX_DROP)
> > - drops++;
> > + list_add_tail(&skb->list, &list);
> > }
> > + netif_receive_skb_list(&list);
> > +
> > /* Feedback loop via tracepoint */
> > trace_xdp_cpumap_kthread(rcpu->map_id, n, drops, sched,
> > &stats);
>
> Given we stop counting drops with the netif_receive_skb_list(), we should then
> also remove drops from trace_xdp_cpumap_kthread(), imho, as otherwise it is
> rather
> misleading (as in: drops actually happening, but 0 are shown from the
> tracepoint).
> Given they are not considered stable API, I would just remove those to make
> it clear
> to users that they cannot rely on this counter anymore anyway.
After Lorenzo's change, the 'drops' still count if kmem_cache_alloc_bulk
cannot alloc SKBs. I guess that will not occur very often. But how
can people/users debug such a case? Maybe the MM-layer can tell us?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
