diff --git a/.gitlab-ci/run-test.sh b/.gitlab-ci/run-test.sh index 44405558c8..b58a1c321b 100755 --- a/.gitlab-ci/run-test.sh +++ b/.gitlab-ci/run-test.sh @@ -155,12 +155,7 @@ test_subtree() { do_clean pushd ./src/$d - ARGS=() - if [ "$d" = n-acd ]; then - ARGS+=('-Debpf=false') - fi - - CC="$cc" CFLAGS="-Werror -Wall" meson build "${ARGS[@]}" + CC="$cc" CFLAGS="-Werror -Wall" meson build ninja -v -C build test popd diff --git a/contrib/fedora/rpm/NetworkManager.spec b/contrib/fedora/rpm/NetworkManager.spec index a4f28f3668..0732fb495f 100644 --- a/contrib/fedora/rpm/NetworkManager.spec +++ b/contrib/fedora/rpm/NetworkManager.spec @@ -154,17 +154,6 @@ %bcond_with ifcfg_migrate %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. @@ -682,11 +671,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 \ diff --git a/contrib/fedora/rpm/configure-for-system.sh b/contrib/fedora/rpm/configure-for-system.sh index f0638591bd..6bdf66825a 100755 --- a/contrib/fedora/rpm/configure-for-system.sh +++ b/contrib/fedora/rpm/configure-for-system.sh @@ -155,7 +155,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-}" @@ -396,7 +395,6 @@ meson setup\ -Dmodify_system=true \ -Dconcheck=true \ -Dlibpsl="$(bool_true "$P_FEDORA")" \ - -Debpf="$(bool_true "$P_EBPF_ENABLED")" \ -Dsession_tracking=systemd \ -Dsuspend_resume=systemd \ -Dsystemdsystemunitdir=/usr/lib/systemd/system \ diff --git a/contrib/scripts/nm-ci-run.sh b/contrib/scripts/nm-ci-run.sh index 13d2f0fffd..57b867c550 100755 --- a/contrib/scripts/nm-ci-run.sh +++ b/contrib/scripts/nm-ci-run.sh @@ -180,8 +180,6 @@ meson setup build \ -D crypto=$_WITH_CRYPTO \ -D docs=$_WITH_DOCS \ \ - -D ebpf=false \ - \ -D iwd=true \ -D ofono=true \ -D teamdctl=$_WITH_LIBTEAM \ diff --git a/data/NetworkManager.service.in b/data/NetworkManager.service.in index 8cd2ac87a3..d0cd8b732e 100644 --- a/data/NetworkManager.service.in +++ b/data/NetworkManager.service.in @@ -19,7 +19,7 @@ KillMode=process # With a huge number of interfaces, starting can take a long time. TimeoutStartSec=600 -CapabilityBoundingSet=CAP_NET_ADMIN CAP_DAC_OVERRIDE CAP_NET_RAW CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT +CapabilityBoundingSet=CAP_NET_ADMIN CAP_DAC_OVERRIDE CAP_NET_RAW CAP_BPF CAP_NET_BIND_SERVICE CAP_SETGID CAP_SETUID CAP_SYS_MODULE CAP_AUDIT_WRITE CAP_KILL CAP_SYS_CHROOT ProtectSystem=true ProtectHome=read-only diff --git a/meson.build b/meson.build index c7313d572d..d1db57b50b 100644 --- a/meson.build +++ b/meson.build @@ -477,19 +477,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') @@ -1160,6 +1147,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 55762f65d5..d28cc76fc8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -44,7 +44,7 @@ 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: true, description: 'Build nm-cloud-setup, a tool for automatically configuring networking in cloud') option('bluez5_dun', type: 'boolean', value: false, description: 'enable Bluez5 DUN support') -option('ebpf', type: 'combo', choices: ['auto', 'true', 'false'], description: 'Enable eBPF support') +option('ebpf', type: 'combo', choices: ['auto', 'true', 'false'], description: 'Enable eBPF support (deprecated)') option('nbft', type: 'boolean', value: true, description: 'Enable NBFT support in the initrd generator') # configuration plugins diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index b200b82087..d2ec113a53 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -3056,9 +3056,10 @@ handle_start_probing: } _LOGT_acd(acd_data, - "%sstart probing (timeout %u msec, %s)", + "%sstart probing (timeout %u msec, ebpf %s; %s)", orig_state == NM_L3_ACD_ADDR_STATE_INIT ? "" : "re", acd_data->probing_timeout_msec, + n_acd_has_bpf(self->priv.p->nacd) ? "enabled" : "disabled", log_reason); return; } @@ -3153,10 +3154,11 @@ handle_start_defending: } _LOGT_acd(acd_data, - "start announcing (defend=%s) (probe created)", + "start announcing (defend=%s) (probe created with ebpf %s)", _l3_acd_defend_type_to_string(acd_data->acd_defend_type_current, sbuf256, - sizeof(sbuf256))); + sizeof(sbuf256)), + n_acd_has_bpf(self->priv.p->nacd) ? "enabled" : "disabled"); acd_data->acd_defend_type_is_active = FALSE; acd_data->nacd_probe = probe; return; diff --git a/src/meson.build b/src/meson.build index 1a0319828b..a5b3441ba7 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**: 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/libnacd.sym b/src/n-acd/src/libnacd.sym index f85e13acf9..537eab3e45 100644 --- a/src/n-acd/src/libnacd.sym +++ b/src/n-acd/src/libnacd.sym @@ -15,6 +15,7 @@ global: n_acd_ref; n_acd_unref; n_acd_get_fd; + n_acd_has_bpf; n_acd_dispatch; n_acd_pop_event; n_acd_probe; diff --git a/src/n-acd/src/meson.build b/src/n-acd/src/meson.build index 3e92681f91..c1582f4216 100644 --- a/src/n-acd/src/meson.build +++ b/src/n-acd/src/meson.build @@ -15,18 +15,9 @@ libnacd_sources = [ 'n-acd.c', 'n-acd-probe.c', 'util/timer.c', + 'n-acd-bpf.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 -#include -#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-bpf.c b/src/n-acd/src/n-acd-bpf.c index 57b29ddf2f..55b44ab94d 100644 --- a/src/n-acd/src/n-acd-bpf.c +++ b/src/n-acd/src/n-acd-bpf.c @@ -170,6 +170,10 @@ int n_acd_bpf_map_add(int mapfd, struct in_addr *addrp) { uint8_t _dummy = 0; int r; + /* If we don't have a map to update, there is nothing to do. */ + if (mapfd == -1) + return 0; + memset(&attr, 0, sizeof(attr)); attr = (union bpf_attr){ .map_fd = mapfd, @@ -190,6 +194,10 @@ int n_acd_bpf_map_remove(int mapfd, struct in_addr *addrp) { union bpf_attr attr; int r; + /* If we don't have a map to update, there is nothing to do. */ + if (mapfd == -1) + return 0; + memset(&attr, 0, sizeof(attr)); attr = (union bpf_attr){ .map_fd = mapfd, diff --git a/src/n-acd/src/n-acd.c b/src/n-acd/src/n-acd.c index c1d9286503..9bc322449e 100644 --- a/src/n-acd/src/n-acd.c +++ b/src/n-acd/src/n-acd.c @@ -284,6 +284,11 @@ int n_acd_ensure_bpf_map_space(NAcd *acd) { max_map = 2 * acd->max_bpf_map; + /* If we didn't succeed in creating a map during n_acd_new(), + * let's assume we are unable to create it here, and skip it. */ + if (!n_acd_has_bpf(acd)) + goto out; + r = n_acd_bpf_map_create(&fd_map, max_map); if (r) return r; @@ -308,6 +313,8 @@ int n_acd_ensure_bpf_map_space(NAcd *acd) { close(acd->fd_bpf_map); acd->fd_bpf_map = fd_map; fd_map = -1; + +out: acd->max_bpf_map = max_map; return 0; } @@ -359,13 +366,16 @@ _c_public_ int n_acd_new(NAcd **acdp, NAcdConfig *config) { acd->max_bpf_map = 8; + /* Let's try to create a BPF map. If we fail, we want to ignore it + * only if we lack permissions to create it, and proceed without eBPF. */ 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) + if (r == 0) { + r = n_acd_bpf_compile(&fd_bpf_prog, acd->fd_bpf_map, (struct ether_addr*) acd->mac); + if (r) + return r; + } else if (r != -EPERM) { return r; + } r = n_acd_socket_new(&acd->fd_socket, fd_bpf_prog, config); if (r) @@ -572,6 +582,20 @@ _c_public_ void n_acd_get_fd(NAcd *acd, int *fdp) { *fdp = acd->fd_epoll; } +/** + * n_acd_has_bpf() - query the usage of eBPF + * @acd: context object to operate on + * + * Checks whether the ACD probe is using eBPF or not. + * + * Return: true if the probe is using eBPF, or + * false if the probe failed to configure eBPF + * (e.g. due to missing capabilities) + */ +_c_public_ bool n_acd_has_bpf(NAcd *acd) { + return acd->fd_bpf_map != -1; +} + static int n_acd_handle_timeout(NAcd *acd) { NAcdProbe *probe; uint64_t now; diff --git a/src/n-acd/src/n-acd.h b/src/n-acd/src/n-acd.h index e2b01270fa..8e5cf59a2f 100644 --- a/src/n-acd/src/n-acd.h +++ b/src/n-acd/src/n-acd.h @@ -93,6 +93,7 @@ NAcd *n_acd_ref(NAcd *acd); NAcd *n_acd_unref(NAcd *acd); void n_acd_get_fd(NAcd *acd, int *fdp); +bool n_acd_has_bpf(NAcd *acd); int n_acd_dispatch(NAcd *acd); int n_acd_pop_event(NAcd *acd, NAcdEvent **eventp); diff --git a/src/n-acd/src/test-api.c b/src/n-acd/src/test-api.c index 70f7520836..009838aada 100644 --- a/src/n-acd/src/test-api.c +++ b/src/n-acd/src/test-api.c @@ -56,6 +56,7 @@ static void test_api_functions(void) { (void *)n_acd_ref, (void *)n_acd_unref, (void *)n_acd_get_fd, + (void *)n_acd_has_bpf, (void *)n_acd_dispatch, (void *)n_acd_pop_event, (void *)n_acd_probe, diff --git a/src/n-acd/src/test-bpf.c b/src/n-acd/src/test-bpf.c index 78f9d0f19c..17e89803b7 100644 --- a/src/n-acd/src/test-bpf.c +++ b/src/n-acd/src/test-bpf.c @@ -19,6 +19,9 @@ #include "n-acd-private.h" #include "test.h" +// https://mesonbuild.com/Unit-tests.html#skipped-tests-and-hard-errors +#define RETURN_TEST_SKIPPED 77 + #define ETHER_ARP_PACKET_INIT(_op, _mac, _sip, _tip) { \ .ea_hdr = { \ .ar_hrd = htobe16(ARPHRD_ETHER), \ @@ -43,11 +46,15 @@ .arp_tpa[3] = be32toh((_tip)->s_addr) & 0xff, \ } -static void test_map(void) { +static int test_map(void) { int r, mapfd = -1; struct in_addr addr = { 1 }; r = n_acd_bpf_map_create(&mapfd, 8); + if (r == -EPERM) { + return RETURN_TEST_SKIPPED; + } + c_assert(r >= 0); c_assert(mapfd >= 0); @@ -67,6 +74,7 @@ static void test_map(void) { c_assert(r == -ENOENT); close(mapfd); + return 0; } static void verify_success(struct ether_arp *packet, int out_fd, int in_fd) { @@ -92,7 +100,7 @@ static void verify_failure(struct ether_arp *packet, int out_fd, int in_fd) { c_assert(errno == EAGAIN); } -static void test_filter(void) { +static int test_filter(void) { uint8_t buf[sizeof(struct ether_arp) + 1] = {}; struct ether_addr mac1 = { { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 } }; struct ether_addr mac2 = { { 0x01, 0x02, 0x03, 0x04, 0x05, 0x07 } }; @@ -103,6 +111,10 @@ 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 RETURN_TEST_SKIPPED; + } + c_assert(r >= 0); r = n_acd_bpf_compile(&progfd, mapfd, &mac1); @@ -214,13 +226,21 @@ static void test_filter(void) { close(pair[1]); close(progfd); close(mapfd); -} - -int main(int argc, char **argv) { - test_setup(); - - test_map(); - test_filter(); + + return 0; +} + +int main(int argc, char **argv) { + int r; + test_setup(); + + if ((r = test_map())) { + return r; + } + + if ((r = test_filter())) { + return r; + } return 0; } diff --git a/src/n-dhcp4/src/n-dhcp4-c-connection.c b/src/n-dhcp4/src/n-dhcp4-c-connection.c index e76ea16592..707c660487 100644 --- a/src/n-dhcp4/src/n-dhcp4-c-connection.c +++ b/src/n-dhcp4/src/n-dhcp4-c-connection.c @@ -379,6 +379,10 @@ void n_dhcp4_c_connection_get_timeout(NDhcp4CConnection *connection, *timeoutp = timeout; } +void n_dhcp4_c_connection_clear_client_ip(NDhcp4CConnection *connection) { + connection->client_ip = INADDR_ANY; +} + static int n_dhcp4_c_connection_packet_broadcast(NDhcp4CConnection *connection, NDhcp4Outgoing *message) { int r; diff --git a/src/n-dhcp4/src/n-dhcp4-c-probe.c b/src/n-dhcp4/src/n-dhcp4-c-probe.c index 58d61e72ba..43dff7b050 100644 --- a/src/n-dhcp4/src/n-dhcp4-c-probe.c +++ b/src/n-dhcp4/src/n-dhcp4-c-probe.c @@ -198,7 +198,7 @@ _c_public_ void n_dhcp4_client_probe_config_set_init_reboot(NDhcp4ClientProbeCon * * This sets the DSCP property of the configuration object, which specifies * the DSCP value to set in the first six bits of the DS field in the IPv4 - * header. If this function is not called, the DSCP will be set to CS0. + * header. If this function is not called, the DSCP will be set to CS0 (0). */ _c_public_ void n_dhcp4_client_probe_config_set_dscp(NDhcp4ClientProbeConfig *config, uint8_t dscp) { config->dscp = dscp & 0x3F; @@ -703,6 +703,8 @@ static int n_dhcp4_client_probe_transition_deferred(NDhcp4ClientProbe *probe, ui switch (probe->state) { case N_DHCP4_CLIENT_PROBE_STATE_INIT: + /* reset client IP (CIADDR) */ + n_dhcp4_c_connection_clear_client_ip(&probe->connection); r = n_dhcp4_c_connection_listen(&probe->connection); if (r) return r; diff --git a/src/n-dhcp4/src/n-dhcp4-private.h b/src/n-dhcp4/src/n-dhcp4-private.h index dbfe9b4ad0..7e71406e18 100644 --- a/src/n-dhcp4/src/n-dhcp4-private.h +++ b/src/n-dhcp4/src/n-dhcp4-private.h @@ -617,6 +617,8 @@ void n_dhcp4_c_connection_close(NDhcp4CConnection *connection); void n_dhcp4_c_connection_get_timeout(NDhcp4CConnection *connection, uint64_t *timeoutp); +void n_dhcp4_c_connection_clear_client_ip(NDhcp4CConnection *connection); + int n_dhcp4_c_connection_discover_new(NDhcp4CConnection *connection, NDhcp4Outgoing **request); int n_dhcp4_c_connection_select_new(NDhcp4CConnection *connection,