All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Daniel Borkmann <daniel@iogearbox.net>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 2/8] samples: bpf: Add common infrastructure for XDP samples
Date: Mon, 16 Aug 2021 22:22:41 +0200	[thread overview]
Message-ID: <87pmudywa6.fsf@toke.dk> (raw)
In-Reply-To: <6a0ba11a-d2a5-38ec-0462-58212c8a4ca7@iogearbox.net>

Daniel Borkmann <daniel@iogearbox.net> writes:

> On 7/28/21 6:55 PM, Kumar Kartikeya Dwivedi wrote:
>> This file implements some common helpers to consolidate differences in
>> features and functionality between the various XDP samples and give them
>> a consistent look, feel, and reporting capabilities.
>> 
>> Some of the key features are:
>>   * A concise output format accompanied by helpful text explaining its
>>     fields.
>>   * An elaborate output format building upon the concise one, and folding
>>     out details in case of errors and staying out of view otherwise.
>>   * Extended reporting of redirect errors by capturing hits for each
>>     errno and displaying them inline (ENETDOWN, EINVAL, ENOSPC, etc.)
>>     to aid debugging.
>>   * Reporting of each xdp_exception action for all samples that use these
>>     helpers (XDP_ABORTED, etc.) to aid debugging.
>>   * Capturing per ifindex pair devmap_xmit counts for decomposing the
>>     total TX count per devmap redirection.
>>   * Ability to jump to source locations invoking tracepoints.
>>   * Faster retrieval of stats per polling interval using mmap'd eBPF
>>     array map (through .bss).
>>   * Printing driver names for devices redirecting packets.
>>   * Printing summarized total statistics for the entire session.
>>   * Ability to dynamically switch between concise and verbose mode, using
>>     SIGQUIT (Ctrl + \).
>> 
>> The goal is sharing these helpers that most of the XDP samples implement
>> in some form but differently for each, lacking in some respect compared
>> to one another.
>> 
>> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> Overall I think it's okay to try to streamline the individual XDP tools, but I
> also tend to wonder whether we keep going beyond the original purpose of kernel
> samples where the main goal is to provide small, easy to hack & stand-alone code
> snippets (like in samples/seccomp ... no doubt we have it more complex in BPF
> land, but still); things people can take away and extend for their purpose. A big
> portion of the samples are still better off in selftests so they can be run in CI,
> and those that are not should generally be simplified for developers to rip out,
> modify, experiment, and build actual applications on top.

FWIW the idea of improving the samples came from Jesper and myself;
we've come to rely on them quite a bit for benchmarking, and our QE
folks run them for testing as well. And I've lost count of the number of
times I had to redo tests because something wasn't working correctly and
I didn't notice that the numbers were off. Kumar took the "improve the
XDP samples" idea and ran with it, and I think the result is much
improved; having it be immediately obvious when something is off is a
huge benefit!

So while I do share your concern about expanding the samples code too
much, in this instance I think it's an improvement. I've toyed with the
idea of also distributing some of the XDP samples with xdp-tools so they
are easier to install as standalone utilities, but I think that is a
secondary concern for later.

-Toke


  parent reply	other threads:[~2021-08-16 20:22 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 16:55 [PATCH bpf-next v3 0/8] Improve XDP samples usability and output Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 1/8] samples: bpf: fix a couple of warnings Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 2/8] samples: bpf: Add common infrastructure for XDP samples Kumar Kartikeya Dwivedi
2021-08-03 23:06   ` Daniel Borkmann
2021-08-03 23:32     ` Kumar Kartikeya Dwivedi
2021-08-16 20:22     ` Toke Høiland-Jørgensen [this message]
2021-07-28 16:55 ` [PATCH bpf-next v3 3/8] samples: bpf: Add BPF support for XDP samples helper Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 4/8] samples: bpf: Convert xdp_monitor to use " Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 5/8] samples: bpf: Convert xdp_redirect " Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 6/8] samples: bpf: Convert xdp_redirect_map to use XDP samples helpers Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 7/8] samples: bpf: Convert xdp_redirect_map_multi " Kumar Kartikeya Dwivedi
2021-07-28 16:55 ` [PATCH bpf-next v3 8/8] samples: bpf: Convert xdp_redirect_cpu " Kumar Kartikeya Dwivedi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pmudywa6.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.