From e2c5634ecd44916dfa28aeb759031f3a28761b60 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jul 2021 15:21:43 +0200 Subject: [PATCH 1/5] glib-aux: add nm_strv_is_same_unordered() helper --- src/libnm-glib-aux/nm-shared-utils.c | 46 ++++++++++++++++++++++++++++ src/libnm-glib-aux/nm-shared-utils.h | 5 +++ 2 files changed, 51 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 106cc70fad..a2a20bd18b 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -2336,6 +2336,52 @@ nm_strv_has_duplicate(const char *const *strv, gssize len, gboolean is_sorted) return FALSE; } +gboolean +nm_strv_is_same_unordered(const char *const *strv1, + gssize len1, + const char *const *strv2, + gssize len2) +{ + gs_free const char **ss1_free = NULL; + gs_free const char **ss2_free = NULL; + gsize l2; + gsize l; + gsize i; + + if (len1 < 0) + l = NM_PTRARRAY_LEN(strv1); + else + l = (gsize) len1; + + if (len2 < 0) + l2 = NM_PTRARRAY_LEN(strv2); + else + l2 = (gsize) len2; + + if (l != l2) + return FALSE; + + if (l == 0) { + /* An empty array. We treat (NULL, -1), (NULL, 0) and ([...], 0) + * all the same. */ + return TRUE; + } + + if (l > 1) { + strv1 = nm_memdup_maybe_a(300, strv1, sizeof(char *) * l, &ss1_free); + strv2 = nm_memdup_maybe_a(300, strv2, sizeof(char *) * l2, &ss2_free); + _nm_utils_strv_sort((const char **) strv1, l); + _nm_utils_strv_sort((const char **) strv2, l); + } + + for (i = 0; i < l; i++) { + if (!nm_streq0(strv1[i], strv2[i])) + return FALSE; + } + + return TRUE; +} + char ** _nm_utils_strv_cleanup(char ** strv, gboolean strip_whitespace, diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index ac522e6d21..56a813c435 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -746,6 +746,11 @@ char **_nm_utils_strv_cleanup(char ** strv, gboolean skip_empty, gboolean skip_repeated); +gboolean nm_strv_is_same_unordered(const char *const *strv1, + gssize len1, + const char *const *strv2, + gssize len2); + /*****************************************************************************/ static inline gpointer From f0ec3d5a56f61a4e1c8b03dc3dda393022324bb2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 28 Jul 2021 23:25:28 +0200 Subject: [PATCH 2/5] glib-aux: add _nm_utils_strv_cleanup_const() helper --- src/libnm-glib-aux/nm-shared-utils.c | 26 +++++++++++++++++++++++++- src/libnm-glib-aux/nm-shared-utils.h | 3 +++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index a2a20bd18b..83f073d39d 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -2382,13 +2382,37 @@ nm_strv_is_same_unordered(const char *const *strv1, return TRUE; } +const char ** +_nm_utils_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repeated) +{ + gsize i; + gsize j; + + if (!strv || !*strv) + return strv; + + if (!skip_empty && !skip_repeated) + return strv; + + j = 0; + for (i = 0; strv[i]; i++) { + if ((skip_empty && !*strv[i]) + || (skip_repeated && nm_utils_strv_find_first((char **) strv, j, strv[i]) >= 0)) + continue; + strv[j++] = strv[i]; + } + strv[j] = NULL; + return strv; +} + char ** _nm_utils_strv_cleanup(char ** strv, gboolean strip_whitespace, gboolean skip_empty, gboolean skip_repeated) { - guint i, j; + gsize i; + gsize j; if (!strv || !*strv) return strv; diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 56a813c435..9ffb28048b 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -741,6 +741,9 @@ gssize nm_utils_strv_find_first(char **list, gssize len, const char *needle); gboolean nm_strv_has_duplicate(const char *const *list, gssize len, gboolean is_sorted); +const char ** +_nm_utils_strv_cleanup_const(const char **strv, gboolean skip_empty, gboolean skip_repeated); + char **_nm_utils_strv_cleanup(char ** strv, gboolean strip_whitespace, gboolean skip_empty, From 4547b4a893bd8a5b42abc7d779b698d934fa996e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jul 2021 15:41:45 +0200 Subject: [PATCH 3/5] std-aux: implement NM_PTRARRAY_LEN() macro via static function We use NM_PTRARRAY_LEN(), and I find it a bit ugly that a macro does so much. Maybe, it's better to have it as a function. But the macro currently lives in "libnm-std-aux/nm-std-aux.h", which is header-only. To add it to a C source file, we would have to move it to another header, but "libnm-std-aux/nm-std-aux.h" is nice because it gets included by default already. Keep it in "libnm-std-aux/nm-std-aux.h", but implement it as an inline function. The macro now only does (as before) some type checking shenanigans to ensure that the argument is a pointer to pointers. In practice, there is probably very little difference compared to the macro before, likely the code will anyway be inlined. --- src/libnm-std-aux/nm-std-aux.h | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/libnm-std-aux/nm-std-aux.h b/src/libnm-std-aux/nm-std-aux.h index 0bc4d5b805..bc74a30997 100644 --- a/src/libnm-std-aux/nm-std-aux.h +++ b/src/libnm-std-aux/nm-std-aux.h @@ -316,22 +316,28 @@ nm_mult_clamped_u(unsigned a, unsigned b) /* macro to return strlen() of a compile time string. */ #define NM_STRLEN(str) (sizeof("" str "") - 1u) +static inline size_t +_nm_ptrarray_len_impl(const void *const *array) +{ + size_t n = 0; + + if (array) { + while (array[n]) + n++; + } + return n; +} + /* returns the length of a NULL terminated array of pointers, * like g_strv_length() does. The difference is: * - it operates on arrays of pointers (of any kind, requiring no cast). * - it accepts NULL to return zero. */ -#define NM_PTRARRAY_LEN(array) \ - ({ \ - typeof(*(array)) *const _array = (array); \ - size_t _n = 0; \ - \ - if (_array) { \ - _nm_unused const void *_type_check_is_pointer = _array[0]; \ - \ - while (_array[_n]) \ - _n++; \ - } \ - _n; \ +#define NM_PTRARRAY_LEN(array) \ + ({ \ + typeof(*(array)) *const _array = (array); \ + _nm_unused const void * _type_check_is_pointer = 0 ? _array[0] : NULL; \ + \ + _nm_ptrarray_len_impl((const void *const *) _array); \ }) /*****************************************************************************/ From 6fa7f2e06c6b1ed2d4272a758440fd4888252fa1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jul 2021 11:49:25 +0200 Subject: [PATCH 4/5] initrd: honor "ip=fw" alias for "ip=ibft" in reader_parse_ip() This alias was introduced by commit [1], without further explaination or documentation. As we already have it, implement it fully. [1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/commit/7a72c705acc23db85ae5fdff250fe42567029476 --- src/nm-initrd-generator/nmi-cmdline-reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c index 8cdabbb312..3b656bc1ba 100644 --- a/src/nm-initrd-generator/nmi-cmdline-reader.c +++ b/src/nm-initrd-generator/nmi-cmdline-reader.c @@ -644,7 +644,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NM_SETTING_IP4_CONFIG_METHOD_DISABLED, NULL); } - } else if (nm_streq0(kind, "ibft")) { + } else if (NM_IN_STRSET(kind, "fw", "ibft")) { NMSettingWired *s_wired; const char * mac = NULL; const char * ifname; From 8b2522168985a959cd8c939f66e2c10793ff5b84 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jul 2021 12:30:20 +0200 Subject: [PATCH 5/5] initrd: rework parsing of ip method from "ip=" Dracut supports several options for the "ip=" method. NetworkManager interprets and handles them in a certain way that aims to give a similar behavior. But as such it maps different settings ("auth6" and "dhcp6") to exactly the same behavior. Add _parse_ip_method() function to normalize these keys, and map their aliases to the keyword that nm-initrd-generator handles. The advantage is that you see now in _parse_ip_method() which methods are mapped to the same behavior, and the later (more complex) code only deals with the normalized kinds. Also, use the same validation code at all 3 places where IP methods can appear, that is ip= ip=:[:...] ip=:...:[:...] Also, dracut supports specifying multiple methods and concatenate them with comma. nm-initrd-generator only did partly, for example, `ip=dhcp,dhcp6" would have worked, but only because the code failed to recognize the string and fell back to the default behavior. It would not have worked as `ip=:dhcp,dhcp6[:...]`. Not all combinations make sense, but some do. So let _parse_ip_method() detect and handle them. Currently, they mostly map to "auto", but in the future it might make sense that `ip=dhcp,local6` is a distinct kind. Try to tighten up the parsing. It's fine to be forgiving and flexible about what we parse, but bogus values should not silently be accepted. However, in order to keep previous behavior, `ip=bogus` and `ip=:...:[:...]` explicitly map invalid method to "auto". --- src/nm-initrd-generator/nmi-cmdline-reader.c | 158 +++++++++++++++--- .../tests/test-cmdline-reader.c | 6 +- 2 files changed, 139 insertions(+), 25 deletions(-) diff --git a/src/nm-initrd-generator/nmi-cmdline-reader.c b/src/nm-initrd-generator/nmi-cmdline-reader.c index 3b656bc1ba..2bf69e6d72 100644 --- a/src/nm-initrd-generator/nmi-cmdline-reader.c +++ b/src/nm-initrd-generator/nmi-cmdline-reader.c @@ -394,6 +394,120 @@ reader_read_all_connections_from_fw(Reader *reader, const char *sysfs_dir) reader_add_connection(reader, "ofw", dt_connection); } +#define _strv_is_same_unordered(strv, ...) \ + nm_strv_is_same_unordered(NM_CAST_STRV_CC(strv), -1, NM_MAKE_STRV(__VA_ARGS__), -1) + +static void +_strv_remove(const char **strv, const char *needle) +{ + gssize idx; + gsize len; + gsize i; + + idx = nm_utils_strv_find_first((char **) strv, -1, needle); + if (idx < 0) + return; + + /* Remove element at idx, by shifting the remaining ones + * (including the terminating NULL). */ + len = NM_PTRARRAY_LEN(strv); + for (i = idx; i < len; i++) + strv[i] = strv[i + 1]; +} + +static const char * +_parse_ip_method(const char *kind) +{ + const char *const KINDS[] = { + "none", + "dhcp", + "dhcp6", + "link6", + "auto", + "ibft", + }; + gs_free char * kind_to_free = NULL; + gs_free const char **strv = NULL; + gsize i; + + kind = nm_strstrip_avoid_copy_a(300, kind, &kind_to_free); + + if (nm_str_is_empty(kind)) { + /* Dracut defaults empty/missing to "dhcp". We treat them differently, as it + * depends on whether we have IP addresses too. + * https://github.com/dracutdevs/dracut/blob/3cc9f1c10c67dcdb5254e0eb69f19e9ab22abf20/modules.d/35network-legacy/parse-ip-opts.sh#L62 */ + return "auto"; + } + + for (i = 0; i < G_N_ELEMENTS(KINDS); i++) { + if (nm_streq(kind, KINDS[i])) + return KINDS[i]; + } + + /* the following are (currently) treated as aliases. */ + if (nm_streq(kind, "fw")) + return "ibft"; + if (nm_streq(kind, "single-dhcp")) + return "dhcp"; + if (nm_streq(kind, "off")) + return "none"; + if (nm_streq(kind, "auto6")) + return "dhcp6"; + if (NM_IN_STRSET(kind, "on", "any")) + return "auto"; + + if (!strchr(kind, ',')) + return NULL; + + /* dracut also supports combinations, separated by comma. We don't + * support arbitrary combinations, but accept specific subsets. */ + strv = nm_utils_strsplit_set_full(kind, ",", NM_UTILS_STRSPLIT_SET_FLAGS_STRSTRIP); + if (!strv) + return NULL; + + /* first normalize the strv array by replacing all entries by their + * normalized kind. */ + for (i = 0; strv[i]; i++) { + strv[i] = _parse_ip_method(strv[i]); + if (!strv[i]) { + /* Unknown key. Not recognized. */ + return NULL; + } + } + + /* sort list and remove duplicates. */ + nm_utils_strv_sort(strv, -1); + _nm_utils_strv_cleanup_const(strv, TRUE, TRUE); + + if (nm_utils_strv_find_first((char **) strv, -1, "auto") >= 0) { + /* if "auto" is present, then "dhcp4", "dhcp6", and "local6" is implied. */ + _strv_remove(strv, "dhcp4"); + _strv_remove(strv, "dhcp6"); + _strv_remove(strv, "local6"); + } else if (nm_utils_strv_find_first((char **) strv, -1, "dhcp6") >= 0) { + /* if "dhcp6" is present, then "local6" is implied. */ + _strv_remove(strv, "local6"); + } + + if (strv[0] && !strv[1]) { + /* there is only one value left. It's good. */ + return strv[0]; + } + + /* only certain combinations are allowed... those are listed + * and mapped to a canonical value. + * + * For the moment, these map all to "auto". This might be revisited + * in the future to add new kinds like "dhcp+local6". */ + if (_strv_is_same_unordered(strv, "dhcp", "dhcp6")) + return "auto"; + if (_strv_is_same_unordered(strv, "dhcp", "local6")) + return "auto"; + + /* undetected. */ + return NULL; +} + static void reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) { @@ -403,7 +517,8 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) gs_unref_hashtable GHashTable *ibft = NULL; const char * tmp; const char * tmp2; - const char * kind = NULL; + const char * tmp3; + const char * kind; const char * client_ip = NULL; const char * peer = NULL; const char * gateway_ip = NULL; @@ -432,24 +547,17 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) tmp = get_word(&argument, ':'); if (!*argument) { /* ip={dhcp|on|any|dhcp6|auto6|link6|ibft} */ - kind = tmp; + kind = _parse_ip_method(tmp); + if (!kind) { + /* invalid method. We treat it as "auto". */ + kind = "auto"; + } } else { tmp2 = get_word(&argument, ':'); - if (NM_IN_STRSET(tmp2, - "none", - "off", - "dhcp", - "single-dhcp", - "on" - "any", - "dhcp6", - "auto", - "auto6", - "link6", - "ibft")) { + if (!nm_str_is_empty(tmp2) && (tmp3 = _parse_ip_method(tmp2))) { /* :{none|off|dhcp|on|any|dhcp6|auto|auto6|link6|ibft} */ iface_spec = tmp; - kind = tmp2; + kind = tmp3; } else { /* :[]:::: */ client_ip = tmp; @@ -466,7 +574,12 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) netmask = get_word(&argument, ':'); client_hostname = get_word(&argument, ':'); iface_spec = get_word(&argument, ':'); - kind = get_word(&argument, ':'); + tmp2 = get_word(&argument, ':'); + kind = _parse_ip_method(tmp2); + if (!kind) { + /* invalid method. We treat that as "auto". */ + kind = "auto"; + } } if (client_hostname && !nm_sd_hostname_is_valid(client_hostname, FALSE)) @@ -495,7 +608,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) } } - if (iface_spec == NULL && NM_IN_STRSET(kind, "fw", "ibft")) { + if (iface_spec == NULL && nm_streq(kind, "ibft")) { reader_read_all_connections_from_fw(reader, sysfs_dir); return; } @@ -592,7 +705,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) } /* Dynamic IP configuration configured explicitly. */ - if (NM_IN_STRSET(kind, "none", "off")) { + if (nm_streq(kind, "none")) { if (nm_setting_ip_config_get_num_addresses(s_ip6) == 0) { g_object_set(s_ip6, NM_SETTING_IP_CONFIG_METHOD, @@ -605,7 +718,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NM_SETTING_IP4_CONFIG_METHOD_DISABLED, NULL); } - } else if (NM_IN_STRSET(kind, "dhcp", "single-dhcp")) { + } else if (nm_streq(kind, "dhcp")) { g_object_set(s_ip4, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO, @@ -618,7 +731,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NM_SETTING_IP6_CONFIG_METHOD_AUTO, NULL); } - } else if (NM_IN_STRSET(kind, "auto6", "dhcp6")) { + } else if (nm_streq(kind, "dhcp6")) { g_object_set(s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO, @@ -631,7 +744,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NM_SETTING_IP4_CONFIG_METHOD_DISABLED, NULL); } - } else if (nm_streq0(kind, "link6")) { + } else if (nm_streq(kind, "link6")) { g_object_set(s_ip6, NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL, @@ -644,7 +757,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) NM_SETTING_IP4_CONFIG_METHOD_DISABLED, NULL); } - } else if (NM_IN_STRSET(kind, "fw", "ibft")) { + } else if (nm_streq(kind, "ibft")) { NMSettingWired *s_wired; const char * mac = NULL; const char * ifname; @@ -684,6 +797,7 @@ reader_parse_ip(Reader *reader, const char *sysfs_dir, char *argument) } } } else { + nm_assert(nm_streq(kind, "auto")); clear_ip4_required_timeout = FALSE; } diff --git a/src/nm-initrd-generator/tests/test-cmdline-reader.c b/src/nm-initrd-generator/tests/test-cmdline-reader.c index 3871ae5c38..57ecfbe061 100644 --- a/src/nm-initrd-generator/tests/test-cmdline-reader.c +++ b/src/nm-initrd-generator/tests/test-cmdline-reader.c @@ -130,7 +130,7 @@ static void test_dhcp_with_hostname(void) { gs_unref_hashtable GHashTable *connections = NULL; - const char *const * ARGV = NM_MAKE_STRV("ip=::::host1::dhcp"); + const char *const * ARGV = NM_MAKE_STRV("ip=::::host1::dhcp,dhcp6"); NMConnection * connection; NMSettingConnection * s_con; NMSettingWired * s_wired; @@ -181,7 +181,7 @@ test_dhcp_with_hostname(void) static void test_dhcp_with_mtu(void) { - const char *const *ARGV0 = NM_MAKE_STRV("ip=:dhcp:1499"); + const char *const *ARGV0 = NM_MAKE_STRV("ip=:dhcp6,dhcp:1499"); const char *const *ARGV1 = NM_MAKE_STRV("ip=::::::dhcp:1499"); const char *const *ARGV[] = {ARGV0, ARGV1}; guint i; @@ -290,7 +290,7 @@ test_dhcp_timeout(void) static void test_if_auto_with_mtu(void) { - const char *const *ARGV = NM_MAKE_STRV("ip=eth0:auto:1666"); + const char *const *ARGV = NM_MAKE_STRV("ip=eth0:dhcp,dhcp6:1666"); gs_unref_object NMConnection *connection = NULL; NMSettingConnection * s_con; NMSettingWired * s_wired;