merge: branch 'bg/vlan-reorder-hdr-bgo762626'

https://bugzilla.gnome.org/show_bug.cgi?id=762626

12 files changed, 95 insertions, 47 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/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/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);
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,33 @@ 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");
+
+	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 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");
 
@@ -7340,8 +7364,8 @@ 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 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);
 
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);
 }