Merge pull request #17341 from yuwata/sd-dhcp-client-fix-eexist-issue-16964

sd-dhcp: make sd_dhcp_client_set_request_option() not return -EEXIST
network: start dynamic addressing clients like DHCP after setting netdevs
2020-10-14 18:55:39 +09:00 · 2020-10-14 10:52:02 +02:00 · 2020-10-14 10:31:59 +02:00 · 2020-10-14 13:02:46 +09:00 · 2020-10-14 12:54:55 +09:00
7 changed files with 94 additions and 74 deletions
--- a/4
+++ b/4
@ -29,10 +29,6 @@ Features:
 * Add service setting to run a service within the specified VRF. i.e. do the
  equivalent of "ip vrf exec".

-* systemd-analyze syscall-filter should show a list of syscalls listed in
-  @known but not in other groups (at least at debug level), since they are
-  candidates to be added to them.
-
 * export action of device object on sd-device, so that monitor becomes useful

 * add root=tmpfs that mounts a tmpfs to /sysroot (to be used in combination
--- a/src/analyze/analyze.c
+++ b/src/analyze/analyze.c
@ -1685,7 +1685,7 @@ static int load_kernel_syscalls(Set **ret) {
        return 0;
 }

-static void kernel_syscalls_remove(Set *s, const SyscallFilterSet *set) {
+static void syscall_set_remove(Set *s, const SyscallFilterSet *set) {
        const char *syscall;

        NULSTR_FOREACH(syscall, set->value) {
@ -1716,9 +1716,14 @@ static int dump_syscall_filters(int argc, char *argv[], void *userdata) {
        (void) pager_open(arg_pager_flags);

        if (strv_isempty(strv_skip(argv, 1))) {
-                _cleanup_set_free_ Set *kernel = NULL;
+                _cleanup_set_free_ Set *kernel = NULL, *known = NULL;
+                const char *sys;
                int i, k;

+                NULSTR_FOREACH(sys, syscall_filter_sets[SYSCALL_FILTER_SET_KNOWN].value)
+                        if (set_put_strdup(&known, sys) < 0)
+                                return log_oom();
+
                k = load_kernel_syscalls(&kernel);

                for (i = 0; i < _SYSCALL_FILTER_SET_MAX; i++) {
@ -1727,10 +1732,30 @@ static int dump_syscall_filters(int argc, char *argv[], void *userdata) {
                                puts("");

                        dump_syscall_filter(set);
-                        kernel_syscalls_remove(kernel, set);
+                        syscall_set_remove(kernel, set);
+                        if (i != SYSCALL_FILTER_SET_KNOWN)
+                                syscall_set_remove(known, set);
                        first = false;
                }

+                if (!set_isempty(known)) {
+                        _cleanup_free_ char **l = NULL;
+                        char **syscall;
+
+                        printf("\n"
+                               "# %sUngrouped System Calls%s (known but not included in any of the groups except @known):\n",
+                               ansi_highlight(), ansi_normal());
+
+                        l = set_get_strv(known);
+                        if (!l)
+                                return log_oom();
+
+                        strv_sort(l);
+
+                        STRV_FOREACH(syscall, l)
+                                printf("#   %s\n", *syscall);
+                }
+
                if (k < 0) {
                        fputc('\n', stdout);
                        fflush(stdout);
--- a/src/basic/macro.h
+++ b/src/basic/macro.h
@ -444,6 +444,9 @@ static inline int __coverity_check_and_return__(int condition) {
 #define PTR_TO_ULONG(p) ((unsigned long) ((uintptr_t) (p)))
 #define ULONG_TO_PTR(u) ((void *) ((uintptr_t) (u)))

+#define PTR_TO_UINT8(p) ((uint8_t) ((uintptr_t) (p)))
+#define UINT8_TO_PTR(u) ((void *) ((uintptr_t) (u)))
+
 #define PTR_TO_INT32(p) ((int32_t) ((intptr_t) (p)))
 #define INT32_TO_PTR(u) ((void *) ((intptr_t) (u)))
 #define PTR_TO_UINT32(p) ((uint32_t) ((uintptr_t) (p)))
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@ -25,6 +25,8 @@
 #include "io-util.h"
 #include "memory-util.h"
 #include "random-util.h"
+#include "set.h"
+#include "sort-util.h"
 #include "string-util.h"
 #include "strv.h"
 #include "utf8.h"
@ -75,9 +77,7 @@ struct sd_dhcp_client {
        union sockaddr_union link;
        sd_event_source *receive_message;
        bool request_broadcast;
-        uint8_t *req_opts;
-        size_t req_opts_allocated;
-        size_t req_opts_size;
+        Set *req_opts;
        bool anonymize;
        be32_t last_addr;
        uint8_t mac_addr[MAX_MAC_ADDR_LEN];
@ -230,8 +230,6 @@ int sd_dhcp_client_set_request_broadcast(sd_dhcp_client *client, int broadcast)
 }

 int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) {
-        size_t i;
-
        assert_return(client, -EINVAL);
        assert_return(IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED), -EBUSY);

@ -248,17 +246,7 @@ int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) {
                break;
        }

-        for (i = 0; i < client->req_opts_size; i++)
-                if (client->req_opts[i] == option)
-                        return -EEXIST;
-
-        if (!GREEDY_REALLOC(client->req_opts, client->req_opts_allocated,
-                            client->req_opts_size + 1))
-                return -ENOMEM;
-
-        client->req_opts[client->req_opts_size++] = option;
-
-        return 0;
+        return set_ensure_put(&client->req_opts, NULL, UINT8_TO_PTR(option));
 }

 int sd_dhcp_client_set_request_address(
@ -725,6 +713,10 @@ static void client_stop(sd_dhcp_client *client, int error) {
        client_initialize(client);
 }

+static int cmp_uint8(const uint8_t *a, const uint8_t *b) {
+        return CMP(*a, *b);
+}
+
 static int client_message_init(
                sd_dhcp_client *client,
                DHCPPacket **ret,
@ -835,10 +827,26 @@ static int client_message_init(
           MAY contain the Parameter Request List option. */
        /* NOTE: in case that there would be an option to do not send
         * any PRL at all, the size should be checked before sending */
-        if (client->req_opts_size > 0 && type != DHCP_RELEASE) {
+        if (!set_isempty(client->req_opts) && type != DHCP_RELEASE) {
+                _cleanup_free_ uint8_t *opts = NULL;
+                size_t n_opts, i = 0;
+                void *val;
+
+                n_opts = set_size(client->req_opts);
+                opts = new(uint8_t, n_opts);
+                if (!opts)
+                        return -ENOMEM;
+
+                SET_FOREACH(val, client->req_opts)
+                        opts[i++] = PTR_TO_UINT8(val);
+                assert(i == n_opts);
+
+                /* For anonymizing the request, let's sort the options. */
+                typesafe_qsort(opts, n_opts, cmp_uint8);
+
                r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0,
                                       SD_DHCP_OPTION_PARAMETER_REQUEST_LIST,
-                                       client->req_opts_size, client->req_opts);
+                                       n_opts, opts);
                if (r < 0)
                        return r;
        }
@ -2187,7 +2195,7 @@ static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) {

        sd_dhcp_lease_unref(client->lease);

-        free(client->req_opts);
+        set_free(client->req_opts);
        free(client->hostname);
        free(client->vendor_class_identifier);
        free(client->mudurl);
@ -2200,6 +2208,10 @@ static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) {
 DEFINE_TRIVIAL_REF_UNREF_FUNC(sd_dhcp_client, sd_dhcp_client, dhcp_client_free);

 int sd_dhcp_client_new(sd_dhcp_client **ret, int anonymize) {
+        const uint8_t *opts;
+        size_t n_opts;
+        int r;
+
        assert_return(ret, -EINVAL);

        _cleanup_(sd_dhcp_client_unrefp) sd_dhcp_client *client = new(sd_dhcp_client, 1);
@ -2219,14 +2231,18 @@ int sd_dhcp_client_new(sd_dhcp_client **ret, int anonymize) {
        };
        /* NOTE: this could be moved to a function. */
        if (anonymize) {
-                client->req_opts_size = ELEMENTSOF(default_req_opts_anonymize);
-                client->req_opts = memdup(default_req_opts_anonymize, client->req_opts_size);
+                n_opts = ELEMENTSOF(default_req_opts_anonymize);
+                opts = default_req_opts_anonymize;
        } else {
-                client->req_opts_size = ELEMENTSOF(default_req_opts);
-                client->req_opts = memdup(default_req_opts, client->req_opts_size);
+                n_opts = ELEMENTSOF(default_req_opts);
+                opts = default_req_opts;
+        }
+
+        for (size_t i = 0; i < n_opts; i++) {
+                r = sd_dhcp_client_set_request_option(client, opts[i]);
+                if (r < 0)
+                        return r;
        }
-        if (!client->req_opts)
-                return -ENOMEM;

        *ret = TAKE_PTR(client);

--- a/src/libsystemd-network/test-dhcp-client.c
+++ b/src/libsystemd-network/test-dhcp-client.c
@ -71,40 +71,29 @@ static void test_request_basic(sd_event *e) {
        assert_se(sd_dhcp_client_set_hostname(client, "~host") == -EINVAL);
        assert_se(sd_dhcp_client_set_hostname(client, "~host.domain") == -EINVAL);

-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_SUBNET_MASK) == -EEXIST);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_ROUTER) == -EEXIST);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_SUBNET_MASK) == 0);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_ROUTER) == 0);
        /* This PRL option is not set when using Anonymize, but in this test
         * Anonymize settings are not being used. */
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_HOST_NAME) == -EEXIST);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_DOMAIN_NAME) == -EEXIST);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == -EEXIST);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_HOST_NAME) == 0);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_DOMAIN_NAME) == 0);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == 0);

-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_PAD) == -EINVAL);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_END) == -EINVAL);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_MESSAGE_TYPE) == -EINVAL);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_OVERLOAD) == -EINVAL);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_PARAMETER_REQUEST_LIST)
-                        == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PAD) == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_END) == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_MESSAGE_TYPE) == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_OVERLOAD) == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) == -EINVAL);

        /* RFC7844: option 33 (SD_DHCP_OPTION_STATIC_ROUTE) is set in the
         * default PRL when using Anonymize, so it is changed to other option
         * that is not set by default, to check that it was set successfully.
         * Options not set by default (using or not anonymize) are option 17
         * (SD_DHCP_OPTION_ROOT_PATH) and 42 (SD_DHCP_OPTION_NTP_SERVER) */
+        assert_se(sd_dhcp_client_set_request_option(client, 17) == 1);
+        assert_se(sd_dhcp_client_set_request_option(client, 17) == 0);
+        assert_se(sd_dhcp_client_set_request_option(client, 42) == 1);
        assert_se(sd_dhcp_client_set_request_option(client, 17) == 0);
-        assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST);
-        assert_se(sd_dhcp_client_set_request_option(client, 42) == 0);
-        assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST);

        sd_dhcp_client_unref(client);
 }
@ -126,19 +115,15 @@ static void test_request_anonymize(sd_event *e) {
        r = sd_dhcp_client_attach_event(client, e, 0);
        assert_se(r >= 0);

-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_NETBIOS_NAMESERVER) == -EEXIST);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_NETBIOS_NAMESERVER) == 0);
        /* This PRL option is not set when using Anonymize */
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_HOST_NAME) == 0);
-        assert_se(sd_dhcp_client_set_request_option(client,
-                                        SD_DHCP_OPTION_PARAMETER_REQUEST_LIST)
-                        == -EINVAL);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_HOST_NAME) == 1);
+        assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) == -EINVAL);

        /* RFC7844: option 101 (SD_DHCP_OPTION_NEW_TZDB_TIMEZONE) is not set in the
         * default PRL when using Anonymize, */
+        assert_se(sd_dhcp_client_set_request_option(client, 101) == 1);
        assert_se(sd_dhcp_client_set_request_option(client, 101) == 0);
-        assert_se(sd_dhcp_client_set_request_option(client, 101) == -EEXIST);

        sd_dhcp_client_unref(client);
 }
--- a/src/network/networkd-dhcp4.c
+++ b/src/network/networkd-dhcp4.c
@ -1357,10 +1357,6 @@ int dhcp4_configure(Link *link) {
                uint32_t option = PTR_TO_UINT32(request_options);

                r = sd_dhcp_client_set_request_option(link->dhcp_client, option);
-                if (r == -EEXIST) {
-                        log_link_debug(link, "DHCP4 CLIENT: Failed to set request flag for '%u' already exists, ignoring.", option);
-                        continue;
-                }
                if (r < 0)
                        return log_link_error_errno(link, r, "DHCP4 CLIENT: Failed to set request flag for '%u': %m", option);
        }
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@ -1758,6 +1758,11 @@ static int link_joined(Link *link) {
                return 0;

        link_set_state(link, LINK_STATE_CONFIGURING);
+
+        r = link_acquire_conf(link);
+        if (r < 0)
+                return r;
+
        return link_set_static_configs(link);
 }

@ -2038,12 +2043,6 @@ static int link_configure_continue(Link *link) {
        if (r < 0)
                log_link_warning_errno(link, r, "Cannot set IPv6 MTU for interface, ignoring: %m");

-        if (link_has_carrier(link) || link->network->configure_without_carrier) {
-                r = link_acquire_conf(link);
-                if (r < 0)
-                        return r;
-        }
-
        return link_enter_join_netdev(link);
 }
Author	SHA1	Message	Date
Yu Watanabe	dfc637d0ff	Merge pull request #17341 from yuwata/sd-dhcp-client-fix-eexist-issue-16964 sd-dhcp: make sd_dhcp_client_set_request_option() not return -EEXIST	2020-10-14 18:55:39 +09:00
Yu Watanabe	0e569a439a	network: start dynamic addressing clients like DHCP after setting netdevs The function link_acquire_conf() may make the link state 'configuring' when DHCP6 PD is enabled. Previously link_acquire_conf() was called before link_enter_join_netdev(), and thus the assertion in the function might be triggered. Fixes #17329.	2020-10-14 10:52:02 +02:00
Lennart Poettering	a748b122be	analyze: show ungrouped syscalls separately (#17343 ) This updates the "systemd-analyze syscall-filter" command to show a special section of syscalls that are included in @known but in no other group. Typically this should show syscalls we either should add to any of the existing groups or where we unsure were they best fit in. Right now, it mostly shows arch-specific compat syscalls, we probably should move "@obsolete". This patch doesn't add thta however.	2020-10-14 10:31:59 +02:00
Yu Watanabe	4eb6a826b8	network: drop unused condition	2020-10-14 13:02:46 +09:00
Yu Watanabe	4081756a63	sd-dhcp-client: make sd_dhcp_client_set_request_option() not return -EEXIST Fixes #16964.	2020-10-14 12:54:55 +09:00