From abec66762abbe28461959f26db1cd95187c66252 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 5 Mar 2019 10:20:25 +0100 Subject: [PATCH] platform: add async sysctl set function Add a function to asynchronously set sysctl values. --- src/platform/nm-linux-platform.c | 227 +++++++++++++++++++++++++++++-- src/platform/nm-platform.c | 34 +++++ src/platform/nm-platform.h | 18 +++ src/platform/tests/test-link.c | 88 ++++++++++++ 4 files changed, 355 insertions(+), 12 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index b3f979185e..8eb30d86f2 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4375,6 +4375,14 @@ _genl_sock (NMLinuxPlatform *platform) } \ } G_STMT_END +/*****************************************************************************/ + +/* core sysctl-set functions can be called from a non-main thread. + * Hence, we require locking from nm-logging. Indicate that by + * setting NM_THREAD_SAFE_ON_MAIN_THREAD to zero. */ +#undef NM_THREAD_SAFE_ON_MAIN_THREAD +#define NM_THREAD_SAFE_ON_MAIN_THREAD 0 + static void _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) { @@ -4409,9 +4417,12 @@ _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, c } G_STMT_END static gboolean -sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) +sysctl_set_internal (NMPlatform *platform, + const char *pathid, + int dirfd, + const char *path, + const char *value) { - nm_auto_pop_netns NMPNetns *netns = NULL; int fd, tries; gssize nwrote; gssize len; @@ -4419,17 +4430,7 @@ sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *pat gs_free char *actual_free = NULL; int errsv; - g_return_val_if_fail (path != NULL, FALSE); - g_return_val_if_fail (value != NULL, FALSE); - - ASSERT_SYSCTL_ARGS (pathid, dirfd, path); - if (dirfd < 0) { - if (!nm_platform_netns_push (platform, &netns)) { - errno = ENETDOWN; - return FALSE; - } - pathid = path; fd = open (path, O_WRONLY | O_TRUNC | O_CLOEXEC); @@ -4529,6 +4530,207 @@ sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *pat return TRUE; } +#undef NM_THREAD_SAFE_ON_MAIN_THREAD +#define NM_THREAD_SAFE_ON_MAIN_THREAD 1 + +/*****************************************************************************/ + +static gboolean +sysctl_set (NMPlatform *platform, + const char *pathid, + int dirfd, + const char *path, + const char *value) +{ + nm_auto_pop_netns NMPNetns *netns = NULL; + + g_return_val_if_fail (path, FALSE); + g_return_val_if_fail (value, FALSE); + + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); + + if ( dirfd < 0 + && !nm_platform_netns_push (platform, &netns)) { + errno = ENETDOWN; + return FALSE; + } + + return sysctl_set_internal (platform, pathid, dirfd, path, value); +} + +typedef struct { + NMPlatform *platform; + char *pathid; + int dirfd; + char *path; + char **values; + GCancellable *cancellable; + NMPlatformAsyncCallback callback; + gpointer callback_data; +} SysctlAsyncInfo; + +static void +sysctl_async_info_free (SysctlAsyncInfo *info) +{ + g_object_unref (info->platform); + g_free (info->pathid); + if (info->dirfd >= 0) + nm_close (info->dirfd); + g_free (info->path); + g_strfreev (info->values); + g_object_unref (info->cancellable); + g_slice_free (SysctlAsyncInfo, info); +} + +static void +sysctl_async_cb (GObject *object, + GAsyncResult *res, + gpointer user_data) +{ + NMPlatform *platform; + GTask *task = G_TASK (res); + SysctlAsyncInfo *info; + gs_free_error GError *error = NULL; + gs_free char *values_str = NULL; + + info = g_task_get_task_data (task); + + if (g_task_propagate_boolean (task, &error)) { + platform = info->platform; + _LOGD ("sysctl: successfully set-async '%s' to values '%s'", + info->pathid ?: info->path, + (values_str = g_strjoinv (", ", info->values))); + } + + if (info->callback) + info->callback (error, info->callback_data); +} + +static void +sysctl_async_thread_fn (GTask *task, + gpointer source_object, + gpointer task_data, + GCancellable *cancellable) +{ + nm_auto_pop_netns NMPNetns *netns = NULL; + SysctlAsyncInfo *info = task_data; + GError *error = NULL; + char **value; + + if (g_task_return_error_if_cancelled (task)) + return; + + if ( info->dirfd < 0 + && !nm_platform_netns_push (info->platform, &netns)) { + g_set_error_literal (&error, + NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "sysctl: failed changing namespace"); + g_task_return_error (task, error); + return; + } + + for (value = info->values; *value; value++) { + if (!sysctl_set_internal (info->platform, + info->pathid, + info->dirfd, + info->path, + *value)) { + g_set_error (&error, + NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "sysctl: failed setting '%s' to value '%s': %s", + info->pathid ?: info->path, + *value, + nm_strerror_native (errno)); + g_task_return_error (task, error); + return; + } + if (g_task_return_error_if_cancelled (task)) + return; + } + g_task_return_boolean (task, TRUE); +} + +static void +sysctl_set_async_return_idle (gpointer user_data, + GCancellable *cancellable) +{ + gs_unref_object NMPlatform *platform = NULL; + gs_free_error GError *cancelled_error = NULL; + gs_free_error GError *error = NULL; + NMPlatformAsyncCallback callback; + gpointer callback_data; + + nm_utils_user_data_unpack (user_data, &platform, &callback, &callback_data, &error); + g_cancellable_set_error_if_cancelled (cancellable, &cancelled_error); + callback (cancelled_error ?: error, callback_data); +} + +static void +sysctl_set_async (NMPlatform *platform, + const char *pathid, + int dirfd, + const char *path, + const char *const *values, + NMPlatformAsyncCallback callback, + gpointer data, + GCancellable *cancellable) +{ + SysctlAsyncInfo *info; + GTask *task; + int dirfd_dup, errsv; + gpointer packed; + GError *error = NULL; + + g_return_if_fail (platform); + g_return_if_fail (path); + g_return_if_fail (values && values[0]); + g_return_if_fail (cancellable); + g_return_if_fail (!data || callback); + + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); + + if (dirfd >= 0) { + dirfd_dup = fcntl (dirfd, F_DUPFD_CLOEXEC, 0); + if (dirfd_dup < 0) { + if (!callback) + return; + errsv = errno; + g_set_error (&error, + NM_UTILS_ERROR, + NM_UTILS_ERROR_UNKNOWN, + "sysctl: failure duplicating directory fd: %s", + nm_strerror_native (errsv)); + packed = nm_utils_user_data_pack (g_object_ref (platform), + callback, + data, + error); + nm_utils_invoke_on_idle (sysctl_set_async_return_idle, + packed, + cancellable); + return; + } + } else + dirfd_dup = -1; + + info = g_slice_new0 (SysctlAsyncInfo); + info->platform = g_object_ref (platform); + info->pathid = g_strdup (pathid); + info->dirfd = dirfd_dup; + info->path = g_strdup (path); + info->values = g_strdupv ((char **) values); + info->callback = callback; + info->callback_data = data; + info->cancellable = g_object_ref (cancellable); + + task = g_task_new (platform, cancellable, sysctl_async_cb, NULL); + g_task_set_task_data (task, info, (GDestroyNotify) sysctl_async_info_free); + g_task_set_return_on_cancel (task, FALSE); + g_task_run_in_thread (task, sysctl_async_thread_fn); + g_object_unref (task); +} + static GSList *sysctl_clear_cache_list; void @@ -8997,6 +9199,7 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass) object_class->finalize = finalize; platform_class->sysctl_set = sysctl_set; + platform_class->sysctl_set_async = sysctl_set_async; platform_class->sysctl_get = sysctl_get; platform_class->link_add = link_add; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 27bdb92f19..de5ccdd8bd 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -481,6 +481,40 @@ nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const c return klass->sysctl_set (self, pathid, dirfd, path, value); } +/** + * nm_platform_sysctl_set_async: + * @self: platform instance + * @pathid: if @dirfd is present, this must be the full path that is looked up + * @dirfd: optional file descriptor for parent directory for openat() + * @path: absolute option path + * @values: NULL-terminated array of strings to be written + * @callback: function called on termination + * @data: data passed to callback function + * @cancellable: to cancel the operation + * + * This function is intended to be used for writing values to sysctl-style + * virtual runtime configuration files. This includes not only /proc/sys + * but also for example /sys/class. The function does not block and returns + * immediately. The callback is always invoked, and asynchronously. The file + * is closed after writing each value and reopened to write the next one so + * that the function can be used safely on all /proc and /sys files, + * independently of how /proc/sys/kernel/sysctl_writes_strict is configured. + */ +void nm_platform_sysctl_set_async (NMPlatform *self, + const char *pathid, + int dirfd, + const char *path, + const char *const *values, + NMPlatformAsyncCallback callback, + gpointer data, + GCancellable *cancellable) +{ + _CHECK_SELF_VOID (self, klass); + + klass->sysctl_set_async (self, pathid, dirfd, path, values, callback, data, cancellable); +} + + gboolean nm_platform_sysctl_ip_conf_set_ipv6_hop_limit_safe (NMPlatform *self, const char *iface, diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 2729680ec6..49d6da5686 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -895,6 +895,8 @@ typedef enum { } NMPlatformWireGuardChangePeerFlags; +typedef void (*NMPlatformAsyncCallback) (GError *error, gpointer user_data); + /*****************************************************************************/ typedef enum { @@ -954,6 +956,14 @@ typedef struct { GObjectClass parent; gboolean (*sysctl_set) (NMPlatform *self, const char *pathid, int dirfd, const char *path, const char *value); + void (*sysctl_set_async) (NMPlatform *self, + const char *pathid, + int dirfd, + const char *path, + const char *const *values, + NMPlatformAsyncCallback callback, + gpointer data, + GCancellable *cancellable); char * (*sysctl_get) (NMPlatform *self, const char *pathid, int dirfd, const char *path); void (*refresh_all) (NMPlatform *self, NMPObjectType obj_type); @@ -1289,6 +1299,14 @@ const char *nm_link_type_to_string (NMLinkType link_type); int nm_platform_sysctl_open_netdir (NMPlatform *self, int ifindex, char *out_ifname); gboolean nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const char *path, const char *value); +void nm_platform_sysctl_set_async (NMPlatform *self, + const char *pathid, + int dirfd, + const char *path, + const char *const *values, + NMPlatformAsyncCallback callback, + gpointer data, + GCancellable *cancellable); char *nm_platform_sysctl_get (NMPlatform *self, const char *pathid, int dirfd, const char *path); gint32 nm_platform_sysctl_get_int32 (NMPlatform *self, const char *pathid, int dirfd, const char *path, gint32 fallback); gint64 nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *pathid, int dirfd, const char *path, guint base, gint64 min, gint64 max, gint64 fallback); diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index a2e3a6efb1..bff030bfd3 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -3015,6 +3015,92 @@ test_sysctl_netns_switch (void) nmtstp_link_delete (PL, FALSE, ifindex, NULL, TRUE); } +static void +sysctl_set_async_cb_assert_success (GError *error, gpointer data) +{ + g_assert_no_error (error); + g_main_loop_quit (data); +} + +static void +sysctl_set_async_cb_assert_failure (GError *error, gpointer data) +{ + g_assert (error); + g_main_loop_quit (data); +} + +static void +test_sysctl_set_async (void) +{ + NMPlatform *const PL = NM_PLATFORM_GET; + const char *const IFNAME = "nm-dummy-0"; + const char *const PATH = "/proc/sys/net/ipv4/conf/nm-dummy-0/rp_filter"; + gs_free GMainLoop *loop = NULL; + gs_unref_object GCancellable *cancellable = NULL; + int ifindex; + + ifindex = nmtstp_link_dummy_add (PL, -1, IFNAME)->ifindex; + loop = g_main_loop_new (NULL, FALSE); + cancellable = g_cancellable_new (); + + nm_platform_sysctl_set_async (PL, + NMP_SYSCTL_PATHID_ABSOLUTE (PATH), + (const char *[]) { "2", NULL}, + sysctl_set_async_cb_assert_success, + loop, + cancellable); + + if (!nmtst_main_loop_run (loop, 1000)) + g_assert_not_reached (); + + g_assert_cmpint (nm_platform_sysctl_get_int32 (PL, NMP_SYSCTL_PATHID_ABSOLUTE (PATH), -1), + ==, + 2); + + nm_platform_sysctl_set_async (PL, + NMP_SYSCTL_PATHID_ABSOLUTE (PATH), + (const char *[]) { "2", "0", "1", "0", "1", NULL}, + sysctl_set_async_cb_assert_success, + loop, + cancellable); + + if (!nmtst_main_loop_run (loop, 2000)) + g_assert_not_reached (); + + g_assert_cmpint (nm_platform_sysctl_get_int32 (PL, NMP_SYSCTL_PATHID_ABSOLUTE (PATH), -1), + ==, + 1); + + nmtstp_link_delete (NULL, -1, ifindex, IFNAME, TRUE); +} + +static void +test_sysctl_set_async_fail (void) +{ + NMPlatform *const PL = NM_PLATFORM_GET; + const char *const IFNAME = "nm-dummy-0"; + const char *const PATH = "/proc/sys/net/ipv4/conf/nm-dummy-0/does-not-exist"; + gs_free GMainLoop *loop = NULL; + gs_unref_object GCancellable *cancellable = NULL; + int ifindex; + + ifindex = nmtstp_link_dummy_add (PL, -1, IFNAME)->ifindex; + loop = g_main_loop_new (NULL, FALSE); + cancellable = g_cancellable_new (); + + nm_platform_sysctl_set_async (PL, + NMP_SYSCTL_PATHID_ABSOLUTE (PATH), + (const char *[]) { "2", NULL}, + sysctl_set_async_cb_assert_failure, + loop, + cancellable); + + if (!nmtst_main_loop_run (loop, 1000)) + g_assert_not_reached (); + + nmtstp_link_delete (NULL, -1, ifindex, IFNAME, TRUE); +} + /*****************************************************************************/ static gpointer @@ -3238,6 +3324,8 @@ _nmtstp_setup_tests (void) g_test_add_func ("/general/sysctl/rename", test_sysctl_rename); g_test_add_func ("/general/sysctl/netns-switch", test_sysctl_netns_switch); + g_test_add_func ("/general/sysctl/set-async", test_sysctl_set_async); + g_test_add_func ("/general/sysctl/set-async-fail", test_sysctl_set_async_fail); g_test_add_func ("/link/ethtool/features/get", test_ethtool_features_get); }