From c2e4e2f1fdde12eab7bd6bd6ca8f30c57af9c099 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() (cherry picked from commit 9080ad696d6b2339db266486b15af2b052265a2f) --- 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 f118156b37..275fab29a5 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 a01da5f95e62ccebc41c1211a0652e927e44039b 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*" (cherry picked from commit 2b518538bec1a0a537cc49672549e5d978716e3b) --- 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 c368e8402c..58f87f0ddf 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -843,6 +843,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) { @@ -853,32 +857,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 @@ -947,8 +976,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) { @@ -962,11 +989,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 275fab29a5..15320191ba 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 3de7acc37acc8f4eebca0ebcf35e9c3d04dbc681 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. (cherry picked from commit 2051944333d0f7d12cbeed90db28a0cb3c4f7132) --- 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 58f87f0ddf..1f2a21a17c 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -830,19 +830,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 ae1edd820b..d11f1fa204 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -94,7 +94,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 86ab3945de..a5059fdf34 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8217,9 +8217,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 aaca52b2614551fa20cf6358fe9ce3fcca58d634 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. (cherry picked from commit 5c2e1afd1bd8442e1b2efc518e7109f516a00bff) --- 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 1f2a21a17c..7ccc5cffd5 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -833,47 +833,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; @@ -887,14 +912,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 @@ -963,34 +991,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 d11f1fa204..c0f313c2f5 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -94,9 +94,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 0bab1f5b97..7fa3d3a41e 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 a5059fdf34..649b1c8ec9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8209,27 +8209,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 c7971b4cb5..67bc18b0fd 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -213,7 +213,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 15320191ba..b26b0e2aec 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 832023fe1c018096f8d2adaafe51af77e2ab313f 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. (cherry picked from commit 3bcc5e4bd0bd88b15ab8f84515f0fca52db62823) --- 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 7ccc5cffd5..d84cd6a475 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -1027,6 +1027,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 c0f313c2f5..884cde9f3f 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -103,6 +103,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 b26b0e2aec..21533cc743 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 2be628334c83440d5155300df551168c1325531a 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'. (cherry picked from commit c6778ad1b76a995deee966af959ab8bf325524d2) --- 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 79f4123e04..a40b683056 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 @@ -74,7 +75,7 @@ typedef struct { char *debug; - char **ignore_carrier; + GSList *ignore_carrier; gboolean configure_and_quit; } NMConfigPrivate; @@ -227,21 +228,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); } /************************************************************************/ @@ -679,6 +669,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 @@ -805,7 +804,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. */ @@ -850,17 +850,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); @@ -900,7 +905,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 8d42eac31c..a559de2c4c 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -91,6 +91,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 edaa7d4566..611192baa5 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 18e29c5e7a89ecd7b020923b09f5bbf2b317e28a 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 (cherry picked from commit b0f9e9bdfb1cbff844115e5d627a9c44a602de9d) --- 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 7c6b95516e..ae4ba88fd6 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. + @@ -287,17 +290,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: @@ -553,6 +550,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