diff options
author | Vojtech Bubela <vbubela@redhat.com> | 2022-11-02 10:51:06 +0100 |
---|---|---|
committer | Vojtech Bubela <vojta@bubela.cz> | 2022-12-02 14:35:32 +0100 |
commit | ff7092f4d70f38be28e4742a3f533e8d2aad79b3 (patch) | |
tree | b77d47d10d34ebf946aa74120b9f6b6cba3051c5 | |
parent | 1d767bb1cddb79aaabccdfa96f65a5c5e6ddf1c3 (diff) | |
download | NetworkManager-fix-eBPF.tar.gz |
n-acd: using ebpf is decided at runtimefix-eBPF
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.
-rwxr-xr-x | .gitlab-ci/run-test.sh | 3 | ||||
-rw-r--r-- | Makefile.am | 7 | ||||
-rw-r--r-- | configure.ac | 16 | ||||
-rw-r--r-- | contrib/fedora/rpm/NetworkManager.spec | 17 | ||||
-rwxr-xr-x | contrib/fedora/rpm/configure-for-system.sh | 3 | ||||
-rwxr-xr-x | contrib/scripts/nm-ci-run.sh | 4 | ||||
-rw-r--r-- | meson.build | 14 | ||||
-rw-r--r-- | meson_options.txt | 1 | ||||
-rw-r--r-- | src/meson.build | 2 | ||||
-rw-r--r-- | src/n-acd/.github/workflows/ci.yml | 36 | ||||
-rw-r--r-- | src/n-acd/README.md | 7 | ||||
-rw-r--r-- | src/n-acd/meson.build | 2 | ||||
-rw-r--r-- | src/n-acd/meson_options.txt | 1 | ||||
-rw-r--r-- | src/n-acd/src/meson.build | 17 | ||||
-rw-r--r-- | src/n-acd/src/n-acd-bpf-fallback.c | 30 | ||||
-rw-r--r-- | src/n-acd/src/n-acd-probe.c | 6 | ||||
-rw-r--r-- | src/n-acd/src/n-acd.c | 12 | ||||
-rw-r--r-- | src/n-acd/src/test-bpf.c | 19 |
18 files changed, 26 insertions, 171 deletions
diff --git a/.gitlab-ci/run-test.sh b/.gitlab-ci/run-test.sh index 1fdb214b85..2bdb8f663f 100755 --- a/.gitlab-ci/run-test.sh +++ b/.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 diff --git a/Makefile.am b/Makefile.am index 8e60a5f5bd..0985a19929 100644 --- a/Makefile.am +++ b/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 diff --git a/configure.ac b/configure.ac index 50f7beaccb..94e814b2ab 100644 --- a/configure.ac +++ b/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 diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index a279b128be..efb34553a6 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/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. @@ -700,11 +689,6 @@ Preferably use nmcli instead. %else -Dlibpsl=false \ %endif -%if %{ebpf_enabled} != "yes" - -Debpf=false \ -%else - -Debpf=true \ -%endif -Dsession_tracking=systemd \ -Dsuspend_resume=systemd \ -Dsystem_ca_path=/etc/pki/tls/cert.pem \ @@ -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 \ diff --git a/contrib/fedora/rpm/configure-for-system.sh b/contrib/fedora/rpm/configure-for-system.sh index ea1639f9d7..3147df6527 100755 --- a/contrib/fedora/rpm/configure-for-system.sh +++ b/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 \ diff --git a/contrib/scripts/nm-ci-run.sh b/contrib/scripts/nm-ci-run.sh index 00adba85e1..e02f6a1d78 100755 --- a/contrib/scripts/nm-ci-run.sh +++ b/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 \ diff --git a/meson.build b/meson.build index 422b29779b..fa9c487c7e 100644 --- a/meson.build +++ b/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) diff --git a/meson_options.txt b/meson_options.txt index 8b1d32e645..a81d35c480 100644 --- a/meson_options.txt +++ b/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') diff --git a/src/meson.build b/src/meson.build index 92e95e68ef..d186b4db4c 100644 --- a/src/meson.build +++ b/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', diff --git a/src/n-acd/.github/workflows/ci.yml b/src/n-acd/.github/workflows/ci.yml index 22fc814187..f6987414ca 100644 --- a/src/n-acd/.github/workflows/ci.yml +++ b/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 diff --git a/src/n-acd/README.md b/src/n-acd/README.md index 089541825d..d207e70041 100644 --- a/src/n-acd/README.md +++ b/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> diff --git a/src/n-acd/meson.build b/src/n-acd/meson.build index 6479eb1a77..ca72e567e3 100644 --- a/src/n-acd/meson.build +++ b/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') diff --git a/src/n-acd/meson_options.txt b/src/n-acd/meson_options.txt deleted file mode 100644 index b024ee1d4c..0000000000 --- a/src/n-acd/meson_options.txt +++ /dev/null @@ -1 +0,0 @@ -option('ebpf', type: 'boolean', value: true, description: 'Enable eBPF packet filtering') diff --git a/src/n-acd/src/meson.build b/src/n-acd/src/meson.build index 3e92681f91..db0bce6201 100644 --- a/src/n-acd/src/meson.build +++ b/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) diff --git a/src/n-acd/src/n-acd-bpf-fallback.c b/src/n-acd/src/n-acd-bpf-fallback.c deleted file mode 100644 index 3cf4eb0679..0000000000 --- a/src/n-acd/src/n-acd-bpf-fallback.c +++ /dev/null @@ -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; -} diff --git a/src/n-acd/src/n-acd-probe.c b/src/n-acd/src/n-acd-probe.c index c1ed59ae9e..2f7d364233 100644 --- a/src/n-acd/src/n-acd-probe.c +++ b/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; diff --git a/src/n-acd/src/n-acd.c b/src/n-acd/src/n-acd.c index c1d9286503..5f49a38ac6 100644 --- a/src/n-acd/src/n-acd.c +++ b/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) diff --git a/src/n-acd/src/test-bpf.c b/src/n-acd/src/test-bpf.c index 78f9d0f19c..f4576012db 100644 --- a/src/n-acd/src/test-bpf.c +++ b/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)); |