fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-04 23:50:16 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

core: fix match spec behavior for a list of all "except:"

Browse source 
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.
This commit is contained in:
Thomas Haller 2018-12-11 12:35:40 +01:00
parent d48904c9a9
commit a7ef23b326
3 changed files with 81 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
man/NetworkManager.conf.xml
View file

@ -1217,6 +1217,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.
<programlisting>
# enable the configuration either when the environment variable
# is present or the version is at least 1.2.0.
@ -1411,7 +1413,11 @@ enable=nm-version-min:1.3,nm-version-min:1.2.6,nm-version-min:1.0.16
<term>except:SPEC</term>
<listitem><para>Negative match of a device. <literal>SPEC</literal> must be explicitly qualified with
a prefix such as <literal>interface-name:</literal>. A negative match has higher priority then the positive
matches above.</para></listitem>
matches above.</para>
<para>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, <literal>"except:interface-name:eth0"</literal> is the same as
<literal>"*,except:interface-name:eth0"</literal>.</para></listitem>
</varlistentry>
<varlistentry>
<term>SPEC[,;]SPEC</term>

96
src/nm-core-utils.c
View file

@ -1303,6 +1303,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)
{
@ -1409,9 +1437,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),
@ -1431,19 +1461,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)
@ -1451,10 +1471,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;
}
@ -1464,11 +1488,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
@ -1538,7 +1563,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;
@ -1553,6 +1581,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))
@ -1562,15 +1601,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

9
src/tests/test-general.c
View file

@ -1236,8 +1236,8 @@ test_match_spec_device (void)
NULL,
NM_MAKE_STRV ("em*"));
_do_test_match_spec_device ("except:interface-name:em*",
NULL,
NM_MAKE_STRV ("", "eth", "eth1", "e1"),
NM_MAKE_STRV (NULL),
NM_MAKE_STRV ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"));
_do_test_match_spec_device ("aa,bb,cc\\,dd,e,,",
NM_MAKE_STRV ("aa", "bb", "cc,dd", "e"),
@ -1310,7 +1310,8 @@ _do_test_match_spec_config (const char *file, int line, const char *spec_str, gu
if (expected != match_result)
g_error ("%s:%d: failed 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);
@ -1318,7 +1319,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);
@ -1402,7 +1403,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);