From 8c63924c8d1eeb5a173669b8b06230bfe721f139 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 02:48:25 +0900 Subject: [PATCH 01/10] network: ignore error on increasing netlink receive buffer size --- src/network/networkd-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 6187434466..1f15d58856 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1347,7 +1347,7 @@ static int manager_connect_genl(Manager *m) { r = sd_netlink_inc_rcvbuf(m->genl, RCVBUF_SIZE); if (r < 0) - return r; + log_warning_errno(r, "Failed to increase receive buffer size for general netlink socket, ignoring: %m"); r = sd_netlink_attach_event(m->genl, m->event, 0); if (r < 0) @@ -1371,7 +1371,7 @@ static int manager_connect_rtnl(Manager *m) { r = sd_netlink_inc_rcvbuf(m->rtnl, RCVBUF_SIZE); if (r < 0) - return r; + log_warning_errno(r, "Failed to increase receive buffer size for rtnl socket, ignoring: %m"); r = sd_netlink_attach_event(m->rtnl, m->event, 0); if (r < 0) From 1263c85ef32ea35969748cd4304cd1a51d19e8d1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 00:07:50 +0900 Subject: [PATCH 02/10] util: refuse to set too large value for socket buffer size --- src/basic/socket-util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index fb12659857..3230d76b02 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -621,6 +621,9 @@ int fd_inc_sndbuf(int fd, size_t n) { int r, value; socklen_t l = sizeof(value); + if (n > INT_MAX) + return -ERANGE; + r = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &value, &l); if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) return 0; @@ -640,6 +643,9 @@ int fd_inc_rcvbuf(int fd, size_t n) { int r, value; socklen_t l = sizeof(value); + if (n > INT_MAX) + return -ERANGE; + r = getsockopt(fd, SOL_SOCKET, SO_RCVBUF, &value, &l); if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) return 0; From b92f350789e33942be0cf85af22a580c1fd483d6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 00:11:14 +0900 Subject: [PATCH 03/10] util: try to set with SO_{RCV,SND}BUFFORCE when requested size is larger than the kernel limit The commit 10ce2e0681ac16e7bb3619b7bb1a72a6f98a2f2c inverts the order of SO_{RCV,SND}BUFFORCE and SO_{RCV,SND}BUF. However, setting buffer size with SO_{RCV,SND}BUF does not fail even if the requested size is larger than the kernel limit. Hence, SO_{RCV,SND}BUFFORCE will not use anymore and the buffer size is always limited by the kernel limit even if we have the priviledge to ignore the limit. This makes the buffer size is checked after configuring it with SO_{RCV,SND}BUF, and if it is still not sufficient, then try to set it with FORCE command. With this commit, if we have enough priviledge, the requested buffer size is correctly set. Hopefully fixes #14417. --- src/basic/socket-util.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 3230d76b02..da207377e0 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -628,13 +628,21 @@ int fd_inc_sndbuf(int fd, size_t n) { if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) return 0; - /* If we have the privileges we will ignore the kernel limit. */ + /* First, try to set the buffer size with SO_SNDBUF. */ + r = setsockopt_int(fd, SOL_SOCKET, SO_SNDBUF, n); + if (r < 0) + return r; - if (setsockopt_int(fd, SOL_SOCKET, SO_SNDBUF, n) < 0) { - r = setsockopt_int(fd, SOL_SOCKET, SO_SNDBUFFORCE, n); - if (r < 0) - return r; - } + /* SO_SNDBUF above may set to the kernel limit, instead of the requested size. + * So, we need to check the actual buffer size here. */ + r = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &value, &l); + if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + return 1; + + /* If we have the privileges we will ignore the kernel limit. */ + r = setsockopt_int(fd, SOL_SOCKET, SO_SNDBUFFORCE, n); + if (r < 0) + return r; return 1; } @@ -650,13 +658,21 @@ int fd_inc_rcvbuf(int fd, size_t n) { if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) return 0; - /* If we have the privileges we will ignore the kernel limit. */ + /* First, try to set the buffer size with SO_RCVBUF. */ + r = setsockopt_int(fd, SOL_SOCKET, SO_RCVBUF, n); + if (r < 0) + return r; - if (setsockopt_int(fd, SOL_SOCKET, SO_RCVBUF, n) < 0) { - r = setsockopt_int(fd, SOL_SOCKET, SO_RCVBUFFORCE, n); - if (r < 0) - return r; - } + /* SO_RCVBUF above may set to the kernel limit, instead of the requested size. + * So, we need to check the actual buffer size here. */ + r = getsockopt(fd, SOL_SOCKET, SO_RCVBUF, &value, &l); + if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + return 1; + + /* If we have the privileges we will ignore the kernel limit. */ + r = setsockopt_int(fd, SOL_SOCKET, SO_RCVBUFFORCE, n); + if (r < 0) + return r; return 1; } From d9d9b2a0ae2befb645ef3aa420831423bcb9f58f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 01:12:38 +0900 Subject: [PATCH 04/10] util: introduce fd_set_{snd,rcv}buf() --- src/basic/socket-util.c | 12 ++++++------ src/basic/socket-util.h | 10 ++++++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index da207377e0..4b3a4f206c 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -617,7 +617,7 @@ bool sockaddr_equal(const union sockaddr_union *a, const union sockaddr_union *b return false; } -int fd_inc_sndbuf(int fd, size_t n) { +int fd_set_sndbuf(int fd, size_t n, bool increase) { int r, value; socklen_t l = sizeof(value); @@ -625,7 +625,7 @@ int fd_inc_sndbuf(int fd, size_t n) { return -ERANGE; r = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &value, &l); - if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + if (r >= 0 && l == sizeof(value) && increase ? (size_t) value >= n*2 : (size_t) value == n*2) return 0; /* First, try to set the buffer size with SO_SNDBUF. */ @@ -636,7 +636,7 @@ int fd_inc_sndbuf(int fd, size_t n) { /* SO_SNDBUF above may set to the kernel limit, instead of the requested size. * So, we need to check the actual buffer size here. */ r = getsockopt(fd, SOL_SOCKET, SO_SNDBUF, &value, &l); - if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + if (r >= 0 && l == sizeof(value) && increase ? (size_t) value >= n*2 : (size_t) value == n*2) return 1; /* If we have the privileges we will ignore the kernel limit. */ @@ -647,7 +647,7 @@ int fd_inc_sndbuf(int fd, size_t n) { return 1; } -int fd_inc_rcvbuf(int fd, size_t n) { +int fd_set_rcvbuf(int fd, size_t n, bool increase) { int r, value; socklen_t l = sizeof(value); @@ -655,7 +655,7 @@ int fd_inc_rcvbuf(int fd, size_t n) { return -ERANGE; r = getsockopt(fd, SOL_SOCKET, SO_RCVBUF, &value, &l); - if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + if (r >= 0 && l == sizeof(value) && increase ? (size_t) value >= n*2 : (size_t) value == n*2) return 0; /* First, try to set the buffer size with SO_RCVBUF. */ @@ -666,7 +666,7 @@ int fd_inc_rcvbuf(int fd, size_t n) { /* SO_RCVBUF above may set to the kernel limit, instead of the requested size. * So, we need to check the actual buffer size here. */ r = getsockopt(fd, SOL_SOCKET, SO_RCVBUF, &value, &l); - if (r >= 0 && l == sizeof(value) && (size_t) value >= n*2) + if (r >= 0 && l == sizeof(value) && increase ? (size_t) value >= n*2 : (size_t) value == n*2) return 1; /* If we have the privileges we will ignore the kernel limit. */ diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 5610a99e53..6a320a976f 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -118,8 +118,14 @@ int netlink_family_from_string(const char *s) _pure_; bool sockaddr_equal(const union sockaddr_union *a, const union sockaddr_union *b); -int fd_inc_sndbuf(int fd, size_t n); -int fd_inc_rcvbuf(int fd, size_t n); +int fd_set_sndbuf(int fd, size_t n, bool increase); +static inline int fd_inc_sndbuf(int fd, size_t n) { + return fd_set_sndbuf(fd, n, true); +} +int fd_set_rcvbuf(int fd, size_t n, bool increase); +static inline int fd_inc_rcvbuf(int fd, size_t n) { + return fd_set_rcvbuf(fd, n, true); +} int ip_tos_to_string_alloc(int i, char **s); int ip_tos_from_string(const char *s); From 2807a79424a0be807bc904ca145f1db03ac7fbb0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 00:22:21 +0900 Subject: [PATCH 05/10] sd-device-monitor: use fd_set_rcvbuf() --- src/libsystemd/sd-device/device-monitor.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c index cb6f1a70f8..43646e1880 100644 --- a/src/libsystemd/sd-device/device-monitor.c +++ b/src/libsystemd/sd-device/device-monitor.c @@ -91,18 +91,9 @@ int device_monitor_allow_unicast_sender(sd_device_monitor *m, sd_device_monitor } _public_ int sd_device_monitor_set_receive_buffer_size(sd_device_monitor *m, size_t size) { - int r, n = (int) size; - assert_return(m, -EINVAL); - assert_return((size_t) n == size, -EINVAL); - if (setsockopt_int(m->sock, SOL_SOCKET, SO_RCVBUFFORCE, n) < 0) { - r = setsockopt_int(m->sock, SOL_SOCKET, SO_RCVBUF, n); - if (r < 0) - return r; - } - - return 0; + return fd_set_rcvbuf(m->sock, size, false); } int device_monitor_disconnect(sd_device_monitor *m) { From ded71ab3bc580852a378b6cfac5e8d4053697b75 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 00:25:23 +0900 Subject: [PATCH 06/10] core/socket: use fd_set_{rcv,snd}buf() --- src/core/socket.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index 855b354dd8..a21584ea48 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1058,20 +1058,15 @@ static void socket_apply_socket_options(Socket *s, int fd) { } if (s->receive_buffer > 0) { - /* We first try with SO_RCVBUFFORCE, in case we have the perms for that */ - if (setsockopt_int(fd, SOL_SOCKET, SO_RCVBUFFORCE, s->receive_buffer) < 0) { - r = setsockopt_int(fd, SOL_SOCKET, SO_RCVBUF, s->receive_buffer); - if (r < 0) - log_unit_warning_errno(UNIT(s), r, "SO_RCVBUF failed: %m"); - } + r = fd_set_rcvbuf(fd, s->receive_buffer, false); + if (r < 0) + log_unit_warning_errno(UNIT(s), r, "SO_RCVBUF/SO_RCVBUFFORCE failed: %m"); } if (s->send_buffer > 0) { - if (setsockopt_int(fd, SOL_SOCKET, SO_SNDBUFFORCE, s->send_buffer) < 0) { - r = setsockopt_int(fd, SOL_SOCKET, SO_SNDBUF, s->send_buffer); - if (r < 0) - log_unit_warning_errno(UNIT(s), r, "SO_SNDBUF failed: %m"); - } + r = fd_set_sndbuf(fd, s->receive_buffer, false); + if (r < 0) + log_unit_warning_errno(UNIT(s), r, "SO_SNDBUF/SO_SNDBUFFORCE failed: %m"); } if (s->mark >= 0) { From e13af7bdb6236d91498a6a5936fc75237ebfa01d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 00:33:11 +0900 Subject: [PATCH 07/10] network: honor the buffer size specified in networkd.socket --- src/network/networkd-manager.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 1f15d58856..5ed9cce1ca 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -1369,9 +1369,14 @@ static int manager_connect_rtnl(Manager *m) { if (r < 0) return r; - r = sd_netlink_inc_rcvbuf(m->rtnl, RCVBUF_SIZE); - if (r < 0) - log_warning_errno(r, "Failed to increase receive buffer size for rtnl socket, ignoring: %m"); + /* 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 + * should take full effect. */ + if (fd < 0) { + r = sd_netlink_inc_rcvbuf(m->rtnl, RCVBUF_SIZE); + if (r < 0) + log_warning_errno(r, "Failed to increase receive buffer size for rtnl socket, ignoring: %m"); + } r = sd_netlink_attach_event(m->rtnl, m->event, 0); if (r < 0) From d31f33e3c9f6ea3bdc873ee52f4398edbec74527 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Sep 2020 23:26:28 +0900 Subject: [PATCH 08/10] network: do not start device monitor if /sys is read-only Follow-up for bf331d87171b7750d1c72ab0b140a240c0cf32c3. --- src/network/networkd-manager.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 5ed9cce1ca..bb1df8cca6 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -36,11 +36,11 @@ #include "path-util.h" #include "set.h" #include "signal-util.h" +#include "stat-util.h" #include "strv.h" #include "sysctl-util.h" #include "tmpfile-util.h" #include "udev-util.h" -#include "virt.h" /* use 128 MB for receive socket kernel queue. */ #define RCVBUF_SIZE (128*1024*1024) @@ -261,10 +261,9 @@ static int manager_udev_process_link(sd_device_monitor *monitor, sd_device *devi static int manager_connect_udev(Manager *m) { int r; - /* udev does not initialize devices inside containers, - * so we rely on them being already initialized before - * entering the container */ - if (detect_container() > 0) + /* udev does not initialize devices inside containers, so we rely on them being already + * initialized before entering the container. */ + if (path_is_read_only_fs("/sys") > 0) return 0; r = sd_device_monitor_new(&m->device_monitor); From a725efb08b068055dcbf163426eef6b04a23aca0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 8 Sep 2020 23:28:22 +0900 Subject: [PATCH 09/10] network: increase receive buffer size for device monitor If networkd creates huge amount of netdevs, then the buffer of device monitor becomes easily flowed. Hopefully fixes #16865. --- src/network/networkd-manager.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index bb1df8cca6..344fc08c5b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -270,6 +270,10 @@ static int manager_connect_udev(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to initialize device monitor: %m"); + r = sd_device_monitor_set_receive_buffer_size(m->device_monitor, RCVBUF_SIZE); + if (r < 0) + log_warning_errno(r, "Failed to increase buffer size for device monitor, ignoring: %m"); + r = sd_device_monitor_filter_add_match_subsystem_devtype(m->device_monitor, "net", NULL); if (r < 0) return log_error_errno(r, "Could not add device monitor filter: %m"); From e77f52e5f951e6b51f3b6eeee69f1f8bb965f4f3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 9 Sep 2020 06:46:54 +0900 Subject: [PATCH 10/10] udev: warn if failed to set buffer size for device monitor --- src/udev/udevd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 48a7774fc9..22dad2acaf 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1686,8 +1686,11 @@ static int manager_new(Manager **ret, int fd_ctrl, int fd_uevent, const char *cg /* 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 * should take full effect. */ - if (fd_uevent < 0) - (void) sd_device_monitor_set_receive_buffer_size(manager->monitor, 128 * 1024 * 1024); + if (fd_uevent < 0) { + r = sd_device_monitor_set_receive_buffer_size(manager->monitor, 128 * 1024 * 1024); + if (r < 0) + log_warning_errno(r, "Failed to set receive buffer size for device monitor, ignoring: %m"); + } r = device_monitor_enable_receiving(manager->monitor); if (r < 0)