From ed30802324365dde6c05d0b7c3ce1a0eff3bf571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 13:28:41 +0100 Subject: [PATCH 1/9] Revert "Always rename an interface to its name specified in config if no NamePolicy= is specified" This reverts commit 55b6530baacf4658a183b15b010a8cf3483fde08. This commit description says "Always rename an interface to its name specified in config if no NamePolicy= is specified", but it does much more: 1. It completely changes the meaning of NamePolicy=kernel. Before, it meant that an interface with type==NAMEPOLICY_KERNEL would not be renamed. After, the kernel name only works as a fallback, if no policy matches. 2. The "if no NamePolicy= is specified" part is not true at all, the interface will be renamed according to the specified NamePolicy=. After 55b6530baacf, the should_rename() function is named very misleadingly: it is only used to mean "respect kernel predictable name if no naming policy matches". Let's revert, and start with a clean slate. This fixes #11436. --- src/udev/net/link-config.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 537cc0cdd99..3a5077ca6aa 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -310,11 +310,16 @@ static bool should_rename(sd_device *device, bool respect_predictable) { return true; switch (type) { + case NET_NAME_USER: + case NET_NAME_RENAMED: + /* these were already named by userspace, do not touch again */ + return false; case NET_NAME_PREDICTABLE: /* the kernel claims to have given a predictable name */ if (respect_predictable) return false; _fallthrough_; + case NET_NAME_ENUM: default: /* the name is known to be bad, or of an unknown type */ return true; @@ -435,8 +440,12 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, } } - if (!new_name && should_rename(device, respect_predictable)) - new_name = config->name; + if (should_rename(device, respect_predictable)) { + /* if not set by policy, fall back manually set name */ + if (!new_name) + new_name = config->name; + } else + new_name = NULL; switch (config->mac_policy) { case MACPOLICY_PERSISTENT: From 7e8bd58eb126324ce796150b076aae5d52ea2072 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 16 Jan 2019 13:02:04 +0900 Subject: [PATCH 2/9] udev: 'val' may be NULL, use strempty() --- src/udev/udev-builtin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c index 3a61be10caa..48ce295a465 100644 --- a/src/udev/udev-builtin.c +++ b/src/udev/udev-builtin.c @@ -139,7 +139,7 @@ int udev_builtin_add_property(sd_device *dev, bool test, const char *key, const key, val ? "=" : "", strempty(val)); if (test) - printf("%s=%s\n", key, val); + printf("%s=%s\n", key, strempty(val)); return 0; } From 29cf0ff82386587e010ae2d417ff903da7537494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 10 Jan 2019 16:13:06 +0100 Subject: [PATCH 3/9] libsystemd-network: use xsprintf in one more place DECIMAL_STR_MAX includes space for NUL, so we don't need 2 here. --- src/libsystemd-network/dhcp-identifier.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/dhcp-identifier.c b/src/libsystemd-network/dhcp-identifier.c index 07496adaa39..4221b9c5044 100644 --- a/src/libsystemd-network/dhcp-identifier.c +++ b/src/libsystemd-network/dhcp-identifier.c @@ -11,6 +11,7 @@ #include "network-internal.h" #include "siphash24.h" #include "sparse-endian.h" +#include "stdio-util.h" #include "virt.h" #define SYSTEMD_PEN 43793 @@ -169,10 +170,10 @@ int dhcp_identifier_set_iaid( if (detect_container() <= 0) { /* not in a container, udev will be around */ - char ifindex_str[2 + DECIMAL_STR_MAX(int)]; + char ifindex_str[1 + DECIMAL_STR_MAX(int)]; int r; - sprintf(ifindex_str, "n%d", ifindex); + xsprintf(ifindex_str, "n%d", ifindex); if (sd_device_new_from_device_id(&device, ifindex_str) >= 0) { r = sd_device_get_is_initialized(device); if (r < 0) From 0b189e8fa72a06ada15096c9b363bddc501c01fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 14:08:02 +0100 Subject: [PATCH 4/9] link-config: unentangle the renaming logic and add logging What policy we dicide to use it rather important, but this bit of information wasn't logged. Let's always do that. The code was also written in a confusing way, which probably contributed to the unintended effects of 55b6530baacf4658a183b15b010a8cf3483fde08 and other commits. We would loop over all policies, and note if "kernel" was specified, and then possibly unset the result at the end. Let's immediately log the result and cut to the end if we can figure out the answer. No functional change intended, except for the new log lines. Using goto is not very elegant, but we can't use break because of the switch, and there are multiple conditions to break the loop, so using goto is cleanest. --- src/udev/net/link-config.c | 127 ++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 64 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 3a5077ca6aa..dc0f02bde93 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -186,6 +186,22 @@ static bool enable_name_policy(void) { return proc_cmdline_get_bool("net.ifnames", &b) <= 0 || b; } +static int link_name_type(sd_device *device, unsigned *type) { + const char *s; + int r; + + r = sd_device_get_sysattr_value(device, "name_assign_type", &s); + if (r < 0) + return log_device_debug_errno(device, r, "Failed to query name_assign_type: %m"); + + r = safe_atou(s, type); + if (r < 0) + return log_device_warning_errno(device, r, "Failed to parse name_assign_type \"%s\": %m", s); + + log_device_debug(device, "Device has name_assign_type=%d", *type); + return 0; +} + int link_config_load(link_config_ctx *ctx) { _cleanup_strv_free_ char **files; char **f; @@ -296,36 +312,6 @@ static bool mac_is_random(sd_device *device) { return type == NET_ADDR_RANDOM; } -static bool should_rename(sd_device *device, bool respect_predictable) { - const char *s; - unsigned type; - int r; - - /* if we can't get the assign type, assume we should rename */ - if (sd_device_get_sysattr_value(device, "name_assign_type", &s) < 0) - return true; - - r = safe_atou(s, &type); - if (r < 0) - return true; - - switch (type) { - case NET_NAME_USER: - case NET_NAME_RENAMED: - /* these were already named by userspace, do not touch again */ - return false; - case NET_NAME_PREDICTABLE: - /* the kernel claims to have given a predictable name */ - if (respect_predictable) - return false; - _fallthrough_; - case NET_NAME_ENUM: - default: - /* the name is known to be bad, or of an unknown type */ - return true; - } -} - static int get_mac(sd_device *device, bool want_random, struct ether_addr *mac) { int r; @@ -352,12 +338,12 @@ static int get_mac(sd_device *device, bool want_random, int link_config_apply(link_config_ctx *ctx, link_config *config, sd_device *device, const char **name) { - bool respect_predictable = false; struct ether_addr generated_mac; struct ether_addr *mac = NULL; const char *new_name = NULL; const char *old_name; - unsigned speed; + unsigned speed, name_type = NET_NAME_UNKNOWN; + NamePolicy policy; int r, ifindex; assert(ctx); @@ -410,42 +396,55 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, if (r < 0) return log_device_warning_errno(device, r, "Could not find ifindex: %m"); - if (ctx->enable_name_policy && config->name_policy) { - NamePolicy *policy; - for (policy = config->name_policy; - !new_name && *policy != _NAMEPOLICY_INVALID; policy++) { - switch (*policy) { - case NAMEPOLICY_KERNEL: - respect_predictable = true; - break; - case NAMEPOLICY_DATABASE: - (void) sd_device_get_property_value(device, "ID_NET_NAME_FROM_DATABASE", &new_name); - break; - case NAMEPOLICY_ONBOARD: - (void) sd_device_get_property_value(device, "ID_NET_NAME_ONBOARD", &new_name); - break; - case NAMEPOLICY_SLOT: - (void) sd_device_get_property_value(device, "ID_NET_NAME_SLOT", &new_name); - break; - case NAMEPOLICY_PATH: - (void) sd_device_get_property_value(device, "ID_NET_NAME_PATH", &new_name); - break; - case NAMEPOLICY_MAC: - (void) sd_device_get_property_value(device, "ID_NET_NAME_MAC", &new_name); - break; - default: - break; - } - } + (void) link_name_type(device, &name_type); + + if (IN_SET(name_type, NET_NAME_USER, NET_NAME_RENAMED)) { + log_device_info(device, "Device already has a name given by userspace, not renaming."); + goto no_rename; } - if (should_rename(device, respect_predictable)) { - /* if not set by policy, fall back manually set name */ - if (!new_name) - new_name = config->name; + if (ctx->enable_name_policy && config->name_policy) + for (NamePolicy *p = config->name_policy; !new_name && *p != _NAMEPOLICY_INVALID; p++) { + policy = *p; + + switch (policy) { + case NAMEPOLICY_KERNEL: + if (name_type != NET_NAME_PREDICTABLE) + continue; + + /* The kernel claims to have given a predictable name, keep it. */ + log_device_debug(device, "Policy *%s*: keeping predictable kernel name", + name_policy_to_string(policy)); + goto no_rename; + case NAMEPOLICY_DATABASE: + (void) sd_device_get_property_value(device, "ID_NET_NAME_FROM_DATABASE", &new_name); + break; + case NAMEPOLICY_ONBOARD: + (void) sd_device_get_property_value(device, "ID_NET_NAME_ONBOARD", &new_name); + break; + case NAMEPOLICY_SLOT: + (void) sd_device_get_property_value(device, "ID_NET_NAME_SLOT", &new_name); + break; + case NAMEPOLICY_PATH: + (void) sd_device_get_property_value(device, "ID_NET_NAME_PATH", &new_name); + break; + case NAMEPOLICY_MAC: + (void) sd_device_get_property_value(device, "ID_NET_NAME_MAC", &new_name); + break; + default: + assert_not_reached("invalid policy"); + } + } + + if (new_name) + log_device_debug(device, "Policy *%s* yields \"%s\".", name_policy_to_string(policy), new_name); + else if (config->name) { + new_name = config->name; + log_device_debug(device, "Policies didn't yield a name, using specified Name=%s.", new_name); } else - new_name = NULL; + log_device_debug(device, "Policies didn't yield a name and Name= is not given, not renaming."); + no_rename: switch (config->mac_policy) { case MACPOLICY_PERSISTENT: From 3907446f02d5966e795a7361860f7189c8b3ccc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 14:26:29 +0100 Subject: [PATCH 5/9] link-config: add "keep" policy and use it by default If "keep" policy is specified, and the interface has a name that is NET_NAME_USER or NET_NAME_RENAMED, we stop processing rules. "keep" should probably be specified either first or last depending on the preference. This partially reimplements 55b6530baacf4658a183b15b010a8cf3483fde08, in the sense that if the "keep" policy is not specified, and if the interface has a NamingPolicy, it will be renamed, even if it had a name previously. So this breaks backwards compatibility in this case, but that's more in line with what users expect. Closes #9006. --- man/systemd.link.xml | 24 +++++++++++++----------- network/99-default.link | 2 +- src/udev/net/link-config.c | 17 ++++++++++------- src/udev/net/link-config.h | 1 + 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/man/systemd.link.xml b/man/systemd.link.xml index f74edd0186b..22713e03167 100644 --- a/man/systemd.link.xml +++ b/man/systemd.link.xml @@ -250,17 +250,12 @@ NamePolicy= - An ordered, space-separated list of policies by which - the interface name should be set. - NamePolicy may be disabled by specifying - net.ifnames=0 on the kernel command line. - Each of the policies may fail, and the first successful one - is used. The name is not set directly, but is exported to - udev as the property ID_NET_NAME, which - is, by default, used by a udev rule to set - NAME. If the name has already been set by - userspace, no renaming is performed. The available policies - are: + An ordered, space-separated list of policies by which the interface name should be set. + NamePolicy may be disabled by specifying net.ifnames=0 on the + kernel command line. Each of the policies may fail, and the first successful one is used. The name + is not set directly, but is exported to udev as the property ID_NET_NAME, which + is, by default, used by a udev rule to set NAME. The available policies are: + @@ -314,6 +309,13 @@ ID_NET_NAME_MAC. + + keep + + If the device already had a name given by userspace (as part of creation of the device + or a rename), keep it. + + diff --git a/network/99-default.link b/network/99-default.link index 561bf329e48..92fcbe83eab 100644 --- a/network/99-default.link +++ b/network/99-default.link @@ -8,5 +8,5 @@ # (at your option) any later version. [Link] -NamePolicy=kernel database onboard slot path +NamePolicy=keep kernel database onboard slot path MACAddressPolicy=persistent diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index dc0f02bde93..dfb00485d12 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -399,11 +399,6 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, (void) link_name_type(device, &name_type); - if (IN_SET(name_type, NET_NAME_USER, NET_NAME_RENAMED)) { - log_device_info(device, "Device already has a name given by userspace, not renaming."); - goto no_rename; - } - if (ctx->enable_name_policy && config->name_policy) for (NamePolicy *p = config->name_policy; !new_name && *p != _NAMEPOLICY_INVALID; p++) { policy = *p; @@ -417,6 +412,13 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, log_device_debug(device, "Policy *%s*: keeping predictable kernel name", name_policy_to_string(policy)); goto no_rename; + case NAMEPOLICY_KEEP: + if (!IN_SET(name_type, NET_NAME_USER, NET_NAME_RENAMED)) + continue; + + log_device_debug(device, "Policy *%s*: keeping existing userspace name", + name_policy_to_string(policy)); + goto no_rename; case NAMEPOLICY_DATABASE: (void) sd_device_get_property_value(device, "ID_NET_NAME_FROM_DATABASE", &new_name); break; @@ -503,7 +505,7 @@ int link_get_driver(link_config_ctx *ctx, sd_device *device, char **ret) { static const char* const mac_policy_table[_MACPOLICY_MAX] = { [MACPOLICY_PERSISTENT] = "persistent", [MACPOLICY_RANDOM] = "random", - [MACPOLICY_NONE] = "none" + [MACPOLICY_NONE] = "none", }; DEFINE_STRING_TABLE_LOOKUP(mac_policy, MACPolicy); @@ -512,11 +514,12 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_mac_policy, mac_policy, MACPolicy, static const char* const name_policy_table[_NAMEPOLICY_MAX] = { [NAMEPOLICY_KERNEL] = "kernel", + [NAMEPOLICY_KEEP] = "keep", [NAMEPOLICY_DATABASE] = "database", [NAMEPOLICY_ONBOARD] = "onboard", [NAMEPOLICY_SLOT] = "slot", [NAMEPOLICY_PATH] = "path", - [NAMEPOLICY_MAC] = "mac" + [NAMEPOLICY_MAC] = "mac", }; DEFINE_STRING_TABLE_LOOKUP(name_policy, NamePolicy); diff --git a/src/udev/net/link-config.h b/src/udev/net/link-config.h index 82049590346..1113b1052e7 100644 --- a/src/udev/net/link-config.h +++ b/src/udev/net/link-config.h @@ -22,6 +22,7 @@ typedef enum MACPolicy { typedef enum NamePolicy { NAMEPOLICY_KERNEL, + NAMEPOLICY_KEEP, NAMEPOLICY_DATABASE, NAMEPOLICY_ONBOARD, NAMEPOLICY_SLOT, From 35b351900fb7b5cf7a86813cb6549afaea2fe423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 14:47:56 +0100 Subject: [PATCH 6/9] udev: move naming-scheme bits into their own file --- src/udev/meson.build | 2 + src/udev/net/naming-scheme.c | 64 ++++++++++++++++++++++ src/udev/net/naming-scheme.h | 47 ++++++++++++++++ src/udev/udev-builtin-net_id.c | 99 +--------------------------------- 4 files changed, 114 insertions(+), 98 deletions(-) create mode 100644 src/udev/net/naming-scheme.c create mode 100644 src/udev/net/naming-scheme.h diff --git a/src/udev/meson.build b/src/udev/meson.build index a9d6c6363f2..9d3f6d1c562 100644 --- a/src/udev/meson.build +++ b/src/udev/meson.build @@ -41,6 +41,8 @@ libudev_core_sources = ''' net/link-config.h net/ethtool-util.c net/ethtool-util.h + net/naming-scheme.c + net/naming-scheme.h '''.split() if conf.get('HAVE_KMOD') == 1 diff --git a/src/udev/net/naming-scheme.c b/src/udev/net/naming-scheme.c new file mode 100644 index 00000000000..27cede5e2e5 --- /dev/null +++ b/src/udev/net/naming-scheme.c @@ -0,0 +1,64 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#include "alloc-util.h" +#include "naming-scheme.h" +#include "proc-cmdline.h" +#include "string-util.h" + +static const NamingScheme naming_schemes[] = { + { "v238", NAMING_V238 }, + { "v239", NAMING_V239 }, + { "v240", NAMING_V240 }, + /* … add more schemes here, as the logic to name devices is updated … */ +}; + +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++) + if (streq(naming_schemes[i].name, name)) + return naming_schemes + i; + + return NULL; +} + +const NamingScheme* naming_scheme(void) { + static const NamingScheme *cache = NULL; + _cleanup_free_ char *buffer = NULL; + const char *e, *k; + + if (cache) + return cache; + + /* Acquire setting from the kernel command line */ + (void) proc_cmdline_get_key("net.naming-scheme", 0, &buffer); + + /* Also acquire it from an env var */ + e = getenv("NET_NAMING_SCHEME"); + if (e) { + if (*e == ':') { + /* If prefixed with ':' the kernel cmdline takes precedence */ + k = buffer ?: e + 1; + } else + k = e; /* Otherwise the env var takes precedence */ + } else + k = buffer; + + if (k) { + cache = naming_scheme_from_name(k); + if (cache) { + log_info("Using interface naming scheme '%s'.", cache->name); + return cache; + } + + log_warning("Unknown interface naming scheme '%s' requested, ignoring.", k); + } + + cache = naming_scheme_from_name(DEFAULT_NET_NAMING_SCHEME); + assert(cache); + log_info("Using default interface naming scheme '%s'.", cache->name); + + return cache; +} diff --git a/src/udev/net/naming-scheme.h b/src/udev/net/naming-scheme.h new file mode 100644 index 00000000000..c7f9505ab7e --- /dev/null +++ b/src/udev/net/naming-scheme.h @@ -0,0 +1,47 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +#include + +#include "macro.h" + +/* So here's the deal: net_id is supposed to be an excercise in providing stable names for network devices. However, we + * also want to keep updating the naming scheme used in future versions of net_id. These two goals of course are + * contradictory: on one hand we want things to not change and on the other hand we want them to improve. Our way out + * of this dilemma is to introduce the "naming scheme" concept: each time we improve the naming logic we define a new + * flag for it. Then, we keep a list of schemes, each identified by a name associated with the flags it implements. Via + * a kernel command line and environment variable we then allow the user to pick the scheme they want us to follow: + * installers could "freeze" the used scheme at the moment of installation this way. + * + * Developers: each time you tweak the naming logic here, define a new flag below, and condition the tweak with + * it. Each time we do a release we'll then add a new scheme entry and include all newly defined flags. + * + * Note that this is only half a solution to the problem though: not only udev/net_id gets updated all the time, the + * kernel gets too. And thus a kernel that previously didn't expose some sysfs attribute we look for might eventually + * do, and thus affect our naming scheme too. Thus, enforcing a naming scheme will make interfacing more stable across + * OS versions, but not fully stabilize them. */ +typedef enum NamingSchemeFlags { + /* First, the individual features */ + NAMING_SR_IOV_V = 1 << 0, /* Use "v" suffix for SR-IOV, see 609948c7043a40008b8299529c978ed8e11de8f6*/ + NAMING_NPAR_ARI = 1 << 1, /* Use NPAR "ARI", see 6bc04997b6eab35d1cb9fa73889892702c27be09 */ + NAMING_INFINIBAND = 1 << 2, /* Use "ib" prefix for infiniband, see 938d30aa98df887797c9e05074a562ddacdcdf5e */ + NAMING_ZERO_ACPI_INDEX = 1 << 3, /* Allow zero acpi_index field, see d81186ef4f6a888a70f20a1e73a812d6acb9e22f */ + + /* And now the masks that combine the features above */ + NAMING_V238 = 0, + NAMING_V239 = NAMING_V238 | NAMING_SR_IOV_V | NAMING_NPAR_ARI, + NAMING_V240 = NAMING_V239 | NAMING_INFINIBAND | NAMING_ZERO_ACPI_INDEX, + + _NAMING_SCHEME_FLAGS_INVALID = -1, +} NamingSchemeFlags; + +typedef struct NamingScheme { + const char *name; + NamingSchemeFlags flags; +} NamingScheme; + +const NamingScheme* naming_scheme(void); + +static inline bool naming_scheme_has(NamingSchemeFlags flags) { + return FLAGS_SET(naming_scheme()->flags, flags); +} diff --git a/src/udev/udev-builtin-net_id.c b/src/udev/udev-builtin-net_id.c index e4d40b149cc..03b281a7710 100644 --- a/src/udev/udev-builtin-net_id.c +++ b/src/udev/udev-builtin-net_id.c @@ -106,6 +106,7 @@ #include "fd-util.h" #include "fileio.h" #include "fs-util.h" +#include "naming-scheme.h" #include "parse-util.h" #include "proc-cmdline.h" #include "stdio-util.h" @@ -116,48 +117,6 @@ #define ONBOARD_INDEX_MAX (16*1024-1) -/* So here's the deal: net_id is supposed to be an excercise in providing stable names for network devices. However, we - * also want to keep updating the naming scheme used in future versions of net_id. These two goals of course are - * contradictory: on one hand we want things to not change and on the other hand we want them to improve. Our way out - * of this dilemma is to introduce the "naming scheme" concept: each time we improve the naming logic we define a new - * flag for it. Then, we keep a list of schemes, each identified by a name associated with the flags it implements. Via - * a kernel command line and environment variable we then allow the user to pick the scheme they want us to follow: - * installers could "freeze" the used scheme at the moment of installation this way. - * - * Developers: each time you tweak the naming logic here, define a new flag below, and condition the tweak with - * it. Each time we do a release we'll then add a new scheme entry and include all newly defined flags. - * - * Note that this is only half a solution to the problem though: not only udev/net_id gets updated all the time, the - * kernel gets too. And thus a kernel that previously didn't expose some sysfs attribute we look for might eventually - * do, and thus affect our naming scheme too. Thus, enforcing a naming scheme will make interfacing more stable across - * OS versions, but not fully stabilize them. */ -typedef enum NamingSchemeFlags { - /* First, the individual features */ - NAMING_SR_IOV_V = 1 << 0, /* Use "v" suffix for SR-IOV, see 609948c7043a40008b8299529c978ed8e11de8f6*/ - NAMING_NPAR_ARI = 1 << 1, /* Use NPAR "ARI", see 6bc04997b6eab35d1cb9fa73889892702c27be09 */ - NAMING_INFINIBAND = 1 << 2, /* Use "ib" prefix for infiniband, see 938d30aa98df887797c9e05074a562ddacdcdf5e */ - NAMING_ZERO_ACPI_INDEX = 1 << 3, /* Allow zero acpi_index field, see d81186ef4f6a888a70f20a1e73a812d6acb9e22f */ - - /* And now the masks that combine the features above */ - NAMING_V238 = 0, - NAMING_V239 = NAMING_V238|NAMING_SR_IOV_V|NAMING_NPAR_ARI, - NAMING_V240 = NAMING_V239|NAMING_INFINIBAND|NAMING_ZERO_ACPI_INDEX, - - _NAMING_SCHEME_FLAGS_INVALID = -1, -} NamingSchemeFlags; - -typedef struct NamingScheme { - const char *name; - NamingSchemeFlags flags; -} NamingScheme; - -static const NamingScheme naming_schemes[] = { - { "v238", NAMING_V238 }, - { "v239", NAMING_V239 }, - { "v240", NAMING_V240 }, - /* … add more schemes here, as the logic to name devices is updated … */ -}; - enum netname_type{ NET_UNDEF, NET_PCI, @@ -193,62 +152,6 @@ struct virtfn_info { char suffix[IFNAMSIZ]; }; -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++) - if (streq(naming_schemes[i].name, name)) - return naming_schemes + i; - - return NULL; -} - -static const NamingScheme* naming_scheme(void) { - static const NamingScheme *cache = NULL; - _cleanup_free_ char *buffer = NULL; - const char *e, *k; - - if (cache) - return cache; - - /* Acquire setting from the kernel command line */ - (void) proc_cmdline_get_key("net.naming-scheme", 0, &buffer); - - /* Also acquire it from an env var */ - e = getenv("NET_NAMING_SCHEME"); - if (e) { - if (*e == ':') { - /* If prefixed with ':' the kernel cmdline takes precedence */ - k = buffer ?: e + 1; - } else - k = e; /* Otherwise the env var takes precedence */ - } else - k = buffer; - - if (k) { - cache = naming_scheme_from_name(k); - if (cache) { - log_info("Using interface naming scheme '%s'.", cache->name); - return cache; - } - - log_warning("Unknown interface naming scheme '%s' requested, ignoring.", k); - } - - cache = naming_scheme_from_name(DEFAULT_NET_NAMING_SCHEME); - assert(cache); - log_info("Using default interface naming scheme '%s'.", cache->name); - - return cache; -} - -static bool naming_scheme_has(NamingSchemeFlags flags) { - return FLAGS_SET(naming_scheme()->flags, flags); -} - /* skip intermediate virtio devices */ static sd_device *skip_virtio(sd_device *dev) { sd_device *parent; From 73d2bb088166ecd11d1bc375c87a13218f89849a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 14:53:49 +0100 Subject: [PATCH 7/9] link-config: default to "keep" policy if naming-scheme<=239 is used This makes the new (>=240) behaviour conditional, restoring backwards compat, as least as long as an old naming scheme is used. --- src/udev/net/link-config.c | 7 +++++++ src/udev/net/naming-scheme.h | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index dfb00485d12..eb2477cea41 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -14,6 +14,7 @@ #include "link-config.h" #include "log.h" #include "missing_network.h" +#include "naming-scheme.h" #include "netlink-util.h" #include "network-internal.h" #include "parse-util.h" @@ -399,6 +400,12 @@ int link_config_apply(link_config_ctx *ctx, link_config *config, (void) link_name_type(device, &name_type); + if (IN_SET(name_type, NET_NAME_USER, NET_NAME_RENAMED) + && !naming_scheme_has(NAMING_ALLOW_RERENAMES)) { + log_device_debug(device, "Device already has a name given by userspace, not renaming."); + goto no_rename; + } + if (ctx->enable_name_policy && config->name_policy) for (NamePolicy *p = config->name_policy; !new_name && *p != _NAMEPOLICY_INVALID; p++) { policy = *p; diff --git a/src/udev/net/naming-scheme.h b/src/udev/net/naming-scheme.h index c7f9505ab7e..0b3d9bff1d7 100644 --- a/src/udev/net/naming-scheme.h +++ b/src/udev/net/naming-scheme.h @@ -26,11 +26,12 @@ typedef enum NamingSchemeFlags { NAMING_NPAR_ARI = 1 << 1, /* Use NPAR "ARI", see 6bc04997b6eab35d1cb9fa73889892702c27be09 */ NAMING_INFINIBAND = 1 << 2, /* Use "ib" prefix for infiniband, see 938d30aa98df887797c9e05074a562ddacdcdf5e */ NAMING_ZERO_ACPI_INDEX = 1 << 3, /* Allow zero acpi_index field, see d81186ef4f6a888a70f20a1e73a812d6acb9e22f */ + NAMING_ALLOW_RERENAMES = 1 << 4, /* Allow re-renaming of devices, see #9006 */ /* And now the masks that combine the features above */ NAMING_V238 = 0, NAMING_V239 = NAMING_V238 | NAMING_SR_IOV_V | NAMING_NPAR_ARI, - NAMING_V240 = NAMING_V239 | NAMING_INFINIBAND | NAMING_ZERO_ACPI_INDEX, + NAMING_V240 = NAMING_V239 | NAMING_INFINIBAND | NAMING_ZERO_ACPI_INDEX | NAMING_ALLOW_RERENAMES, _NAMING_SCHEME_FLAGS_INVALID = -1, } NamingSchemeFlags; From 08e1fe4249b15862d688c8e11ec254f24032def1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 16 Jan 2019 22:38:22 +0100 Subject: [PATCH 8/9] NEWS: describe the naming scheme updates --- NEWS | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/NEWS b/NEWS index ee926a12033..8cc45365042 100644 --- a/NEWS +++ b/NEWS @@ -22,6 +22,23 @@ CHANGES WITH 241 in spe: * $DBUS_SESSION_BUS_ADDRESS environment variable is set by pam_systemd again. + * A new network device NamePolicy "keep" is implemented for link files, + and used by default in 99-default.link (the fallback configuration + provided by systemd). With this policy, if the network device name + was already set by userspace, the device will not be renamed again. + This matches the naming scheme that was implemented before + systemd-240. If naming-scheme < 240 is specified, the "keep" policy + is also enabled by default, even if not specified. Effectively, this + means that if naming-scheme >= 240 is specified, network devices will + be renamed according to the configuration, even if they have been + renamed already, if "keep" is not specified as the naming policy in + the .link file. The 99-default.link file provided by systemd includes + "keep" for backwards compatibility, but it is recommended for user + installed .link files to *not* include it. + + The "kernel" policy, which keeps kernel names declared to be + "persistent", now works again as documented. + * kernel-install script now optionally takes a path to an initrd file, and passes it to all plugins. @@ -492,6 +509,11 @@ CHANGES WITH 240: * $DBUS_SESSION_BUS_ADDRESS environment variable is not set by pam_systemd anymore. + * The naming scheme for network devices was changed to always rename + devices, even if they were already renamed by userspace. The "kernel" + policy was changed to only apply as a fallback, if no other naming + policy took effect. + * The requirements to build systemd is bumped to meson-0.46 and python-3.5. From d7dce7b6fb079948f5b5bf449b20548487b546a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 17 Jan 2019 13:53:00 +0100 Subject: [PATCH 9/9] man: use and