From acaa6368663a8972e3722751bd67bb1e0b3f272f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Sep 2021 08:55:18 +0200 Subject: [PATCH 1/3] netif-naming: inline one iterator variable --- src/shared/netif-naming-scheme.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 4dcf2889ce..0a80931b0f 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -19,12 +19,10 @@ static const NamingScheme naming_schemes[] = { }; static const NamingScheme* naming_scheme_from_name(const char *name) { - size_t i; - if (streq(name, "latest")) return naming_schemes + ELEMENTSOF(naming_schemes) - 1; - for (i = 0; i < ELEMENTSOF(naming_schemes); i++) + for (size_t i = 0; i < ELEMENTSOF(naming_schemes); i++) if (streq(naming_schemes[i].name, name)) return naming_schemes + i; From 77faadfdd3d2d934f52d1d35b60e774c494d84b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Sep 2021 09:33:30 +0200 Subject: [PATCH 2/3] meson: drop the list of valid net naming schemes We used 'combo' type for the scheme list. For a while we forgot to add new names, and recently aa0a23ec86 added v241, v243, v245, and v247. I want to allow defining new values during configuration, which means that we can't use meson to verify the list of options. So any value is allowed, but then two tests are added: one that will fail compilation if some invalid name is given (other than "latest"), and one that converts DEFAULT_NET_NAMING_SCHEME to a NamingScheme pointer. --- meson.build | 4 ++++ meson_options.txt | 3 +-- src/shared/netif-naming-scheme.c | 10 ++++++++-- src/shared/netif-naming-scheme.h | 1 + src/test/meson.build | 2 ++ src/test/test-net-naming-scheme.c | 31 +++++++++++++++++++++++++++++++ 6 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 src/test/test-net-naming-scheme.c diff --git a/meson.build b/meson.build index 9db035c3b7..67695a4368 100644 --- a/meson.build +++ b/meson.build @@ -709,6 +709,10 @@ endif default_net_naming_scheme = get_option('default-net-naming-scheme') conf.set_quoted('DEFAULT_NET_NAMING_SCHEME', default_net_naming_scheme) +if default_net_naming_scheme != 'latest' + conf.set('_DEFAULT_NET_NAMING_SCHEME_TEST', + 'NAMING_' + default_net_naming_scheme.underscorify().to_upper()) +endif time_epoch = get_option('time-epoch') if time_epoch == -1 diff --git a/meson_options.txt b/meson_options.txt index dcd3c01ae4..15be1959d1 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -200,8 +200,7 @@ option('fallback-hostname', type : 'string', value : 'localhost', option('default-hierarchy', type : 'combo', choices : ['legacy', 'hybrid', 'unified'], value : 'unified', description : 'default cgroup hierarchy') -option('default-net-naming-scheme', type : 'combo', - choices : ['latest', 'v238', 'v239', 'v240', 'v241', 'v243', 'v245', 'v247', 'v249'], +option('default-net-naming-scheme', type : 'string', value : 'latest', description : 'default net.naming-scheme= value') option('status-unit-format-default', type : 'combo', choices : ['description', 'name', 'combined'], diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 0a80931b0f..287a942014 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -5,6 +5,13 @@ #include "proc-cmdline.h" #include "string-util.h" +#ifdef _DEFAULT_NET_NAMING_SCHEME_TEST +/* The primary purpose of this check is to verify that _DEFAULT_NET_NAMING_SCHEME_TEST + * is a valid identifier. If an invalid name is given during configuration, this will + * fail with a name error. */ +assert_cc(_DEFAULT_NET_NAMING_SCHEME_TEST >= 0); +#endif + static const NamingScheme naming_schemes[] = { { "v238", NAMING_V238 }, { "v239", NAMING_V239 }, @@ -15,10 +22,9 @@ static const NamingScheme naming_schemes[] = { { "v247", NAMING_V247 }, { "v249", NAMING_V249 }, /* … add more schemes here, as the logic to name devices is updated … */ - /* also remember to update the list of options in meson_options.txt */ }; -static const NamingScheme* naming_scheme_from_name(const char *name) { +const NamingScheme* naming_scheme_from_name(const char *name) { if (streq(name, "latest")) return naming_schemes + ELEMENTSOF(naming_schemes) - 1; diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h index 119b80178f..f5d040cc09 100644 --- a/src/shared/netif-naming-scheme.h +++ b/src/shared/netif-naming-scheme.h @@ -54,6 +54,7 @@ typedef struct NamingScheme { NamingSchemeFlags flags; } NamingScheme; +const NamingScheme* naming_scheme_from_name(const char *name); const NamingScheme* naming_scheme(void); static inline bool naming_scheme_has(NamingSchemeFlags flags) { diff --git a/src/test/meson.build b/src/test/meson.build index f58cf2d843..0223086c69 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -429,6 +429,8 @@ tests += [ [['src/test/test-firewall-util.c']], + [['src/test/test-net-naming-scheme.c']], + [['src/test/test-netlink-manual.c'], [], [libkmod], diff --git a/src/test/test-net-naming-scheme.c b/src/test/test-net-naming-scheme.c new file mode 100644 index 0000000000..693b2f6604 --- /dev/null +++ b/src/test/test-net-naming-scheme.c @@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "netif-naming-scheme.h" +#include "string-util.h" +#include "tests.h" + +static void test_default_net_naming_scheme(void) { + log_info("/* %s */", __func__); + + const NamingScheme *n; + assert_se(n = naming_scheme_from_name(DEFAULT_NET_NAMING_SCHEME)); + log_info("default → %s", n->name); +} + +static void test_naming_scheme_conversions(void) { + log_info("/* %s */", __func__); + + const NamingScheme *n; + assert_se(n = naming_scheme_from_name("latest")); + log_info("latest → %s", n->name); + + assert_se(n = naming_scheme_from_name("v238")); + assert_se(streq(n->name, "v238")); +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_INFO); + + test_default_net_naming_scheme(); + test_naming_scheme_conversions(); +} From 681cb84a63c14410449469c580ea3cc54f590ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Sep 2021 10:12:36 +0200 Subject: [PATCH 3/3] meson: allow extra net naming schemes to be defined during configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In upstream, we have a linearly-growing list of net-naming-scheme defines; we add a new one for every release where we make user-visible changes to the naming scheme. But the general idea was that downstream distributions could define their own combinations (or even just their own names for existing combinations), so provide stability for their users. So far this required patching of the netif-naming-scheme.c and .h files to add the new lines. With this patch, patching is not required: $ meson configure build \ -Dextra-net-naming-schemes=gargoyle=v238+npar_ari+allow_rerenames,gargoyle2=gargoyle+nspawn_long_hash \ -Ddefault-net-naming-scheme=gargoyle2 or even $ meson configure build \ -Dextra-net-naming-schemes=gargoyle=v238+npar_ari+allow_rerenames,gargoyle2=gargoyle+nspawn_long_hash,latest=v249 \ -Ddefault-net-naming-scheme=gargoyle2 The syntax is a comma-separated list of NAME=name+name+… This syntax is a bit scary, but any typos result in compilation errors, so I think it should be OK in practice. With this approach, we don't allow users to define arbitrary combinations: what is allowed is still defined at compilation time, so it's up to the distribution maintainers to provide reasonable combinations. In this regard, the only difference from status quo is that it's much easier to do (and harder to do incorrectly, for example by forgetting to add a name to one of the maps). --- meson.build | 18 ++++++++++++++++++ meson_options.txt | 2 ++ src/shared/netif-naming-scheme.c | 10 ++++++++-- src/shared/netif-naming-scheme.h | 2 ++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index 67695a4368..0d716d5485 100644 --- a/meson.build +++ b/meson.build @@ -707,6 +707,24 @@ else conf.set('DEFAULT_HIERARCHY', 'CGROUP_UNIFIED_ALL') endif +extra_net_naming_schemes = [] +extra_net_naming_map = [] +foreach scheme: get_option('extra-net-naming-schemes').split(',') + if scheme != '' + name = scheme.split('=')[0] + value = scheme.split('=')[1] + NAME = name.underscorify().to_upper() + VALUE = [] + foreach field: value.split('+') + VALUE += 'NAMING_' + field.underscorify().to_upper() + endforeach + extra_net_naming_schemes += 'NAMING_@0@ = @1@,'.format(NAME, '|'.join(VALUE)) + extra_net_naming_map += '{ "@0@", NAMING_@1@ },'.format(name, NAME) + endif +endforeach +conf.set('EXTRA_NET_NAMING_SCHEMES', ' '.join(extra_net_naming_schemes)) +conf.set('EXTRA_NET_NAMING_MAP', ' '.join(extra_net_naming_map)) + default_net_naming_scheme = get_option('default-net-naming-scheme') conf.set_quoted('DEFAULT_NET_NAMING_SCHEME', default_net_naming_scheme) if default_net_naming_scheme != 'latest' diff --git a/meson_options.txt b/meson_options.txt index 15be1959d1..c09a23dc03 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -200,6 +200,8 @@ option('fallback-hostname', type : 'string', value : 'localhost', option('default-hierarchy', type : 'combo', choices : ['legacy', 'hybrid', 'unified'], value : 'unified', description : 'default cgroup hierarchy') +option('extra-net-naming-schemes', type : 'string', + description : 'comma-separated list of extra net.naming-scheme= definitions') option('default-net-naming-scheme', type : 'string', value : 'latest', description : 'default net.naming-scheme= value') option('status-unit-format-default', type : 'combo', diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 287a942014..d949f7381a 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -22,16 +22,22 @@ static const NamingScheme naming_schemes[] = { { "v247", NAMING_V247 }, { "v249", NAMING_V249 }, /* … add more schemes here, as the logic to name devices is updated … */ + + EXTRA_NET_NAMING_MAP }; const NamingScheme* naming_scheme_from_name(const char *name) { - if (streq(name, "latest")) - return naming_schemes + ELEMENTSOF(naming_schemes) - 1; + /* "latest" may either be defined explicitly by the extra map, in which case we we will find it in + * the table like any other name. After iterating through the table, we check for "latest" again, + * which means that if not mapped explicitly, it maps to the last defined entry, whatever that is. */ for (size_t i = 0; i < ELEMENTSOF(naming_schemes); i++) if (streq(naming_schemes[i].name, name)) return naming_schemes + i; + if (streq(name, "latest")) + return naming_schemes + ELEMENTSOF(naming_schemes) - 1; + return NULL; } diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h index f5d040cc09..0141a4fb90 100644 --- a/src/shared/netif-naming-scheme.h +++ b/src/shared/netif-naming-scheme.h @@ -46,6 +46,8 @@ typedef enum NamingSchemeFlags { NAMING_V247 = NAMING_V245 | NAMING_BRIDGE_NO_SLOT, NAMING_V249 = NAMING_V247 | NAMING_SLOT_FUNCTION_ID | NAMING_16BIT_INDEX | NAMING_REPLACE_STRICTLY, + EXTRA_NET_NAMING_SCHEMES + _NAMING_SCHEME_FLAGS_INVALID = -EINVAL, } NamingSchemeFlags;