Re: [PATCH bpf-next 1/2] libbpf: separate XDP program load with xsk socket creation

On Wed, Nov 4, 2020 at 10:07 PM Andrii Nakryiko
[off-list ref] wrote:

On Wed, Nov 4, 2020 at 1:47 AM [off-list ref] wrote:

quoted

From: Mariusz Dudek <redacted>

        Add support for separation of eBPF program load and xsk socket
        creation.

        This is needed for use-case when you want to privide as little
        privileges as possible to the data plane application that will
        handle xsk socket creation and incoming traffic.

        With this patch the data entity container can be run with only
        CAP_NET_RAW capability to fulfill its purpose of creating xsk
        socket and handling packages. In case your umem is larger or
        equal process limit for MEMLOCK you need either increase the
        limit or CAP_IPC_LOCK capability.

        To resolve privileges issue two APIs are introduced:

        - xsk_setup_xdp_prog - prepares bpf program if given and
        loads it on a selected network interface or loads the built in
        XDP program, if no XDP program is supplied. It can also return
        xsks_map_fd which is needed by unprivileged process to update
        xsks_map with AF_XDP socket "fd"

        - xsk_update_xskmap - inserts an AF_XDP socket into an xskmap
        for a particular xsk_socket

Your commit message seems to be heavily shifted right...

Will be fixed

quoted

Signed-off-by: Mariusz Dudek <redacted>
---
 tools/lib/bpf/libbpf.map |   2 +
 tools/lib/bpf/xsk.c      | 157 ++++++++++++++++++++++++++++++++-------
 tools/lib/bpf/xsk.h      |  13 ++++
 3 files changed, 146 insertions(+), 26 deletions(-)

[...]

quoted

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 1069c46364ff..c42b91935d3c 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h

@@ -201,6 +201,19 @@ struct xsk_umem_config {
        __u32 flags;
 };

+struct bpf_prog_cfg {
+       struct bpf_insn *prog;
+       const char *license;
+       size_t insns_cnt;
+       int xsks_map_fd;
+};

This config will have problems with backward/forward compatibility.
Please check how xxx_opts are done and use them for extensible options
structs.

I will add struct size as first parameter and #define for __last_field
to be inline with xxx_opts

quoted

+
+LIBBPF_API int xsk_setup_xdp_prog(int ifindex,
+                                 struct bpf_prog_cfg *cfg,
+                                 int *xsks_map_fd);
+LIBBPF_API int xsk_update_xskmap(struct xsk_socket *xsk,
+                                int xsks_map_fd);

this should be called xsk_socket__update_map? BTW, what's xskmap? Is
that a special BPF map type?

I will change the API name as you suggested. XSKMAP is a special
BPF_MAP_TYPE_XSKMAP.
It defines how packets are being distributed from an XDP program to the XSKs.

quoted

+
 /* Flags for the libbpf_flags field. */
 #define XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD (1 << 0)

--
2.20.1