Re: [PATCH v3 bpf-next] cpumap: bulk skb using netif_receive_skb_list

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Tue, 20 Apr 2021 16:05:14 +0200
Lorenzo Bianconi <lorenzo@kernel.org> 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. Packets are discarded by the
> UDP layer.
> 
> $xdp_redirect_cpu  --cpu <cpu> --progname xdp_cpu_map0 --dev <eth>
> 
> bpf-next: ~2.35Mpps
> bpf-next + cpumap skb-list: ~2.72Mpps
> 
> Since netif_receive_skb_list does not return number of discarded packets,
> remove drop counter from xdp_cpumap_kthread tracepoint and update related
> xdp samples.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> Changes since v2:
> - remove drop counter and update related xdp samples
> - rebased on top of bpf-next
> 
> Changes since v1:
> - fixed comment
> - rebased on top of bpf-next tree
> ---
>  include/trace/events/xdp.h          | 14 +++++---------
>  kernel/bpf/cpumap.c                 | 16 +++++++---------
>  samples/bpf/xdp_monitor_kern.c      |  6 ++----
>  samples/bpf/xdp_monitor_user.c      | 14 ++++++--------
>  samples/bpf/xdp_redirect_cpu_kern.c | 12 +++++-------
>  samples/bpf/xdp_redirect_cpu_user.c | 10 ++++------
>  6 files changed, 29 insertions(+), 43 deletions(-)
> 
> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> index fcad3645a70b..52ecfe9c7e25 100644
> --- a/include/trace/events/xdp.h
> +++ b/include/trace/events/xdp.h
> @@ -184,16 +184,15 @@ DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map_err,
>  
>  TRACE_EVENT(xdp_cpumap_kthread,
>  
> -	TP_PROTO(int map_id, unsigned int processed,  unsigned int drops,
> -		 int sched, struct xdp_cpumap_stats *xdp_stats),
> +	TP_PROTO(int map_id, unsigned int processed, int sched,
> +		 struct xdp_cpumap_stats *xdp_stats),
>  
> -	TP_ARGS(map_id, processed, drops, sched, xdp_stats),
> +	TP_ARGS(map_id, processed, sched, xdp_stats),
>  
>  	TP_STRUCT__entry(
>  		__field(int, map_id)
>  		__field(u32, act)
>  		__field(int, cpu)
> -		__field(unsigned int, drops)
>  		__field(unsigned int, processed)

So, struct member @processed will takeover the room for @drops.

Can you please test how an old xdp_monitor program will react to this?
Will it fail, or extract and show wrong values?

The xdp_mointor tool is in several external git repos:

 https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/samples/bpf/xdp_monitor_kern.c
 https://github.com/xdp-project/xdp-tutorial/tree/master/tracing02-xdp-monitor

Do you have any plans for fixing those tools?


>  		__field(int, sched)
>  		__field(unsigned int, xdp_pass)
> @@ -205,7 +204,6 @@ TRACE_EVENT(xdp_cpumap_kthread,
>  		__entry->map_id		= map_id;
>  		__entry->act		= XDP_REDIRECT;
>  		__entry->cpu		= smp_processor_id();
> -		__entry->drops		= drops;
>  		__entry->processed	= processed;
>  		__entry->sched	= sched;
>  		__entry->xdp_pass	= xdp_stats->pass;
> @@ -215,13 +213,11 @@ TRACE_EVENT(xdp_cpumap_kthread,
>  
>  	TP_printk("kthread"
>  		  " cpu=%d map_id=%d action=%s"
> -		  " processed=%u drops=%u"
> -		  " sched=%d"
> +		  " processed=%u sched=%u"
>  		  " xdp_pass=%u xdp_drop=%u xdp_redirect=%u",
>  		  __entry->cpu, __entry->map_id,
>  		  __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
> -		  __entry->processed, __entry->drops,
> -		  __entry->sched,
> +		  __entry->processed, __entry->sched,
>  		  __entry->xdp_pass, __entry->xdp_drop, __entry->xdp_redirect)
>  );



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer