n-acd: using ebpf is decided at runtime

Modify n-acd library to use ebpf whenever it is possible.
This is done by always making a `sys_call` for bpf resources. If this
call fails program continues to run normally the only difference is that
ebpf is not used.

Edit the test for bpf map to only run when bpf map is successfully
created.

.gitlab-ci/run-test.sh

@@ -77,9 +77,6 @@ test_subtree() {
     pushd ./src/$d
 
     ARGS=()
-    if [ "$d" = n-acd ]; then
-        ARGS+=('-Debpf=false')
-    fi
 
     CC="$cc" CFLAGS="-Werror -Wall" meson build "${ARGS[@]}"
     ninja -v -C build test

Makefile.am

@@ -321,18 +321,13 @@ src_n_acd_libn_acd_la_LDFLAGS = \
 src_n_acd_libn_acd_la_SOURCES = \
 	src/n-acd/src/n-acd.c \
 	src/n-acd/src/n-acd.h \
+	src/n-acd/src/n-acd-bpf.c \
 	src/n-acd/src/n-acd-private.h \
 	src/n-acd/src/n-acd-probe.c \
 	src/n-acd/src/util/timer.c \
 	src/n-acd/src/util/timer.h \
 	$(NULL)
 
-if WITH_EBPF
-src_n_acd_libn_acd_la_SOURCES += src/n-acd/src/n-acd-bpf.c
-else
-src_n_acd_libn_acd_la_SOURCES += src/n-acd/src/n-acd-bpf-fallback.c
-endif
-
 ###############################################################################
 
 noinst_LTLIBRARIES += src/n-dhcp4/libn-dhcp4.la

configure.ac

@@ -541,21 +541,6 @@ case $with_suspend_resume in
 		;;
 esac
 
-# eBPF support
-AC_ARG_WITH(ebpf,
-            AS_HELP_STRING([--with-ebpf=yes|no|auto], [Build with eBPF support [default=auto]]),
-            [], [with_ebpf=auto])
-# 'auto' means 'false' because there are still some issues.
-if test "$with_ebpf" = "yes" ; then
-	AC_CHECK_HEADER(linux/bpf.h, [have_ebpf=yes], [have_ebpf=no])
-else
-	have_ebpf=no
-fi
-if test "$with_ebpf" = "yes" -a "$have_ebpf" = "no"; then
-	AC_MSG_ERROR([--with-ebpf=yes requires eBPF kernel header])
-fi
-AM_CONDITIONAL(WITH_EBPF, test "${have_ebpf}" = "yes")
-
 # SELinux support
 AC_ARG_WITH(selinux,
             AS_HELP_STRING([--with-selinux=yes|no|auto], [Build with SELinux [default=auto]]),
@@ -1442,7 +1427,6 @@ echo "  linker garbage collection: $enable_ld_gc"
 echo "  crypto: $with_crypto (have-gnutls: $have_crypto_gnutls, have-nss: $have_crypto_nss)"
 echo "  sanitizers: $sanitizers"
 echo "  Mozilla Public Suffix List: $with_libpsl"
-echo "  eBPF: $have_ebpf"
 echo "  readline: $with_readline"
 echo "  python: $PYTHON"
 echo

contrib/fedora/rpm/NetworkManager.spec

@@ -161,17 +161,6 @@
 %global ifcfg_warning 0
 %endif
 
-%if 0%{?fedora}
-# Although eBPF would be available on Fedora's kernel, it seems
-# we often get SELinux denials (rh#1651654). But even aside them,
-# bpf(BPF_MAP_CREATE, ...) randomly fails with EPERM. That might
-# be related to `ulimit -l`. Anyway, this is not usable at the
-# moment.
-%global ebpf_enabled "no"
-%else
-%global ebpf_enabled "no"
-%endif
-
 # Fedora 33 enables LTO by default by setting CFLAGS="-flto -ffat-lto-objects".
 # However, we also require "-flto -flto-partition=none", so disable Fedora's
 # default and use our configure option --with-lto instead.
@@ -699,11 +688,6 @@ Preferably use nmcli instead.
 	-Dlibpsl=true \
 %else
 	-Dlibpsl=false \
-%endif
-%if %{ebpf_enabled} != "yes"
-	-Debpf=false \
-%else
-	-Debpf=true \
 %endif
 	-Dsession_tracking=systemd \
 	-Dsuspend_resume=systemd \
@@ -840,7 +824,6 @@ autoreconf --install --force
 %else
 	--with-libpsl=no \
 %endif
-	--with-ebpf=%{ebpf_enabled} \
 	--with-session-tracking=systemd \
 	--with-suspend-resume=systemd \
 	--with-system-ca-path=/etc/pki/tls/cert.pem \

contrib/fedora/rpm/configure-for-system.sh

@@ -159,7 +159,6 @@ P_CRYPTO="${CRYPTO-}"
 P_DBUS_SYS_DIR="${DBUS_SYS_DIR-}"
 P_DHCP_DEFAULT="${DHCP_DEFAULT-}"
 P_DNS_RC_MANAGER_DEFAULT="${DNS_RC_MANAGER_DEFAULT-}"
-P_EBPF_ENABLED="${EBPF_ENABLED-no}"
 P_FIREWALLD_ZONE="${FIREWALLD_ZONE-}"
 P_IWD="${IWD-}"
 P_LOGGING_BACKEND_DEFAULT="${LOGGING_BACKEND_DEFAULT-}"
@@ -404,7 +403,6 @@ if [ "$P_BUILD_TYPE" == meson ] ; then
         -Dmodify_system=true \
         -Dconcheck=true \
         -Dlibpsl="$(bool_true "$P_FEDORA")" \
-        -Debpf="$(bool_true "$P_EBPF_ENABLED")" \
         -Dsession_tracking=systemd \
         -Dsuspend_resume=systemd \
         -Dsystem_ca_path=/etc/pki/tls/cert.pem \
@@ -487,7 +485,6 @@ else
         --enable-modify-system=yes \
         --enable-concheck=yes \
         --with-libpsl="$(bool_yes "$P_FEDORA")" \
-        --with-ebpf="$(bool_yes "$P_EBPF_ENABLED")" \
         --with-session-tracking=systemd \
         --with-suspend-resume=systemd \
         --with-system-ca-path=/etc/pki/tls/cert.pem \

contrib/scripts/nm-ci-run.sh

@@ -169,8 +169,6 @@ run_autotools() {
             --enable-tests=yes \
             --with-crypto=$_WITH_CRYPTO \
             \
-            --with-ebpf=no \
-            \
             --with-iwd=yes \
             --with-ofono=yes \
             --enable-teamdctl=$_WITH_LIBTEAM \
@@ -241,8 +239,6 @@ run_meson() {
         -D crypto=$_WITH_CRYPTO \
         -D docs=$_WITH_DOCS \
         \
-        -D ebpf=false \
-        \
         -D iwd=true \
         -D ofono=true \
         -D teamdctl=$_WITH_LIBTEAM \

meson.build

@@ -463,19 +463,6 @@ if enable_selinux
 endif
 config_h.set10('HAVE_SELINUX', enable_selinux)
 
-# eBPF support
-ebpf_opt = get_option('ebpf')
-# 'auto' means 'false', because there are still issues.
-if ebpf_opt != 'true'
-  enable_ebpf = false
-else
-  enable_ebpf = true
-  if not cc.has_header('linux/bpf.h')
-    assert(ebpf_opt != 'true', 'eBPF requires kernel support')
-    enable_ebpf = false
-  endif
-endif
-
 # libaudit support
 libaudit = get_option('libaudit')
 enable_libaudit = libaudit.contains('yes')
@@ -1093,6 +1080,5 @@ output +=      'have-nss: ' + crypto_nss_dep.found().to_string() + ')\n'
 output += '  sanitizers: ' + get_option('b_sanitize') + '\n'
 output += '  Mozilla Public Suffix List: ' + enable_libpsl.to_string() + '\n'
 output += '  vapi: ' + enable_vapi.to_string() + '\n'
-output += '  ebpf: ' + enable_ebpf.to_string() + '\n'
 output += '  readline: ' + with_readline + '\n'
 message(output)

meson_options.txt

@@ -40,7 +40,6 @@ option('nmcli', type: 'boolean', value: true, description: 'Build nmcli')
 option('nmtui', type: 'boolean', value: true, description: 'Build nmtui')
 option('nm_cloud_setup', type: 'boolean', value: false, description: 'Build nm-cloud-setup, a tool for automatically configure networking in cloud (EXPERIMENTAL!)')
 option('bluez5_dun', type: 'boolean', value: false, description: 'enable Bluez5 DUN support')
-option('ebpf', type: 'combo', choices: ['auto', 'true', 'false'], description: 'Enable eBPF support')
 
 # configuration plugins
 option('config_plugins_default', type: 'string', value: '', description: 'Default configuration option for main.plugins setting, used as fallback if the configuration option is unset')

src/meson.build

@@ -24,7 +24,7 @@ libn_acd = static_library(
     'n-acd/src/n-acd.c',
     'n-acd/src/n-acd-probe.c',
     'n-acd/src/util/timer.c',
-    enable_ebpf ? 'n-acd/src/n-acd-bpf.c' : 'n-acd/src/n-acd-bpf-fallback.c',
+    'n-acd/src/n-acd-bpf.c',
   ),
   include_directories: include_directories(
     'c-list/src',

src/n-acd/.github/workflows/ci.yml

@@ -49,42 +49,6 @@ jobs:
                 "--m32=1" \
                 "--source=/github/workspace"
 
-  ci-no-ebpf:
-    name: CI without eBPF
-    runs-on: ubuntu-latest
-
-    steps:
-    # See above in 'ci' job.
-    - name: Fetch CI
-      uses: actions/checkout@v2
-      with:
-        repository: c-util/automation
-        ref: v1
-        path: automation
-    - name: Build CI
-      working-directory: automation/src/ci-c-util
-      run: docker build --tag ci-c-util:v1 .
-
-    #
-    # Run CI
-    #
-    # This again runs the CI, but this time disables eBPF. We do support the
-    # legacy BPF fallback, so lets make sure we test for it.
-    #
-    - name: Fetch Sources
-      uses: actions/checkout@v2
-      with:
-        path: source
-    - name: Run through C-Util CI
-      run: |
-        docker run \
-                --privileged \
-                -v "$(pwd)/source:/github/workspace" \
-                "ci-c-util:v1" \
-                "--m32=1" \
-                "--mesonargs=-Debpf=false" \
-                "--source=/github/workspace"
-
   ci-valgrind:
     name: CI through Valgrind
     runs-on: ubuntu-latest

src/n-acd/README.md

@@ -41,13 +41,6 @@ meson test
 ninja install
 ```
 
-The following configuration options are available:
-
- * `ebpf`: This boolean controls whether `ebpf` features are used to improve
-           the package filtering performance. If disabled, classic bpf will be
-           used. This feature requires a rather recent kernel (>=3.19).
-           Default is: true
-
 ### Repository:
 
  - **web**:   <https://github.com/nettools/n-acd>

src/n-acd/meson.build

@@ -22,6 +22,4 @@ dep_crbtree = sub_crbtree.get_variable('libcrbtree_dep')
 dep_csiphash = sub_csiphash.get_variable('libcsiphash_dep')
 dep_cstdaux = sub_cstdaux.get_variable('libcstdaux_dep')
 
-use_ebpf = get_option('ebpf')
-
 subdir('src')

src/n-acd/meson_options.txt

@@ -1 +0,0 @@
-option('ebpf', type: 'boolean', value: true, description: 'Enable eBPF packet filtering')

src/n-acd/src/meson.build

@@ -13,20 +13,11 @@ libnacd_deps = [
 
 libnacd_sources = [
         'n-acd.c',
+        'n-acd-bpf.c',
         'n-acd-probe.c',
         'util/timer.c',
 ]
 
-if use_ebpf
-        libnacd_sources += [
-                'n-acd-bpf.c',
-        ]
-else
-        libnacd_sources += [
-                'n-acd-bpf-fallback.c',
-        ]
-endif
-
 libnacd_private = static_library(
         'nacd-private',
         libnacd_sources,
@@ -77,10 +68,8 @@ endif
 test_api = executable('test-api', ['test-api.c'], link_with: libnacd_shared)
 test('API Symbol Visibility', test_api)
 
-if use_ebpf
-        test_bpf = executable('test-bpf', ['test-bpf.c'], dependencies: libnacd_dep)
-        test('eBPF socket filtering', test_bpf)
-endif
+test_bpf = executable('test-bpf', ['test-bpf.c'], dependencies: libnacd_dep)
+test('eBPF socket filtering', test_bpf)
 
 test_loopback = executable('test-loopback', ['test-loopback.c'], dependencies: libnacd_dep)
 test('Echo Suppression via Loopback', test_loopback)

src/n-acd/src/n-acd-bpf-fallback.c

@@ -1,30 +0,0 @@
-/*
- * A noop implementation of eBPF filter for IPv4 Address Conflict Detection
- *
- * These are a collection of dummy functions that have no effect, but allows
- * n-acd to compile without eBPF support.
- *
- * See n-acd-bpf.c for documentation.
- */
-
-#include <c-stdaux.h>
-#include <stddef.h>
-#include "n-acd-private.h"
-
-int n_acd_bpf_map_create(int *mapfdp, size_t max_entries) {
-        *mapfdp = -1;
-        return 0;
-}
-
-int n_acd_bpf_map_add(int mapfd, struct in_addr *addrp) {
-        return 0;
-}
-
-int n_acd_bpf_map_remove(int mapfd, struct in_addr *addrp) {
-        return 0;
-}
-
-int n_acd_bpf_compile(int *progfdp, int mapfd, struct ether_addr *macp) {
-        *progfdp = -1;
-        return 0;
-}

src/n-acd/src/n-acd-probe.c

@@ -208,8 +208,6 @@ static int n_acd_probe_link(NAcdProbe *probe) {
          * entry.
          */
         r = n_acd_ensure_bpf_map_space(probe->acd);
-        if (r)
-                return r;
 
         /*
          * Link entry into context, indexed by its IP. Note that we allow
@@ -238,7 +236,7 @@ static int n_acd_probe_link(NAcdProbe *probe) {
         /*
          * Add the ip address to the map, if it is not already there.
          */
-        if (n_acd_probe_is_unique(probe)) {
+        if (probe->acd->fd_bpf_map >= 0 && n_acd_probe_is_unique(probe)) {
                 r = n_acd_bpf_map_add(probe->acd->fd_bpf_map, &probe->ip);
                 if (r) {
                         /*
@@ -261,7 +259,7 @@ static void n_acd_probe_unlink(NAcdProbe *probe) {
          * If this is the only probe for a given IP, remove the IP from the
          * kernel BPF map.
          */
-        if (n_acd_probe_is_unique(probe)) {
+        if (probe->acd->fd_bpf_map >= 0 && n_acd_probe_is_unique(probe)) {
                 r = n_acd_bpf_map_remove(probe->acd->fd_bpf_map, &probe->ip);
                 c_assert(r >= 0);
                 --probe->acd->n_bpf_map;

src/n-acd/src/n-acd.c

@@ -296,7 +296,7 @@ int n_acd_ensure_bpf_map_space(NAcd *acd) {
 
         r = n_acd_bpf_compile(&fd_prog, fd_map, (struct ether_addr*) acd->mac);
         if (r)
-                return r;
+                fd_prog = -1;
 
         if (fd_prog >= 0) {
                 r = setsockopt(acd->fd_socket, SOL_SOCKET, SO_ATTACH_BPF, &fd_prog, sizeof(fd_prog));
@@ -360,12 +360,12 @@ _c_public_ int n_acd_new(NAcd **acdp, NAcdConfig *config) {
         acd->max_bpf_map = 8;
 
         r = n_acd_bpf_map_create(&acd->fd_bpf_map, acd->max_bpf_map);
-        if (r)
-                return r;
 
-        r = n_acd_bpf_compile(&fd_bpf_prog, acd->fd_bpf_map, (struct ether_addr*) acd->mac);
-        if (r)
-                return r;
+        if (acd->fd_bpf_map >= 0) {
+                r = n_acd_bpf_compile(&fd_bpf_prog, acd->fd_bpf_map, (struct ether_addr*) acd->mac);
+                if (r)
+                        fd_bpf_prog = -1;
+        }
 
         r = n_acd_socket_new(&acd->fd_socket, fd_bpf_prog, config);
         if (r)

src/n-acd/src/test-bpf.c

@@ -48,8 +48,9 @@ static void test_map(void) {
         struct in_addr addr = { 1 };
 
         r = n_acd_bpf_map_create(&mapfd, 8);
-        c_assert(r >= 0);
-        c_assert(mapfd >= 0);
+
+        if (r == -EPERM)
+                return;
 
         r = n_acd_bpf_map_remove(mapfd, &addr);
         c_assert(r == -ENOENT);
@@ -103,7 +104,13 @@ static void test_filter(void) {
         int r, mapfd = -1, progfd = -1, pair[2];
 
         r = n_acd_bpf_map_create(&mapfd, 1);
+
+        if (r == -EPERM)
+                return;
+
         c_assert(r >= 0);
+        c_assert(mapfd >= 0);
+
 
         r = n_acd_bpf_compile(&progfd, mapfd, &mac1);
         c_assert(r >= 0);
@@ -113,7 +120,7 @@ static void test_filter(void) {
         c_assert(r >= 0);
 
         r = setsockopt(pair[1], SOL_SOCKET, SO_ATTACH_BPF, &progfd,
-                       sizeof(progfd));
+                      sizeof(progfd));
         c_assert(r >= 0);
 
         r = n_acd_bpf_map_add(mapfd, &ip1);
@@ -190,9 +197,9 @@ static void test_filter(void) {
         c_assert(errno == EAGAIN);
 
         /*
-         * Send one packet before and one packet after modifying the map,
-         * verify that the modification applies at the time of send(), not recv().
-         */
+        * Send one packet before and one packet after modifying the map,
+        * verify that the modification applies at the time of send(), not recv().
+        */
         *packet = (struct ether_arp)ETHER_ARP_PACKET_INIT(ARPOP_REQUEST, &mac2, &ip1, &ip2);
         r = send(pair[0], buf, sizeof(struct ether_arp), 0);
         c_assert(r == sizeof(struct ether_arp));