From 9080ad696d6b2339db266486b15af2b052265a2f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Jan 2015 18:58:20 +0100 Subject: [PATCH 1/7] core/test: add test for nm_match_spec() --- src/tests/test-general.c | 84 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/src/tests/test-general.c b/src/tests/test-general.c index f118156b3..275fab29a 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -774,6 +774,89 @@ test_nm_utils_uuid_generate_from_strings (void) /*******************************************/ +static const char *_test_match_spec_all[] = { + "e", + "em", + "em*", + "em\\", + "em\\*", + "em\\1", + "em\\11", + "em\\2", + "em1", + "em11", + "em2", + "=em*", + NULL +}; + +static gboolean +_test_match_spec_contains (const char **matches, const char *match) +{ + guint i; + + for (i = 0; matches && matches[i]; i++) { + if (strcmp (match, matches[i]) == 0) + return TRUE; + } + return FALSE; +} + +static void +test_match_spec_ifname (const char *spec_str, const char **matches) +{ + const char *m; + char **spec_str_split; + GSList *specs, *specs_reverse = NULL; + guint i; + + g_assert (spec_str); + spec_str_split = g_strsplit_set (spec_str, ";,", -1); + for (i = 0; spec_str_split[i]; i++) { + if (spec_str_split[i]) + specs_reverse = g_slist_prepend (specs_reverse, spec_str_split[i]); + } + specs = g_slist_reverse (g_slist_copy (specs_reverse)); + + for (i = 0; matches && matches[i]; i++) { + g_assert (nm_match_spec_interface_name (specs, matches[i])); + g_assert (nm_match_spec_interface_name (specs_reverse, matches[i])); + } + for (i = 0; (m = _test_match_spec_all[i]); i++) { + if (_test_match_spec_contains (matches, m)) + continue; + g_assert (!nm_match_spec_interface_name (specs, m)); + g_assert (!nm_match_spec_interface_name (specs_reverse, m)); + } + + g_slist_free (specs_reverse); + g_slist_free (specs); + g_strfreev (spec_str_split); +} + +static void +test_nm_match_spec_interface_name (void) +{ +#define S(...) ((const char *[]) { __VA_ARGS__, NULL } ) + test_match_spec_ifname ("em1", + S ("em1")); + test_match_spec_ifname ("em1,em2", + S ("em1", "em2")); + test_match_spec_ifname ("em1,em2,interface-name:em2", + S ("em1", "em2")); + test_match_spec_ifname ("interface-name:em1", + S ("em1")); + test_match_spec_ifname ("interface-name:em*", + S ("em*")); + test_match_spec_ifname ("interface-name:em\\*", + S ("em\\*")); + test_match_spec_ifname ("interface-name:=em*", + S ("=em*")); +#undef S +} + +/*******************************************/ + NMTST_DEFINE (); int @@ -797,6 +880,7 @@ main (int argc, char **argv) g_test_add_func ("/general/connection-sort/autoconnect-priority", test_connection_sort_autoconnect_priority); g_test_add_func ("/general/nm_utils_uuid_generate_from_strings", test_nm_utils_uuid_generate_from_strings); + g_test_add_func ("/general/nm_match_spec_interface_name", test_nm_match_spec_interface_name); return g_test_run (); } From 2b518538bec1a0a537cc49672549e5d978716e3b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jan 2015 16:42:58 +0100 Subject: [PATCH 2/7] core: rework matching of nm_match_spec() This includes several changes how to match device specs: - matching the interface name is no longer case-insenstive as interface names themselves are case-sensitive. - Now we skip patterns that start with "mac:" or "s390-subchannels:" for comparing interface names. Previously a spec "mac:1" would have matched an interface named "mac:1", now it doesn't. To match such an interface, you would have to specify "interface-name:mac:1". - previously, a pattern "a" would have matched an interface named "interface-name:a", now it doesn't. Since valid interface name (in the kernel) can be at most 15 characters long, this is however no problem. - if the spec has the prefix "interface-name:", we support simple globbing using GPatternSpec. Globbing without exact spec type will still not match "vboxnet*" -- with the exception of "*". You can disable globbing by putting an '=' immediately after the ':'. (a) "interface-name:em1" | matches "em1" (b) "interface-name:em*" | matches "em", "em1", "em2", etc. (c) "interface-name:em\*" | matches "em\", "em\1", etc. (d) "interface-name:=em*" | matches "em*" (e) "em*" | matches "em*" --- src/NetworkManagerUtils.c | 66 ++++++++++++++++++++++++++++----------- src/tests/test-general.c | 8 +++-- 2 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 5b4cac94e..e5ff3c265 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -877,6 +877,10 @@ nm_match_spec_string (const GSList *specs, const char *match) return FALSE; } +#define MAC_TAG "mac:" +#define INTERFACE_NAME_TAG "interface-name:" +#define SUBCHAN_TAG "s390-subchannels:" + gboolean nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr) { @@ -887,32 +891,57 @@ nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr) for (iter = specs; iter; iter = g_slist_next (iter)) { const char *spec_str = iter->data; - if ( !g_ascii_strncasecmp (spec_str, "mac:", 4) - && nm_utils_hwaddr_matches (spec_str + 4, -1, hwaddr, -1)) - return TRUE; + if (!spec_str || !*spec_str) + continue; + + if ( !g_ascii_strncasecmp (spec_str, INTERFACE_NAME_TAG, STRLEN (INTERFACE_NAME_TAG)) + || !g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) + continue; + + if (!g_ascii_strncasecmp (spec_str, MAC_TAG, STRLEN (MAC_TAG))) + spec_str += STRLEN (MAC_TAG); if (nm_utils_hwaddr_matches (spec_str, -1, hwaddr, -1)) return TRUE; } - return FALSE; } gboolean nm_match_spec_interface_name (const GSList *specs, const char *interface_name) { - char *iface_match; - gboolean matched; + const GSList *iter; g_return_val_if_fail (interface_name != NULL, FALSE); - if (nm_match_spec_string (specs, interface_name)) - return TRUE; + for (iter = specs; iter; iter = g_slist_next (iter)) { + const char *spec_str = iter->data; + gboolean use_pattern = FALSE; - iface_match = g_strdup_printf ("interface-name:%s", interface_name); - matched = nm_match_spec_string (specs, iface_match); - g_free (iface_match); - return matched; + if (!spec_str || !*spec_str) + continue; + + if ( !g_ascii_strncasecmp (spec_str, MAC_TAG, STRLEN (MAC_TAG)) + || !g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) + continue; + + if (!g_ascii_strncasecmp (spec_str, INTERFACE_NAME_TAG, STRLEN (INTERFACE_NAME_TAG))) { + spec_str += STRLEN (INTERFACE_NAME_TAG); + if (spec_str[0] == '=') + spec_str += 1; + else { + if (spec_str[0] == '~') + spec_str += 1; + use_pattern=TRUE; + } + } + + if (!strcmp (spec_str, interface_name)) + return TRUE; + if (use_pattern && g_pattern_match_simple (spec_str, interface_name)) + return TRUE; + } + return FALSE; } #define BUFSIZE 10 @@ -981,8 +1010,6 @@ parse_subchannels (const char *subchannels, guint32 *a, guint32 *b, guint32 *c) return TRUE; } -#define SUBCHAN_TAG "s390-subchannels:" - gboolean nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels) { @@ -996,11 +1023,14 @@ nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels) return FALSE; for (iter = specs; iter; iter = g_slist_next (iter)) { - const char *spec = iter->data; + const char *spec_str = iter->data; - if (!strncmp (spec, SUBCHAN_TAG, strlen (SUBCHAN_TAG))) { - spec += strlen (SUBCHAN_TAG); - if (parse_subchannels (spec, &spec_a, &spec_b, &spec_c)) { + if (!spec_str || !*spec_str) + continue; + + if (!g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) { + spec_str += STRLEN (SUBCHAN_TAG); + if (parse_subchannels (spec_str, &spec_a, &spec_b, &spec_c)) { if (a == spec_a && b == spec_b && c == spec_c) return TRUE; } diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 275fab29a..15320191b 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -847,11 +847,13 @@ test_nm_match_spec_interface_name (void) test_match_spec_ifname ("interface-name:em1", S ("em1")); test_match_spec_ifname ("interface-name:em*", - S ("em*")); + S ("em", "em*", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3")); test_match_spec_ifname ("interface-name:em\\*", - S ("em\\*")); + S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2")); + test_match_spec_ifname ("interface-name:~em\\*", + S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2")); test_match_spec_ifname ("interface-name:=em*", - S ("=em*")); + S ("em*")); #undef S } From 2051944333d0f7d12cbeed90db28a0cb3c4f7132 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jan 2015 17:05:22 +0100 Subject: [PATCH 3/7] core: remove nm_match_spec_string() It was only used to match against "*", in a case-insensitive way. --- src/NetworkManagerUtils.c | 13 ------------- src/NetworkManagerUtils.h | 1 - src/devices/nm-device.c | 7 +++++-- 3 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index e5ff3c265..0e4e17b00 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -864,19 +864,6 @@ nm_utils_find_helper(const char *progname, const char *try_first, GError **error /******************************************************************************************/ -gboolean -nm_match_spec_string (const GSList *specs, const char *match) -{ - const GSList *iter; - - for (iter = specs; iter; iter = g_slist_next (iter)) { - if (!g_ascii_strcasecmp ((const char *) iter->data, match)) - return TRUE; - } - - return FALSE; -} - #define MAC_TAG "mac:" #define INTERFACE_NAME_TAG "interface-name:" #define SUBCHAN_TAG "s390-subchannels:" diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 049856556..7ed0c7aa9 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -96,7 +96,6 @@ const char *nm_utils_find_helper (const char *progname, const char *try_first, GError **error); -gboolean nm_match_spec_string (const GSList *specs, const char *string); gboolean nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr); gboolean nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); gboolean nm_match_spec_interface_name (const GSList *specs, const char *interface_name); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 9df3712ed..08ff47f8a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8088,9 +8088,12 @@ spec_match_list (NMDevice *self, const GSList *specs) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); gboolean matched = FALSE; + const GSList *iter; - if (nm_match_spec_string (specs, "*")) - return TRUE; + for (iter = specs; iter; iter = g_slist_next (iter)) { + if (!strcmp ((const char *) iter->data, "*")) + return TRUE; + } if (priv->hw_addr_len) matched = nm_match_spec_hwaddr (specs, priv->hw_addr); From 5c2e1afd1bd8442e1b2efc518e7109f516a00bff Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 23 Jan 2015 17:02:16 +0100 Subject: [PATCH 4/7] core: support "except:" spec to negate match Extend nm_match_spec_*() to support an "except:" prefix to negate the result of a match. "except:" only works when followed by an exact match type, for example "except:interface-name:vboxnet0", but not "except:vboxnet0". A matching "except:" spec always wins, regardless of other positive matchings. --- src/NetworkManagerUtils.c | 74 +++++++++++++++++++++++--------- src/NetworkManagerUtils.h | 12 ++++-- src/devices/nm-device-ethernet.c | 14 +++--- src/devices/nm-device.c | 27 ++++++------ src/devices/nm-device.h | 2 +- src/tests/test-general.c | 44 +++++++++++++------ 6 files changed, 120 insertions(+), 53 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 0e4e17b00..1f6cda4f6 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -867,47 +867,72 @@ nm_utils_find_helper(const char *progname, const char *try_first, GError **error #define MAC_TAG "mac:" #define INTERFACE_NAME_TAG "interface-name:" #define SUBCHAN_TAG "s390-subchannels:" +#define EXCEPT_TAG "except:" -gboolean +static const char * +_match_except (const char *spec_str, gboolean *out_except) +{ + if (!g_ascii_strncasecmp (spec_str, EXCEPT_TAG, STRLEN (EXCEPT_TAG))) { + spec_str += STRLEN (EXCEPT_TAG); + *out_except = TRUE; + } else + *out_except = FALSE; + return spec_str; +} + +NMMatchSpecMatchType nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr) { const GSList *iter; + NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH; - g_return_val_if_fail (hwaddr != NULL, FALSE); + g_return_val_if_fail (hwaddr != NULL, NM_MATCH_SPEC_NO_MATCH); for (iter = specs; iter; iter = g_slist_next (iter)) { const char *spec_str = iter->data; + gboolean except; if (!spec_str || !*spec_str) continue; + spec_str = _match_except (spec_str, &except); + if ( !g_ascii_strncasecmp (spec_str, INTERFACE_NAME_TAG, STRLEN (INTERFACE_NAME_TAG)) || !g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) continue; if (!g_ascii_strncasecmp (spec_str, MAC_TAG, STRLEN (MAC_TAG))) spec_str += STRLEN (MAC_TAG); + else if (except) + continue; - if (nm_utils_hwaddr_matches (spec_str, -1, hwaddr, -1)) - return TRUE; + if (nm_utils_hwaddr_matches (spec_str, -1, hwaddr, -1)) { + if (except) + return NM_MATCH_SPEC_NEG_MATCH; + match = NM_MATCH_SPEC_MATCH; + } } - return FALSE; + return match; } -gboolean +NMMatchSpecMatchType nm_match_spec_interface_name (const GSList *specs, const char *interface_name) { const GSList *iter; + NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH; - g_return_val_if_fail (interface_name != NULL, FALSE); + g_return_val_if_fail (interface_name != NULL, NM_MATCH_SPEC_NO_MATCH); for (iter = specs; iter; iter = g_slist_next (iter)) { const char *spec_str = iter->data; gboolean use_pattern = FALSE; + gboolean except; if (!spec_str || !*spec_str) continue; + spec_str = _match_except (spec_str, &except); + if ( !g_ascii_strncasecmp (spec_str, MAC_TAG, STRLEN (MAC_TAG)) || !g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) continue; @@ -921,14 +946,17 @@ nm_match_spec_interface_name (const GSList *specs, const char *interface_name) spec_str += 1; use_pattern=TRUE; } - } + } else if (except) + continue; - if (!strcmp (spec_str, interface_name)) - return TRUE; - if (use_pattern && g_pattern_match_simple (spec_str, interface_name)) - return TRUE; + if ( !strcmp (spec_str, interface_name) + || (use_pattern && g_pattern_match_simple (spec_str, interface_name))) { + if (except) + return NM_MATCH_SPEC_NEG_MATCH; + match = NM_MATCH_SPEC_MATCH; + } } - return FALSE; + return match; } #define BUFSIZE 10 @@ -997,34 +1025,40 @@ parse_subchannels (const char *subchannels, guint32 *a, guint32 *b, guint32 *c) return TRUE; } -gboolean +NMMatchSpecMatchType nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels) { const GSList *iter; guint32 a = 0, b = 0, c = 0; guint32 spec_a = 0, spec_b = 0, spec_c = 0; + NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH; - g_return_val_if_fail (subchannels != NULL, FALSE); + g_return_val_if_fail (subchannels != NULL, NM_MATCH_SPEC_NO_MATCH); if (!parse_subchannels (subchannels, &a, &b, &c)) - return FALSE; + return NM_MATCH_SPEC_NO_MATCH; for (iter = specs; iter; iter = g_slist_next (iter)) { const char *spec_str = iter->data; + gboolean except; if (!spec_str || !*spec_str) continue; + spec_str = _match_except (spec_str, &except); + if (!g_ascii_strncasecmp (spec_str, SUBCHAN_TAG, STRLEN (SUBCHAN_TAG))) { spec_str += STRLEN (SUBCHAN_TAG); if (parse_subchannels (spec_str, &spec_a, &spec_b, &spec_c)) { - if (a == spec_a && b == spec_b && c == spec_c) - return TRUE; + if (a == spec_a && b == spec_b && c == spec_c) { + if (except) + return NM_MATCH_SPEC_NEG_MATCH; + match = NM_MATCH_SPEC_MATCH; + } } } } - - return FALSE; + return match; } const char * diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 7ed0c7aa9..de691f138 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -96,9 +96,15 @@ const char *nm_utils_find_helper (const char *progname, const char *try_first, GError **error); -gboolean nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr); -gboolean nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); -gboolean nm_match_spec_interface_name (const GSList *specs, const char *interface_name); +typedef enum { + NM_MATCH_SPEC_NO_MATCH = 0, + NM_MATCH_SPEC_MATCH = 1, + NM_MATCH_SPEC_NEG_MATCH = 2, +} NMMatchSpecMatchType; + +NMMatchSpecMatchType nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr); +NMMatchSpecMatchType nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); +NMMatchSpecMatchType nm_match_spec_interface_name (const GSList *specs, const char *interface_name); const char *nm_utils_get_shared_wifi_permission (NMConnection *connection); diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index af24f8be6..24aa8967d 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -1495,15 +1495,19 @@ new_default_connection (NMDevice *self) return connection; } -static gboolean +static NMMatchSpecMatchType spec_match_list (NMDevice *device, const GSList *specs) { + NMMatchSpecMatchType matched = NM_MATCH_SPEC_NO_MATCH, m; NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (device); - if (priv->subchannels && nm_match_spec_s390_subchannels (specs, priv->subchannels)) - return TRUE; - - return NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->spec_match_list (device, specs); + if (priv->subchannels) + matched = nm_match_spec_s390_subchannels (specs, priv->subchannels); + if (matched != NM_MATCH_SPEC_NEG_MATCH) { + m = NM_DEVICE_CLASS (nm_device_ethernet_parent_class)->spec_match_list (device, specs); + matched = MAX (matched, m); + } + return matched; } static void diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 08ff47f8a..64140de7e 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8080,27 +8080,30 @@ nm_device_spec_match_list (NMDevice *self, const GSList *specs) if (!specs) return FALSE; - return NM_DEVICE_GET_CLASS (self)->spec_match_list (self, specs); + return NM_DEVICE_GET_CLASS (self)->spec_match_list (self, specs) == NM_MATCH_SPEC_MATCH; } -static gboolean +static NMMatchSpecMatchType spec_match_list (NMDevice *self, const GSList *specs) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - gboolean matched = FALSE; + NMMatchSpecMatchType matched = NM_MATCH_SPEC_NO_MATCH, m; const GSList *iter; for (iter = specs; iter; iter = g_slist_next (iter)) { - if (!strcmp ((const char *) iter->data, "*")) - return TRUE; + if (!strcmp ((const char *) iter->data, "*")) { + matched = NM_MATCH_SPEC_MATCH; + break; + } + } + if (priv->hw_addr_len) { + m = nm_match_spec_hwaddr (specs, priv->hw_addr); + matched = MAX (matched, m); + } + if (matched != NM_MATCH_SPEC_NEG_MATCH) { + m = nm_match_spec_interface_name (specs, nm_device_get_iface (self)); + matched = MAX (matched, m); } - - if (priv->hw_addr_len) - matched = nm_match_spec_hwaddr (specs, priv->hw_addr); - - if (!matched) - matched = nm_match_spec_interface_name (specs, nm_device_get_iface (self)); - return matched; } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index e6f07f2ef..d3b4139d3 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -187,7 +187,7 @@ typedef struct { /* Sync deactivating (in the DISCONNECTED phase) */ void (* deactivate) (NMDevice *self); - gboolean (* spec_match_list) (NMDevice *self, const GSList *specs); + NMMatchSpecMatchType (* spec_match_list) (NMDevice *self, const GSList *specs); /* Update the connection with currently configured L2 settings */ void (* update_connection) (NMDevice *device, NMConnection *connection); diff --git a/src/tests/test-general.c b/src/tests/test-general.c index 15320191b..b26b0e2ae 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -803,7 +803,7 @@ _test_match_spec_contains (const char **matches, const char *match) } static void -test_match_spec_ifname (const char *spec_str, const char **matches) +test_match_spec_ifname (const char *spec_str, const char **matches, const char **neg_matches) { const char *m; char **spec_str_split; @@ -819,14 +819,20 @@ test_match_spec_ifname (const char *spec_str, const char **matches) specs = g_slist_reverse (g_slist_copy (specs_reverse)); for (i = 0; matches && matches[i]; i++) { - g_assert (nm_match_spec_interface_name (specs, matches[i])); - g_assert (nm_match_spec_interface_name (specs_reverse, matches[i])); + g_assert (nm_match_spec_interface_name (specs, matches[i]) == NM_MATCH_SPEC_MATCH); + g_assert (nm_match_spec_interface_name (specs_reverse, matches[i]) == NM_MATCH_SPEC_MATCH); + } + for (i = 0; neg_matches && neg_matches[i]; i++) { + g_assert (nm_match_spec_interface_name (specs, neg_matches[i]) == NM_MATCH_SPEC_NEG_MATCH); + g_assert (nm_match_spec_interface_name (specs_reverse, neg_matches[i]) == NM_MATCH_SPEC_NEG_MATCH); } for (i = 0; (m = _test_match_spec_all[i]); i++) { if (_test_match_spec_contains (matches, m)) continue; - g_assert (!nm_match_spec_interface_name (specs, m)); - g_assert (!nm_match_spec_interface_name (specs_reverse, m)); + if (_test_match_spec_contains (neg_matches, m)) + continue; + g_assert (nm_match_spec_interface_name (specs, m) == NM_MATCH_SPEC_NO_MATCH); + g_assert (nm_match_spec_interface_name (specs_reverse, m) == NM_MATCH_SPEC_NO_MATCH); } g_slist_free (specs_reverse); @@ -839,20 +845,34 @@ test_nm_match_spec_interface_name (void) { #define S(...) ((const char *[]) { __VA_ARGS__, NULL } ) test_match_spec_ifname ("em1", - S ("em1")); + S ("em1"), + NULL); test_match_spec_ifname ("em1,em2", - S ("em1", "em2")); + S ("em1", "em2"), + NULL); test_match_spec_ifname ("em1,em2,interface-name:em2", - S ("em1", "em2")); + S ("em1", "em2"), + NULL); test_match_spec_ifname ("interface-name:em1", - S ("em1")); + S ("em1"), + NULL); test_match_spec_ifname ("interface-name:em*", - S ("em", "em*", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3")); + S ("em", "em*", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"), + NULL); test_match_spec_ifname ("interface-name:em\\*", - S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2")); + S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2"), + NULL); test_match_spec_ifname ("interface-name:~em\\*", - S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2")); + S ("em\\", "em\\*", "em\\1", "em\\11", "em\\2"), + NULL); test_match_spec_ifname ("interface-name:=em*", + S ("em*"), + NULL); + test_match_spec_ifname ("interface-name:em*,except:interface-name:em1*", + S ("em", "em*", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em2", "em3"), + S ("em1", "em11")); + test_match_spec_ifname ("interface-name:em*,except:interface-name:=em*", + S ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"), S ("em*")); #undef S } From 3bcc5e4bd0bd88b15ab8f84515f0fca52db62823 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Feb 2015 16:57:14 +0100 Subject: [PATCH 5/7] core: add nm_match_spec_split() function There are currently three device spec properties: 'main.ignore-carrier', 'main.no-auto-default' and 'keyfile.unmanaged-devices'. The first two, called g_key_file_parse_value_as_string() to split the string into individual device specs. This uses ',' as separator and supports escaping using '\\'. 'keyfile.unmanaged-devices' is split using ',' or ';' as separator without supporting escaping. Add a new function nm_match_spec_split(), to unify these two behaviors and support both formats. That is, both previous formats are mostly supported, but obviously there are some behavioral changes if the string contains one of '\\', ',', or ';'. nm_match_spec_split() is copied from glibs g_key_file_parse_value_as_string() and adjusted. --- src/NetworkManagerUtils.c | 68 +++++++++++++++++++++++++++++++++++++++ src/NetworkManagerUtils.h | 1 + src/tests/test-general.c | 25 ++++++++------ 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index 1f6cda4f6..04ef2aa76 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1061,6 +1061,74 @@ nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels) return match; } +GSList * +nm_match_spec_split (const char *value) +{ + char *string_value, *p, *q0, *q; + GSList *pieces = NULL; + + if (!value || !*value) + return NULL; + + /* Copied from glibs g_key_file_parse_value_as_string() function + * and adjusted. */ + + string_value = g_new (gchar, strlen (value) + 1); + + p = (gchar *) value; + q0 = q = string_value; + while (*p) { + if (*p == '\\') { + p++; + + switch (*p) { + case 's': + *q = ' '; + break; + case 'n': + *q = '\n'; + break; + case 't': + *q = '\t'; + break; + case 'r': + *q = '\r'; + break; + case '\\': + *q = '\\'; + break; + case '\0': + break; + default: + if (NM_IN_SET (*p, ',', ';')) + *q = *p; + else { + *q++ = '\\'; + *q = *p; + } + break; + } + } else { + *q = *p; + if (NM_IN_SET (*p, ',', ';')) { + if (q0 < q) + pieces = g_slist_prepend (pieces, g_strndup (q0, q - q0)); + q0 = q + 1; + } + } + if (*p == '\0') + break; + q++; + p++; + } + + *q = '\0'; + if (q0 < q) + pieces = g_slist_prepend (pieces, g_strndup (q0, q - q0)); + g_free (string_value); + return g_slist_reverse (pieces); +} + const char * nm_utils_get_shared_wifi_permission (NMConnection *connection) { diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index de691f138..4d797fafe 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -105,6 +105,7 @@ typedef enum { NMMatchSpecMatchType nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr); NMMatchSpecMatchType nm_match_spec_s390_subchannels (const GSList *specs, const char *subchannels); NMMatchSpecMatchType nm_match_spec_interface_name (const GSList *specs, const char *interface_name); +GSList *nm_match_spec_split (const char *value); const char *nm_utils_get_shared_wifi_permission (NMConnection *connection); diff --git a/src/tests/test-general.c b/src/tests/test-general.c index b26b0e2ae..21533cc74 100644 --- a/src/tests/test-general.c +++ b/src/tests/test-general.c @@ -806,17 +806,13 @@ static void test_match_spec_ifname (const char *spec_str, const char **matches, const char **neg_matches) { const char *m; - char **spec_str_split; GSList *specs, *specs_reverse = NULL; guint i; g_assert (spec_str); - spec_str_split = g_strsplit_set (spec_str, ";,", -1); - for (i = 0; spec_str_split[i]; i++) { - if (spec_str_split[i]) - specs_reverse = g_slist_prepend (specs_reverse, spec_str_split[i]); - } - specs = g_slist_reverse (g_slist_copy (specs_reverse)); + + specs = nm_match_spec_split (spec_str); + specs_reverse = g_slist_reverse (g_slist_copy (specs)); for (i = 0; matches && matches[i]; i++) { g_assert (nm_match_spec_interface_name (specs, matches[i]) == NM_MATCH_SPEC_MATCH); @@ -836,8 +832,7 @@ test_match_spec_ifname (const char *spec_str, const char **matches, const char * } g_slist_free (specs_reverse); - g_slist_free (specs); - g_strfreev (spec_str_split); + g_slist_free_full (specs, g_free); } static void @@ -874,6 +869,18 @@ test_nm_match_spec_interface_name (void) test_match_spec_ifname ("interface-name:em*,except:interface-name:=em*", S ("em", "em\\", "em\\*", "em\\1", "em\\11", "em\\2", "em1", "em11", "em2", "em3"), S ("em*")); + test_match_spec_ifname ("aa,bb,cc\\,dd,e,,", + S ("aa", "bb", "cc,dd", "e"), + NULL); + test_match_spec_ifname ("aa;bb;cc\\;dd;e,;", + S ("aa", "bb", "cc;dd", "e"), + NULL); + test_match_spec_ifname ("interface-name:em\\;1,em\\,2,\\,,\\\\,,em\\\\x", + S ("em;1", "em,2", ",", "\\", "em\\x"), + NULL); + test_match_spec_ifname (" , interface-name:a, ,", + S (" ", " ", " interface-name:a"), + NULL); #undef S } From c6778ad1b76a995deee966af959ab8bf325524d2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Feb 2015 16:36:53 +0100 Subject: [PATCH 6/7] core: unify parsing of device specs using nm_match_spec_split() There are three configuration options that contain device specs: 'main.ignore-carrier', 'main.no-auto-default', and 'keyfile.unmanaged-devices'. Unify the parsing of them by splitting the device spec with nm_match_spec_split(). This changes behavior for parsing of these properties. Also get rid of logging warnings when parsing 'keyfile.unmanaged-devices'. --- src/nm-config.c | 47 +++++++++++++++------------ src/nm-config.h | 1 + src/settings/plugins/keyfile/plugin.c | 34 +++---------------- 3 files changed, 31 insertions(+), 51 deletions(-) diff --git a/src/nm-config.c b/src/nm-config.c index 6b9887ad1..594d05f2e 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -32,6 +32,7 @@ #include "NetworkManagerUtils.h" #include "gsystem-local-alloc.h" #include "nm-enum-types.h" +#include "nm-core-internal.h" #include #include @@ -76,7 +77,7 @@ typedef struct { char *debug; - char **ignore_carrier; + GSList *ignore_carrier; gboolean configure_and_quit; } NMConfigPrivate; @@ -237,21 +238,10 @@ nm_config_get_configure_and_quit (NMConfig *config) gboolean nm_config_get_ignore_carrier (NMConfig *config, NMDevice *device) { - NMConfigPrivate *priv = NM_CONFIG_GET_PRIVATE (config); - GSList *specs = NULL; - int i; - gboolean match; + g_return_val_if_fail (NM_IS_CONFIG (config), FALSE); + g_return_val_if_fail (NM_IS_DEVICE (device), FALSE); - if (!priv->ignore_carrier) - return FALSE; - - for (i = 0; priv->ignore_carrier[i]; i++) - specs = g_slist_prepend (specs, priv->ignore_carrier[i]); - - match = nm_device_spec_match_list (device, specs); - - g_slist_free (specs); - return match; + return nm_device_spec_match_list (device, NM_CONFIG_GET_PRIVATE (config)->ignore_carrier); } /************************************************************************/ @@ -689,6 +679,15 @@ read_entire_config (const NMConfigCmdLineOptions *cli, return keyfile; } +GSList * +nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key) +{ + gs_free char *value = NULL; + + value = g_key_file_get_string ((GKeyFile *) keyfile, group, key, NULL); + return nm_match_spec_split (value); +} + /************************************************************************/ void @@ -813,7 +812,8 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) char *config_main_file = NULL; char *config_description = NULL; char **no_auto_default; - char **no_auto_default_orig; + GSList *no_auto_default_orig_list; + GPtrArray *no_auto_default_orig; if (priv->config_dir) { /* Object is already initialized. */ @@ -859,17 +859,22 @@ init_sync (GInitable *initable, GCancellable *cancellable, GError **error) priv->debug = g_key_file_get_value (keyfile, "main", "debug", NULL); - priv->ignore_carrier = g_key_file_get_string_list (keyfile, "main", "ignore-carrier", NULL, NULL); + priv->ignore_carrier = nm_config_get_device_match_spec (keyfile, "main", "ignore-carrier"); priv->configure_and_quit = _get_bool_value (keyfile, "main", "configure-and-quit", FALSE); - no_auto_default_orig = g_key_file_get_string_list (keyfile, "main", "no-auto-default", NULL, NULL); - no_auto_default = no_auto_default_merge_from_file (priv->no_auto_default_file, (const char *const *) no_auto_default_orig); + no_auto_default_orig_list = nm_config_get_device_match_spec (keyfile, "main", "no-auto-default"); + + no_auto_default_orig = _nm_utils_copy_slist_to_array (no_auto_default_orig_list, NULL, NULL); + g_ptr_array_add (no_auto_default_orig, NULL); + no_auto_default = no_auto_default_merge_from_file (priv->no_auto_default_file, (const char *const *) no_auto_default_orig->pdata); + g_ptr_array_unref (no_auto_default_orig); + + g_slist_free_full (no_auto_default_orig_list, g_free); priv->config_data_orig = nm_config_data_new (config_main_file, config_description, (const char *const*) no_auto_default, keyfile); g_strfreev (no_auto_default); - g_strfreev (no_auto_default_orig); priv->config_data = g_object_ref (priv->config_data_orig); @@ -910,7 +915,7 @@ finalize (GObject *gobject) g_free (priv->log_level); g_free (priv->log_domains); g_free (priv->debug); - g_strfreev (priv->ignore_carrier); + g_slist_free_full (priv->ignore_carrier, g_free); _nm_config_cmd_line_options_clear (&priv->cli); diff --git a/src/nm-config.h b/src/nm-config.h index 530cbade9..46388b08e 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -90,6 +90,7 @@ NMConfig *nm_config_setup (const NMConfigCmdLineOptions *cli, GError **error); void nm_config_reload (NMConfig *config); GKeyFile *nm_config_create_keyfile (void); +GSList *nm_config_get_device_match_spec (const GKeyFile *keyfile, const char *group, const char *key); G_END_DECLS diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index edaa7d456..611192baa 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -585,45 +585,19 @@ get_unmanaged_specs (NMSystemConfigInterface *config) GKeyFile *key_file; GSList *specs = NULL; GError *error = NULL; - char *str; if (!priv->conf_file) return NULL; - key_file = g_key_file_new (); - if (!parse_key_file_allow_none (priv, key_file, &error)) - goto out; + key_file = nm_config_create_keyfile (); + if (parse_key_file_allow_none (priv, key_file, &error)) + specs = nm_config_get_device_match_spec (key_file, "keyfile", "unmanaged-devices"); - str = g_key_file_get_value (key_file, "keyfile", "unmanaged-devices", NULL); - if (str) { - char **udis; - int i; - - udis = g_strsplit_set (str, ";,", -1); - g_free (str); - - for (i = 0; udis[i] != NULL; i++) { - /* Verify unmanaged specification and add it to the list */ - if (!strncmp (udis[i], "mac:", 4) && nm_utils_hwaddr_valid (udis[i] + 4, -1)) { - specs = g_slist_append (specs, udis[i]); - } else if (!strncmp (udis[i], "interface-name:", 15) && nm_utils_iface_valid_name (udis[i] + 15)) { - specs = g_slist_append (specs, udis[i]); - } else { - nm_log_warn (LOGD_SETTINGS, "keyfile: error in file '%s': invalid unmanaged-devices entry: '%s'", priv->conf_file, udis[i]); - g_free (udis[i]); - } - } - - g_free (udis); /* Yes, g_free, not g_strfreev because we need the strings in the list */ - } - - out: if (error) { nm_log_warn (LOGD_SETTINGS, "keyfile: error getting unmanaged specs: %s", error->message); g_error_free (error); } - if (key_file) - g_key_file_free (key_file); + g_key_file_free (key_file); return specs; } From b0f9e9bdfb1cbff844115e5d627a9c44a602de9d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 9 Feb 2015 15:33:28 +0100 Subject: [PATCH 7/7] man: explain the format for device specifier in manual page NetworkManager.conf --- man/NetworkManager.conf.xml.in | 110 ++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/man/NetworkManager.conf.xml.in b/man/NetworkManager.conf.xml.in index d9d51b846..ecba84439 100644 --- a/man/NetworkManager.conf.xml.in +++ b/man/NetworkManager.conf.xml.in @@ -148,7 +148,7 @@ Copyright 2010 - 2014 Red Hat, Inc. no-auto-default - Comma-separated list of devices for which + Specify devices for which NetworkManager shouldn't create default wired connection (Auto eth0). By default, NetworkManager creates a temporary wired connection for any Ethernet device that is managed and @@ -162,11 +162,15 @@ Copyright 2010 - 2014 Red Hat, Inc. /var/run/NetworkManager/no-auto-default.state to prevent creating the default connection for that device again. + See for the syntax how to + specify a device. + + Example: - no-auto-default=00:22:68:5c:5d:c4,00:1e:65:ff:aa:ee - no-auto-default=eth0,eth1 - no-auto-default=* +no-auto-default=00:22:68:5c:5d:c4,00:1e:65:ff:aa:ee +no-auto-default=eth0,eth1 +no-auto-default=* @@ -176,8 +180,8 @@ Copyright 2010 - 2014 Red Hat, Inc. ignore-carrier - Comma-separated list of devices for which NetworkManager - will (partially) ignore the carrier state. Normally, for + Specify devices for which NetworkManager will (partially) + ignore the carrier state. Normally, for device types that support carrier-detect, such as Ethernet and InfiniBand, NetworkManager will only allow a connection to be activated on the device if carrier is @@ -193,15 +197,14 @@ Copyright 2010 - 2014 Red Hat, Inc. connection (whether static or dynamic) to remain active on the device when carrier is lost. - - May have the special value * to apply - to all devices. - Note that the "carrier" property of NMDevices and device D-Bus interfaces will still reflect the actual device state; it's just that NetworkManager will not make use of that information. + See for the syntax how to + specify a device. + @@ -279,17 +282,11 @@ Copyright 2010 - 2014 Red Hat, Inc. unmanaged-devices Set devices that should be ignored by - NetworkManager when using the keyfile - plugin. Devices are specified in the following - format: - mac:<hwaddr> or - interface-name:<ifname>. Here - hwaddr is the MAC address of the device - to be ignored, in hex-digits-and-colons notation. - ifname is the interface name of the - ignored device. - Multiple entries are separated with semicolons. No - spaces are allowed in the value. + NetworkManager. + + See for the syntax how to + specify a device. + Example: @@ -545,6 +542,77 @@ unmanaged-devices=mac:00:22:68:1c:59:b1;mac:00:1E:65:30:D1:C4;interface-name:eth + + Appendix + + Device List Format + + The configuration options main.no-auto-default, main.ignore-carrier, + and keyfile.unmanaged-devices select devices based on a list of matchings. + Devices can be specified using the following format: + + + + + * + Matches every device. + + + IFNAME + Case sensitive match of interface name of the device. Globbing is not supported. + + + HWADDR + Match the MAC address of the device. Globbing is not supported + + + interface-name:IFNAME + interface-name:~IFNAME + Case sensitive match of interface name of the device. Simple globbing is supported with + * and ?. Ranges and escaping is not supported. + + + interface-name:=IFNAME + Case sensitive match of interface name of the device. Globbing is disabled and IFNAME + is taken literally. + + + mac:HWADDR + Match the MAC address of the device. Globbing is not supported + + + s390-subchannels:HWADDR + Match the device based on the subchannel address. Globbing is not supported + + + 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. + + + SPEC[,;]SPEC + Multiple specs can be concatenated with comman or semicolon. The order does not matter as + matches are either positive (inclusive) or negative, with negative matches having higher priority. + Backslash is supported to escape the separators ';' and ',', and to express special + characters such as newline ('\n'), tabulator ('\t'), whitespace ('\s') and backslash ('\\'). The globbing of + interface names cannot be escaped. Whitespace is taken literally so usually the specs will be concatenated + without spaces. + + + + + Example: + +interface-name:em4 +mac:00:22:68:1c:59:b1;mac:00:1E:65:30:D1:C4;interface-name:eth2 +interface-name:vboxnet*,except:interface-name:vboxnet2 +*,except:mac:00:22:68:1c:59:b1 + + + + + See Also