GNOME.org
 

Re: [PATCH 1/9] bonding: settings parser for ifcfg plugin + NMSettingBond class

On Fri, 2011-09-23 at 14:52 +0200, Thomas Graf wrote:
> Introduced a new TYPE=bond for ifcfg-rh configuration files.
> Alternatively BONDING_MASTER=yes can be specified instead of
> setting the type explicitely.
> 
> Bonding device files require a DEVICE= line to be present which
> specifies the virtual bonding device in the kernel. We do not
> allow auto-generation of the name in order to keep confusion to
> a minimum when reusing existing bonding devices.
> 
> The BONDING_OPTS= parameter can be used to specify various bonding
> related options, such as:
>   - mode
>   - miimon
>   - updelay
>   - downdelay
>   - arp_interval
>   - arp_ip_target
> 
> By default, the NMSettingBond class uses a miimon value of 100 which
> seems like a sensible default value for 99% of all configurations.
> If this is not suitable, an arp_ip_target needs to be specified
> manually.
> 
> A writer is not yet implemented.
> 
> Signed-off-by: Thomas Graf <tgraf redhat com>
> ---
>  libnm-util/Makefile.am                 |    2 +
>  libnm-util/libnm-util.ver              |   11 +
>  libnm-util/nm-connection.c             |    8 +-
>  libnm-util/nm-setting-bond.c           |  554 ++++++++++++++++++++++++++++++++
>  libnm-util/nm-setting-bond.h           |   94 ++++++
>  src/settings/plugins/ifcfg-rh/common.h |    3 +-
>  src/settings/plugins/ifcfg-rh/reader.c |  163 +++++++++-
>  7 files changed, 828 insertions(+), 7 deletions(-)
>  create mode 100644 libnm-util/nm-setting-bond.c
>  create mode 100644 libnm-util/nm-setting-bond.h
> 
> diff --git a/libnm-util/Makefile.am b/libnm-util/Makefile.am
> index 1529b4a..324847c 100644
> --- a/libnm-util/Makefile.am
> +++ b/libnm-util/Makefile.am
> @@ -15,6 +15,7 @@ libnm_util_include_HEADERS = 		\
>  	nm-setting.h			\
>  	nm-setting-8021x.h		\
>  	nm-setting-bluetooth.h		\
> +	nm-setting-bond.h		\
>  	nm-setting-connection.h		\
>  	nm-setting-ip4-config.h		\
>  	nm-setting-ip6-config.h		\
> @@ -44,6 +45,7 @@ libnm_util_la_csources = \
>  	nm-setting.c			\
>  	nm-setting-8021x.c		\
>  	nm-setting-bluetooth.c		\
> +	nm-setting-bond.c		\
>  	nm-setting-connection.c		\
>  	nm-setting-ip4-config.c		\
>  	nm-setting-ip6-config.c		\
> diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver
> index edf5d29..8c28edf 100644
> --- a/libnm-util/libnm-util.ver
> +++ b/libnm-util/libnm-util.ver
> @@ -166,6 +166,17 @@ global:
>  	nm_setting_bluetooth_get_connection_type;
>  	nm_setting_bluetooth_get_type;
>  	nm_setting_bluetooth_new;
> +	nm_setting_bond_error_get_type;
> +	nm_setting_bond_error_quark;
> +	nm_setting_bond_new;
> +	nm_setting_bond_get_type;
> +	nm_setting_bond_get_device_name;
> +	nm_setting_bond_get_mode;
> +	nm_setting_bond_get_miimon;
> +	nm_setting_bond_get_downdelay;
> +	nm_setting_bond_get_updelay;
> +	nm_setting_bond_get_arp_interval;
> +	nm_setting_bond_get_arp_ip_target;
>  	nm_setting_cdma_error_get_type;
>  	nm_setting_cdma_error_quark;
>  	nm_setting_cdma_get_number;
> diff --git a/libnm-util/nm-connection.c b/libnm-util/nm-connection.c
> index d59c8b4..c444885 100644
> --- a/libnm-util/nm-connection.c
> +++ b/libnm-util/nm-connection.c
> @@ -44,6 +44,7 @@
>  #include "nm-setting-wireless-security.h"
>  #include "nm-setting-vpn.h"
>  #include "nm-setting-olpc-mesh.h"
> +#include "nm-setting-bond.h"
>  
>  #include "nm-setting-serial.h"
>  #include "nm-setting-gsm.h"
> @@ -135,7 +136,7 @@ static guint signals[LAST_SIGNAL] = { 0 };
>  
>  static GHashTable *registered_settings = NULL;
>  
> -#define DEFAULT_MAP_SIZE 16
> +#define DEFAULT_MAP_SIZE 17
>  
>  static struct SettingInfo {
>  	const char *name;
> @@ -242,6 +243,11 @@ register_default_settings (void)
>  	                      NM_SETTING_WIMAX_ERROR,
>  	                      1, TRUE);
>  
> +	register_one_setting (NM_SETTING_BOND_SETTING_NAME,
> +			      NM_TYPE_SETTING_BOND,
> +			      NM_SETTING_BOND_ERROR,
> +			      1, TRUE);
> +
>  	register_one_setting (NM_SETTING_WIRELESS_SECURITY_SETTING_NAME,
>  	                      NM_TYPE_SETTING_WIRELESS_SECURITY,
>  	                      NM_SETTING_WIRELESS_SECURITY_ERROR,
> diff --git a/libnm-util/nm-setting-bond.c b/libnm-util/nm-setting-bond.c
> new file mode 100644
> index 0000000..74d736d
> --- /dev/null
> +++ b/libnm-util/nm-setting-bond.c
> @@ -0,0 +1,554 @@
> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
> +
> +/*
> + * Thomas Graf <tgraf redhat com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the
> + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> + * Boston, MA 02110-1301 USA.
> + *
> + * (C) Copyright 2011 Red Hat, Inc.
> + */
> +
> +#include <string.h>
> +#include <ctype.h>
> +#include <dbus/dbus-glib.h>
> +
> +#include "nm-setting-bond.h"
> +#include "nm-param-spec-specialized.h"
> +#include "nm-utils.h"
> +#include "nm-utils-private.h"
> +#include "nm-dbus-glib-types.h"
> +
> +/**
> + * SECTION:nm-setting-bond
> + * @short_description: Describes connection properties for bonds
> + * @include: nm-setting-bond.h
> + *
> + * The #NMSettingBond object is a #NMSetting subclass that describes properties
> + * necessary for bond connections.
> + **/
> +
> +/**
> + * nm_setting_bond_error_quark:
> + *
> + * Registers an error quark for #NMSettingBond if necessary.
> + *
> + * Returns: the error quark used for #NMSettingBond errors.
> + **/
> +GQuark
> +nm_setting_bond_error_quark (void)
> +{
> +	static GQuark quark;
> +
> +	if (G_UNLIKELY (!quark))
> +		quark = g_quark_from_static_string ("nm-setting-bond-error-quark");
> +	return quark;
> +}
> +
> +/* This should really be standard. */
> +#define ENUM_ENTRY(NAME, DESC) { NAME, "" #NAME "", DESC }
> +
> +GType
> +nm_setting_bond_error_get_type (void)
> +{
> +	static GType etype = 0;
> +
> +	if (etype == 0) {
> +		static const GEnumValue values[] = {
> +			/* Unknown error. */
> +			ENUM_ENTRY (NM_SETTING_BOND_ERROR_UNKNOWN, "UnknownError"),
> +			/* The specified property was invalid. */
> +			ENUM_ENTRY (NM_SETTING_BOND_ERROR_INVALID_PROPERTY, "InvalidProperty"),
> +			/* The specified property was missing and is required. */
> +			ENUM_ENTRY (NM_SETTING_BOND_ERROR_MISSING_PROPERTY, "MissingProperty"),
> +			{ 0, 0, 0 }
> +		};
> +
> +		etype = g_enum_register_static ("NMSettingBondError", values);
> +	}
> +
> +	return etype;
> +}
> +
> +
> +G_DEFINE_TYPE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING)
> +
> +#define NM_SETTING_BOND_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SETTING_BOND, NMSettingBondPrivate))
> +
> +typedef struct {
> +	char *	device_name;
> +	char *	mode;
> +	guint32	miimon;
> +	guint32	downdelay;
> +	guint32	updelay;
> +	guint32	arp_interval;
> +	char *	arp_ip_target;
> +} NMSettingBondPrivate;
> +
> +enum {
> +	PROP_0,
> +	PROP_DEVICE_NAME,
> +	PROP_MODE,
> +	PROP_MIIMON,
> +	PROP_DOWNDELAY,
> +	PROP_UPDELAY,
> +	PROP_ARP_INTERVAL,
> +	PROP_ARP_IP_TARGET,
> +	LAST_PROP
> +};
> +
> +/**
> + * nm_setting_bond_new:
> + *
> + * Creates a new #NMSettingBond object with default values.
> + *
> + * Returns: (transfer full): the new empty #NMSettingBond object
> + **/
> +NMSetting *
> +nm_setting_bond_new (void)
> +{
> +	return (NMSetting *) g_object_new (NM_TYPE_SETTING_BOND, NULL);
> +}
> +
> +/**
> + * nm_setting_bond_get_device_name
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:device-name property of the setting
> + **/
> +const char *
> +nm_setting_bond_get_device_name (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->device_name;
> +}
> +
> +/**
> + * nm_setting_bond_get_mode:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:mode property of the setting
> + **/
> +const char *
> +nm_setting_bond_get_mode (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->mode;
> +}
> +
> +/**
> + * nm_setting_bond_get_miimon:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:miimon property of the setting
> + **/
> +guint32
> +nm_setting_bond_get_miimon (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->miimon;
> +}
> +
> +/**
> + * nm_setting_bond_get_downdelay:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:downdelay property of the setting
> + **/
> +guint32
> +nm_setting_bond_get_downdelay (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->downdelay;
> +}
> +
> +/**
> + * nm_setting_bond_get_updelay:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:updelay property of the setting
> + **/
> +guint32
> +nm_setting_bond_get_updelay (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->updelay;
> +}
> +
> +/**
> + * nm_setting_bond_get_arp_interval:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:arp_interval property of the setting
> + **/
> +guint32
> +nm_setting_bond_get_arp_interval (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->arp_interval;
> +}
> +
> +/**
> + * nm_setting_bond_get_arp_ip_target:
> + * @setting: the #NMSettingBond
> + *
> + * Returns: the #NMSettingBond:arp_ip_target property of the setting
> + **/
> +const char *
> +nm_setting_bond_get_arp_ip_target (NMSettingBond *setting)
> +{
> +	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> +
> +	return NM_SETTING_BOND_GET_PRIVATE (setting)->arp_ip_target;
> +}
> +
> +static gboolean
> +verify (NMSetting *setting, GSList *all_settings, GError **error)
> +{
> +	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
> +	const char *valid_modes[] = {	"balance-rr",
> +					"active-backup",
> +					"balance-xor",
> +					"broadcast",
> +					"802.3ad",
> +					"balance-tlb",
> +					"balance-alb",
> +					NULL };
> +
> +	if (!priv->device_name || !strlen(priv->device_name)) {
> +		g_set_error (error,
> +		             NM_SETTING_BOND_ERROR,
> +		             NM_SETTING_BOND_ERROR_MISSING_PROPERTY,
> +		             NM_SETTING_BOND_DEVICE_NAME);
> +		return FALSE;
> +	}
> +
> +	if (strlen(priv->device_name) >= 16 ||
> +	    !strcmp(priv->device_name, ".") ||
> +	    !strcmp(priv->device_name, "..")) {

Do we want to check isalnum() here for every character?  What are the
valid characters for a kernel device name?

> +		g_set_error (error,
> +		             NM_SETTING_BOND_ERROR,
> +		             NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> +		             NM_SETTING_BOND_DEVICE_NAME);
> +		return FALSE;
> +	} else {
> +		char *name = priv->device_name;
> +
> +		while (*name) {
> +			if (*name == '/' || isspace(*name)) {
> +				g_set_error (error,
> +					     NM_SETTING_BOND_ERROR,
> +					     NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> +					     NM_SETTING_BOND_DEVICE_NAME);
> +				return FALSE;
> +			}
> +
> +			name++;
> +		}
> +	}
> +
> +	if (priv->mode && !_nm_utils_string_in_list (priv->mode, valid_modes)) {
> +		g_set_error (error,
> +		             NM_SETTING_BOND_ERROR,
> +		             NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> +		             NM_SETTING_BOND_MODE);
> +		return FALSE;
> +	}
> +
> +	/* XXX: Validate arp-ip-target */

Does this just require inet_pton() or something?

> +
> +	return TRUE;
> +}
> +
> +static void
> +nm_setting_bond_init (NMSettingBond *setting)
> +{
> +	g_object_set (setting, NM_SETTING_NAME, NM_SETTING_BOND_SETTING_NAME,
> +			       NM_SETTING_BOND_MIIMON, 100, /* default: miimon=100 */
> +			       NULL);
> +}
> +
> +static void
> +finalize (GObject *object)
> +{
> +	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
> +
> +	g_free (priv->device_name);
> +	g_free (priv->mode);
> +	g_free (priv->arp_ip_target);
> +
> +	G_OBJECT_CLASS (nm_setting_bond_parent_class)->finalize (object);
> +}
> +
> +static void
> +set_property (GObject *object, guint prop_id,
> +              const GValue *value, GParamSpec *pspec)
> +{
> +	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
> +
> +	switch (prop_id) {
> +	case PROP_DEVICE_NAME:
> +		priv->device_name = g_value_dup_string (value);
> +		break;
> +	case PROP_MODE:
> +		priv->mode = g_value_dup_string (value);
> +		break;
> +	case PROP_MIIMON:
> +		priv->miimon = g_value_get_uint (value);
> +		break;
> +	case PROP_DOWNDELAY:
> +		priv->downdelay = g_value_get_uint (value);
> +		break;
> +	case PROP_UPDELAY:
> +		priv->updelay = g_value_get_uint (value);
> +		break;
> +	case PROP_ARP_INTERVAL:
> +		priv->arp_interval = g_value_get_uint (value);
> +		break;
> +	case PROP_ARP_IP_TARGET:
> +		g_free (priv->arp_ip_target);
> +		priv->arp_ip_target = g_value_dup_string (value);
> +		break;
> +	default:
> +		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> +		break;
> +	}
> +}
> +
> +static void
> +get_property (GObject *object, guint prop_id,
> +              GValue *value, GParamSpec *pspec)
> +{
> +	NMSettingBond *setting = NM_SETTING_BOND (object);
> +
> +	switch (prop_id) {
> +	case PROP_DEVICE_NAME:
> +		g_value_set_string (value, nm_setting_bond_get_device_name (setting));
> +		break;
> +	case PROP_MODE:
> +		g_value_set_string (value, nm_setting_bond_get_mode (setting));
> +		break;
> +	case PROP_MIIMON:
> +		g_value_set_uint (value, nm_setting_bond_get_miimon (setting));
> +		break;
> +	case PROP_DOWNDELAY:
> +		g_value_set_uint (value, nm_setting_bond_get_downdelay (setting));
> +		break;
> +	case PROP_UPDELAY:
> +		g_value_set_uint (value, nm_setting_bond_get_updelay (setting));
> +		break;
> +	case PROP_ARP_INTERVAL:
> +		g_value_set_uint (value, nm_setting_bond_get_arp_interval (setting));
> +		break;
> +	case PROP_ARP_IP_TARGET:
> +		g_value_set_string (value, nm_setting_bond_get_arp_ip_target (setting));
> +		break;
> +	default:
> +		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> +		break;
> +	}
> +}
> +
> +static void
> +nm_setting_bond_class_init (NMSettingBondClass *setting_class)
> +{
> +	GObjectClass *object_class = G_OBJECT_CLASS (setting_class);
> +	NMSettingClass *parent_class = NM_SETTING_CLASS (setting_class);
> +
> +	g_type_class_add_private (setting_class, sizeof (NMSettingBondPrivate));
> +
> +	/* virtual methods */
> +	object_class->set_property = set_property;
> +	object_class->get_property = get_property;
> +	object_class->finalize     = finalize;
> +	parent_class->verify       = verify;
> +
> +	/* Properties */
> +	/**
> +	 * NMSettingBond:device-name:
> +	 *
> +	 * Name of virtual kernel device
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_DEVICE_NAME,

We could use IFACE_NAME or INTERFACE_NAME or such here; NM's usage of
"device" is an anachronism from long ago which we don't really need to
propagate if we don't want to.  Up to you.

> +		 g_param_spec_string (NM_SETTING_BOND_DEVICE_NAME,
> +						  "DeviceName",
> +						  "The name of the virtual in-kernel bonding nework device",
> +						  NULL,
> +						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:mode:
> +	 *
> +	 * Bonding policy
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_MODE,
> +		 g_param_spec_string (NM_SETTING_BOND_MODE,
> +						  "Mode",
> +						  "The bonding policy to use. One of 'balance-rr' (default), "
> +						  "'active-backup', 'balance-xor', 'broadcast', '802.3ad', "
> +						  "'balance-tlb', 'balance-alb'.",
> +						  NULL,
> +						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:miimon:
> +	 *
> +	 * Specifies the MII link monitoring frequency in milliseconds.
> +	 * This determines how often the link state of each slave is
> +	 * inspected for link failures.  A value of zero disables MII
> +	 * link monitoring.  A value of 100 is a good starting point.
> +	 * The use_carrier option, below, affects how the link state is
> +	 * determined.
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_MIIMON,
> +		 g_param_spec_uint (NM_SETTING_BOND_MIIMON,
> +						"MiiMon",
> +						"Specifies the MII link monitoring frequency in milliseconds. "
> +						"This determines how often the link state of each slave is "
> +						"inspected for link failures.  A value of zero disables MII "
> +						"link monitoring.  A value of 100 is a good starting point. "
> +						"The use_carrier option, below, affects how the link state is "
> +						"determined. The default value is 0.",
> +						0, G_MAXUINT32, 0,
> +						G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:downdelay:
> +	 *
> +	 * Specifies the time, in milliseconds, to wait before disabling
> +	 * a slave after a link failure has been detected.  This option
> +	 * is only valid for the miimon link monitor.  The downdelay
> +	 * value should be a multiple of the miimon value; if not, it
> +	 * will be rounded down to the nearest multiple.  The default
> +	 * value is 0.
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_DOWNDELAY,
> +		 g_param_spec_uint (NM_SETTING_BOND_DOWNDELAY,
> +						"DownDelay",
> +						"Specifies the time, in milliseconds, to wait before disabling "
> +						"a slave after a link failure has been detected.  This option "
> +						"is only valid for the miimon link monitor.  The downdelay "
> +						"value should be a multiple of the miimon value; if not, it "
> +						"will be rounded down to the nearest multiple.  The default "
> +						"value is 0.",
> +						0, G_MAXUINT32, 0,
> +						G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:updelay:
> +	 *
> +	 * Specifies the time, in milliseconds, to wait before enabling a
> +	 * slave after a link recovery has been detected.  This option is
> +	 * only valid for the miimon link monitor.  The updelay value
> +	 * should be a multiple of the miimon value; if not, it will be
> +	 * rounded down to the nearest multiple.  The default value is 0.
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_UPDELAY,
> +		 g_param_spec_uint (NM_SETTING_BOND_UPDELAY,
> +						"UpDelay",
> +						"Specifies the time, in milliseconds, to wait before enabling a "
> +						"slave after a link recovery has been detected.  This option is "
> +						"only valid for the miimon link monitor.  The updelay value "
> +						"should be a multiple of the miimon value; if not, it will be "
> +						"rounded down to the nearest multiple.  The default value is 0.",
> +						0, G_MAXUINT32, 0,
> +						G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:arp_interval:
> +	 *
> +	 * Specifies the ARP link monitoring frequency in milliseconds.
> +	 *
> +	 * The ARP monitor works by periodically checking the slave
> +	 * devices to determine whether they have sent or received
> +	 * traffic recently (the precise criteria depends upon the
> +	 * bonding mode, and the state of the slave).  Regular traffic is
> +	 * generated via ARP probes issued for the addresses specified by
> +	 * the arp-ip-target option.
> +	 *
> +	 * This behavior can be modified by the arp-validate option.
> +	 *
> +	 * If ARP monitoring is used in an etherchannel compatible mode
> +	 * (modes 0 and 2), the switch should be configured in a mode
> +	 * that evenly distributes packets across all links. If the
> +	 * switch is configured to distribute the packets in an XOR
> +	 * fashion, all replies from the ARP targets will be received on
> +	 * the same link which could cause the other team members to
> +	 * fail.  ARP monitoring should not be used in conjunction with
> +	 * miimon.  A value of 0 disables ARP monitoring.  The default
> +	 * value is 0.
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_ARP_INTERVAL,
> +		 g_param_spec_uint (NM_SETTING_BOND_ARP_INTERVAL,
> +						"ArpInterval",
> +						"Specifies the ARP link monitoring frequency in milliseconds. "
> +						"The ARP monitor works by periodically checking the slave "
> +						"devices to determine whether they have sent or received "
> +						"traffic recently (the precise criteria depends upon the "
> +						"bonding mode, and the state of the slave).  Regular traffic is "
> +						"generated via ARP probes issued for the addresses specified by "
> +						"the arp-ip-target option. "
> +						"This behavior can be modified by the arp-validate option. "
> +						"If ARP monitoring is used in an etherchannel compatible mode "
> +						"(modes 0 and 2), the switch should be configured in a mode "
> +						"that evenly distributes packets across all links. If the "
> +						"switch is configured to distribute the packets in an XOR "
> +						"fashion, all replies from the ARP targets will be received on "
> +						"the same link which could cause the other team members to "
> +						"fail.  ARP monitoring should not be used in conjunction with "
> +						"miimon.  A value of 0 disables ARP monitoring.  The default "
> +						"value is 0.",
> +						0, G_MAXUINT32, 0,
> +						G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> +
> +	/**
> +	 * NMSettingBond:arp_ip_target:
> +	 *
> +	 * Specifies the IP addresses to use as ARP monitoring peers when
> +	 * arp_interval is > 0.  These are the targets of the ARP request
> +	 * sent to determine the health of the link to the targets.
> +	 * Specify these values in ddd.ddd.ddd.ddd format.  Multiple IP
> +	 * addresses must be separated by a comma.  At least one IP
> +	 * address must be given for ARP monitoring to function.  The
> +	 * maximum number of targets that can be specified is 16.  The
> +	 * default value is no IP addresses.
> +	 **/
> +	g_object_class_install_property
> +		(object_class, PROP_ARP_IP_TARGET,
> +		 g_param_spec_string (NM_SETTING_BOND_ARP_IP_TARGET,
> +						  "ArpIpTarget",
> +						  "Specifies the IP addresses to use as ARP monitoring peers when "
> +						  "arp-interval is > 0.  These are the targets of the ARP request "
> +						  "sent to determine the health of the link to the targets. "
> +						  "Specify these values in ddd.ddd.ddd.ddd format.  Multiple IP "
> +						  "addresses must be separated by a comma.  At least one IP "
> +						  "address must be given for ARP monitoring to function.  The "
> +						  "maximum number of targets that can be specified is 16.  The "
> +						  "default value is no IP addresses.",
> +						  NULL,
> +						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> +}
> diff --git a/libnm-util/nm-setting-bond.h b/libnm-util/nm-setting-bond.h
> new file mode 100644
> index 0000000..54ddf82
> --- /dev/null
> +++ b/libnm-util/nm-setting-bond.h
> @@ -0,0 +1,94 @@
> +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
> +
> +/*
> + * Thomas Graf <tgraf redhat com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the
> + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> + * Boston, MA 02110-1301 USA.
> + *
> + * (C) Copyright 2011 Red Hat, Inc.
> + */
> +
> +#ifndef NM_SETTING_BOND_H
> +#define NM_SETTING_BOND_H
> +
> +#include <nm-setting.h>
> +
> +G_BEGIN_DECLS
> +
> +#define NM_TYPE_SETTING_BOND            (nm_setting_bond_get_type ())
> +#define NM_SETTING_BOND(obj)            (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SETTING_BOND, NMSettingBond))
> +#define NM_SETTING_BOND_CLASS(klass)    (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_SETTING_BOND, NMSettingBondClass))
> +#define NM_IS_SETTING_BOND(obj)         (G_TYPE_CHECK_INSTANCE_TYPE ((obj), NM_TYPE_SETTING_BOND))
> +#define NM_IS_SETTING_BOND_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((obj), NM_TYPE_SETTING_BOND))
> +#define NM_SETTING_BOND_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_SETTING_BOND, NMSettingBondClass))
> +
> +#define NM_SETTING_BOND_SETTING_NAME "bond"
> +
> +/**
> + * NMSettingBondError:
> + * @NM_SETTING_BOND_ERROR_UNKNOWN: unknown or unclassified error
> + * @NM_SETTING_BOND_ERROR_INVALID_PROPERTY: the property was invalid
> + * @NM_SETTING_BOND_ERROR_MISSING_PROPERTY: the property was missing and is
> + * required
> + */
> +typedef enum {
> +	NM_SETTING_BOND_ERROR_UNKNOWN = 0,
> +	NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> +	NM_SETTING_BOND_ERROR_MISSING_PROPERTY
> +} NMSettingBondError;
> +
> +#define NM_TYPE_SETTING_BOND_ERROR (nm_setting_bond_error_get_type ()) 
> +GType nm_setting_bond_error_get_type (void);
> +
> +#define NM_SETTING_BOND_ERROR nm_setting_bond_error_quark ()
> +GQuark nm_setting_bond_error_quark (void);
> +
> +#define NM_SETTING_BOND_DEVICE_NAME "device-name"
> +#define NM_SETTING_BOND_MODE "mode"
> +#define NM_SETTING_BOND_MIIMON "miimon"
> +#define NM_SETTING_BOND_DOWNDELAY "down-delay"
> +#define NM_SETTING_BOND_UPDELAY "up-delay"
> +#define NM_SETTING_BOND_ARP_INTERVAL "arp-interval"
> +#define NM_SETTING_BOND_ARP_IP_TARGET "arp-ip-target"
> +
> +typedef struct {
> +	NMSetting parent;
> +} NMSettingBond;
> +
> +typedef struct {
> +	NMSettingClass parent;
> +
> +	/* Padding for future expansion */
> +	void (*_reserved1) (void);
> +	void (*_reserved2) (void);
> +	void (*_reserved3) (void);
> +	void (*_reserved4) (void);
> +} NMSettingBondClass;
> +
> +GType nm_setting_bond_get_type (void);
> +
> +NMSetting *       nm_setting_bond_new                  (void);
> +const char *      nm_setting_bond_get_device_name      (NMSettingBond *setting);
> +const char *      nm_setting_bond_get_mode             (NMSettingBond *setting);
> +guint32           nm_setting_bond_get_miimon           (NMSettingBond *setting);
> +guint32           nm_setting_bond_get_downdelay        (NMSettingBond *setting);
> +guint32           nm_setting_bond_get_updelay          (NMSettingBond *setting);
> +guint32           nm_setting_bond_get_arp_interval     (NMSettingBond *setting);
> +const char *      nm_setting_bond_get_arp_ip_target    (NMSettingBond *setting);
> +
> +G_END_DECLS
> +
> +#endif /* NM_SETTING_BOND_H */
> diff --git a/src/settings/plugins/ifcfg-rh/common.h b/src/settings/plugins/ifcfg-rh/common.h
> index 74a77c4..d801be1 100644
> --- a/src/settings/plugins/ifcfg-rh/common.h
> +++ b/src/settings/plugins/ifcfg-rh/common.h
> @@ -15,7 +15,7 @@
>   * with this program; if not, write to the Free Software Foundation, Inc.,
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   *
> - * (C) Copyright 2008 - 2010 Red Hat, Inc.
> + * (C) Copyright 2008 - 2011 Red Hat, Inc.
>   */
>  
>  #ifndef __COMMON_H__
> @@ -44,6 +44,7 @@
>  #define TYPE_ETHERNET "Ethernet"
>  #define TYPE_WIRELESS "Wireless"
>  #define TYPE_BRIDGE   "Bridge"
> +#define TYPE_BOND     "Bond"
>  
>  #define SECRET_FLAG_AGENT "user"
>  #define SECRET_FLAG_NOT_SAVED "ask"
> diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
> index ab94db6..f20b6cc 100644
> --- a/src/settings/plugins/ifcfg-rh/reader.c
> +++ b/src/settings/plugins/ifcfg-rh/reader.c
> @@ -50,6 +50,7 @@
>  #include <nm-setting-wired.h>
>  #include <nm-setting-wireless.h>
>  #include <nm-setting-8021x.h>
> +#include <nm-setting-bond.h>
>  #include <nm-utils.h>
>  
>  #include "common.h"
> @@ -3339,6 +3340,150 @@ is_wireless_device (const char *iface)
>  	return is_wireless;
>  }
>  
> +static void
> +handle_bond_option (NMSettingBond *s_bond,
> +		    const char *key,
> +		    const char *value)
> +{
> +	if (!g_strcmp0 (key, "mode"))
> +		g_object_set (s_bond, NM_SETTING_BOND_MODE, value, NULL);
> +	else if (!g_strcmp0 (key, "miimon"))
> +		g_object_set (s_bond, NM_SETTING_BOND_MIIMON, strtoul(value, NULL, 0), NULL);
> +	else if (!g_strcmp0 (key, "updelay"))
> +		g_object_set (s_bond, NM_SETTING_BOND_UPDELAY, strtoul(value, NULL, 0), NULL);
> +	else if (!g_strcmp0 (key, "downdelay"))
> +		g_object_set (s_bond, NM_SETTING_BOND_DOWNDELAY, strtoul(value, NULL, 0), NULL);
> +	else if (!g_strcmp0 (key, "arp_interval"))
> +		g_object_set (s_bond, NM_SETTING_BOND_ARP_INTERVAL, strtoul(value, NULL, 0), NULL);
> +	else if (!g_strcmp0 (key, "arp_ip_target"))
> +		g_object_set (s_bond, NM_SETTING_BOND_ARP_IP_TARGET, value, NULL);
> +	else
> +		PLUGIN_WARN (IFCFG_PLUGIN_NAME, "    warning: invalid bonding option '%s'", key);
> +}
> +
> +static NMSetting *
> +make_bond_setting (shvarFile *ifcfg,
> +                   const char *file,
> +		   gboolean nm_controlled,
> +		   char **unmanaged,
> +		   GError **error)
> +{
> +	NMSettingBond *s_bond;
> +	char *value;
> +
> +	s_bond = NM_SETTING_BOND (nm_setting_bond_new ());
> +
> +	value = svGetValue (ifcfg, "DEVICE", FALSE);
> +	if (!value || !strlen (value)) {
> +		g_set_error (error, IFCFG_PLUGIN_ERROR, 0, "mandatory DEVICE keyword missing");
> +		goto error;
> +	}
> +
> +	g_object_set (s_bond, NM_SETTING_BOND_DEVICE_NAME, value, NULL);
> +	g_free (value);
> +
> +	value = svGetValue (ifcfg, "BONDING_OPTS", FALSE);
> +	if (value) {
> +		char **items, **iter;
> +
> +		items = g_strsplit_set (value, " ", -1);
> +		for (iter = items; iter && *iter; iter++) {
> +			if (strlen (*iter)) {
> +				char **keys, *key, *val;
> +
> +				keys = g_strsplit_set (*iter, "=", 2);
> +				if (keys && *keys) {
> +					key = *keys;
> +					val = *(keys + 1);
> +					if (val && strlen(key) && strlen(val))
> +						handle_bond_option (s_bond, key, val);
> +				}
> +
> +				g_strfreev (keys);
> +			}
> +		}
> +		g_free (value);
> +		g_strfreev (items);
> +	}
> +
> +	return (NMSetting *) s_bond;
> +
> +error:
> +	g_object_unref (s_bond);
> +	return NULL;
> +}
> +
> +static NMConnection *
> +bond_connection_from_ifcfg (const char *file,
> +			    shvarFile *ifcfg,
> +			    gboolean nm_controlled,
> +			    char **unmanaged,
> +			    GError **error)
> +{
> +	NMConnection *connection = NULL;
> +	NMSetting *con_setting = NULL;
> +	NMSetting *bond_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);
> +
> +	connection = nm_connection_new ();
> +	if (!connection) {
> +		g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
> +		             "Failed to allocate new connection for %s.", file);
> +		return NULL;
> +	}
> +
> +	con_setting = make_connection_setting (file, ifcfg, NM_SETTING_BOND_SETTING_NAME, NULL);
> +	if (!con_setting) {
> +		g_set_error (error, IFCFG_PLUGIN_ERROR, 0,
> +		             "Failed to create connection setting.");
> +		g_object_unref (connection);
> +		return NULL;
> +	}
> +	nm_connection_add_setting (connection, con_setting);
> +
> +	bond_setting = make_bond_setting (ifcfg, file, nm_controlled, unmanaged, error);
> +	if (!bond_setting) {
> +		g_object_unref (connection);
> +		return NULL;
> +	}
> +	nm_connection_add_setting (connection, bond_setting);
> +
> +	wired_setting = make_wired_setting (ifcfg, file, nm_controlled, unmanaged, &s_8021x, error);
> +	if (!wired_setting) {
> +		g_object_unref (connection);
> +		return NULL;
> +	}
> +	nm_connection_add_setting (connection, wired_setting);
> +
> +	if (s_8021x)
> +		nm_connection_add_setting (connection, NM_SETTING (s_8021x));
> +
> +	if (!nm_connection_verify (connection, error)) {
> +		g_object_unref (connection);
> +		return NULL;
> +	}
> +
> +	return connection;
> +}
> +
> +static gboolean
> +is_bond_device (const char *name, shvarFile *parsed)
> +{
> +	g_return_val_if_fail (name != NULL, FALSE);
> +	g_return_val_if_fail (parsed != NULL, FALSE);
> +
> +	if (svTrueValue (parsed, "BONDING_MASTER", FALSE))
> +		return TRUE;
> +	
> +	/* XXX: Check for "bond[\d]+"? */
> +
> +	return FALSE;
> +}
> +
>  enum {
>  	IGNORE_REASON_NONE = 0x00,
>  	IGNORE_REASON_BRIDGE = 0x01,
> @@ -3402,9 +3547,6 @@ connection_from_file (const char *filename,
>  	if (!type) {
>  		char *device;
>  
> -		/* If no type, if the device has wireless extensions, it's wifi,
> -		 * otherwise it's ethernet.
> -		 */
>  		device = svGetValue (parsed, "DEVICE", FALSE);
>  		if (!device) {
>  			g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
> @@ -3422,8 +3564,10 @@ connection_from_file (const char *filename,
>  		}
>  
>  		if (!test_type) {
> +			if (is_bond_device (device, parsed))
> +				type = g_strdup (TYPE_BOND);
>  			/* Test wireless extensions */
> -			if (is_wireless_device (device))
> +			else if (is_wireless_device (device))
>  				type = g_strdup (TYPE_WIRELESS);
>  			else
>  				type = g_strdup (TYPE_ETHERNET);
> @@ -3456,6 +3600,13 @@ connection_from_file (const char *filename,
>  		g_free (lower);
>  	}
>  
> +	if (svTrueValue (parsed, "BONDING_MASTER", FALSE) &&
> +	    strcasecmp (type, TYPE_BOND)) {
> +		g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
> +			     "BONDING_MASTER=yes key only allowed in TYPE=bond connections");
> +		goto done;
> +	}
> +
>  	/* Ignore BRIDGE= and VLAN= connections for now too (rh #619863) */
>  	tmp = svGetValue (parsed, "BRIDGE", FALSE);
>  	if (tmp) {
> @@ -3481,7 +3632,9 @@ connection_from_file (const char *filename,
>  	else if (!strcasecmp (type, TYPE_BRIDGE)) {
>  		g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
>  		             "Bridge connections are not yet supported");
> -	} else {
> +	} else if (!strcasecmp (type, TYPE_BOND))
> +		connection = bond_connection_from_ifcfg (filename, parsed, nm_controlled, unmanaged, &error);
> +	else {
>  		g_set_error (&error, IFCFG_PLUGIN_ERROR, 0,
>  		             "Unknown connection type '%s'", type);
>  	}

The rest of this looks good.

Dan
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]