xdp-project / xdp-tools

Utilities and example programs for use with XDP
Other
644 stars 139 forks source link

libxdp: Failed to attach program to dispatcher: Operation not supported #353

Closed vincentmli closed 1 year ago

vincentmli commented 1 year ago

Hi,

I have an idea to allow suricata to attach suricata's xdp_hashfilter with libxdp so I could attach another XDP program to the same NIC with libxdp multi XDP attachment capability. here is a small patch to suricata I come up with: Add multi XDP attachment with libxdp, my intention is to make minimal code changes to suricata's ebpf/xdp load/attachement code so the patch could be accepted by suricata, suricata calls libbpf function to load the xdp_hashfilter XDP program and returns a XDP fd, so I assigned the XDP fd with xdp_opts.fd = fd to create the xdp_program.

    struct xdp_program *p = NULL;
    DECLARE_LIBXDP_OPTS(xdp_program_opts, xdp_opts, 0);
    xdp_opts.fd = fd;

but when I run suricata, I got:

21/8/2023 -- 17:40:14 - <Info> - Successfully loaded eBPF file '/usr/libexec/suricata/ebpf/xdp_filter.bpf' on 'eno1'
libbpf: elf: skipping unrecognized data section(7) xdp_metadata
libbpf: elf: skipping unrecognized data section(7) xdp_metadata
libbpf: elf: skipping unrecognized data section(7) xdp_metadata
libbpf: elf: skipping unrecognized data section(7) xdp_metadata
libxdp: Failed to attach program xdp_hashfilter to dispatcher: Operation not supported
libxdp: Falling back to loading single prog without dispatcher

I think the libbpf:elf message could be ignored, the Operation not supported is concerning.

the libxdp code line 2860 below logs above error Failed to attach program xdp_hashfilter:

2833         /* The attach will disappear once this fd is closed */
2834         lfd = bpf_link_create(new_prog->prog_fd, mp->main_prog->prog_fd, 0, &opts);
2835         if (lfd < 0) {
2836                 err = -errno;
2837                 if (err == -EINVAL) {
2838                         if (!was_loaded) {
2839                                 pr_debug("Kernel doesn't support re-attaching "
2840                                          "freplace programs.\n");
2841                                 err = -EOPNOTSUPP;
2842                         } else {
2843                                 pr_debug("Got EINVAL, retrying "
2844                                          "raw_tracepoint_open() without target\n");
2845                                 /* we just loaded the program, so should be able
2846                                  * to attach the old way */
2847                                 lfd = bpf_raw_tracepoint_open(NULL, new_prog->prog_fd);
2848                                 if (lfd < 0)
2849                                         err = -errno;
2850                                 else
2851                                         goto attach_ok;
2852                         }
2853                 }           
2854                 if (err == -EPERM) {
2855                         pr_debug("Got 'permission denied' error while "
2856                                  "attaching program to dispatcher.\n%s\n",
2857                                 dispatcher_feature_err);
2858                         err = -EOPNOTSUPP;
2859                 } else {
2860                         pr_warn("Failed to attach program %s to dispatcher: %s\n",
2861                                 xdp_program__name(new_prog), strerror(-err));
2862                 }
2863                 goto err_free;
2864         }

am I doing it wrong to assign an already loaded XDP program FD by libbpf to libxdp?

vincentmli commented 1 year ago

I looked at the lfd = bpf_link_create(new_prog->prog_fd, mp->main_prog->prog_fd, 0, &opts);

here is the strace to show the bpf BPF_LINK_CREATE

120010 bpf(BPF_LINK_CREATE, {link_create={prog_fd=32, target_fd=31, attach_type=BPF_CGROUP_INET_INGRESS, flags=0}, ...}, 144) = -1 EINVAL (Invalid argument) 120015 <... clock_nanosleep resumed>NULL) = 0 120010 write(2, "libxdp: Failed to attach program xdp_hashfilter to dispatcher: Operation not supported\n", 87 <unfinished ...>

the Invalid argument is interesting, is the attach_type BPF_CGROUP_INET_INGRESS correct?

vincentmli commented 1 year ago

I changed the pr_debug to pr_warn in libxdp.c, now I see:

libxdp: Linking prog xdp_hashfilter as multiprog entry 0
libxdp: Kernel doesn't support re-attaching freplace programs.
libxdp: Failed to attach program xdp_hashfilter to dispatcher: Operation not supported
libxdp: Falling back to loading single prog without dispatcher

Kernel doesn't support re-attaching freplace programs., my kernel is very recent (6.2.0-1008-lowlatency), I guess it is just kernel does not have this capability yet?

vincentmli commented 1 year ago

I could workaround the problem by passing the bpf object to xdp_opts instead of XDP program fd

diff --git a/configure.ac b/configure.ac
index 969030d63..4e52e1e1a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1845,6 +1845,7 @@
         AC_CHECK_LIB(xdp, xdp_program__open_file,have_multixdp="yes")
         if test "$have_multixdp" = "yes"; then
             AC_DEFINE([HAVE_MULTI_XDP],[1],[MULTI XDP support is available])
+           LIBS="${LIBS} -lxdp"
         fi
         AC_CHECK_FUNCS(bpf_program__section_name)
     fi;
diff --git a/src/util-ebpf.c b/src/util-ebpf.c
index 447e1a910..521ed4a8a 100644
--- a/src/util-ebpf.c
+++ b/src/util-ebpf.c
@@ -390,6 +390,37 @@ int EBPFLoadFile(const char *iface, const char *path, const char * section,
         return -1;
     }

+#ifdef HAVE_MULTI_XDP
+    char buf[129];
+    struct xdp_program *p = NULL;
+    DECLARE_LIBXDP_OPTS(xdp_program_opts, xdp_opts, 0);
+
+    unsigned int ifindex = if_nametoindex(iface);
+    if (ifindex == 0) {
+        SCLogError(SC_ERR_INVALID_VALUE,
+                "Unknown interface '%s'", iface);
+        return -1;
+    }
+
+    xdp_opts.obj = bpfobj;
+    p = xdp_program__create(&xdp_opts);
+    err = libxdp_get_error(p);
+    if (err) {
+        libxdp_strerror(err, buf, sizeof(buf));
+        SCLogError(SC_ERR_INVALID_VALUE, "Unable to attach multi XDP on '%s': %s (%d)",
+            iface, buf, err);
+       p = NULL;
+       return -1;
+     }
+
+    err = xdp_program__attach(p, ifindex, 0, 0);
+    if (err != 0) {
+        libxdp_strerror(err, buf, sizeof(buf));
+        SCLogError(SC_ERR_INVALID_VALUE, "Unable to attach multi XDP on '%s': %s (%d)",
+                iface, buf, err);
+        return -1;
+    }
+#else
     err = bpf_object__load(bpfobj);
     if (err < 0) {
         if (err == -EPERM) {
@@ -406,6 +437,7 @@ int EBPFLoadFile(const char *iface, const char *path, const char * section,
         }
         return -1;
     }
+#endif

     /* Kernel and userspace are sharing data via map. Userspace access to the
      * map via a file descriptor. So we need to store the map to fd info. For
@@ -460,6 +492,7 @@ int EBPFLoadFile(const char *iface, const char *path, const char * section,
     /* Finally we get the file descriptor for our eBPF program. We will use
      * the fd to attach the program to the socket (eBPF case) or to the device
      * (XDP case). */
+#ifndef HAVE_MULTI_XDP
     pfd = bpf_program__fd(bpfprog);
     if (pfd == -1) {
         SCLogError(SC_ERR_INVALID_VALUE,
@@ -469,6 +502,7 @@ int EBPFLoadFile(const char *iface, const char *path, const char * section,

     SCLogInfo("Successfully loaded eBPF file '%s' on '%s'", path, iface);
     *val = pfd;
+#endif
     return 0;
 }

@@ -490,28 +524,7 @@ int EBPFSetupXDP(const char *iface, int fd, uint8_t flags)
                 "Unknown interface '%s'", iface);
         return -1;
     }
-#ifdef HAVE_MULTI_XDP
-    struct xdp_program *p = NULL;
-    DECLARE_LIBXDP_OPTS(xdp_program_opts, xdp_opts, 0);
-    xdp_opts.fd = fd;
-    p = xdp_program__create(&xdp_opts);
-    err = libxdp_get_error(p);
-    if (err) {
-        libxdp_strerror(err, buf, sizeof(buf));
-        SCLogError(SC_ERR_INVALID_VALUE, "Unable to attach multi XDP on '%s': %s (%d)",
-            iface, buf, err);
-       p = NULL;
-       return -1;
-     }
-
-    err = xdp_program__attach(p, ifindex, 0, 0);
-    if (err != 0) {
-        libxdp_strerror(err, buf, sizeof(buf));
-        SCLogError(SC_ERR_INVALID_VALUE, "Unable to attach multi XDP on '%s': %s (%d)",
-                iface, buf, err);
-        return -1;
-    }
-#elif HAVE_PACKET_XDP
+#ifdef HAVE_PACKET_XDP
     err = bpf_set_link_xdp_fd(ifindex, fd, flags);
     if (err != 0) {
         libbpf_strerror(err, buf, sizeof(buf));
tohojo commented 1 year ago

Yeah, as you have discovered the multiprog support requires that libxdp does the loading. This is because we need to change the program type and provide it with an attach target, which can only be done at load time. It's a bit awkward, but the way the kernel support works we don't have a choice, unfortunately.

One way of dealing with this is, as you've discovered, to pass the bpf_obj into libxdp and have it construct an xdp_prog around it. An alternative would be to modify the EBPFLoadFile() function to use libxdp to load the file from disk. You can then extract the bpf object using xdp_program__bpf_obj() afterwards. The benefit of the latter is that in this mode libxdp will "own" the bpf_obj and take care of closing it along with the xdp_prog object itself, whereas if you pass in the object, libxdp won't close it so the caller has to take responsibility for this.

Speaking of closing objects, AFAICT, the EBPFLoadFile() currently doesn't close the bpf object at all, meaning it leaks memory. Also, the teardown code in ReceiveAFPThreadDeinit() does a call to EBPFSetupXDP() with a -1 file descriptor, which is not compatible with libxdp - you'll end up detaching the dispatcher itselt, not just the Suricata progam. From a libxdp PoV the easiest thing is to just keep the xdp_prog object around and call xdp_program__detach() followed by xdp_program__close() during teardown :)

vincentmli commented 1 year ago

Thank you very much for not only providing answer for libxdp, but also how to better use it in Suricata :)

tohojo commented 1 year ago

You're welcome! :)