fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-06 07:20:14 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ifcfg-rh: use distinct variables for bridge and wired mac address

Browse source 
Currently both bridge.mac-address and ethernet.cloned-mac-address get
written to the same MACADDR ifcfg-rh variable; the ethernet property
wins if both are present.

When one property is set and the connection is saved (and thus reread)
both properties are populated with the same value. This is wrong
because, even if the properties have the same meaning, the setting
plugin should not read something different from what was written. Also
consider that after the following steps:

 $ nmcli con mod c ethernet.cloned-mac-address 00:11:22:33:44:55
 $ nmcli con mod c ethernet.cloned-mac-address ""

the connection will still have the new mac address set in the
bridge.mac-address property, which is certainly unexpected.

In general, mapping multiple properties to the same variable is
harmful and must be avoided. Therefore, let's use a different variable
for bridge.mac-address. This changes behavior, but not so much:

 - connections that have MACADDR set will behave as before; the only
   difference will be that the MAC will be present in the wired
   setting instead of the bridge one;

 - initscripts compatibility is not relevant because MACADDR for
   bridges was a NM extension;

 - if someone creates a new connection and sets bridge.mac-address NM
   will set the BRIDGE_MACADDR property instead of MACADDR. But this
   shouldn't be a big concern as bridge.mac-address is documented as
   deprecated and should not be used for new connections.

https://bugzilla.redhat.com/show_bug.cgi?id=1516659
This commit is contained in:
Beniamino Galvani 2017-11-22 13:55:53 +01:00
parent 56a02c9baf
commit fb191fc282
4 changed files with 8 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
libnm-core/nm-setting-bridge.c
View file

@ -445,10 +445,10 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
* ---end---
* ---ifcfg-rh---
* property: mac-address
* variable: MACADDR(+)
* variable: BRIDGE_MACADDR(+)
* description: MAC address of the bridge. Note that this requires a recent
* kernel support, originally introduced in 3.15 upstream kernel)
* MACADDR for bridges is an NM extension.
* BRIDGE_MACADDR for bridges is an NM extension.
* ---end---
*/
g_object_class_install_property

2
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
View file

@ -4750,7 +4750,7 @@ make_bridge_setting (shvarFile *ifcfg,
s_bridge = NM_SETTING_BRIDGE (nm_setting_bridge_new ());
value = svGetValueStr_cp (ifcfg, "MACADDR");
value = svGetValueStr_cp (ifcfg, "BRIDGE_MACADDR");
if (value) {
value = g_strstrip (value);
g_object_set (s_bridge, NM_SETTING_BRIDGE_MAC_ADDRESS, value, NULL);

2
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
View file

@ -1408,7 +1408,7 @@ write_bridge_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wire
svUnsetValue (ifcfg, "DELAY");
mac = nm_setting_bridge_get_mac_address (s_bridge);
svSetValueStr (ifcfg, "MACADDR", mac);
svSetValueStr (ifcfg, "BRIDGE_MACADDR", mac);
/* Bridge options */
opts = g_string_sized_new (32);

5
src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
View file

@ -7403,6 +7403,7 @@ test_read_bridge_main (void)
{
NMConnection *connection;
NMSettingBridge *s_bridge;
NMSettingWired *s_wired;
const char *mac;
char expected_mac_address[ETH_ALEN] = { 0x00, 0x16, 0x41, 0x11, 0x22, 0x33 };
@ -7425,7 +7426,9 @@ test_read_bridge_main (void)
g_assert (!nm_setting_bridge_get_multicast_snooping (s_bridge));
/* MAC address */
mac = nm_setting_bridge_get_mac_address (s_bridge);
s_wired = nm_connection_get_setting_wired (connection);
g_assert (s_wired);
mac = nm_setting_wired_get_cloned_mac_address (s_wired);
g_assert (mac);
g_assert (nm_utils_hwaddr_matches (mac, -1, expected_mac_address, ETH_ALEN));