From 50e23ac6671ea1eb00cde2a2bd1ee5ee69895f3b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 15 Aug 2022 20:05:21 +0900 Subject: [PATCH 1/6] daemon-util: introduce several helper functions for fd store --- src/shared/daemon-util.c | 76 ++++++++++++++++++++++++++++++++++++++++ src/shared/daemon-util.h | 6 ++++ src/shared/meson.build | 1 + 3 files changed, 83 insertions(+) create mode 100644 src/shared/daemon-util.c diff --git a/src/shared/daemon-util.c b/src/shared/daemon-util.c new file mode 100644 index 0000000000..32180a14bb --- /dev/null +++ b/src/shared/daemon-util.c @@ -0,0 +1,76 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "daemon-util.h" +#include "fd-util.h" +#include "log.h" +#include "string-util.h" + +static int notify_remove_fd_warn(const char *name) { + int r; + + assert(name); + + r = sd_notifyf(/* unset_environment = */ false, + "FDSTOREREMOVE=1\n" + "FDNAME=%s", name); + if (r < 0) + return log_warning_errno(r, + "Failed to remove file descriptor \"%s\" from the store, ignoring: %m", + name); + + return 0; +} + +int notify_remove_fd_warnf(const char *format, ...) { + _cleanup_free_ char *p = NULL; + va_list ap; + int r; + + assert(format); + + va_start(ap, format); + r = vasprintf(&p, format, ap); + va_end(ap); + if (r < 0) + return log_oom(); + + return notify_remove_fd_warn(p); +} + +int close_and_notify_warn(int fd, const char *name) { + if (name) + (void) notify_remove_fd_warn(name); + + return safe_close(fd); +} + +static int notify_push_fd(int fd, const char *name) { + _cleanup_free_ char *state = NULL; + + assert(fd >= 0); + assert(name); + + state = strjoin("FDSTORE=1\n" + "FDNAME=", name); + if (!state) + return -ENOMEM; + + return sd_pid_notify_with_fds(0, /* unset_environment = */ false, state, &fd, 1); +} + +int notify_push_fdf(int fd, const char *format, ...) { + _cleanup_free_ char *name = NULL; + va_list ap; + int r; + + assert(fd >= 0); + assert(format); + + va_start(ap, format); + r = vasprintf(&name, format, ap); + va_end(ap); + if (r < 0) + return -ENOMEM; + + return notify_push_fd(fd, name); +} diff --git a/src/shared/daemon-util.h b/src/shared/daemon-util.h index 585e4894a0..711885bba4 100644 --- a/src/shared/daemon-util.h +++ b/src/shared/daemon-util.h @@ -5,6 +5,8 @@ #include "sd-daemon.h" +#include "macro.h" + #define NOTIFY_READY "READY=1\n" "STATUS=Processing requests..." #define NOTIFY_STOPPING "STOPPING=1\n" "STATUS=Shutting down..." @@ -20,3 +22,7 @@ static inline void notify_on_cleanup(const char **p) { if (*p) (void) sd_notify(false, *p); } + +int notify_remove_fd_warnf(const char *format, ...) _printf_(1, 2); +int close_and_notify_warn(int fd, const char *name); +int notify_push_fdf(int fd, const char *format, ...) _printf_(2, 3); diff --git a/src/shared/meson.build b/src/shared/meson.build index 598a64593b..426a87b70c 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -85,6 +85,7 @@ shared_sources = files( 'creds-util.h', 'cryptsetup-util.c', 'cryptsetup-util.h', + 'daemon-util.c', 'daemon-util.h', 'data-fd-util.c', 'data-fd-util.h', From 2720b6f23cbf87235339dd3cec32ab790f1b5bf5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Mon, 15 Aug 2022 20:05:32 +0900 Subject: [PATCH 2/6] login: use helper functions for fd store --- src/login/logind-session-device.c | 22 ++++------------------ src/login/logind.c | 14 ++++---------- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index 003dbc0a95..dc6a0d5407 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -10,6 +10,7 @@ #include "alloc-util.h" #include "bus-util.h" +#include "daemon-util.h" #include "fd-util.h" #include "logind-session-dbus.h" #include "logind-session-device.h" @@ -376,19 +377,11 @@ error: } void session_device_free(SessionDevice *sd) { - int r; - assert(sd); /* Make sure to remove the pushed fd. */ - if (sd->pushed_fd) { - r = sd_notifyf(false, - "FDSTOREREMOVE=1\n" - "FDNAME=session-%s-device-%u-%u", - sd->session->id, major(sd->dev), minor(sd->dev)); - if (r < 0) - log_warning_errno(r, "Failed to remove file descriptor from the store, ignoring: %m"); - } + if (sd->pushed_fd) + (void) notify_remove_fd_warnf("session-%s-device-%u-%u", sd->session->id, major(sd->dev), minor(sd->dev)); session_device_stop(sd); session_device_notify(sd, SESSION_DEVICE_RELEASE); @@ -469,7 +462,6 @@ unsigned session_device_try_pause_all(Session *s) { } int session_device_save(SessionDevice *sd) { - _cleanup_free_ char *m = NULL; const char *id; int r; @@ -489,13 +481,7 @@ int session_device_save(SessionDevice *sd) { id = sd->session->id; assert(*(id + strcspn(id, "-\n")) == '\0'); - r = asprintf(&m, "FDSTORE=1\n" - "FDNAME=session-%s-device-%u-%u\n", - id, major(sd->dev), minor(sd->dev)); - if (r < 0) - return r; - - r = sd_pid_notify_with_fds(0, false, m, &sd->fd, 1); + r = notify_push_fdf(sd->fd, "session-%s-device-%u-%u", id, major(sd->dev), minor(sd->dev)); if (r < 0) return r; diff --git a/src/login/logind.c b/src/login/logind.c index d14a17274b..3a6a6e0748 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -438,7 +438,7 @@ static int deliver_fd(Manager *m, const char *fdname, int fd) { static int manager_attach_fds(Manager *m) { _cleanup_strv_free_ char **fdnames = NULL; - int r, n; + int n; /* Upon restart, PID1 will send us back all fds of session devices that we previously opened. Each * file descriptor is associated with a given session. The session ids are passed through FDNAMES. */ @@ -455,15 +455,9 @@ static int manager_attach_fds(Manager *m) { if (deliver_fd(m, fdnames[i], fd) >= 0) continue; - /* Hmm, we couldn't deliver the fd to any session device object? If so, let's close the fd */ - safe_close(fd); - - /* Remove from fdstore as well */ - r = sd_notifyf(false, - "FDSTOREREMOVE=1\n" - "FDNAME=%s", fdnames[i]); - if (r < 0) - log_warning_errno(r, "Failed to remove file descriptor from the store, ignoring: %m"); + /* Hmm, we couldn't deliver the fd to any session device object? If so, let's close the fd + * and remove it from fdstore. */ + close_and_notify_warn(fd, fdnames[i]); } return 0; From 8f388c4e4607f7e8ce1807e8c7dfcc17d68b9c55 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Aug 2022 07:35:21 +0900 Subject: [PATCH 3/6] network/tuntap: code cleanups - merge unnecessarily split functions, - drop unnecessary initializations, - tighten variable scopes, - introduce TUNTAP() helper function. --- src/network/netdev/tuntap.c | 99 ++++++++++++++----------------------- 1 file changed, 38 insertions(+), 61 deletions(-) diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c index 6f099a1448..8e9c1ca9b3 100644 --- a/src/network/netdev/tuntap.c +++ b/src/network/netdev/tuntap.c @@ -16,111 +16,88 @@ #define TUN_DEV "/dev/net/tun" -static int netdev_fill_tuntap_message(NetDev *netdev, struct ifreq *ifr) { - TunTap *t; - +static TunTap* TUNTAP(NetDev *netdev) { assert(netdev); - assert(netdev->ifname); - assert(ifr); - if (netdev->kind == NETDEV_KIND_TAP) { - t = TAP(netdev); - ifr->ifr_flags |= IFF_TAP; - } else { - t = TUN(netdev); - ifr->ifr_flags |= IFF_TUN; + switch (netdev->kind) { + case NETDEV_KIND_TAP: + return TAP(netdev); + case NETDEV_KIND_TUN: + return TUN(netdev); + default: + return NULL; } - - if (!t->packet_info) - ifr->ifr_flags |= IFF_NO_PI; - - if (t->multi_queue) - ifr->ifr_flags |= IFF_MULTI_QUEUE; - - if (t->vnet_hdr) - ifr->ifr_flags |= IFF_VNET_HDR; - - strncpy(ifr->ifr_name, netdev->ifname, IFNAMSIZ-1); - - return 0; } -static int netdev_tuntap_add(NetDev *netdev, struct ifreq *ifr) { +static int netdev_create_tuntap(NetDev *netdev) { _cleanup_close_ int fd = -1; - TunTap *t = NULL; - const char *user; - const char *group; - uid_t uid; - gid_t gid; + struct ifreq ifr = {}; + TunTap *t; int r; assert(netdev); - assert(ifr); + t = TUNTAP(netdev); + assert(t); fd = open(TUN_DEV, O_RDWR|O_CLOEXEC); if (fd < 0) - return log_netdev_error_errno(netdev, errno, "Failed to open tun dev: %m"); - - if (ioctl(fd, TUNSETIFF, ifr) < 0) - return log_netdev_error_errno(netdev, errno, "TUNSETIFF failed on tun dev: %m"); + return log_netdev_error_errno(netdev, errno, "Failed to open " TUN_DEV ": %m"); if (netdev->kind == NETDEV_KIND_TAP) - t = TAP(netdev); + ifr.ifr_flags |= IFF_TAP; else - t = TUN(netdev); + ifr.ifr_flags |= IFF_TUN; - assert(t); + if (!t->packet_info) + ifr.ifr_flags |= IFF_NO_PI; + + if (t->multi_queue) + ifr.ifr_flags |= IFF_MULTI_QUEUE; + + if (t->vnet_hdr) + ifr.ifr_flags |= IFF_VNET_HDR; + + strncpy(ifr.ifr_name, netdev->ifname, IFNAMSIZ-1); + + if (ioctl(fd, TUNSETIFF, &ifr) < 0) + return log_netdev_error_errno(netdev, errno, "TUNSETIFF failed: %m"); if (t->user_name) { - user = t->user_name; + const char *user = t->user_name; + uid_t uid; r = get_user_creds(&user, &uid, NULL, NULL, NULL, USER_CREDS_ALLOW_MISSING); if (r < 0) return log_netdev_error_errno(netdev, r, "Cannot resolve user name %s: %m", t->user_name); if (ioctl(fd, TUNSETOWNER, uid) < 0) - return log_netdev_error_errno(netdev, errno, "TUNSETOWNER failed on tun dev: %m"); + return log_netdev_error_errno(netdev, errno, "TUNSETOWNER failed: %m"); } if (t->group_name) { - group = t->group_name; + const char *group = t->group_name; + gid_t gid; r = get_group_creds(&group, &gid, USER_CREDS_ALLOW_MISSING); if (r < 0) return log_netdev_error_errno(netdev, r, "Cannot resolve group name %s: %m", t->group_name); if (ioctl(fd, TUNSETGROUP, gid) < 0) - return log_netdev_error_errno(netdev, errno, "TUNSETGROUP failed on tun dev: %m"); + return log_netdev_error_errno(netdev, errno, "TUNSETGROUP failed: %m"); } if (ioctl(fd, TUNSETPERSIST, 1) < 0) - return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed on tun dev: %m"); + return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed: %m"); return 0; } -static int netdev_create_tuntap(NetDev *netdev) { - struct ifreq ifr = {}; - int r; - - r = netdev_fill_tuntap_message(netdev, &ifr); - if (r < 0) - return r; - - return netdev_tuntap_add(netdev, &ifr); -} - static void tuntap_done(NetDev *netdev) { - TunTap *t = NULL; + TunTap *t; assert(netdev); - - if (netdev->kind == NETDEV_KIND_TUN) - t = TUN(netdev); - else - t = TAP(netdev); - + t = TUNTAP(netdev); assert(t); t->user_name = mfree(t->user_name); From f8b7c177640e19a0146fa26c4263e713b7045222 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Aug 2022 07:45:49 +0900 Subject: [PATCH 4/6] network/tuntap: introduce KeepCarrier= setting Closes #24267. --- man/systemd.netdev.xml | 9 +++++++++ src/network/netdev/netdev-gperf.gperf | 2 ++ src/network/netdev/tuntap.c | 16 ++++++++++++++++ src/network/netdev/tuntap.h | 2 ++ test/fuzz/fuzz-netdev-parser/directives.netdev | 2 ++ 5 files changed, 31 insertions(+) diff --git a/man/systemd.netdev.xml b/man/systemd.netdev.xml index 94c9308e46..266ad52df2 100644 --- a/man/systemd.netdev.xml +++ b/man/systemd.netdev.xml @@ -1558,6 +1558,15 @@ /dev/net/tun device. + + KeepCarrier= + + Takes a boolean. If enabled, to make the interface maintain its carrier status, the file + descriptor of the interface is kept open. This may be useful to keep the interface in running + state, for example while the backing process is temporarily shutdown. Defaults to + no. + + diff --git a/src/network/netdev/netdev-gperf.gperf b/src/network/netdev/netdev-gperf.gperf index 162664ecf1..3cfcd51e63 100644 --- a/src/network/netdev/netdev-gperf.gperf +++ b/src/network/netdev/netdev-gperf.gperf @@ -185,12 +185,14 @@ Tun.PacketInfo, config_parse_bool, Tun.VNetHeader, config_parse_bool, 0, offsetof(TunTap, vnet_hdr) Tun.User, config_parse_string, CONFIG_PARSE_STRING_SAFE, offsetof(TunTap, user_name) Tun.Group, config_parse_string, CONFIG_PARSE_STRING_SAFE, offsetof(TunTap, group_name) +Tun.KeepCarrier, config_parse_bool, 0, offsetof(TunTap, keep_fd) Tap.OneQueue, config_parse_warn_compat, DISABLED_LEGACY, 0 Tap.MultiQueue, config_parse_bool, 0, offsetof(TunTap, multi_queue) Tap.PacketInfo, config_parse_bool, 0, offsetof(TunTap, packet_info) Tap.VNetHeader, config_parse_bool, 0, offsetof(TunTap, vnet_hdr) Tap.User, config_parse_string, CONFIG_PARSE_STRING_SAFE, offsetof(TunTap, user_name) Tap.Group, config_parse_string, CONFIG_PARSE_STRING_SAFE, offsetof(TunTap, group_name) +Tap.KeepCarrier, config_parse_bool, 0, offsetof(TunTap, keep_fd) Bond.Mode, config_parse_bond_mode, 0, offsetof(Bond, mode) Bond.TransmitHashPolicy, config_parse_bond_xmit_hash_policy, 0, offsetof(Bond, xmit_hash_policy) Bond.LACPTransmitRate, config_parse_bond_lacp_rate, 0, offsetof(Bond, lacp_rate) diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c index 8e9c1ca9b3..f48ebf268a 100644 --- a/src/network/netdev/tuntap.c +++ b/src/network/netdev/tuntap.c @@ -90,9 +90,22 @@ static int netdev_create_tuntap(NetDev *netdev) { if (ioctl(fd, TUNSETPERSIST, 1) < 0) return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed: %m"); + if (t->keep_fd) + t->fd = TAKE_FD(fd); + return 0; } +static void tuntap_init(NetDev *netdev) { + TunTap *t; + + assert(netdev); + t = TUNTAP(netdev); + assert(t); + + t->fd = -1; +} + static void tuntap_done(NetDev *netdev) { TunTap *t; @@ -100,6 +113,7 @@ static void tuntap_done(NetDev *netdev) { t = TUNTAP(netdev); assert(t); + t->fd = safe_close(t->fd); t->user_name = mfree(t->user_name); t->group_name = mfree(t->group_name); } @@ -126,6 +140,7 @@ const NetDevVTable tun_vtable = { .object_size = sizeof(TunTap), .sections = NETDEV_COMMON_SECTIONS "Tun\0", .config_verify = tuntap_verify, + .init = tuntap_init, .done = tuntap_done, .create = netdev_create_tuntap, .create_type = NETDEV_CREATE_INDEPENDENT, @@ -136,6 +151,7 @@ const NetDevVTable tap_vtable = { .object_size = sizeof(TunTap), .sections = NETDEV_COMMON_SECTIONS "Tap\0", .config_verify = tuntap_verify, + .init = tuntap_init, .done = tuntap_done, .create = netdev_create_tuntap, .create_type = NETDEV_CREATE_INDEPENDENT, diff --git a/src/network/netdev/tuntap.h b/src/network/netdev/tuntap.h index 4d1e643f43..52f2da9fab 100644 --- a/src/network/netdev/tuntap.h +++ b/src/network/netdev/tuntap.h @@ -8,11 +8,13 @@ typedef struct TunTap TunTap; struct TunTap { NetDev meta; + int fd; char *user_name; char *group_name; bool multi_queue; bool packet_info; bool vnet_hdr; + bool keep_fd; }; DEFINE_NETDEV_CAST(TUN, TunTap); diff --git a/test/fuzz/fuzz-netdev-parser/directives.netdev b/test/fuzz/fuzz-netdev-parser/directives.netdev index d5f3228065..309941f58d 100644 --- a/test/fuzz/fuzz-netdev-parser/directives.netdev +++ b/test/fuzz/fuzz-netdev-parser/directives.netdev @@ -171,6 +171,7 @@ User= Group= PacketInfo= VNetHeader= +KeepCarrier= [IPVLAN] Mode= Flags= @@ -184,6 +185,7 @@ PacketInfo= VNetHeader= Group= User= +KeepCarrier= [NetDev] Kind= MACAddress= From af7a86b8a6b6510264b7ac0ae6a1e1d37d510ef5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Aug 2022 17:18:55 +0900 Subject: [PATCH 5/6] network/tuntap: save tun or tap file descriptor in fd store --- src/network/netdev/netdev.c | 3 + src/network/netdev/netdev.h | 3 + src/network/netdev/tuntap.c | 93 ++++++++++++++++++++++++++++++- src/network/netdev/tuntap.h | 3 + src/network/networkd-link.c | 1 + src/network/networkd-manager.c | 62 ++++++++++++++++----- src/network/networkd-manager.h | 2 + units/systemd-networkd.service.in | 1 + 8 files changed, 153 insertions(+), 15 deletions(-) diff --git a/src/network/netdev/netdev.c b/src/network/netdev/netdev.c index 464f47f2cf..212df3daa0 100644 --- a/src/network/netdev/netdev.c +++ b/src/network/netdev/netdev.c @@ -237,6 +237,9 @@ void netdev_drop(NetDev *netdev) { return; } + if (NETDEV_VTABLE(netdev) && NETDEV_VTABLE(netdev)->drop) + NETDEV_VTABLE(netdev)->drop(netdev); + netdev->state = NETDEV_STATE_LINGER; log_netdev_debug(netdev, "netdev removed"); diff --git a/src/network/netdev/netdev.h b/src/network/netdev/netdev.h index 1c7dc0f7e5..49eadbb7a4 100644 --- a/src/network/netdev/netdev.h +++ b/src/network/netdev/netdev.h @@ -142,6 +142,9 @@ typedef struct NetDevVTable { * to be set != 0. */ void (*init)(NetDev *n); + /* This is called when the interface is removed. */ + void (*drop)(NetDev *n); + /* This should free all kind-specific variables. It should be * idempotent. */ void (*done)(NetDev *n); diff --git a/src/network/netdev/tuntap.c b/src/network/netdev/tuntap.c index f48ebf268a..39ea7c1d73 100644 --- a/src/network/netdev/tuntap.c +++ b/src/network/netdev/tuntap.c @@ -10,7 +10,11 @@ #include #include "alloc-util.h" +#include "daemon-util.h" #include "fd-util.h" +#include "networkd-link.h" +#include "networkd-manager.h" +#include "socket-util.h" #include "tuntap.h" #include "user-util.h" @@ -29,6 +33,73 @@ static TunTap* TUNTAP(NetDev *netdev) { } } +static void *close_fd_ptr(void *p) { + safe_close(PTR_TO_FD(p)); + return NULL; +} + +DEFINE_PRIVATE_HASH_OPS_FULL(named_fd_hash_ops, char, string_hash_func, string_compare_func, free, void, close_fd_ptr); + +int manager_add_tuntap_fd(Manager *m, int fd, const char *name) { + _cleanup_free_ char *tuntap_name = NULL; + const char *p; + int r; + + assert(m); + assert(fd >= 0); + assert(name); + + p = startswith(name, "tuntap-"); + if (!p) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received unknown fd (%s).", name); + + if (!ifname_valid(p)) + return log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Received tuntap fd with invalid name (%s).", p); + + tuntap_name = strdup(p); + if (!tuntap_name) + return log_oom_debug(); + + r = hashmap_ensure_put(&m->tuntap_fds_by_name, &named_fd_hash_ops, tuntap_name, FD_TO_PTR(fd)); + if (r < 0) + return log_debug_errno(r, "Failed to store tuntap fd: %m"); + + TAKE_PTR(tuntap_name); + return 0; +} + +void manager_clear_unmanaged_tuntap_fds(Manager *m) { + char *name; + void *p; + + assert(m); + + while ((p = hashmap_steal_first_key_and_value(m->tuntap_fds_by_name, (void**) &name))) { + close_and_notify_warn(PTR_TO_FD(p), name); + name = mfree(name); + } +} + +static int tuntap_take_fd(NetDev *netdev) { + _cleanup_free_ char *name = NULL; + void *p; + int r; + + assert(netdev); + assert(netdev->manager); + + r = link_get_by_name(netdev->manager, netdev->ifname, NULL); + if (r < 0) + return r; + + p = hashmap_remove2(netdev->manager->tuntap_fds_by_name, netdev->ifname, (void**) &name); + if (!p) + return -ENOENT; + + log_netdev_debug(netdev, "Found file descriptor in fd store."); + return PTR_TO_FD(p); +} + static int netdev_create_tuntap(NetDev *netdev) { _cleanup_close_ int fd = -1; struct ifreq ifr = {}; @@ -39,7 +110,11 @@ static int netdev_create_tuntap(NetDev *netdev) { t = TUNTAP(netdev); assert(t); - fd = open(TUN_DEV, O_RDWR|O_CLOEXEC); + fd = TAKE_FD(t->fd); + if (fd < 0) + fd = tuntap_take_fd(netdev); + if (fd < 0) + fd = open(TUN_DEV, O_RDWR|O_CLOEXEC); if (fd < 0) return log_netdev_error_errno(netdev, errno, "Failed to open " TUN_DEV ": %m"); @@ -90,8 +165,10 @@ static int netdev_create_tuntap(NetDev *netdev) { if (ioctl(fd, TUNSETPERSIST, 1) < 0) return log_netdev_error_errno(netdev, errno, "TUNSETPERSIST failed: %m"); - if (t->keep_fd) + if (t->keep_fd) { t->fd = TAKE_FD(fd); + (void) notify_push_fdf(t->fd, "tuntap-%s", netdev->ifname); + } return 0; } @@ -106,6 +183,16 @@ static void tuntap_init(NetDev *netdev) { t->fd = -1; } +static void tuntap_drop(NetDev *netdev) { + TunTap *t; + + assert(netdev); + t = TUNTAP(netdev); + assert(t); + + t->fd = close_and_notify_warn(t->fd, netdev->ifname); +} + static void tuntap_done(NetDev *netdev) { TunTap *t; @@ -141,6 +228,7 @@ const NetDevVTable tun_vtable = { .sections = NETDEV_COMMON_SECTIONS "Tun\0", .config_verify = tuntap_verify, .init = tuntap_init, + .drop = tuntap_drop, .done = tuntap_done, .create = netdev_create_tuntap, .create_type = NETDEV_CREATE_INDEPENDENT, @@ -152,6 +240,7 @@ const NetDevVTable tap_vtable = { .sections = NETDEV_COMMON_SECTIONS "Tap\0", .config_verify = tuntap_verify, .init = tuntap_init, + .drop = tuntap_drop, .done = tuntap_done, .create = netdev_create_tuntap, .create_type = NETDEV_CREATE_INDEPENDENT, diff --git a/src/network/netdev/tuntap.h b/src/network/netdev/tuntap.h index 52f2da9fab..88e0ce5f97 100644 --- a/src/network/netdev/tuntap.h +++ b/src/network/netdev/tuntap.h @@ -21,3 +21,6 @@ DEFINE_NETDEV_CAST(TUN, TunTap); DEFINE_NETDEV_CAST(TAP, TunTap); extern const NetDevVTable tun_vtable; extern const NetDevVTable tap_vtable; + +int manager_add_tuntap_fd(Manager *m, int fd, const char *name); +void manager_clear_unmanaged_tuntap_fds(Manager *m); diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 20eb43eb09..06e9822251 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -64,6 +64,7 @@ #include "strv.h" #include "tc.h" #include "tmpfile-util.h" +#include "tuntap.h" #include "udev-util.h" #include "util.h" #include "vrf.h" diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 9b77f536c8..52c62d7297 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -8,7 +8,6 @@ #include #include -#include "sd-daemon.h" #include "sd-netlink.h" #include "alloc-util.h" @@ -18,6 +17,7 @@ #include "bus-polkit.h" #include "bus-util.h" #include "conf-parser.h" +#include "daemon-util.h" #include "def.h" #include "device-private.h" #include "device-util.h" @@ -58,6 +58,7 @@ #include "sysctl-util.h" #include "tclass.h" #include "tmpfile-util.h" +#include "tuntap.h" #include "udev-util.h" /* use 128 MB for receive socket kernel queue. */ @@ -243,22 +244,45 @@ static int manager_connect_udev(Manager *m) { return 0; } -static int systemd_netlink_fd(void) { - int n, fd, rtnl_fd = -EINVAL; +static int manager_listen_fds(Manager *m, int *ret_rtnl_fd) { + _cleanup_strv_free_ char **names = NULL; + int n, rtnl_fd = -1; - n = sd_listen_fds(true); - if (n <= 0) + assert(m); + assert(ret_rtnl_fd); + + n = sd_listen_fds_with_names(/* unset_environment = */ true, &names); + if (n < 0) + return n; + + if (strv_length(names) != (size_t) n) return -EINVAL; - for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd ++) + for (int i = 0; i < n; i++) { + int fd = i + SD_LISTEN_FDS_START; + if (sd_is_socket(fd, AF_NETLINK, SOCK_RAW, -1) > 0) { - if (rtnl_fd >= 0) - return -EINVAL; + if (rtnl_fd >= 0) { + log_debug("Received multiple netlink socket, ignoring."); + safe_close(fd); + continue; + } rtnl_fd = fd; + continue; } - return rtnl_fd; + if (manager_add_tuntap_fd(m, fd, names[i]) >= 0) + continue; + + if (m->test_mode) + safe_close(fd); + else + close_and_notify_warn(fd, names[i]); + } + + *ret_rtnl_fd = rtnl_fd; + return 0; } static int manager_connect_genl(Manager *m) { @@ -325,18 +349,21 @@ static int manager_setup_rtnl_filter(Manager *manager) { return sd_netlink_attach_filter(manager->rtnl, ELEMENTSOF(filter), filter); } -static int manager_connect_rtnl(Manager *m) { - int fd, r; +static int manager_connect_rtnl(Manager *m, int fd) { + _unused_ _cleanup_close_ int fd_close = fd; + int r; assert(m); - fd = systemd_netlink_fd(); + /* This takes input fd. */ + if (fd < 0) r = sd_netlink_open(&m->rtnl); else r = sd_netlink_open_fd(&m->rtnl, fd); if (r < 0) return r; + TAKE_FD(fd_close); /* Bump receiver buffer, but only if we are not called via socket activation, as in that * case systemd sets the receive buffer size for us, and the value in the .socket unit @@ -487,6 +514,7 @@ static int manager_set_keep_configuration(Manager *m) { } int manager_setup(Manager *m) { + _cleanup_close_ int rtnl_fd = -1; int r; assert(m); @@ -510,7 +538,11 @@ int manager_setup(Manager *m) { if (r < 0) return r; - r = manager_connect_rtnl(m); + r = manager_listen_fds(m, &rtnl_fd); + if (r < 0) + return r; + + r = manager_connect_rtnl(m, TAKE_FD(rtnl_fd)); if (r < 0) return r; @@ -600,6 +632,8 @@ Manager* manager_free(Manager *m) { m->netdevs = hashmap_free_with_destructor(m->netdevs, netdev_unref); + m->tuntap_fds_by_name = hashmap_free(m->tuntap_fds_by_name); + m->wiphy_by_name = hashmap_free(m->wiphy_by_name); m->wiphy_by_index = hashmap_free_with_destructor(m->wiphy_by_index, wiphy_free); @@ -678,6 +712,8 @@ int manager_load_config(Manager *m) { if (r < 0) return r; + manager_clear_unmanaged_tuntap_fds(m); + r = network_load(m, &m->networks); if (r < 0) return r; diff --git a/src/network/networkd-manager.h b/src/network/networkd-manager.h index 2191d0d955..e16f2b854b 100644 --- a/src/network/networkd-manager.h +++ b/src/network/networkd-manager.h @@ -100,6 +100,8 @@ struct Manager { FirewallContext *fw_ctx; OrderedSet *request_queue; + + Hashmap *tuntap_fds_by_name; }; int manager_new(Manager **ret, bool test_mode); diff --git a/units/systemd-networkd.service.in b/units/systemd-networkd.service.in index 95dd2665b2..d15129e7f0 100644 --- a/units/systemd-networkd.service.in +++ b/units/systemd-networkd.service.in @@ -25,6 +25,7 @@ CapabilityBoundingSet=CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_BROADCAST CAP_N DeviceAllow=char-* rw ExecStart=!!{{ROOTLIBEXECDIR}}/systemd-networkd ExecReload=networkctl reload +FileDescriptorStoreMax=512 LockPersonality=yes MemoryDenyWriteExecute=yes NoNewPrivileges=yes From ae014ecb3dd8a31286db1316cecf26495a3c75ce Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Aug 2022 07:46:47 +0900 Subject: [PATCH 6/6] test-network: add tests for KeepCarrier= for tuntap interfaces --- test/test-network/conf/25-tap.netdev | 1 + test/test-network/conf/25-tun.netdev | 1 + ...6-netdev-link-local-addressing-yes.network | 2 + test/test-network/systemd-networkd-tests.py | 73 ++++++++++++++++++- 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/test/test-network/conf/25-tap.netdev b/test/test-network/conf/25-tap.netdev index f52d255560..5cbfd0e5b1 100644 --- a/test/test-network/conf/25-tap.netdev +++ b/test/test-network/conf/25-tap.netdev @@ -7,3 +7,4 @@ Kind=tap MultiQueue=true PacketInfo=true VNetHeader=true +KeepCarrier=yes diff --git a/test/test-network/conf/25-tun.netdev b/test/test-network/conf/25-tun.netdev index 8ab2f5a038..a354ebb2b2 100644 --- a/test/test-network/conf/25-tun.netdev +++ b/test/test-network/conf/25-tun.netdev @@ -7,3 +7,4 @@ Kind=tun MultiQueue=true PacketInfo=true VNetHeader=true +KeepCarrier=yes diff --git a/test/test-network/conf/26-netdev-link-local-addressing-yes.network b/test/test-network/conf/26-netdev-link-local-addressing-yes.network index 8ec0190bcb..71521258d3 100644 --- a/test/test-network/conf/26-netdev-link-local-addressing-yes.network +++ b/test/test-network/conf/26-netdev-link-local-addressing-yes.network @@ -22,6 +22,8 @@ Name=nlmon99 Name=xfrm98 xfrm99 Name=vxlan98 Name=hogehogehogehogehogehoge +Name=testtun99 +Name=testtap99 [Network] LinkLocalAddressing=yes diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index f3ec22aeee..580ba384d7 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -13,6 +13,7 @@ import errno import itertools import os import pathlib +import psutil import re import shutil import signal @@ -607,6 +608,9 @@ def restart_networkd(show_logs=True): if show_logs: print(check_output('journalctl _SYSTEMD_INVOCATION_ID=' + invocation_id)) +def networkd_pid(): + return int(check_output('systemctl show --value -p MainPID systemd-networkd.service')) + def networkctl_reconfigure(*links): check_output(*networkctl_cmd, 'reconfigure', *links, env=env) @@ -1321,20 +1325,83 @@ class NetworkdNetDevTests(unittest.TestCase, Utilities): self.assertRegex(output, 'mtu 1800') def test_tuntap(self): - copy_network_unit('25-tun.netdev', '25-tap.netdev') + copy_network_unit('25-tun.netdev', '25-tap.netdev', '26-netdev-link-local-addressing-yes.network') start_networkd() - self.wait_online(['testtun99:off', 'testtap99:off'], setup_state='unmanaged') + self.wait_online(['testtun99:degraded', 'testtap99:degraded']) + + pid = networkd_pid() + name = psutil.Process(pid).name()[:15] + + output = check_output('ip -d tuntap show') + print(output) + self.assertRegex(output, f'(?m)testtap99: tap pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:{name}\({pid}\)systemd\(1\)$') + self.assertRegex(output, f'(?m)testtun99: tun pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:{name}\({pid}\)systemd\(1\)$') output = check_output('ip -d link show testtun99') print(output) # Old ip command does not support IFF_ flags self.assertRegex(output, 'tun (type tun pi on vnet_hdr on multi_queue|addrgenmode) ') + self.assertIn('UP,LOWER_UP', output) output = check_output('ip -d link show testtap99') print(output) - # Old ip command does not support IFF_ flags self.assertRegex(output, 'tun (type tap pi on vnet_hdr on multi_queue|addrgenmode) ') + self.assertIn('UP,LOWER_UP', output) + + remove_network_unit('26-netdev-link-local-addressing-yes.network') + + restart_networkd() + self.wait_online(['testtun99:degraded', 'testtap99:degraded'], setup_state='unmanaged') + + pid = networkd_pid() + name = psutil.Process(pid).name()[:15] + + output = check_output('ip -d tuntap show') + print(output) + self.assertRegex(output, f'(?m)testtap99: tap pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:{name}\({pid}\)systemd\(1\)$') + self.assertRegex(output, f'(?m)testtun99: tun pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:{name}\({pid}\)systemd\(1\)$') + + output = check_output('ip -d link show testtun99') + print(output) + self.assertRegex(output, 'tun (type tun pi on vnet_hdr on multi_queue|addrgenmode) ') + self.assertIn('UP,LOWER_UP', output) + + output = check_output('ip -d link show testtap99') + print(output) + self.assertRegex(output, 'tun (type tap pi on vnet_hdr on multi_queue|addrgenmode) ') + self.assertIn('UP,LOWER_UP', output) + + clear_network_units() + restart_networkd() + self.wait_online(['testtun99:off', 'testtap99:off'], setup_state='unmanaged') + + output = check_output('ip -d tuntap show') + print(output) + self.assertRegex(output, f'(?m)testtap99: tap pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:$') + self.assertRegex(output, f'(?m)testtun99: tun pi (multi_queue |)vnet_hdr persist filter *(0x100|)\n\tAttached to processes:$') + + for i in range(10): + if i != 0: + time.sleep(1) + output = check_output('ip -d link show testtun99') + print(output) + self.assertRegex(output, 'tun (type tun pi on vnet_hdr on multi_queue|addrgenmode) ') + if 'NO-CARRIER' in output: + break + else: + self.fail() + + for i in range(10): + if i != 0: + time.sleep(1) + output = check_output('ip -d link show testtap99') + print(output) + self.assertRegex(output, 'tun (type tap pi on vnet_hdr on multi_queue|addrgenmode) ') + if 'NO-CARRIER' in output: + break + else: + self.fail() @expectedFailureIfModuleIsNotAvailable('vrf') def test_vrf(self):