summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVojtech Bubela <vbubela@redhat.com>2022-11-02 10:51:06 +0100
committerVojtech Bubela <vojta@bubela.cz>2022-12-02 14:35:32 +0100
commitff7092f4d70f38be28e4742a3f533e8d2aad79b3 (patch)
treeb77d47d10d34ebf946aa74120b9f6b6cba3051c5
parent1d767bb1cddb79aaabccdfa96f65a5c5e6ddf1c3 (diff)
downloadNetworkManager-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.sh3
-rw-r--r--Makefile.am7
-rw-r--r--configure.ac16
-rw-r--r--contrib/fedora/rpm/NetworkManager.spec17
-rwxr-xr-xcontrib/fedora/rpm/configure-for-system.sh3
-rwxr-xr-xcontrib/scripts/nm-ci-run.sh4
-rw-r--r--meson.build14
-rw-r--r--meson_options.txt1
-rw-r--r--src/meson.build2
-rw-r--r--src/n-acd/.github/workflows/ci.yml36
-rw-r--r--src/n-acd/README.md7
-rw-r--r--src/n-acd/meson.build2
-rw-r--r--src/n-acd/meson_options.txt1
-rw-r--r--src/n-acd/src/meson.build17
-rw-r--r--src/n-acd/src/n-acd-bpf-fallback.c30
-rw-r--r--src/n-acd/src/n-acd-probe.c6
-rw-r--r--src/n-acd/src/n-acd.c12
-rw-r--r--src/n-acd/src/test-bpf.c19
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));