From 51bd7dc4aa8b69719c17d8c322a5413274a6e7b0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 11 Dec 2018 12:14:34 +0100 Subject: [PATCH 1/4] device: fix matching device-spec for DHCP plugin https://bugzilla.redhat.com/show_bug.cgi?id=1658057 Fixes: b9eb264efe6dec856d5e30f0c48a62017bad1466 (cherry picked from commit 1bff602c460a483e817741fc7811bc6cfcb3ed64) --- src/devices/nm-device.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index c1737afa3e..df1b28d39b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -15778,8 +15778,8 @@ nm_device_spec_match_list_full (NMDevice *self, const GSList *specs, int no_matc nm_device_get_driver (self), nm_device_get_driver_version (self), nm_device_get_permanent_hw_address (self), - nm_dhcp_manager_get_config (nm_dhcp_manager_get ()), - klass->get_s390_subchannels ? klass->get_s390_subchannels (self) : NULL); + klass->get_s390_subchannels ? klass->get_s390_subchannels (self) : NULL, + nm_dhcp_manager_get_config (nm_dhcp_manager_get ())); switch (m) { case NM_MATCH_SPEC_MATCH: From 593af85d31dab55e70c55551a118515dad601d0d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 11 Dec 2018 13:20:55 +0100 Subject: [PATCH 2/4] src/tests: add test for except match spec (cherry picked from commit d48904c9a90bed99eb3d7cb84fc2da016e15190b) --- src/tests/test-general.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 71efe5fb6d..1010467043 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -1233,6 +1233,10 @@ test_match_spec_device (void) S ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"), NULL, S ("em*")); + _do_test_match_spec_device ("except:interface-name:em*", + NULL, + S ("", "eth", "eth1", "e1"), + S ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3")); _do_test_match_spec_device ("aa,bb,cc\\,dd,e,,", S ("aa", "bb", "cc,dd", "e"), NULL, From 8d34d23d529ecc58641563fd0796228719b61d46 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 11 Dec 2018 12:35:40 +0100 Subject: [PATCH 3/4] core: fix match spec behavior for a list of all "except:" If the spec specifies only negative matches (and none of them matches), then the result shall be positive. Meaning: [connection*] match-device=except:dhcp-plugin:dhclient [connection*] match-device=except:interface-name:eth0 [.config] enabled=except:nm-version:1.14 should be the same as: [connection*] match-device=*,except:dhcp-plugin:dhclient [connection*] match-device=*,except:interface-name:eth0 [.config] enabled=*,except:nm-version:1.14 and match by default. Previously, such specs would never yield a positive match, which seems wrong. Note that "except:" already has a special meaning. It is not merely "not:". That is because we don't support "and:" nor grouping, but all matches are combined by an implicit "or:". With such a meaning, having a "not:" would be unclear to define. Instead it is defined that any "except:" match always wins and makes the entire condition to explicitly not match. As such, it makes sense to treat a match that only consists of "except:" matches special. This is a change in behavior, but the alternative meaning makes little sense. (cherry picked from commit a7ef23b326f1163adbe3b573ce78d71bfb186c1f) --- man/NetworkManager.conf.xml | 8 +++- src/nm-core-utils.c | 96 ++++++++++++++++++++++++++----------- src/tests/test-general.c | 9 ++-- 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index 71c217ae22..94a23fe8b3 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -1170,6 +1170,8 @@ enable=env:TAG1 be used to negate the match. Note that if one except-predicate matches, the entire configuration will be disabled. In other words, a except predicate always wins over other predicates. + If the setting only consists of "except:" matches and none of the + negative conditions are satisfied, the configuration is still enabled. # enable the configuration either when the environment variable # is present or the version is at least 1.2.0. @@ -1364,7 +1366,11 @@ enable=nm-version-min:1.3,nm-version-min:1.2.6,nm-version-min:1.0.16 except:SPEC Negative match of a device. SPEC must be explicitly qualified with a prefix such as interface-name:. A negative match has higher priority then the positive - matches above. + matches above. + If there is a list consisting only of negative matches, the behavior is the same as if there + is also match-all. That means, if none of all the negative matches is satisfied, the overall result is + still a positive match. That means, "except:interface-name:eth0" is the same as + "*,except:interface-name:eth0". SPEC[,;]SPEC diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index ec5dbe635d..f0efee461f 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -1295,6 +1295,34 @@ match_device_hwaddr_eval (const char *spec_str, _has; \ }) +static NMMatchSpecMatchType +_match_result (gboolean has_except, + gboolean has_not_except, + gboolean has_match, + gboolean has_match_except) +{ + if ( has_except + && !has_not_except) { + /* a match spec that only consists of a list of except matches is treated specially. */ + nm_assert (!has_match); + if (has_match_except) { + /* one of the "except:" matches matched. The result is an explicit + * negative match. */ + return NM_MATCH_SPEC_NEG_MATCH; + } else { + /* none of the "except:" matches matched. The result is a positive match, + * despite there being no positive match. */ + return NM_MATCH_SPEC_MATCH; + } + } + + if (has_match_except) + return NM_MATCH_SPEC_NEG_MATCH; + if (has_match) + return NM_MATCH_SPEC_MATCH; + return NM_MATCH_SPEC_NO_MATCH; +} + static const char * match_except (const char *spec_str, gboolean *out_except) { @@ -1401,9 +1429,11 @@ nm_match_spec_device (const GSList *specs, const char *dhcp_plugin) { const GSList *iter; - NMMatchSpecMatchType match; + gboolean has_match = FALSE; + gboolean has_match_except = FALSE; + gboolean has_except = FALSE; + gboolean has_not_except = FALSE; const char *spec_str; - gboolean except; MatchDeviceData match_data = { .interface_name = interface_name, .device_type = nm_str_not_empty (device_type), @@ -1423,19 +1453,9 @@ nm_match_spec_device (const GSList *specs, if (!specs) return NM_MATCH_SPEC_NO_MATCH; - match = NM_MATCH_SPEC_NO_MATCH; - - /* pre-search for "*" */ for (iter = specs; iter; iter = iter->next) { - spec_str = iter->data; + gboolean except; - if (spec_str && spec_str[0] == '*' && spec_str[1] == '\0') { - match = NM_MATCH_SPEC_MATCH; - break; - } - } - - for (iter = specs; iter; iter = iter->next) { spec_str = iter->data; if (!spec_str || !*spec_str) @@ -1443,10 +1463,14 @@ nm_match_spec_device (const GSList *specs, spec_str = match_except (spec_str, &except); - if ( !except - && match == NM_MATCH_SPEC_MATCH) { - /* we have no "except-match" but already match. No need to evaluate - * the match, we cannot match stronger. */ + if (except) + has_except = TRUE; + else + has_not_except = TRUE; + + if ( ( except && has_match_except) + || (!except && has_match)) { + /* evaluating the match does not give new information. Skip it. */ continue; } @@ -1456,11 +1480,12 @@ nm_match_spec_device (const GSList *specs, continue; if (except) - return NM_MATCH_SPEC_NEG_MATCH; - match = NM_MATCH_SPEC_MATCH; + has_match_except = TRUE; + else + has_match = TRUE; } - return match; + return _match_result (has_except, has_not_except, has_match, has_match_except); } static gboolean @@ -1530,7 +1555,10 @@ NMMatchSpecMatchType nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env) { const GSList *iter; - NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH; + gboolean has_match = FALSE; + gboolean has_match_except = FALSE; + gboolean has_except = FALSE; + gboolean has_not_except = FALSE; if (!specs) return NM_MATCH_SPEC_NO_MATCH; @@ -1545,6 +1573,17 @@ nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env spec_str = match_except (spec_str, &except); + if (except) + has_except = TRUE; + else + has_not_except = TRUE; + + if ( ( except && has_match_except) + || (!except && has_match)) { + /* evaluating the match does not give new information. Skip it. */ + continue; + } + if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_NM_VERSION)) v_match = match_config_eval (spec_str, MATCH_TAG_CONFIG_NM_VERSION, cur_nm_version); else if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_NM_VERSION_MIN)) @@ -1554,15 +1593,18 @@ nm_match_spec_config (const GSList *specs, guint cur_nm_version, const char *env else if (_MATCH_CHECK (spec_str, MATCH_TAG_CONFIG_ENV)) v_match = env && env[0] && !strcmp (spec_str, env); else + v_match = FALSE; + + if (!v_match) continue; - if (v_match) { - if (except) - return NM_MATCH_SPEC_NEG_MATCH; - match = NM_MATCH_SPEC_MATCH; - } + if (except) + has_match_except = TRUE; + else + has_match = TRUE; } - return match; + + return _match_result (has_except, has_not_except, has_match, has_match_except); } #undef _MATCH_CHECK diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 1010467043..9dce943527 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -1234,8 +1234,8 @@ test_match_spec_device (void) NULL, S ("em*")); _do_test_match_spec_device ("except:interface-name:em*", - NULL, S ("", "eth", "eth1", "e1"), + S (NULL), S ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3")); _do_test_match_spec_device ("aa,bb,cc\\,dd,e,,", S ("aa", "bb", "cc,dd", "e"), @@ -1309,7 +1309,8 @@ _do_test_match_spec_config (const char *file, int line, const char *spec_str, gu if (expected != match_result) g_error ("%s:%d: faild comparing \"%s\" with %u.%u.%u. Expected %d, but got %d", file, line, spec_str, v_maj, v_min, v_mic, (int) expected, (int) match_result); - if (g_slist_length (specs) == 1 && match_result != NM_MATCH_SPEC_NEG_MATCH) { + if ( g_slist_length (specs) == 1 + && !g_str_has_prefix (specs->data, "except:")) { /* there is only one spec in the list... test that we match except: */ char *sss = g_strdup_printf ("except:%s", (char *) specs->data); GSList *specs2 = g_slist_append (NULL, sss); @@ -1317,7 +1318,7 @@ _do_test_match_spec_config (const char *file, int line, const char *spec_str, gu match_result2 = nm_match_spec_config (specs2, version, NULL); if (match_result == NM_MATCH_SPEC_NO_MATCH) - g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_NO_MATCH); + g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_MATCH); else g_assert_cmpint (match_result2, ==, NM_MATCH_SPEC_NEG_MATCH); @@ -1401,7 +1402,7 @@ test_match_spec_config (void) do_test_match_spec_config ("nm-version-max:1", 1, 4, 30, NM_MATCH_SPEC_MATCH); do_test_match_spec_config ("nm-version-max:1", 2, 4, 30, NM_MATCH_SPEC_NO_MATCH); - do_test_match_spec_config ("except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_NO_MATCH); + do_test_match_spec_config ("except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_MATCH); do_test_match_spec_config ("nm-version-min:1.6,except:nm-version:1.4.8", 1, 6, 0, NM_MATCH_SPEC_MATCH); do_test_match_spec_config ("nm-version-min:1.6,nm-version-min:1.4.6,nm-version-min:1.2.16,except:nm-version:1.4.8", 1, 2, 0, NM_MATCH_SPEC_NO_MATCH); From 0fb8a25a61a440bb7479c416835d44f1bd888ea3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 11 Dec 2018 12:56:53 +0100 Subject: [PATCH 4/4] contrib/rpm: adjust match-device spec for 00-server-dhcp-client-id.conf For older NetworkManager versions, a match spec that only contained except: specifiers could never yield a positive match. That is not very useful and got fixed by commit 242de347adbf653a709607979d36a0da1ca3ff0b (core: fix device spec matching for a list of "except:"). Still, adjust the configuration snippet so that it also works with configurations that predate the fix. (cherry picked from commit 3627601bde023e5fc3c55a70025b687d2af20d66) --- contrib/fedora/rpm/00-server-dhcp-client-id.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/fedora/rpm/00-server-dhcp-client-id.conf b/contrib/fedora/rpm/00-server-dhcp-client-id.conf index afaf0ca9a1..0cd3cd7167 100644 --- a/contrib/fedora/rpm/00-server-dhcp-client-id.conf +++ b/contrib/fedora/rpm/00-server-dhcp-client-id.conf @@ -2,5 +2,5 @@ # But don't do so for dhclient DHCP plugin, as the default of dhclient may # be specified via /etc/dhcp (and anyway defaults to "hardware" already). [connection-00-server-dhcp-client-id] -match-device=except:dhcp-plugin:dhclient +match-device=*,except:dhcp-plugin:dhclient ipv4.dhcp-client-id=mac