system/NetworkManager
system/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager synced 2024-07-09 04:05:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

ifcfg-rh: always honor "$VLAN_ID" in ifcfg files

Browse Source 
initscripts don't support "$VLAN_ID". They actually support "$VID",
which NetworkManager doesn't.

"$VLAN_ID" was introduced by commit 10b32be37b ('ifcfg-rh: various VLAN
cleanups'). It has a comment about "backward compatibility" for the case
where the reader would ignore "$VLAN_ID" if "$DEVICE"'s name contains
a suffix that is parsable as VLAN ID.

That is wrong. If a new feature gets introduce (like NetworkManager
supporting "$VLAN_ID"), then there is no way that an older version of the
tool -- which doesn't know the new feature yet (initscripts) -- supports it.
This is not what backward compatibility means. Backward compatibility
means that if a user has an old ifcfg-file without "$VLAN_ID", then we
continue parsing it as before.

Consider, when a user (or NetworkManager) writes a configuration

  DEVICE=vlan9
  PHYSDEV=eth0
  VLAN_ID=10

then it makes no sense to ignore VLAN_ID=10 and use "9" instead.
Otherwise the user (or NetworkManager) should not have written the
file this way.

Also, NetworkManager profiles support "connection.interface-name=vlan9"
together with "vlan.id=10". Such a configuration is valid and must be
expressible in ifcfg-rh format. The ifcfg-rh writer code did not somehow
restrict the setting of "$VLAN_ID" to account for this odd behavior. Whenever
NetworkManager in the past wrote VLAN_ID variable to file, it really meant
it.

https://bugzilla.redhat.com/show_bug.cgi?id=1907960

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/794
This commit is contained in:
Thomas Haller 2021-03-25 11:55:22 +01:00
parent ba6552c472
commit 5d6532f2d7
No known key found for this signature in database
GPG Key ID: 29C2366E4DFC5728
6 changed files with 67 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Makefile.am
View File

@ -3434,6 +3434,8 @@ EXTRA_DIST += \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-1 \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-reorder-hdr-2 \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-trailing-spaces \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-vlanid-use \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-vlanid-use.cexpected \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-band-a \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-band-a-channel-mismatch \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-band-bg-channel-mismatch \
@ -3463,9 +3465,9 @@ EXTRA_DIST += \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wep-eap-ttls-chap \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wep-no-keys \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wep-passphrase \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-eap-suite-b-192-tls \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-eap-tls \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-eap-ttls-tls \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-eap-suite-b-192-tls \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-psk \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-psk-2 \
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wifi-wpa-psk-adhoc \

15
src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
View File

@ -6071,15 +6071,14 @@ make_vlan_setting(shvarFile *ifcfg, const char *file, GError **error)
v = iface_name + 4;
}
if (v) {
int device_vlan_id;
/* Grab VLAN ID from interface name; this takes precedence over the
* separate VLAN_ID property for backwards compat.
if (vlan_id == -1 && v) {
/* Grab VLAN ID from interface name; The explicit VLAN_ID option takes precedence
* over detecting the ID based on PHYSDEV.
*
* Note that older versions of NetworkManager had a bug and this would overwrite the
* VLAN_ID in this case.
*/
device_vlan_id = _nm_utils_ascii_str_to_int64(v, 10, 0, 4095, -1);
if (device_vlan_id != -1)
vlan_id = device_vlan_id;
vlan_id = _nm_utils_ascii_str_to_int64(v, 10, 0, 4095, -1);
}
}

4
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-vlanid-use Normal file
View File

@ -0,0 +1,4 @@
VLAN=yes
TYPE=Vlan
DEVICE=eth0.9
VLAN_ID=10

15
src/core/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-vlan-vlanid-use.cexpected Normal file
View File

@ -0,0 +1,15 @@
VLAN=yes
TYPE=Vlan
PHYSDEV=eth0
VLAN_ID=10
REORDER_HDR=yes
GVRP=no
MVRP=no
HWADDR=
PROXY_METHOD=none
BROWSER_ONLY=no
IPV6INIT=no
NAME="Vlan test-vlan-vlanid-use"
UUID=${UUID}
DEVICE=eth0.9
ONBOOT=yes

33
src/core/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
View File

@ -8786,6 +8786,38 @@ test_read_vlan_only_vlan_id(void)
g_object_unref(connection);
}
static void
test_read_vlan_vlanid_use(void)
{
nmtst_auto_unlinkfile char *testfile = NULL;
gs_unref_object NMConnection *connection = NULL;
gs_unref_object NMConnection *reread = NULL;
NMSettingVlan * s_vlan;
connection = _connection_from_file(TEST_IFCFG_DIR "/ifcfg-test-vlan-vlanid-use",
NULL,
TYPE_ETHERNET,
NULL);
g_assert_cmpstr(nm_connection_get_interface_name(connection), ==, "eth0.9");
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), ==, 10);
g_assert_cmpint(nm_setting_vlan_get_flags(s_vlan), ==, NM_VLAN_FLAG_REORDER_HEADERS);
_writer_new_connec_exp(connection,
TEST_SCRATCH_DIR,
TEST_IFCFG_DIR "/ifcfg-test-vlan-vlanid-use.cexpected",
&testfile);
reread = _connection_from_file(testfile, NULL, TYPE_ETHERNET, NULL);
nmtst_assert_connection_equals(connection, TRUE, reread, FALSE);
}
static void
test_read_vlan_only_device(void)
{
@ -11626,6 +11658,7 @@ main(int argc, char **argv)
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 "vlan/read/vlanid-use", test_read_vlan_vlanid_use);
g_test_add_func(TPATH "wired/read/read-wake-on-lan", test_read_wired_wake_on_lan);
g_test_add_func(TPATH "wired/read/read-auto-negotiate-off", test_read_wired_auto_negotiate_off);
g_test_add_func(TPATH "wired/read/read-auto-negotiate-on", test_read_wired_auto_negotiate_on);

7
src/libnm-core-impl/nm-setting-vlan.c
View File

@ -862,8 +862,11 @@ nm_setting_vlan_class_init(NMSettingVlanClass *klass)
**/
/* ---ifcfg-rh---
* property: id
* variable: VLAN_ID or DEVICE
* description: VLAN identifier.
* variable: VLAN_ID, DEVICE.
* description: VLAN identifier. If VLAN_ID is not set, it is attempted
* to be detected from the suffix of DEVICE=.
* Note that older versions of NetworkManager had a bug where they would
* prefer the detected ID from the DEVICE over VLAN_ID.
* ---end---
*/
obj_properties[PROP_ID] =