From 64f2cf77d1aed62da549ca3573e2c8aabec10d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 16 Oct 2023 11:54:21 +0200 Subject: [PATCH 1/3] NEWS, man: move description of SR-IOV-R net naming to v255 https://github.com/systemd/systemd/pull/29582 adds the "v254" name. This also changes what the default is and what "latest" refers to. Without the name, the code could be enabled via runtime configuration. Nevertheless, it could be enabled at compilation time. In other words: meson setup build -Ddefault-net-naming-scheme=v254 would work, but net.naming-scheme=v254 would fail. It is possible that people were using the compile-time override, so I think we should allow "v254" scheme to stay and clearly document that it wasn't the default. Unfortunately, unless people manually introduced the compile-time override, we were never actually testing the new code too. So all the pull request testing was not useful. --- NEWS | 14 ++++++++++++-- man/systemd.net-naming-scheme.xml | 20 +++++++++++++++----- src/shared/netif-naming-scheme.c | 1 + src/shared/netif-naming-scheme.h | 6 +++++- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 7ab2c88db9..c469ad650c 100644 --- a/NEWS +++ b/NEWS @@ -60,6 +60,13 @@ CHANGES WITH 255 in spe: is now dropped, as it never worked, hence it should not be used by anyone. + * The predictable network interface naming logic is extended to include + the SR-IOV-R "representor" information in network interface names. + This feature was intended for v254, but even though the code was + merged, the part that actually enabled the feature was forgotten. + It is now enabled by default and is part of the new "v255" naming + scheme. + Changes in systemd-analyze: * "systemd-analyze plot" has gained tooltips on each unit name with @@ -576,8 +583,11 @@ CHANGES WITH 254: selects the default value of the per-network setting of the same name. - * The predictable network interface naming logic will now include - SR-IOV-R "representor" information in network interface names. + * The predictable network interface naming logic was extended to + include SR-IOV-R "representor" information in network interface + names. Unfortunately, this feature was not enabled by default and can + only be enabled at compilation time by setting + -Ddefault-net-naming-scheme=v254. * The DHCPv4 + DHCPv6 + IPv6 RA logic in networkd gained support for the RFC8910 captive portal option. diff --git a/man/systemd.net-naming-scheme.xml b/man/systemd.net-naming-scheme.xml index 8932c11f05..0eba646804 100644 --- a/man/systemd.net-naming-scheme.xml +++ b/man/systemd.net-naming-scheme.xml @@ -505,13 +505,23 @@ v254 - Naming was changed for SR-IOV virtual device representors. + Naming was changed for SR-IOV virtual device representors, optionally settable at + compilation time. The rslot suffix was added to + differentiate SR-IOV virtual device representors attached to a single physical device interface. + Because of a mistake, this scheme was not the the default scheme for systemd version + 254. - The rslot suffix was added to differentiate SR-IOV - virtual device representors attached to a single physical device interface. - + + + - + + v255 + + Naming was changed for SR-IOV virtual device representors to enable the + change introduced in v254 by default. + + diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 26f012fe02..3118ca3efb 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -27,6 +27,7 @@ static const NamingScheme naming_schemes[] = { { "v252", NAMING_V252 }, { "v253", NAMING_V253 }, { "v254", NAMING_V254 }, + { "v255", NAMING_V255 }, /* … add more schemes here, as the logic to name devices is updated … */ EXTRA_NET_NAMING_MAP diff --git a/src/shared/netif-naming-scheme.h b/src/shared/netif-naming-scheme.h index 707c0d26f3..9160e457ed 100644 --- a/src/shared/netif-naming-scheme.h +++ b/src/shared/netif-naming-scheme.h @@ -54,7 +54,11 @@ typedef enum NamingSchemeFlags { NAMING_V251 = NAMING_V250 | NAMING_BRIDGE_MULTIFUNCTION_SLOT, NAMING_V252 = NAMING_V251 | NAMING_DEVICETREE_ALIASES, NAMING_V253 = NAMING_V252 | NAMING_USB_HOST, - NAMING_V254 = NAMING_V253 | NAMING_SR_IOV_R, + NAMING_V254 = NAMING_V253 | NAMING_SR_IOV_R, /* Despite the name, "v254" is NOT the default scheme + * for systemd version 254. It was added in a follow-up + * patch later. NAMING_SR_IOV_R is enabled by default in + * systemd version 255, naming scheme "v255". */ + NAMING_V255 = NAMING_V254, EXTRA_NET_NAMING_SCHEMES From 8b01831950481b3b5b1b47da9e0495bc60df822e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 16 Oct 2023 12:35:33 +0200 Subject: [PATCH 2/3] shared/netif-naming-scheme: align tables --- src/shared/netif-naming-scheme.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 3118ca3efb..7fe19d4877 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -88,23 +88,23 @@ const NamingScheme* naming_scheme(void) { } static const char* const name_policy_table[_NAMEPOLICY_MAX] = { - [NAMEPOLICY_KERNEL] = "kernel", - [NAMEPOLICY_KEEP] = "keep", + [NAMEPOLICY_KERNEL] = "kernel", + [NAMEPOLICY_KEEP] = "keep", [NAMEPOLICY_DATABASE] = "database", - [NAMEPOLICY_ONBOARD] = "onboard", - [NAMEPOLICY_SLOT] = "slot", - [NAMEPOLICY_PATH] = "path", - [NAMEPOLICY_MAC] = "mac", + [NAMEPOLICY_ONBOARD] = "onboard", + [NAMEPOLICY_SLOT] = "slot", + [NAMEPOLICY_PATH] = "path", + [NAMEPOLICY_MAC] = "mac", }; DEFINE_STRING_TABLE_LOOKUP(name_policy, NamePolicy); static const char* const alternative_names_policy_table[_NAMEPOLICY_MAX] = { [NAMEPOLICY_DATABASE] = "database", - [NAMEPOLICY_ONBOARD] = "onboard", - [NAMEPOLICY_SLOT] = "slot", - [NAMEPOLICY_PATH] = "path", - [NAMEPOLICY_MAC] = "mac", + [NAMEPOLICY_ONBOARD] = "onboard", + [NAMEPOLICY_SLOT] = "slot", + [NAMEPOLICY_PATH] = "path", + [NAMEPOLICY_MAC] = "mac", }; DEFINE_STRING_TABLE_LOOKUP(alternative_names_policy, NamePolicy); From 386256e69900d02d1413603a0066eb945ca2ec3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 16 Oct 2023 12:53:10 +0200 Subject: [PATCH 3/3] test: make sure that the default naming scheme name maps back to itself We were testing the that C constant is defined, but we weren't actually testing that the string name maps back to itself. This would catch the issue fixed by the grandparent commit. The test for the default name is moved to the test file to keep the tests together. The define is renamed to not have "_TEST" in the name. The issue here is complicated by the fact that we allow downstreams to inject additional fields, so we don't know the name of the default scheme if it not set with -Ddefault-net-naming-scheme=, so _DEFAULT_NET_NAMING_SCHEME[_TEST] is not defined in all cases, but at least in principle it could be used in other places. If it exists, it is fully valid. --- meson.build | 8 +++++--- src/shared/netif-naming-scheme.c | 7 ------- src/test/test-net-naming-scheme.c | 9 +++++++++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/meson.build b/meson.build index 5b6b928276..77684b5caf 100644 --- a/meson.build +++ b/meson.build @@ -767,10 +767,12 @@ 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) +conf.set_quoted('DEFAULT_NET_NAMING_SCHEME', default_net_naming_scheme, + description : 'Default naming scheme as a string') if default_net_naming_scheme != 'latest' - conf.set('_DEFAULT_NET_NAMING_SCHEME_TEST', - 'NAMING_' + default_net_naming_scheme.underscorify().to_upper()) + conf.set('_DEFAULT_NET_NAMING_SCHEME', + 'NAMING_' + default_net_naming_scheme.underscorify().to_upper(), + description : 'Default naming scheme as a constant') endif time_epoch = get_option('time-epoch') diff --git a/src/shared/netif-naming-scheme.c b/src/shared/netif-naming-scheme.c index 7fe19d4877..fbaf5c5a60 100644 --- a/src/shared/netif-naming-scheme.c +++ b/src/shared/netif-naming-scheme.c @@ -6,13 +6,6 @@ #include "string-util.h" #include "string-table.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 }, diff --git a/src/test/test-net-naming-scheme.c b/src/test/test-net-naming-scheme.c index 0766170757..f7ec5a6d72 100644 --- a/src/test/test-net-naming-scheme.c +++ b/src/test/test-net-naming-scheme.c @@ -4,10 +4,19 @@ #include "string-util.h" #include "tests.h" +#ifdef _DEFAULT_NET_NAMING_SCHEME +/* 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 >= 0); +#endif + TEST(default_net_naming_scheme) { const NamingScheme *n; assert_se(n = naming_scheme_from_name(DEFAULT_NET_NAMING_SCHEME)); log_info("default → %s", n->name); + + assert_se(naming_scheme_from_name(n->name) == n); } TEST(naming_scheme_conversions) {