nm-setting-bridge: add 'group-address' bridge option

Also add related unit test.

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

clients/common/nm-meta-setting-desc.c

@@ -4890,6 +4890,9 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = {
 	    .prompt =                       N_("MAC address ageing time [300]"),
 	    .property_type =                &_pt_gobject_int,
 	),
+	PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_GROUP_ADDRESS,
+	    .property_type =                &_pt_gobject_mac,
+	),
 	PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_GROUP_FORWARD_MASK,
 	    .is_cli_option =                TRUE,
 	    .property_alias =               "group-forward-mask",

clients/common/settings-docs.h.in

@@ -114,6 +114,7 @@
 #define DESCRIBE_DOC_NM_SETTING_BOND_OPTIONS N_("Dictionary of key/value pairs of bonding options.  Both keys and values must be strings. Option names must contain only alphanumeric characters (ie, [a-zA-Z0-9]).")
 #define DESCRIBE_DOC_NM_SETTING_BRIDGE_AGEING_TIME N_("The Ethernet MAC address aging time, in seconds.")
 #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_ADDRESS N_("If specified, The MAC address of the multicast group this bridge uses for STP. The address must be a link-local address in standard Ethernet MAC address format, ie an address of the form 01:80:C2:00:00:0X, with X in [0, 4..F]. If not specified the default value is 01:80:C2:00:00:00.")
 #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. Deprecated: 1")

libnm-core/nm-setting-bridge.c

@@ -41,6 +41,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMSettingBridge,
 	PROP_HELLO_TIME,
 	PROP_MAX_AGE,
 	PROP_AGEING_TIME,
+	PROP_GROUP_ADDRESS,
 	PROP_GROUP_FORWARD_MASK,
 	PROP_MULTICAST_SNOOPING,
 	PROP_VLAN_FILTERING,
@@ -51,6 +52,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMSettingBridge,
 typedef struct {
 	GPtrArray *vlans;
 	char *   mac_address;
+	char *   group_address;
 	guint32  ageing_time;
 	guint16  priority;
 	guint16  forward_delay;
@@ -899,6 +901,22 @@ nm_setting_bridge_clear_vlans (NMSettingBridge *setting)
 	}
 }
 
+/**
+ * nm_setting_bridge_get_group_address:
+ * @setting: the #NMSettingBridge
+ *
+ * Returns: the #NMSettingBridge:group-address property of the setting
+ *
+ * Since 1.24
+ **/
+const char *
+nm_setting_bridge_get_group_address (const NMSettingBridge *setting)
+{
+	g_return_val_if_fail (NM_IS_SETTING_BRIDGE (setting), NULL);
+
+	return NM_SETTING_BRIDGE_GET_PRIVATE (setting)->group_address;
+}
+
 /*****************************************************************************/
 
 static gboolean
@@ -989,6 +1007,16 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	                                        NM_SETTING_BRIDGE_VLANS))
 		return FALSE;
 
+	if (   priv->group_address
+	    && !_nm_utils_hwaddr_link_local_valid (priv->group_address)) {
+		g_set_error_literal (error,
+		                     NM_CONNECTION_ERROR,
+		                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
+		                     _("is not a valid link local MAC address"));
+		g_prefix_error (error, "%s.%s: ", NM_SETTING_BRIDGE_SETTING_NAME, NM_SETTING_BRIDGE_GROUP_ADDRESS);
+		return FALSE;
+	}
+
 	/* Failures from here on are NORMALIZABLE... */
 
 	if (!_nm_utils_bridge_vlan_verify_list (priv->vlans,
@@ -1069,6 +1097,9 @@ get_property (GObject *object, guint prop_id,
 	case PROP_AGEING_TIME:
 		g_value_set_uint (value, priv->ageing_time);
 		break;
+	case PROP_GROUP_ADDRESS:
+		g_value_set_string (value, priv->group_address);
+		break;
 	case PROP_GROUP_FORWARD_MASK:
 		g_value_set_uint (value, priv->group_forward_mask);
 		break;
@@ -1122,6 +1153,11 @@ set_property (GObject *object, guint prop_id,
 	case PROP_AGEING_TIME:
 		priv->ageing_time = g_value_get_uint (value);
 		break;
+	case PROP_GROUP_ADDRESS:
+		g_free (priv->group_address);
+		priv->group_address = _nm_utils_hwaddr_canonical_or_invalid (g_value_get_string (value),
+		                                                             ETH_ALEN);
+		break;
 	case PROP_GROUP_FORWARD_MASK:
 		priv->group_forward_mask = (guint16) g_value_get_uint (value);
 		break;
@@ -1184,6 +1220,7 @@ finalize (GObject *object)
 	NMSettingBridgePrivate *priv = NM_SETTING_BRIDGE_GET_PRIVATE (object);
 
 	g_free (priv->mac_address);
+	g_free (priv->group_address);
 	g_ptr_array_unref (priv->vlans);
 
 	G_OBJECT_CLASS (nm_setting_bridge_parent_class)->finalize (object);
@@ -1493,6 +1530,32 @@ nm_setting_bridge_class_init (NMSettingBridgeClass *klass)
 	 */
 	_nm_properties_override_dbus (properties_override, "interface-name", &nm_sett_info_propert_type_deprecated_interface_name);
 
+	/**
+	 * NMSettingBridge:group-address:
+	 *
+	 * If specified, The MAC address of the multicast group this bridge uses for STP.
+	 *
+	 * The address must be a link-local address in standard Ethernet MAC address format,
+	 * ie an address of the form 01:80:C2:00:00:0X, with X in [0, 4..F].
+	 * If not specified the default value is 01:80:C2:00:00:00.
+	 *
+	 * Since: 1.24
+	 **/
+	/* ---ifcfg-rh---
+	 * property: group-address
+	 * variable: BRIDGING_OPTS: group_address=
+	 * description: STP group address.
+	 * example: BRIDGING_OPTS="group_address=01:80:C2:00:00:0A"
+	 * ---end---
+	 */
+	obj_properties[PROP_GROUP_ADDRESS] =
+	    g_param_spec_string (NM_SETTING_BRIDGE_GROUP_ADDRESS, "", "",
+	                         NULL,
+	                         G_PARAM_READWRITE |
+	                         NM_SETTING_PARAM_INFERRABLE |
+	                         G_PARAM_STATIC_STRINGS);
+	_nm_properties_override_gobj (properties_override, obj_properties[PROP_GROUP_ADDRESS], &nm_sett_info_propert_type_mac_address);
+
 	g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties);
 
 	_nm_setting_class_commit_full (setting_class, NM_META_SETTING_TYPE_BRIDGE,

libnm-core/nm-setting-bridge.h

@@ -35,6 +35,7 @@ G_BEGIN_DECLS
 #define NM_SETTING_BRIDGE_VLAN_FILTERING     "vlan-filtering"
 #define NM_SETTING_BRIDGE_VLAN_DEFAULT_PVID  "vlan-default-pvid"
 #define NM_SETTING_BRIDGE_VLANS              "vlans"
+#define NM_SETTING_BRIDGE_GROUP_ADDRESS  "group-address"
 
 #define NM_BRIDGE_VLAN_VID_MIN            1
 #define NM_BRIDGE_VLAN_VID_MAX            4094
@@ -116,6 +117,9 @@ char         * nm_bridge_vlan_to_str (const NMBridgeVlan *vlan, GError **error);
 NM_AVAILABLE_IN_1_18
 NMBridgeVlan * nm_bridge_vlan_from_str (const char *str, GError **error);
 
+NM_AVAILABLE_IN_1_24
+const char *   nm_setting_bridge_get_group_address (const NMSettingBridge *setting);
+
 G_END_DECLS
 
 #endif /* __NM_SETTING_BRIDGE_H__ */

libnm-core/nm-utils-private.h

@@ -65,6 +65,8 @@ void        _nm_utils_bytes_from_dbus   (GVariant *dbus_value,
 
 char *      _nm_utils_hwaddr_canonical_or_invalid (const char *mac, gssize length);
 
+gboolean    _nm_utils_hwaddr_link_local_valid (const char *mac);
+
 void        _nm_utils_format_variant_attributes_full (GString *str,
                                                       const NMUtilsNamedValue *values,
                                                       guint num_values,

libnm-core/nm-utils.c

@@ -4186,6 +4186,41 @@ _nm_utils_hwaddr_canonical_or_invalid (const char *mac, gssize length)
 		return g_strdup (mac);
 }
 
+/*
+ * Determine if given Ethernet address is link-local
+ *
+ * Return value: %TRUE if @mac is link local
+ * reserved addr (01:80:c2:00:00:0X) per IEEE 802.1Q 8.6.3 Frame filtering, %FALSE if not.
+ */
+gboolean
+_nm_utils_hwaddr_link_local_valid (const char *mac)
+{
+	guint8 mac_net[ETH_ALEN];
+	static const guint8 eth_reserved_addr_base[] = {
+		0x01, 0x80,
+		0xc2, 0x00,
+		0x00
+	};
+
+	if (!mac)
+		return FALSE;
+
+	if (!nm_utils_hwaddr_aton (mac, mac_net, ETH_ALEN))
+		return FALSE;
+
+	if (   memcmp (mac_net,
+	               eth_reserved_addr_base, ETH_ALEN - 1)
+	    || (mac_net[5] & 0xF0))
+		return FALSE;
+
+	if (   mac_net[5] == 1  /* 802.3x Pause address */
+	    || mac_net[5] == 2  /* 802.3ad Slow protocols */
+	    || mac_net[5] == 3) /* 802.1X PAE address */
+		return FALSE;
+
+	return TRUE;
+}
+
 /**
  * nm_utils_hwaddr_matches:
  * @hwaddr1: (nullable): pointer to a binary or ASCII hardware address, or %NULL

libnm-core/tests/test-setting.c

@@ -500,8 +500,6 @@ create_bond_connection (NMConnection **con, NMSettingBond **s_bond)
 	                                        NULL,
 	                                        NM_SETTING_BOND_SETTING_NAME,
 	                                        &s_con);
-	g_assert (*con);
-	g_assert (s_con);
 
 	g_object_set (s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, "bond0", NULL);
 
@@ -512,28 +510,25 @@ create_bond_connection (NMConnection **con, NMSettingBond **s_bond)
 }
 
 #define test_verify_options(exp, ...) \
-	_test_verify_options (NM_MAKE_STRV (__VA_ARGS__), exp)
+	_test_verify_options (exp, NM_MAKE_STRV (__VA_ARGS__))
 
 static void
-_test_verify_options (const char *const *options,
+_test_verify_options (gboolean expected_result,
-                      gboolean expected_result)
+                      const char *const *options)
 {
 	gs_unref_object NMConnection *con = NULL;
 	NMSettingBond *s_bond;
-	GError *error = NULL;
-	gboolean success;
 	const char *const *option;
 
+	g_assert (NM_PTRARRAY_LEN (options) % 2 == 0);
+
 	create_bond_connection (&con, &s_bond);
 
-	for (option = options; option[0] && option[1]; option += 2)
+	for (option = options; option[0]; option += 2)
 		g_assert (nm_setting_bond_add_option (s_bond, option[0], option[1]));
 
 	if (expected_result) {
 		nmtst_assert_connection_verifies_and_normalizable (con);
-		nmtst_connection_normalize (con);
-		success = nm_setting_verify ((NMSetting *) s_bond, con, &error);
-		nmtst_assert_success (success, error);
 	} else {
 		nmtst_assert_connection_unnormalizable (con,
 		                                        NM_CONNECTION_ERROR,
@@ -1874,6 +1869,105 @@ test_bridge_vlans (void)
 	nm_bridge_vlan_unref (v2);
 }
 
+static void
+create_bridge_connection (NMConnection **con, NMSettingBridge **s_bridge)
+{
+	NMSettingConnection *s_con;
+
+	g_assert (con);
+	g_assert (s_bridge);
+
+	*con = nmtst_create_minimal_connection ("bridge",
+	                                        NULL,
+	                                        NM_SETTING_BOND_SETTING_NAME,
+	                                        &s_con);
+
+	g_object_set (s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, "bridge0", NULL);
+
+	*s_bridge = (NMSettingBridge *) nm_setting_bridge_new ();
+	g_assert (*s_bridge);
+
+	nm_connection_add_setting (*con, NM_SETTING (*s_bridge));
+}
+
+#define test_verify_options_bridge(exp, ...) \
+	_test_verify_options_bridge (exp, NM_MAKE_STRV (__VA_ARGS__))
+
+static void
+_test_verify_options_bridge (gboolean expected_result,
+                             const char *const *options)
+{
+	gs_unref_object NMConnection *con = NULL;
+	NMSettingBridge *s_bridge;
+	const char *const *option;
+
+	g_assert (NM_PTRARRAY_LEN (options) % 2 == 0);
+
+	create_bridge_connection (&con, &s_bridge);
+
+	for (option = options; option[0]; option += 2) {
+		const char *option_key = option[0];
+		const char *option_val = option[1];
+		GParamSpec *pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (s_bridge), option_key);
+
+		g_assert (pspec);
+		g_assert (option_val);
+
+		switch (G_PARAM_SPEC_VALUE_TYPE (pspec)) {
+		case G_TYPE_UINT: {
+				guint uvalue;
+
+				uvalue = _nm_utils_ascii_str_to_uint64 (option_val, 10, 0, G_MAXUINT, 0);
+				g_object_set (s_bridge, option_key, uvalue, NULL);
+			}
+			break;
+		case G_TYPE_BOOLEAN: {
+				gboolean bvalue;
+
+				bvalue = _nm_utils_ascii_str_to_bool (option_val, FALSE);
+				g_object_set (s_bridge, option_key, bvalue, NULL);
+			}
+			break;
+		case G_TYPE_STRING:
+			g_object_set (s_bridge, option_key, option_val, NULL);
+			break;
+		default:
+			g_assert_not_reached();
+			break;
+		}
+	}
+
+	if (expected_result)
+		nmtst_assert_connection_verifies_and_normalizable (con);
+	else {
+		nmtst_assert_connection_unnormalizable (con,
+		                                        NM_CONNECTION_ERROR,
+		                                        NM_CONNECTION_ERROR_INVALID_PROPERTY);
+	}
+}
+
+static void
+test_bridge_verify (void)
+{
+	/* group-address */
+	test_verify_options_bridge (FALSE,
+	                            "group-address", "nonsense");
+	test_verify_options_bridge (FALSE,
+	                            "group-address", "FF:FF:FF:FF:FF:FF");
+	test_verify_options_bridge (FALSE,
+	                            "group-address", "01:02:03:04:05:06");
+	test_verify_options_bridge (TRUE,
+	                            "group-address", "01:80:C2:00:00:00");
+	test_verify_options_bridge (FALSE,
+	                            "group-address", "01:80:C2:00:00:02");
+	test_verify_options_bridge (FALSE,
+	                            "group-address", "01:80:C2:00:00:03");
+	test_verify_options_bridge (TRUE,
+	                            "group-address", "01:80:C2:00:00:00");
+	test_verify_options_bridge (TRUE,
+	                            "group-address", "01:80:C2:00:00:0A");
+}
+
 /*****************************************************************************/
 
 static void
@@ -3653,6 +3747,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/libnm/settings/tc_config/dbus", test_tc_config_dbus);
 
 	g_test_add_func ("/libnm/settings/bridge/vlans", test_bridge_vlans);
+	g_test_add_func ("/libnm/settings/bridge/verify", test_bridge_verify);
 
 	g_test_add_func ("/libnm/settings/team/sync_runner_from_config_roundrobin",
 	                 test_runner_roundrobin_sync_from_config);

libnm/libnm.ver

@@ -1685,6 +1685,7 @@ global:
 	nm_setting_802_1x_get_domain_match;
 	nm_setting_802_1x_get_phase2_domain_match;
 	nm_setting_bond_get_option_normalized;
+	nm_setting_bridge_get_group_address;
 	nm_setting_vrf_get_table;
 	nm_setting_vrf_get_type;
 	nm_setting_vrf_new;

shared/nm-utils/nm-test-utils.h

@@ -1803,8 +1803,12 @@ nmtst_create_minimal_connection (const char *id, const char *uuid, const char *t
 	con = nm_connection_new ();
 #endif
 
+	g_assert (con);
+
 	s_con = NM_SETTING_CONNECTION (nm_setting_connection_new ());
 
+	g_assert (s_con);
+
 	g_object_set (s_con,
 	              NM_SETTING_CONNECTION_ID, id,
 	              NM_SETTING_CONNECTION_UUID, uuid,

src/devices/nm-device-bridge.c

@@ -158,11 +158,26 @@ complete_connection (NMDevice *device,
 	return TRUE;
 }
 
+static void
+from_sysfs_group_address (const char *value, GValue *out)
+{
+	if (!nm_utils_hwaddr_matches (value, -1, "01:80:C2:00:00:00", -1))
+		g_value_set_string (out, value);
+}
+
+static const char *
+to_sysfs_group_address (GValue *value)
+{
+	return g_value_get_string (value) ?: "01:80:C2:00:00:00";
+}
+
 /*****************************************************************************/
 
 typedef struct {
 	const char *name;
 	const char *sysname;
+	const char *(*to_sysfs) (GValue *value);
+	void (*from_sysfs) (const char *value, GValue *out);
 	uint nm_min;
 	uint nm_max;
 	uint nm_default;
@@ -173,40 +188,55 @@ typedef struct {
 
 static const Option master_options[] = {
 	{ NM_SETTING_BRIDGE_STP,                "stp_state", /* this must stay as the first item */
+	                                        NULL, NULL,
 	                                        0, 1, 1,
 	                                        FALSE, FALSE, FALSE },
 	{ NM_SETTING_BRIDGE_PRIORITY,           "priority",
+	                                        NULL, NULL,
 	                                        0, G_MAXUINT16, 0x8000,
 	                                        TRUE, FALSE, TRUE },
 	{ NM_SETTING_BRIDGE_FORWARD_DELAY,      "forward_delay",
+	                                        NULL, NULL,
 	                                        0, NM_BR_MAX_FORWARD_DELAY, 15,
 	                                        TRUE, TRUE, TRUE},
 	{ NM_SETTING_BRIDGE_HELLO_TIME,         "hello_time",
+	                                        NULL, NULL,
 	                                        0, NM_BR_MAX_HELLO_TIME, 2,
 	                                        TRUE, TRUE, TRUE },
 	{ NM_SETTING_BRIDGE_MAX_AGE,            "max_age",
+	                                        NULL, NULL,
 	                                        0, NM_BR_MAX_MAX_AGE, 20,
 	                                        TRUE, TRUE, TRUE },
 	{ NM_SETTING_BRIDGE_AGEING_TIME,        "ageing_time",
+	                                        NULL, NULL,
 	                                        NM_BR_MIN_AGEING_TIME, NM_BR_MAX_AGEING_TIME, 300,
 	                                        TRUE, TRUE, FALSE },
 	{ NM_SETTING_BRIDGE_GROUP_FORWARD_MASK, "group_fwd_mask",
+	                                        NULL, NULL,
 	                                        0, 0xFFFF, 0,
 	                                        TRUE, FALSE, FALSE },
 	{ NM_SETTING_BRIDGE_MULTICAST_SNOOPING, "multicast_snooping",
+	                                        NULL, NULL,
 	                                        0, 1, 1,
 	                                        FALSE, FALSE, FALSE },
+	{ NM_SETTING_BRIDGE_GROUP_ADDRESS,      "group_addr",
+	                                        to_sysfs_group_address, from_sysfs_group_address,
+	                                        0, 0, 0,
+	                                        FALSE, FALSE, FALSE },
 	{ NULL, NULL }
 };
 
 static const Option slave_options[] = {
 	{ NM_SETTING_BRIDGE_PORT_PRIORITY,     "priority",
+	                                       NULL, NULL,
 	                                       0, NM_BR_PORT_MAX_PRIORITY, NM_BR_PORT_DEF_PRIORITY,
 	                                       TRUE, FALSE },
 	{ NM_SETTING_BRIDGE_PORT_PATH_COST,    "path_cost",
+	                                       NULL, NULL,
 	                                       0, NM_BR_PORT_MAX_PATH_COST, 100,
 	                                       TRUE, FALSE },
 	{ NM_SETTING_BRIDGE_PORT_HAIRPIN_MODE, "hairpin_mode",
+	                                       NULL, NULL,
 	                                       0, 1, 0,
 	                                       FALSE, FALSE },
 	{ NULL, NULL }
@@ -216,10 +246,9 @@ static void
 commit_option (NMDevice *device, NMSetting *setting, const Option *option, gboolean slave)
 {
 	int ifindex = nm_device_get_ifindex (device);
+	nm_auto_unset_gvalue GValue val = G_VALUE_INIT;
 	GParamSpec *pspec;
-	GValue val = G_VALUE_INIT;
+	const char *value;
-	guint32 uval = 0;
-	char value[100];
 
 	g_assert (setting);
 
@@ -229,37 +258,67 @@ commit_option (NMDevice *device, NMSetting *setting, const Option *option, gbool
 	/* Get the property's value */
 	g_value_init (&val, G_PARAM_SPEC_VALUE_TYPE (pspec));
 	g_object_get_property ((GObject *) setting, option->name, &val);
-	if (G_VALUE_HOLDS_BOOLEAN (&val))
-		uval = g_value_get_boolean (&val) ? 1 : 0;
-	else if (G_VALUE_HOLDS_UINT (&val)) {
-		uval = g_value_get_uint (&val);
 
-		/* zero means "unspecified" for some NM properties but isn't in the
+	if (option->to_sysfs) {
-		 * allowed kernel range, so reset the property to the default value.
+		value = option->to_sysfs(&val);
-		 */
+		goto out;
-		if (option->default_if_zero && uval == 0) {
+	}
-			g_value_unset (&val);
-			g_value_init (&val, G_PARAM_SPEC_VALUE_TYPE (pspec));
-			g_param_value_set_default (pspec, &val);
-			uval = g_value_get_uint (&val);
-		}
 
-		/* Linux kernel bridge interfaces use 'centiseconds' for time-based values.
+	switch (pspec->value_type) {
-		 * In reality it's not centiseconds, but depends on HZ and USER_HZ, which
+		case G_TYPE_BOOLEAN:
-		 * is almost always works out to be a multiplier of 100, so we can assume
+			value = g_value_get_boolean (&val) ? "1" : "0";
-		 * centiseconds.  See clock_t_to_jiffies().
+			break;
-		 */
+		case G_TYPE_UINT: {
-		if (option->user_hz_compensate)
+				char value_buf[100];
-			uval *= 100;
+				guint uval;
-	} else
-		nm_assert_not_reached ();
-	g_value_unset (&val);
 
-	nm_sprintf_buf (value, "%u", uval);
+				uval = g_value_get_uint (&val);
-	if (slave)
+
-		nm_platform_sysctl_slave_set_option (nm_device_get_platform (device), ifindex, option->sysname, value);
+				/* zero means "unspecified" for some NM properties but isn't in the
-	else
+				 * allowed kernel range, so reset the property to the default value.
-		nm_platform_sysctl_master_set_option (nm_device_get_platform (device), ifindex, option->sysname, value);
+				 */
+				if (option->default_if_zero && uval == 0) {
+					g_value_unset (&val);
+					g_value_init (&val, G_PARAM_SPEC_VALUE_TYPE (pspec));
+					g_param_value_set_default (pspec, &val);
+					uval = g_value_get_uint (&val);
+				}
+
+				/* Linux kernel bridge interfaces use 'centiseconds' for time-based values.
+				 * In reality it's not centiseconds, but depends on HZ and USER_HZ, which
+				 * is almost always works out to be a multiplier of 100, so we can assume
+				 * centiseconds.  See clock_t_to_jiffies().
+				 */
+				if (option->user_hz_compensate)
+					uval *= 100;
+
+				nm_sprintf_buf (value_buf, "%u", uval);
+				value = value_buf;
+			}
+			break;
+		case G_TYPE_STRING:
+			value = g_value_get_string (&val);
+			break;
+		default:
+			nm_assert_not_reached ();
+			break;
+	}
+
+out:
+	if (!value)
+		return;
+
+	if (slave) {
+		nm_platform_sysctl_slave_set_option (nm_device_get_platform (device),
+		                                     ifindex,
+		                                     option->sysname,
+		                                     value);
+	} else {
+		nm_platform_sysctl_master_set_option (nm_device_get_platform (device),
+		                                      ifindex,
+		                                      option->sysname,
+		                                      value);
+	}
 }
 
 static const NMPlatformBridgeVlan **
@@ -335,29 +394,68 @@ update_connection (NMDevice *device, NMConnection *connection)
 	option++;
 
 	for (; option->name; option++) {
-		gs_free char *str = nm_platform_sysctl_master_get_option (nm_device_get_platform (device), ifindex, option->sysname);
+		nm_auto_unset_gvalue GValue value = G_VALUE_INIT;
-		uint value;
+		gs_free char *str = NULL;
+		GParamSpec *pspec;
+
+		str = nm_platform_sysctl_master_get_option (nm_device_get_platform (device), ifindex, option->sysname);
+		pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (s_bridge), option->name);
 
 		if (!stp_value && option->only_with_stp)
 			continue;
 
-		if (str) {
+		if (!str) {
-			/* See comments in set_sysfs_uint() about centiseconds. */
-			if (option->user_hz_compensate) {
-				value = _nm_utils_ascii_str_to_int64 (str, 10,
-				                                      option->nm_min * 100,
-				                                      option->nm_max * 100,
-				                                      option->nm_default * 100);
-				value /= 100;
-			} else {
-				value = _nm_utils_ascii_str_to_int64 (str, 10,
-				                                      option->nm_min,
-				                                      option->nm_max,
-				                                      option->nm_default);
-			}
-			g_object_set (s_bridge, option->name, value, NULL);
-		} else
 			_LOGW (LOGD_BRIDGE, "failed to read bridge setting '%s'", option->sysname);
+			continue;
+		}
+
+		g_value_init (&value, G_PARAM_SPEC_VALUE_TYPE (pspec));
+
+		if (option->from_sysfs) {
+			option->from_sysfs(str, &value);
+			goto out;
+		}
+
+		switch (pspec->value_type) {
+		case G_TYPE_UINT: {
+				guint uvalue;
+
+				/* See comments in set_sysfs_uint() about centiseconds. */
+				if (option->user_hz_compensate) {
+					uvalue = _nm_utils_ascii_str_to_int64 (str, 10,
+					                                       option->nm_min * 100,
+					                                       option->nm_max * 100,
+					                                       option->nm_default * 100);
+					uvalue /= 100;
+				} else {
+					uvalue = _nm_utils_ascii_str_to_int64 (str, 10,
+					                                       option->nm_min,
+					                                       option->nm_max,
+					                                       option->nm_default);
+				}
+				g_value_set_uint (&value, uvalue);
+			}
+			break;
+		case G_TYPE_BOOLEAN: {
+				gboolean bvalue;
+
+				bvalue = _nm_utils_ascii_str_to_int64 (str, 10,
+				                                       option->nm_min,
+				                                       option->nm_max,
+				                                       option->nm_default);
+				g_value_set_boolean (&value, bvalue);
+			}
+			break;
+		case G_TYPE_STRING:
+			g_value_set_string (&value, str);
+			break;
+		default:
+			nm_assert_not_reached();
+			break;
+		}
+
+out:
+		g_object_set_property (G_OBJECT (s_bridge), option->name, &value);
 	}
 }
 

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c

@@ -5160,6 +5160,7 @@ handle_bridge_option (NMSetting *setting,
 		{ "multicast_snooping", NM_SETTING_BRIDGE_MULTICAST_SNOOPING, BRIDGE_OPT_TYPE_OPTION },
 		{ "vlan_filtering",     NM_SETTING_BRIDGE_VLAN_FILTERING,     BRIDGE_OPT_TYPE_OPTION },
 		{ "default_pvid",       NM_SETTING_BRIDGE_VLAN_DEFAULT_PVID,  BRIDGE_OPT_TYPE_OPTION },
+		{ "group_address",      NM_SETTING_BRIDGE_GROUP_ADDRESS,      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 },
@@ -5216,6 +5217,9 @@ handle_bridge_option (NMSetting *setting,
 				goto warn;
 			}
 			return;
+		case G_TYPE_STRING:
+			nm_g_object_set_property_string (G_OBJECT (setting), m[i].property_name, value, NULL);
+			return;
 		default:
 			nm_assert_not_reached ();
 			continue;

src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c

@@ -1438,8 +1438,8 @@ write_bridge_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wire
 	NMSettingBridge *s_bridge;
 	guint32 i;
 	gboolean b;
+	const char *s;
 	GString *opts;
-	const char *mac;
 
 	s_bridge = nm_connection_get_setting_bridge (connection);
 	if (!s_bridge) {
@@ -1450,8 +1450,8 @@ write_bridge_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wire
 
 	svSetValueBoolean (ifcfg, "STP", FALSE);
 
-	mac = nm_setting_bridge_get_mac_address (s_bridge);
+	s = nm_setting_bridge_get_mac_address (s_bridge);
-	svSetValueStr (ifcfg, "BRIDGE_MACADDR", mac);
+	svSetValueStr (ifcfg, "BRIDGE_MACADDR", s);
 
 	/* Bridge options */
 	opts = g_string_sized_new (32);
@@ -1487,6 +1487,13 @@ write_bridge_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wire
 		g_string_append_printf (opts, "ageing_time=%u", i);
 	}
 
+	s = nm_setting_bridge_get_group_address (s_bridge);
+	if (s) {
+		if (opts->len)
+			g_string_append_c (opts, ' ');
+		g_string_append_printf (opts, "group_address=%s", s);
+	}
+
 	i = nm_setting_bridge_get_group_forward_mask (s_bridge);
 	if (i != get_setting_default_uint (NM_SETTING (s_bridge), NM_SETTING_BRIDGE_GROUP_FORWARD_MASK)) {
 		if (opts->len)