From eb80f690981fe849c3bfc2477f1d3350b6e35f6d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 26 Feb 2016 17:02:11 +0100 Subject: [PATCH 1/3] utils: add nm_utils_strbuf_init() macro --- src/NetworkManagerUtils.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h index 87c898771c..8ca50736df 100644 --- a/src/NetworkManagerUtils.h +++ b/src/NetworkManagerUtils.h @@ -301,6 +301,19 @@ fcn_name (lookup_type val) \ /*****************************************************************************/ +static inline void +_nm_utils_strbuf_init (char *buf, gsize len, char **p_buf_ptr, gsize *p_buf_len) +{ + NM_SET_OUT (p_buf_len, len); + NM_SET_OUT (p_buf_ptr, buf); + buf[0] = '\0'; +} + +#define nm_utils_strbuf_init(buf, p_buf_ptr, p_buf_len) \ + G_STMT_START { \ + G_STATIC_ASSERT (G_N_ELEMENTS (buf) == sizeof (buf) && sizeof (buf) > sizeof (char *)); \ + _nm_utils_strbuf_init ((buf), sizeof (buf), (p_buf_ptr), (p_buf_len)); \ + } G_STMT_END void nm_utils_strbuf_append (char **buf, gsize *len, const char *format, ...) __attribute__((__format__ (__printf__, 3, 4))); void nm_utils_strbuf_append_c (char **buf, gsize *len, char c); void nm_utils_strbuf_append_str (char **buf, gsize *len, const char *str); From 2e5e7285a875f84377da4e5035795286a6f13c9b Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 26 Feb 2016 15:29:19 +0100 Subject: [PATCH 2/3] ifcfg-rh: change the handling of REORDER_HDR flag On NM 1.0 connections were created by default without the REORDER_HDR flag, but then due to a bug in platform code (fixed in [1]), the kernel interface always had the flag set. Now that the setting is honored, users upgrading to the new version of NM will see a change from the previous behavior, since interfaces will not have REORDER_HDR and this will certainly break functionality. The only solution here seems to be to ignore the REORDER_HDR variable in ifcfg files (since it never had any effect) and introduce a new NO_REORDER_HDR option for the VLAN_FLAGS variable which allows to turn the flag off. The consequence is that the flag will be set for all old connections. This change introduces an incompatibility with initscripts, however is necessary to avoid breaking user functionality upon upgrade. Connections created through NetworkManager will still be parsed correctly by initscripts (since we always write the REORDER_HDR variable). [1] db62fc9d72fa ("platform: fix adding VLAN flags") https://bugzilla.gnome.org/show_bug.cgi?id=762626 --- libnm-core/nm-setting-vlan.c | 4 +- src/settings/plugins/ifcfg-rh/reader.c | 28 +++++++---- .../tests/network-scripts/Makefile.am | 1 + .../network-scripts/ifcfg-test-vlan-flags-2 | 1 - .../network-scripts/ifcfg-test-vlan-interface | 1 - .../ifcfg-test-vlan-reorder-hdr-1 | 3 +- .../ifcfg-test-vlan-reorder-hdr-2 | 6 +++ .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 48 +++++++++++++++---- src/settings/plugins/ifcfg-rh/writer.c | 14 ++++-- 9 files changed, 79 insertions(+), 27 deletions(-) create mode 100644 src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-2 diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 90798c40d3..39587d59f9 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -893,8 +893,8 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) **/ /* ---ifcfg-rh--- * property: flags - * variable: REORDER_HDR, GVRP, MVRP, VLAN_FLAGS - * values: "yes or "no" for REORDER_HDR, GVRP and MVRP; "LOOSE_BINDING" for VLAN_FLAGS + * variable: GVRP, MVRP, VLAN_FLAGS + * values: "yes or "no" for GVRP and MVRP; "LOOSE_BINDING" and "NO_REORDER_HDR" for VLAN_FLAGS * description: VLAN flags. * ---end--- */ diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c index c8871a55a8..ca512fc6c2 100644 --- a/src/settings/plugins/ifcfg-rh/reader.c +++ b/src/settings/plugins/ifcfg-rh/reader.c @@ -4647,7 +4647,7 @@ make_vlan_setting (shvarFile *ifcfg, char *end = NULL; gint vlan_id = -1; guint32 vlan_flags = 0; - gint gvrp; + gint gvrp, reorder_hdr; value = svGetValue (ifcfg, "VLAN_ID", FALSE); if (value) { @@ -4722,8 +4722,7 @@ make_vlan_setting (shvarFile *ifcfg, g_object_set (s_vlan, NM_SETTING_VLAN_PARENT, parent, NULL); g_clear_pointer (&parent, g_free); - if (svGetValueBoolean (ifcfg, "REORDER_HDR", FALSE)) - vlan_flags |= NM_VLAN_FLAG_REORDER_HEADERS; + vlan_flags |= NM_VLAN_FLAG_REORDER_HEADERS; gvrp = svGetValueBoolean (ifcfg, "GVRP", -1); if (gvrp > 0) @@ -4731,13 +4730,26 @@ make_vlan_setting (shvarFile *ifcfg, value = svGetValue (ifcfg, "VLAN_FLAGS", FALSE); if (value) { - /* Prefer GVRP variable; only take VLAN_FLAG=GVRP when GVRP is not specified */ - if (g_strstr_len (value, -1, "GVRP") && gvrp == -1) - vlan_flags |= NM_VLAN_FLAG_GVRP; - if (g_strstr_len (value, -1, "LOOSE_BINDING")) - vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING; + gs_strfreev char **strv = NULL; + char **ptr; + + strv = g_strsplit_set (value, ", ", 0); + + for (ptr = strv; ptr && *ptr; ptr++) { + if (nm_streq (*ptr, "GVRP") && gvrp == -1) + vlan_flags |= NM_VLAN_FLAG_GVRP; + if (nm_streq (*ptr, "LOOSE_BINDING")) + vlan_flags |= NM_VLAN_FLAG_LOOSE_BINDING; + if (nm_streq (*ptr, "NO_REORDER_HDR")) + vlan_flags &= ~NM_VLAN_FLAG_REORDER_HEADERS; + } } + reorder_hdr = svGetValueBoolean (ifcfg, "REORDER_HDR", -1); + if ( reorder_hdr != -1 + && reorder_hdr != NM_FLAGS_HAS (vlan_flags, NM_VLAN_FLAG_REORDER_HEADERS)) + PARSE_WARNING ("REORDER_HDR key is deprecated, use VLAN_FLAGS"); + if (svGetValueBoolean (ifcfg, "MVRP", FALSE)) vlan_flags |= NM_VLAN_FLAG_MVRP; diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am index ab8579762a..36e52af4d1 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/Makefile.am @@ -107,6 +107,7 @@ EXTRA_DIST = \ ifcfg-test-vlan-only-device \ ifcfg-test-vlan-physdev \ ifcfg-test-vlan-reorder-hdr-1 \ + ifcfg-test-vlan-reorder-hdr-2 \ ifcfg-test-vlan-flags-1 \ ifcfg-test-vlan-flags-2 \ ifcfg-test-wifi-wep-no-keys \ diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-2 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-2 index 3b536a6628..2c01be71fc 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-2 +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-flags-2 @@ -3,7 +3,6 @@ TYPE=Vlan DEVICE=super-vlan VLAN_ID=44 PHYSDEV=eth9 -REORDER_HDR=no VLAN_FLAGS="GVRP LOOSE_BINDING" ONBOOT=yes BOOTPROTO=static diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface index ccd75d7ffb..d8d9193fe4 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-interface @@ -2,7 +2,6 @@ VLAN=yes TYPE=Vlan DEVICE=vlan43 PHYSDEV=eth9 -REORDER_HDR=0 VLAN_FLAGS=GVRP,LOOSE_BINDING VLAN_INGRESS_PRIORITY_MAP=0:1,2:5 VLAN_EGRESS_PRIORITY_MAP=12:3,14:7,3:1 diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-1 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-1 index ca38f839b1..0dc539712b 100644 --- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-1 +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-1 @@ -3,5 +3,4 @@ TYPE=Vlan DEVICE=vlan0.3 PHYSDEV=eth0 VLAN_ID=3 -REORDER_HDR=1 - +REORDER_HDR=0 diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-2 b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-2 new file mode 100644 index 0000000000..d98a9d364d --- /dev/null +++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-2 @@ -0,0 +1,6 @@ +VLAN=yes +TYPE=Vlan +DEVICE=vlan0.3 +PHYSDEV=eth0 +VLAN_ID=3 +VLAN_FLAGS="LOOSE_BINDING,NO_REORDER_HDR" diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 9f663c9c87..8ca5c8bbe0 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -7230,7 +7230,7 @@ test_read_vlan_interface (void) g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 43); g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, - NM_VLAN_FLAG_GVRP | NM_VLAN_FLAG_LOOSE_BINDING); + NM_VLAN_FLAG_GVRP | NM_VLAN_FLAG_LOOSE_BINDING | NM_VLAN_FLAG_REORDER_HEADERS); /* Ingress map */ g_assert_cmpint (nm_setting_vlan_get_num_priorities (s_vlan, NM_VLAN_INGRESS_MAP), ==, 2); @@ -7278,8 +7278,7 @@ test_read_vlan_only_vlan_id (void) g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 43); - /* Ensure that flags are 0 if both REORDER_HDR and VLAN_FLAGS are missing */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 0); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); } @@ -7330,8 +7329,11 @@ test_read_vlan_reorder_hdr_1 (void) NMConnection *connection; NMSettingVlan *s_vlan; + g_test_expect_message ("NetworkManager", G_LOG_LEVEL_WARNING, + "*REORDER_HDR key is deprecated, use VLAN_FLAGS*"); connection = _connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-reorder-hdr-1", NULL, TYPE_ETHERNET, NULL); + g_test_assert_expected_messages (); g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "vlan0.3"); @@ -7340,8 +7342,30 @@ test_read_vlan_reorder_hdr_1 (void) g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth0"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 3); - /* Check correct read of REORDER_HDR=1 */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 1); + /* Check that REORDER_HDR=0 is ignored */ + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS); + + g_object_unref (connection); +} + +static void +test_read_vlan_reorder_hdr_2 (void) +{ + NMConnection *connection; + NMSettingVlan *s_vlan; + + connection = _connection_from_file (TEST_IFCFG_DIR"/network-scripts/ifcfg-test-vlan-reorder-hdr-2", + NULL, TYPE_ETHERNET, NULL); + + g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "vlan0.3"); + + s_vlan = nm_connection_get_setting_vlan (connection); + g_assert (s_vlan); + + g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth0"); + g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 3); + /* Check that VLAN_FLAGS=NO_REORDER_HDR works */ + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_LOOSE_BINDING); g_object_unref (connection); } @@ -7362,8 +7386,9 @@ test_read_vlan_flags_1 (void) g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 44); - /* reorder_hdr and loose_binding */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 5); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, + NM_VLAN_FLAG_LOOSE_BINDING | + NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); } @@ -7384,8 +7409,10 @@ test_read_vlan_flags_2 (void) g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "eth9"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 44); - /* gvrp and loose_binding */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 6); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, + NM_VLAN_FLAG_GVRP | + NM_VLAN_FLAG_LOOSE_BINDING | + NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); } @@ -8706,7 +8733,7 @@ test_read_vlan_trailing_spaces (void) g_assert_cmpstr (nm_connection_get_interface_name (connection), ==, "vlan201"); g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "enccw0.0.fb00"); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 201); - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 0); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); } @@ -8836,6 +8863,7 @@ int main (int argc, char **argv) g_test_add_func (TPATH "vlan/read/only-device", test_read_vlan_only_device); g_test_add_func (TPATH "vlan/read/physdev", test_read_vlan_physdev); g_test_add_func (TPATH "vlan/read/reorder-hdr-1", test_read_vlan_reorder_hdr_1); + g_test_add_func (TPATH "vlan/read/reorder-hdr-2", test_read_vlan_reorder_hdr_2); g_test_add_func (TPATH "wired/read/read-wake-on-lan", test_read_wired_wake_on_lan); g_test_add_func (TPATH "wired/write/static", test_write_wired_static); diff --git a/src/settings/plugins/ifcfg-rh/writer.c b/src/settings/plugins/ifcfg-rh/writer.c index 53e2f56b22..e7d64379da 100644 --- a/src/settings/plugins/ifcfg-rh/writer.c +++ b/src/settings/plugins/ifcfg-rh/writer.c @@ -45,6 +45,7 @@ #include "nm-core-internal.h" #include "nm-utils.h" #include "nm-core-internal.h" +#include "NetworkManagerUtils.h" #include "common.h" #include "shvar.h" @@ -1260,6 +1261,8 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wired, NMSettingConnection *s_con; char *tmp; guint32 vlan_flags = 0; + gsize s_buf_len; + char s_buf[50], *s_buf_ptr; s_con = nm_connection_get_setting_connection (connection); if (!s_con) { @@ -1292,9 +1295,14 @@ write_vlan_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wired, svSetValue (ifcfg, "GVRP", vlan_flags & NM_VLAN_FLAG_GVRP ? "yes" : "no", FALSE); - svSetValue (ifcfg, "VLAN_FLAGS", NULL, FALSE); - if (vlan_flags & NM_VLAN_FLAG_LOOSE_BINDING) - svSetValue (ifcfg, "VLAN_FLAGS", "LOOSE_BINDING", FALSE); + nm_utils_strbuf_init (s_buf, &s_buf_ptr, &s_buf_len); + + if (NM_FLAGS_HAS (vlan_flags, NM_VLAN_FLAG_LOOSE_BINDING)) + nm_utils_strbuf_append_str (&s_buf_ptr, &s_buf_len, "LOOSE_BINDING"); + if (!NM_FLAGS_HAS (vlan_flags, NM_VLAN_FLAG_REORDER_HEADERS)) + nm_utils_strbuf_append (&s_buf_ptr, &s_buf_len, "%sNO_REORDER_HDR", s_buf[0] ? "," : ""); + + svSetValue (ifcfg, "VLAN_FLAGS", s_buf, FALSE); svSetValue (ifcfg, "MVRP", vlan_flags & NM_VLAN_FLAG_MVRP ? "yes" : "no", FALSE); From da70fbd7d56f6b6f7c885d57660f1ed30c5064f2 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 26 Feb 2016 16:00:25 +0100 Subject: [PATCH 3/3] libnm-core: remove special handling of missing VLAN_FLAGS On older NM versions the default value for vlan.flags was 0, but then the actual value set on interfaces was REORDER_HDR. In order to maintain backwards compatibility in behavior, remove the special handling of vlan.flags so that a missing key is treated as the default value REORDER_HDR. https://bugzilla.gnome.org/show_bug.cgi?id=762626 --- libnm-core/nm-keyfile-reader.c | 17 +---------------- .../plugins/keyfile/tests/test-keyfile.c | 6 ++---- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c index 3936a4a7a6..4a17d2c488 100644 --- a/libnm-core/nm-keyfile-reader.c +++ b/libnm-core/nm-keyfile-reader.c @@ -1278,10 +1278,7 @@ set_default_for_missing_key (NMSetting *setting, const char *property) { /* Set a value different from the default value of the property's spec */ - if (NM_IS_SETTING_VLAN (setting)) { - if (!strcmp (property, NM_SETTING_VLAN_FLAGS)) - g_object_set (setting, property, (NMVlanFlags) 0, NULL); - } else if (NM_IS_SETTING_WIRELESS (setting)) { + if (NM_IS_SETTING_WIRELESS (setting)) { if (!strcmp (property, NM_SETTING_WIRELESS_MAC_ADDRESS_RANDOMIZATION)) g_object_set (setting, property, (NMSettingMacRandomization) NM_SETTING_MAC_RANDOMIZATION_NEVER, NULL); } @@ -1689,18 +1686,6 @@ nm_keyfile_read (GKeyFile *keyfile, } } - /* Make sure that if [vlan] group was missing we set vlan.flags to 0 - * for backwards compatibility */ - if (nm_connection_is_type (connection, NM_SETTING_VLAN_SETTING_NAME)) { - if (!nm_connection_get_setting_vlan (connection)) { - NMSettingVlan *s_vlan; - - s_vlan = NM_SETTING_VLAN (nm_setting_vlan_new ()); - g_object_set (s_vlan, NM_SETTING_VLAN_FLAGS, 0, NULL); - nm_connection_add_setting (connection, NM_SETTING (s_vlan)); - } - } - return connection; out_error: g_propagate_error (error, info.error); diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 7758724246..c1b79ef7d7 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -2630,8 +2630,7 @@ test_read_missing_vlan_setting (void) s_vlan = nm_connection_get_setting_vlan (connection); g_assert (s_vlan); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 0); - /* Ensure the VLAN flags are not set (0) */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 0); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); } @@ -2657,8 +2656,7 @@ test_read_missing_vlan_flags (void) g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 444); g_assert_cmpstr (nm_setting_vlan_get_parent (s_vlan), ==, "em1"); - /* Ensure the VLAN flags are not set (0) */ - g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, 0); + g_assert_cmpint (nm_setting_vlan_get_flags (s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS); g_object_unref (connection); }