ifcfg-rh: merge branch 'bg/ifcfg-rh-bridge-mac-rh1516659'

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

8 files changed, 208 insertions, 149 deletions

diff --git a/clients/common/settings-docs.c.in b/clients/common/settings-docs.c.in
index 3d6f012bf7..79cbb715b1 100644
--- a/clients/common/settings-docs.c.in
+++ b/clients/common/settings-docs.c.in
@@ -118,7 +118,7 @@
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_FORWARD_DELAY N_("The Spanning Tree Protocol (STP) forwarding delay, in seconds.")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_GROUP_FORWARD_MASK N_("A mask of group addresses to forward. Usually, group addresses in the range from 01:80:C2:00:00:00 to 01:80:C2:00:00:0F are not forwarded according to standards. This property is a mask of 16 bits, each corresponding to a group address in that range that must be forwarded. The mask can't have bits 0, 1 or 2 set because they are used for STP, MAC pause frames and LACP.")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_HELLO_TIME N_("The Spanning Tree Protocol (STP) hello time, in seconds.")
-#define DESCRIBE_DOC_NM_SETTING_BRIDGE_MAC_ADDRESS N_("If specified, the MAC address of bridge. When creating a new bridge, this MAC address will be set. If this field is left unspecified, the \"ethernet.cloned-mac-address\" is referred instead to generate the initial MAC address. Note that setting \"ethernet.cloned-mac-address\" anyway overwrites the MAC address of the bridge later while activating the bridge. Hence, this property is deprecated.")
+#define DESCRIBE_DOC_NM_SETTING_BRIDGE_MAC_ADDRESS N_("If specified, the MAC address of bridge. When creating a new bridge, this MAC address will be set. If this field is left unspecified, the \"ethernet.cloned-mac-address\" is referred instead to generate the initial MAC address. Note that setting \"ethernet.cloned-mac-address\" anyway overwrites the MAC address of the bridge later while activating the bridge. Hence, this property is deprecated. Deprecated: 1")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_MAX_AGE N_("The Spanning Tree Protocol (STP) maximum message age, in seconds.")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_MULTICAST_SNOOPING N_("Controls whether IGMP snooping is enabled for this bridge. Note that if snooping was automatically disabled due to hash collisions, the system may refuse to enable the feature until the collisions are resolved.")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_NAME N_("The setting's name, which uniquely identifies the setting within the connection.  Each setting type has a name unique to that type, for example \"ppp\" or \"wireless\" or \"wired\".")
diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h
index 5144a05077..de39deabd9 100644
--- a/libnm-core/nm-core-internal.h
+++ b/libnm-core/nm-core-internal.h
@@ -80,6 +80,20 @@
 #include "nm-core-types-internal.h"
 #include "nm-vpn-editor-plugin.h"
 
+/* IEEE 802.1D-1998 timer values */
+#define NM_BR_MIN_HELLO_TIME    1
+#define NM_BR_MAX_HELLO_TIME    10
+
+#define NM_BR_MIN_FORWARD_DELAY 2
+#define NM_BR_MAX_FORWARD_DELAY 30
+
+#define NM_BR_MIN_MAX_AGE       6
+#define NM_BR_MAX_MAX_AGE       40
+
+/* IEEE 802.1D-1998 Table 7.4 */
+#define NM_BR_MIN_AGEING_TIME   0
+#define NM_BR_MAX_AGEING_TIME   1000000
+
 /* NM_SETTING_COMPARE_FLAG_INFERRABLE: check whether a device-generated
  * connection can be replaced by a already-defined connection. This flag only
  * takes into account properties marked with the %NM_SETTING_PARAM_INFERRABLE
diff --git a/libnm-core/nm-setting-bridge.c b/libnm-core/nm-setting-bridge.c
index 5330038425..112b499f31 100644
--- a/libnm-core/nm-setting-bridge.c
+++ b/libnm-core/nm-setting-bridge.c
@@ -213,20 +213,6 @@ nm_setting_bridge_get_multicast_snooping (NMSettingBridge *setting)
 	return NM_SETTING_BRIDGE_GET_PRIVATE (setting)->multicast_snooping;
 }
 
-/* IEEE 802.1D-1998 timer values */
-#define BR_MIN_HELLO_TIME    1
-#define BR_MAX_HELLO_TIME    10
-
-#define BR_MIN_FORWARD_DELAY 2
-#define BR_MAX_FORWARD_DELAY 30
-
-#define BR_MIN_MAX_AGE       6
-#define BR_MAX_MAX_AGE       40
-
-/* IEEE 802.1D-1998 Table 7.4 */
-#define BR_MIN_AGEING_TIME   0
-#define BR_MAX_AGEING_TIME   1000000
-
 static inline gboolean
 check_range (guint32 val,
              guint32 min,
@@ -265,32 +251,32 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	}
 
 	if (!check_range (priv->forward_delay,
-	                  BR_MIN_FORWARD_DELAY,
-	                  BR_MAX_FORWARD_DELAY,
+	                  NM_BR_MIN_FORWARD_DELAY,
+	                  NM_BR_MAX_FORWARD_DELAY,
 	                  !priv->stp,
 	                  NM_SETTING_BRIDGE_FORWARD_DELAY,
 	                  error))
 		return FALSE;
 
 	if (!check_range (priv->hello_time,
-	                  BR_MIN_HELLO_TIME,
-	                  BR_MAX_HELLO_TIME,
+	                  NM_BR_MIN_HELLO_TIME,
+	                  NM_BR_MAX_HELLO_TIME,
 	                  !priv->stp,
 	                  NM_SETTING_BRIDGE_HELLO_TIME,
 	                  error))
 		return FALSE;
 
 	if (!check_range (priv->max_age,
-	                  BR_MIN_MAX_AGE,
-	                  BR_MAX_MAX_AGE,
+	                  NM_BR_MIN_MAX_AGE,
+	                  NM_BR_MAX_MAX_AGE,
 	                  !priv->stp,
 	                  NM_SETTING_BRIDGE_MAX_AGE,
 	                  error))
 		return FALSE;
 
 	if (!check_range (priv->ageing_time,
-	                  BR_MIN_AGEING_TIME,
-	                  BR_MAX_AGEING_TIME,
+	                  NM_BR_MIN_AGEING_TIME,
+	                  NM_BR_MAX_AGEING_TIME,
 	                  !priv->stp,
 	                  NM_SETTING_BRIDGE_AGEING_TIME,
 	                  error))
@@ -432,6 +418,8 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 	 * "ethernet.cloned-mac-address" anyway overwrites the MAC address of
 	 * the bridge later while activating the bridge. Hence, this property
 	 * is deprecated.
+	 *
+	 * Deprecated: 1.12: Use the ethernet.cloned-mac-address property instead.
 	 **/
 	/* ---keyfile---
 	 * property: mac-address
@@ -443,10 +431,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
@@ -522,7 +510,7 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 	g_object_class_install_property
 		(object_class, PROP_FORWARD_DELAY,
 		 g_param_spec_uint (NM_SETTING_BRIDGE_FORWARD_DELAY, "", "",
-		                    0, BR_MAX_FORWARD_DELAY, 15,
+		                    0, NM_BR_MAX_FORWARD_DELAY, 15,
 		                    G_PARAM_READWRITE |
 		                    G_PARAM_CONSTRUCT |
 		                    NM_SETTING_PARAM_INFERRABLE |
@@ -544,7 +532,7 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 	g_object_class_install_property
 		(object_class, PROP_HELLO_TIME,
 		 g_param_spec_uint (NM_SETTING_BRIDGE_HELLO_TIME, "", "",
-		                    0, BR_MAX_HELLO_TIME, 2,
+		                    0, NM_BR_MAX_HELLO_TIME, 2,
 		                    G_PARAM_READWRITE |
 		                    G_PARAM_CONSTRUCT |
 		                    NM_SETTING_PARAM_INFERRABLE |
@@ -566,7 +554,7 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 	g_object_class_install_property
 		(object_class, PROP_MAX_AGE,
 		 g_param_spec_uint (NM_SETTING_BRIDGE_MAX_AGE, "", "",
-		                    0, BR_MAX_MAX_AGE, 20,
+		                    0, NM_BR_MAX_MAX_AGE, 20,
 		                    G_PARAM_READWRITE |
 		                    G_PARAM_CONSTRUCT |
 		                    NM_SETTING_PARAM_INFERRABLE |
@@ -588,7 +576,7 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *setting_class)
 	g_object_class_install_property
 		(object_class, PROP_AGEING_TIME,
 		 g_param_spec_uint (NM_SETTING_BRIDGE_AGEING_TIME, "", "",
-		                    0, BR_MAX_AGEING_TIME, 300,
+		                    NM_BR_MIN_AGEING_TIME, NM_BR_MAX_AGEING_TIME, 300,
 		                    G_PARAM_READWRITE |
 		                    G_PARAM_CONSTRUCT |
 		                    NM_SETTING_PARAM_INFERRABLE |
diff --git a/shared/nm-utils/nm-shared-utils.c b/shared/nm-utils/nm-shared-utils.c
index 79d1e6aa9c..2e42dc0cac 100644
--- a/shared/nm-utils/nm-shared-utils.c
+++ b/shared/nm-utils/nm-shared-utils.c
@@ -845,6 +845,32 @@ nm_g_object_set_property (GObject *object,
 	return TRUE;
 }
 
+gboolean
+nm_g_object_set_property_boolean (GObject *object,
+                                  const gchar  *property_name,
+                                  gboolean value,
+                                  GError **error)
+{
+	nm_auto_unset_gvalue GValue gvalue = { 0 };
+
+	g_value_init (&gvalue, G_TYPE_BOOLEAN);
+	g_value_set_boolean (&gvalue, !!value);
+	return nm_g_object_set_property (object, property_name, &gvalue, error);
+}
+
+gboolean
+nm_g_object_set_property_uint (GObject *object,
+                               const gchar  *property_name,
+                               guint value,
+                               GError **error)
+{
+	nm_auto_unset_gvalue GValue gvalue = { 0 };
+
+	g_value_init (&gvalue, G_TYPE_UINT);
+	g_value_set_uint (&gvalue, value);
+	return nm_g_object_set_property (object, property_name, &gvalue, error);
+}
+
 GParamSpec *
 nm_g_object_class_find_property_from_gtype (GType gtype,
                                             const char *property_name)
diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h
index 1e80b35a93..d6d829cd57 100644
--- a/shared/nm-utils/nm-shared-utils.h
+++ b/shared/nm-utils/nm-shared-utils.h
@@ -386,6 +386,16 @@ gboolean nm_g_object_set_property (GObject *object,
                                    const GValue *value,
                                    GError **error);
 
+gboolean nm_g_object_set_property_boolean (GObject *object,
+                                           const gchar  *property_name,
+                                           gboolean value,
+                                           GError **error);
+
+gboolean nm_g_object_set_property_uint (GObject *object,
+                                        const gchar  *property_name,
+                                        guint value,
+                                        GError **error);
+
 GParamSpec *nm_g_object_class_find_property_from_gtype (GType gtype,
                                                         const char *property_name);
 
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index b1cf80ad91..50d0bc723a 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -77,18 +77,6 @@
 
 /*****************************************************************************/
 
-static gboolean
-get_uint (const char *str, guint32 *value)
-{
-	gint64 tmp;
-
-	tmp = _nm_utils_ascii_str_to_int64 (str, 0, 0, G_MAXUINT32, -1);
-	if (tmp == -1)
-		return FALSE;
-	*value = tmp;
-	return TRUE;
-}
-
 static void
 check_if_bond_slave (shvarFile *ifcfg,
                      NMSettingConnection *s_con)
@@ -4650,66 +4638,114 @@ team_connection_from_ifcfg (const char *file,
 	return connection;
 }
 
+typedef enum {
+	BRIDGE_OPT_TYPE_MAIN,
+	BRIDGE_OPT_TYPE_OPTION,
+	BRIDGE_OPT_TYPE_PORT_MAIN,
+	BRIDGE_OPT_TYPE_PORT_OPTION,
+} BridgeOptType;
+
 typedef void (*BridgeOptFunc) (NMSetting *setting,
                                gboolean stp,
                                const char *key,
-                               const char *value);
+                               const char *value,
+                               BridgeOptType opt_type);
 
 static void
 handle_bridge_option (NMSetting *setting,
                       gboolean stp,
                       const char *key,
-                      const char *value)
+                      const char *value,
+                      BridgeOptType opt_type)
 {
-	guint32 u = 0;
+	static const struct {
+		const char *key;
+		const char *property_name;
+		BridgeOptType opt_type;
+		gboolean only_with_stp;
+		gboolean extended_bool;
+	} m/*etadata*/[] = {
+		{ "DELAY",              NM_SETTING_BRIDGE_FORWARD_DELAY,      BRIDGE_OPT_TYPE_MAIN,   .only_with_stp = TRUE },
+		{ "priority",           NM_SETTING_BRIDGE_PRIORITY,           BRIDGE_OPT_TYPE_OPTION, .only_with_stp = TRUE },
+		{ "hello_time",         NM_SETTING_BRIDGE_HELLO_TIME,         BRIDGE_OPT_TYPE_OPTION, .only_with_stp = TRUE },
+		{ "max_age",            NM_SETTING_BRIDGE_MAX_AGE,            BRIDGE_OPT_TYPE_OPTION, .only_with_stp = TRUE },
+		{ "ageing_time",        NM_SETTING_BRIDGE_AGEING_TIME,        BRIDGE_OPT_TYPE_OPTION },
+		{ "multicast_snooping", NM_SETTING_BRIDGE_MULTICAST_SNOOPING, BRIDGE_OPT_TYPE_OPTION },
+		{ "group_fwd_mask",     NM_SETTING_BRIDGE_GROUP_FORWARD_MASK, BRIDGE_OPT_TYPE_OPTION },
+		{ "priority",           NM_SETTING_BRIDGE_PORT_PRIORITY,      BRIDGE_OPT_TYPE_PORT_OPTION },
+		{ "path_cost",          NM_SETTING_BRIDGE_PORT_PATH_COST,     BRIDGE_OPT_TYPE_PORT_OPTION },
+		{ "hairpin_mode",       NM_SETTING_BRIDGE_PORT_HAIRPIN_MODE,  BRIDGE_OPT_TYPE_PORT_OPTION, .extended_bool = TRUE, },
+	};
+	const char *error_message = NULL;
+	int i;
+	gint64 v;
 
-	if (!strcmp (key, "priority")) {
-		if (stp == FALSE)
-			PARSE_WARNING ("'priority' invalid when STP is disabled");
-		else if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_PRIORITY, u, NULL);
-		else
-			PARSE_WARNING ("invalid priority value '%s'", value);
-	} else if (!strcmp (key, "hello_time")) {
-		if (stp == FALSE)
-			PARSE_WARNING ("'hello_time' invalid when STP is disabled");
-		else if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_HELLO_TIME, u, NULL);
-		else
-			PARSE_WARNING ("invalid hello_time value '%s'", value);
-	} else if (!strcmp (key, "max_age")) {
-		if (stp == FALSE)
-			PARSE_WARNING ("'max_age' invalid when STP is disabled");
-		else if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_MAX_AGE, u, NULL);
-		else
-			PARSE_WARNING ("invalid max_age value '%s'", value);
-	} else if (!strcmp (key, "ageing_time")) {
-		if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_AGEING_TIME, u, NULL);
-		else
-			PARSE_WARNING ("invalid ageing_time value '%s'", value);
-	} else if (!strcmp (key, "multicast_snooping")) {
-		if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_MULTICAST_SNOOPING,
-			              (gboolean) u, NULL);
-		else
-			PARSE_WARNING ("invalid multicast_snooping value '%s'", value);
-	} else if (!strcmp (key, "group_fwd_mask")) {
-		if (get_uint (value, &u) && u <= 0xFFFF && !NM_FLAGS_ANY (u, 7))
-			g_object_set (setting, NM_SETTING_BRIDGE_GROUP_FORWARD_MASK,
-			              (gboolean) u, NULL);
-		else
-			PARSE_WARNING ("invalid group_fwd_mask value '%s'", value);
-	} else
-			PARSE_WARNING ("unhandled bridge option '%s'", key);
+	for (i = 0; i < G_N_ELEMENTS (m); i++) {
+		GParamSpec *param_spec;
+
+		if (opt_type != m[i].opt_type)
+			continue;
+		if (!nm_streq (key, m[i].key))
+			continue;
+		if (m[i].only_with_stp && !stp) {
+			PARSE_WARNING ("'%s' invalid when STP is disabled", key);
+			return;
+		}
+
+		param_spec = g_object_class_find_property (G_OBJECT_GET_CLASS (setting), m[i].property_name);
+		switch (param_spec->value_type) {
+		case G_TYPE_BOOLEAN:
+			if (m[i].extended_bool) {
+				if (!strcasecmp (value, "on") || !strcasecmp (value, "yes") || !strcmp (value, "1"))
+					v = TRUE;
+				else if (!strcasecmp (value, "off") || !strcasecmp (value, "no"))
+					v = FALSE;
+				else {
+					error_message = "is not a boolean";
+					goto warn;
+				}
+			} else {
+				v = _nm_utils_ascii_str_to_int64 (value, 10, 0, 1, -1);
+				if (v == -1) {
+					error_message = g_strerror (errno);
+					goto warn;
+				}
+			}
+			if (!nm_g_object_set_property_boolean (G_OBJECT (setting), m[i].property_name, v, NULL)) {
+				error_message = "number is out of range";
+				goto warn;
+			}
+			return;
+		case G_TYPE_UINT:
+			v = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXUINT, -1);
+			if (v == -1) {
+				error_message = g_strerror (errno);
+				goto warn;
+			}
+			if (!nm_g_object_set_property_uint (G_OBJECT (setting), m[i].property_name, v, NULL)) {
+				error_message = "number is out of range";
+				goto warn;
+			}
+			return;
+		default:
+			nm_assert_not_reached ();
+			continue;
+		}
+
+warn:
+		PARSE_WARNING ("invalid %s value '%s': %s", key, value, error_message);
+		return;
+	}
+
+	PARSE_WARNING ("unhandled bridge option '%s'", key);
 }
 
 static void
 handle_bridging_opts (NMSetting *setting,
                       gboolean stp,
                       const char *value,
-                      BridgeOptFunc func)
+                      BridgeOptFunc func,
+                      BridgeOptType opt_type)
 {
 	gs_free const char **items = NULL;
 	const char *const *iter;
@@ -4724,7 +4760,7 @@ handle_bridging_opts (NMSetting *setting,
 			key = *keys;
 			val = *(keys + 1);
 			if (val && key[0] && val[0])
-				func (setting, stp, key, val);
+				func (setting, stp, key, val, opt_type);
 		}
 	}
 }
@@ -4734,30 +4770,29 @@ make_bridge_setting (shvarFile *ifcfg,
                      const char *file,
                      GError **error)
 {
-	NMSettingBridge *s_bridge;
-	char *value;
-	guint32 u;
+	gs_unref_object NMSettingBridge *s_bridge = NULL;
+	gs_free char *value_to_free = NULL;
+	const char *value;
 	gboolean stp = FALSE;
 	gboolean stp_set = FALSE;
 
-	value = svGetValueStr_cp (ifcfg, "DEVICE");
+	value = svGetValueStr (ifcfg, "DEVICE", &value_to_free);
 	if (!value) {
 		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
 		             "mandatory DEVICE keyword missing");
 		return NULL;
 	}
-	g_free (value);
+	nm_clear_g_free (&value_to_free);
 
 	s_bridge = NM_SETTING_BRIDGE (nm_setting_bridge_new ());
 
-	value = svGetValueStr_cp (ifcfg, "MACADDR");
+	value = svGetValueStr (ifcfg, "BRIDGE_MACADDR", &value_to_free);
 	if (value) {
-		value = g_strstrip (value);
 		g_object_set (s_bridge, NM_SETTING_BRIDGE_MAC_ADDRESS, value, NULL);
-		g_free (value);
+		nm_clear_g_free (&value_to_free);
 	}
 
-	value = svGetValueStr_cp (ifcfg, "STP");
+	value = svGetValueStr (ifcfg, "STP", &value_to_free);
 	if (value) {
 		if (!strcasecmp (value, "on") || !strcasecmp (value, "yes")) {
 			g_object_set (s_bridge, NM_SETTING_BRIDGE_STP, TRUE, NULL);
@@ -4768,7 +4803,7 @@ make_bridge_setting (shvarFile *ifcfg,
 			stp_set = TRUE;
 		} else
 			PARSE_WARNING ("invalid STP value '%s'", value);
-		g_free (value);
+		nm_clear_g_free (&value_to_free);
 	}
 
 	if (!stp_set) {
@@ -4776,25 +4811,19 @@ make_bridge_setting (shvarFile *ifcfg,
 		g_object_set (s_bridge, NM_SETTING_BRIDGE_STP, FALSE, NULL);
 	}
 
-	value = svGetValueStr_cp (ifcfg, "DELAY");
+	value = svGetValueStr (ifcfg, "DELAY", &value_to_free);
 	if (value) {
-		if (stp) {
-			if (get_uint (value, &u))
-				g_object_set (s_bridge, NM_SETTING_BRIDGE_FORWARD_DELAY, u, NULL);
-			else
-				PARSE_WARNING ("invalid forward delay value '%s'", value);
-		} else
-			PARSE_WARNING ("DELAY invalid when STP is disabled");
-		g_free (value);
+		handle_bridge_option (NM_SETTING (s_bridge), stp, "DELAY", value, BRIDGE_OPT_TYPE_MAIN);
+		nm_clear_g_free (&value_to_free);
 	}
 
-	value = svGetValueStr_cp (ifcfg, "BRIDGING_OPTS");
+	value = svGetValueStr (ifcfg, "BRIDGING_OPTS", &value_to_free);
 	if (value) {
-		handle_bridging_opts (NM_SETTING (s_bridge), stp, value, handle_bridge_option);
-		g_free (value);
+		handle_bridging_opts (NM_SETTING (s_bridge), stp, value, handle_bridge_option, BRIDGE_OPT_TYPE_OPTION);
+		nm_clear_g_free (&value_to_free);
 	}
 
-	return (NMSetting *) s_bridge;
+	return (NMSetting *) g_steal_pointer (&s_bridge);
 }
 
 static NMConnection *
@@ -4805,6 +4834,8 @@ bridge_connection_from_ifcfg (const char *file,
 	NMConnection *connection = NULL;
 	NMSetting *con_setting = NULL;
 	NMSetting *bridge_setting = NULL;
+	NMSetting *wired_setting = NULL;
+	NMSetting8021x *s_8021x = NULL;
 
 	g_return_val_if_fail (file != NULL, NULL);
 	g_return_val_if_fail (ifcfg != NULL, NULL);
@@ -4827,57 +4858,40 @@ bridge_connection_from_ifcfg (const char *file,
 	}
 	nm_connection_add_setting (connection, bridge_setting);
 
-	return connection;
-}
+	wired_setting = make_wired_setting (ifcfg, file, &s_8021x, error);
+	if (!wired_setting) {
+		g_object_unref (connection);
+		return NULL;
+	}
+	nm_connection_add_setting (connection, wired_setting);
 
-static void
-handle_bridge_port_option (NMSetting *setting,
-                           gboolean stp,
-                           const char *key,
-                           const char *value)
-{
-	guint32 u = 0;
+	if (s_8021x)
+		nm_connection_add_setting (connection, NM_SETTING (s_8021x));
 
-	if (!strcmp (key, "priority")) {
-		if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_PORT_PRIORITY, u, NULL);
-		else
-			PARSE_WARNING ("invalid priority value '%s'", value);
-	} else if (!strcmp (key, "path_cost")) {
-		if (get_uint (value, &u))
-			g_object_set (setting, NM_SETTING_BRIDGE_PORT_PATH_COST, u, NULL);
-		else
-			PARSE_WARNING ("invalid path_cost value '%s'", value);
-	} else if (!strcmp (key, "hairpin_mode")) {
-		if (!strcasecmp (value, "on") || !strcasecmp (value, "yes") || !strcmp (value, "1"))
-			g_object_set (setting, NM_SETTING_BRIDGE_PORT_HAIRPIN_MODE, TRUE, NULL);
-		else if (!strcasecmp (value, "off") || !strcasecmp (value, "no"))
-			g_object_set (setting, NM_SETTING_BRIDGE_PORT_HAIRPIN_MODE, FALSE, NULL);
-		else
-			PARSE_WARNING ("invalid hairpin_mode value '%s'", value);
-	} else
-			PARSE_WARNING ("unhandled bridge port option '%s'", key);
+	return connection;
 }
 
 static NMSetting *
 make_bridge_port_setting (shvarFile *ifcfg)
 {
 	NMSetting *s_port = NULL;
-	char *value;
+	gs_free char *value_to_free = NULL;
+	const char *value;
 
 	g_return_val_if_fail (ifcfg != NULL, FALSE);
 
-	value = svGetValueStr_cp (ifcfg, "BRIDGE_UUID");
+	value = svGetValueStr (ifcfg, "BRIDGE_UUID", &value_to_free);
 	if (!value)
-		value = svGetValueStr_cp (ifcfg, "BRIDGE");
+		value = svGetValueStr (ifcfg, "BRIDGE", &value_to_free);
 	if (value) {
-		g_free (value);
+		nm_clear_g_free (&value_to_free);
 
 		s_port = nm_setting_bridge_port_new ();
-		value = svGetValueStr_cp (ifcfg, "BRIDGING_OPTS");
-		if (value)
-			handle_bridging_opts (s_port, FALSE, value, handle_bridge_port_option);
-		g_free (value);
+		value = svGetValueStr (ifcfg, "BRIDGING_OPTS", &value_to_free);
+		if (value) {
+			handle_bridging_opts (s_port, FALSE, value, handle_bridge_option, BRIDGE_OPT_TYPE_PORT_OPTION);
+			nm_clear_g_free (&value_to_free);
+		}
 	}
 
 	return s_port;
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index c277cb2ad5..d5cbcd3555 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -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);
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 10a0f68396..7df0c4efad 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -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));
 
@@ -7440,8 +7443,8 @@ test_write_bridge_main (void)
 	gs_unref_object NMConnection *reread = NULL;
 	NMSettingConnection *s_con;
 	NMSettingBridge *s_bridge;
-	NMSettingIPConfig *s_ip4;
-	NMSettingIPConfig *s_ip6;
+	NMSettingIPConfig *s_ip4, *s_ip6;
+	NMSettingWired *s_wired;
 	NMIPAddress *addr;
 	static const char *mac = "31:33:33:37:be:cd";
 	GError *error = NULL;
@@ -7493,6 +7496,10 @@ test_write_bridge_main (void)
 	              NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_IGNORE,
 	              NULL);
 
+	/* Wired setting */
+	s_wired = (NMSettingWired *) nm_setting_wired_new ();
+	nm_connection_add_setting (connection, NM_SETTING (s_wired));
+
 	nm_connection_add_setting (connection, nm_setting_proxy_new ());
 
 	nmtst_assert_connection_verifies_without_normalization (connection);