doc: add initial BPF and XDP documentation #546

borkmann · 2017-04-14T01:13:34Z

We're currently lacking a guide on this, therefore add a start of it
that we can further extend over time.

Rendered version from wip branch:

http://cilium.readthedocs.io/en/doc-bpf-wip/

Signed-off-by: Daniel Borkmann daniel@cilium.io

aalemayhu

Other than the minor nitpick this looks very good and was fun to read 🌮

aalemayhu · 2017-04-14T11:25:05Z

Documentation/bpf.rst

+generic and flexible enough that there are many kernel subsystems which use eBPF
+apart from only networking. Nowadays, the Linux kernel runs eBPF only and loaded
+cBPF bytecode is transparently translated into an eBPF representation in the
+kernel before program execution.


s/into an/into a/ ?

I think it's probably fine as is, but I'll double check to make sure.

(an eBPF representation is the correct form)

aalemayhu · 2017-04-14T11:28:44Z

Documentation/bpf.rst

+  these packet load instructions are less relevant nowadays. ``BPF_LDX`` class
+  holds instructions for byte / half-word / word / double-word loads out of
+  memory. Memory in this context is generic can could be stack memory, map value
+  data, packet data, etc.


s/can could/and could/

aalemayhu · 2017-04-14T11:36:36Z

Documentation/bpf.rst

+   included into the main programs. For example, Cilium makes heavy use of
+   this (see ``bpf/lib/``). However, this still allows for including header
+   files, for example, from the kernel or other libraries and resuse their
+   static inline functions or macros / definitions.


s/resuse/reuse/

aalemayhu · 2017-04-14T11:37:39Z

Documentation/bpf.rst

+
+   The remaining section names are specific for eBPF program code, for example,
+   the below code has been modified to contain to two program sections, ``ingress``
+   and ``egress``. The toy example code demonstrates that both can share a map


s/to contain to/to contain/ ?

aalemayhu · 2017-04-14T11:39:38Z

Documentation/bpf.rst

+  value size. This works, because during execution, eBPF programs are guaranteed
+  to never get preempted by the kernel and therefore can use the single map entry
+  as a scratch buffer for temporariy data, for example, to extend beyond the stack
+  limitation. This also works accross tail calls, since it has the same guarantees


s/temporariy/temporary/ ?

aalemayhu · 2017-04-14T11:47:30Z

Documentation/bpf.rst

+  program attached. ``ip link | grep xdp`` can thus be used to find all interfaces
+  that have XDP running. Further introspection facilities will be provided through
+  the detailed view through ``ip -d link`` once the kernel API gains support for
+  dumping additional attributes.


s/view through/view/ ? The extra through adds nothing extra IMO, but either way is fine.

Wrote it as view with.

aalemayhu · 2017-04-14T11:48:37Z

Documentation/bpf.rst

+  For removing the entire ``clsact`` qdisc from the netdevice, which implicitly also
+  removes all attached programs from the ``ingress`` and ``egress`` hooks, the
+  below command is provided:
+


s/instace/instance/ ?

tgraf · 2017-04-14T08:32:17Z

Documentation/bpf.rst

+.. _bpf_guide:
+
+*****************
+BPF and XDP Guide


Maybe rename this to "BPF and XDP Reference Guide" to make it a bit clearer that this is deep dive material for developers and not required for users.

tgraf · 2017-04-14T08:33:38Z

Documentation/bpf.rst

+types such as tc and XDP, and to aide developing Cilium's eBPF templates.
+
+**This purely serves as a developer's guide and is explicitly not a requirement
+for Cilium users to read, since Cilium abstracts eBPF internals away entirely.**


Let's move this and make it the first paragraph. You can use:

.. note:: This purely serves as a ...

prefix so it will appear as a note widget so it stands out a bit more.

tgraf · 2017-04-14T08:41:19Z

Documentation/bpf.rst

+**This purely serves as a developer's guide and is explicitly not a requirement
+for Cilium users to read, since Cilium abstracts eBPF internals away entirely.**
+
+The older cBPF architecture is not covered by this document, since Cilium does


I would drop the 'since Cilium' part

tgraf · 2017-04-14T08:42:53Z

Documentation/bpf.rst

+
+eBPF does not define itself by only providing its instruction set, but also by
+offering further infrastructure around it such as maps that act as efficient
+key / value stores, helper functions to interact with the kernel, tail calls for


helper functions to interact with and leverage kernel functionaly

tgraf · 2017-04-14T09:10:54Z

Documentation/bpf.rst

+
+* In case of networking (e.g., tc and XDP), eBPF programs can be updated atomically
+  without having to restart the kernel, system services or containers, and without
+  traffic interruptions.


Let's add that any program state can be maintained throughout updates via bpf maps.

tgraf · 2017-04-14T09:23:31Z

Documentation/bpf.rst

+  calls with regard to user space applications.
+
+* eBPF programs work in concert with the kernel, they make use of existing kernel
+  infrastructure and tooling (e.g., iproute2) as well as the safety guarantees that


Would be great to add an (e.g. ...) with a couple of examples of used kernel infrastructure.

tgraf · 2017-04-14T09:24:42Z

Documentation/bpf.rst

+  infrastructure and tooling (e.g., iproute2) as well as the safety guarantees that
+  the kernel provides. Unlike kernel modules, eBPF programs are verified through an
+  in-kernel verifier in order to ensure that they cannot crash the kernel, always
+  terminate, etc. XDP programs, for example, reuse the existing in-kernel drivers


Maybe move XDP down until you have properly introduced it. The XDP concept is completely unknown to the reader at this point.

Given it's kind of used throughout the doc (also in the more generic LLVM / iproute2 section), I tried to introduce XDP and tc terms at the very beginning now, but we could later on also still rearrange this a bit if we think a strict separation seems better.

tgraf · 2017-04-14T09:25:10Z

Documentation/bpf.rst

+
+The execution of an eBPF program inside the kernel is always event driven! For example,
+a networking device that has an eBPF program attached on its ingress path will trigger
+the execution of the program once a packet is received.


Add a second example for kprobe attachment.

tgraf · 2017-04-14T09:26:59Z

Documentation/bpf.rst

+handing execution back to the kernel, the exit value is passed as a 32 bit value.
+
+Registers ``r1`` - ``r5`` are scratch registers, meaning the eBPF program needs to
+either spill them to the eBPF stack or move them to callee saved registers if these


Add a sentence to explains spilling.

We're currently lacking a guide on this, therefore add a start of it that we can further extend over time. Rendered version from wip branch: http://cilium.readthedocs.io/en/doc-bpf-wip/ Signed-off-by: Daniel Borkmann <daniel@cilium.io>

tgraf · 2017-04-15T07:33:35Z

I'll provide more feedback directly as a PR but this is in great shape to go in already.

borkmann · 2017-04-15T08:57:59Z

Perfect, thanks. I'll continue working on the remaining bits.

borkmann requested review from tgraf, aanm, aalemayhu, mchalla and ianvernon April 14, 2017 01:13

aalemayhu approved these changes Apr 14, 2017

View reviewed changes

tgraf requested changes Apr 14, 2017

View reviewed changes

borkmann force-pushed the doc-bpf-wip branch 4 times, most recently from bfe9836 to 44cd590 Compare April 14, 2017 23:35

doc: add initial BPF and XDP documentation

c5cbd77

We're currently lacking a guide on this, therefore add a start of it that we can further extend over time. Rendered version from wip branch: http://cilium.readthedocs.io/en/doc-bpf-wip/ Signed-off-by: Daniel Borkmann <daniel@cilium.io>

borkmann force-pushed the doc-bpf-wip branch from 44cd590 to c5cbd77 Compare April 15, 2017 00:00

tgraf approved these changes Apr 15, 2017

View reviewed changes

borkmann merged commit e70ff85 into master Apr 15, 2017

borkmann deleted the doc-bpf-wip branch April 15, 2017 08:58

doc: add initial BPF and XDP documentation #546

doc: add initial BPF and XDP documentation #546

Conversation

borkmann commented Apr 14, 2017

aalemayhu left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgraf commented Apr 15, 2017

borkmann commented Apr 15, 2017

aalemayhu left a comment •

edited