Commit 2e5e7285 authored by Beniamino Galvani's avatar Beniamino Galvani

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] db62fc9d ("platform: fix adding VLAN flags")

https://bugzilla.gnome.org/show_bug.cgi?id=762626
parent eb80f690
......@@ -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---
*/
......
......@@ -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;
......
......@@ -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 \
......
......@@ -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
......
......@@ -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
......
......@@ -3,5 +3,4 @@ TYPE=Vlan
DEVICE=vlan0.3
PHYSDEV=eth0
VLAN_ID=3
REORDER_HDR=1
REORDER_HDR=0
VLAN=yes
TYPE=Vlan
DEVICE=vlan0.3
PHYSDEV=eth0
VLAN_ID=3
VLAN_FLAGS="LOOSE_BINDING,NO_REORDER_HDR"
......@@ -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);
......
......@@ -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);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment