merge: branch 'bg/bond-options-rh1299103'
authorBeniamino Galvani <bgalvani@redhat.com>
Tue, 29 Mar 2016 16:10:35 +0000 (18:10 +0200)
committerBeniamino Galvani <bgalvani@redhat.com>
Tue, 29 Mar 2016 16:10:35 +0000 (18:10 +0200)
Add missing bond options and improve connection matching for bond
connections.

.gitignore
clients/cli/settings.c
libnm-core/Makefile.am
libnm-core/nm-core-internal.h
libnm-core/nm-setting-bond.c
libnm-core/nm-setting-bond.h
libnm-core/tests/Makefile.am
libnm-core/tests/test-setting-bond.c [new file with mode: 0644]
libnm/Makefile.am
src/devices/nm-device-bond.c

index 8991127..fbf35bd 100644 (file)
@@ -153,6 +153,7 @@ test-*.trs
 /libnm-core/tests/test-need-secrets
 /libnm-core/tests/test-secrets
 /libnm-core/tests/test-setting-8021x
+/libnm-core/tests/test-setting-bond
 /libnm-core/tests/test-setting-dcb
 
 /libnm-glib/nm-secret-agent-glue.h
index 3fb841a..06e9fb4 100644 (file)
@@ -1150,8 +1150,19 @@ nmc_property_bond_get_options (NMSetting *setting, NmcPropertyGetType get_type)
        bond_options_s = g_string_new (NULL);
        for (i = 0; i < nm_setting_bond_get_num_options (s_bond); i++) {
                const char *key, *value;
+               gs_free char *tmp_value = NULL;
+               char *p;
 
                nm_setting_bond_get_option (s_bond, i, &key, &value);
+
+               if (nm_streq0 (key, NM_SETTING_BOND_OPTION_ARP_IP_TARGET)) {
+                       value = tmp_value = g_strdup (value);
+                       for (p = tmp_value; p && *p; p++) {
+                               if (*p == ',')
+                                       *p = ' ';
+                       }
+               }
+
                g_string_append_printf (bond_options_s, "%s=%s,", key, value);
        }
        g_string_truncate (bond_options_s, bond_options_s->len-1);  /* chop off trailing ',' */
@@ -3651,10 +3662,28 @@ _validate_bond_option_value (const char *option, const char *value, GError **err
        return value;
 }
 
+static gboolean
+_bond_add_option (NMSettingBond *setting,
+                  const char *name,
+                  const char *value)
+{
+       gs_free char *tmp_value = NULL;
+       char *p;
+
+       if (nm_streq0 (name, NM_SETTING_BOND_OPTION_ARP_IP_TARGET)) {
+               value = tmp_value = g_strdup (value);
+               for (p = tmp_value; p && *p; p++)
+                       if (*p == ' ')
+                               *p = ',';
+       }
+
+       return nm_setting_bond_add_option (setting, name, value);
+}
+
 DEFINE_SETTER_OPTIONS (nmc_property_bond_set_options,
                        NM_SETTING_BOND,
                        NMSettingBond,
-                       nm_setting_bond_add_option,
+                       _bond_add_option,
                        nm_setting_bond_get_valid_options,
                        _validate_bond_option_value)
 DEFINE_REMOVER_OPTION (nmc_property_bond_remove_option_options,
index 527a1c2..1d80744 100644 (file)
@@ -1,5 +1,7 @@
 include $(GLIB_MAKEFILE)
 
+@GNOME_CODE_COVERAGE_RULES@
+
 SUBDIRS = . tests
 
 AM_CPPFLAGS = \
@@ -11,7 +13,8 @@ AM_CPPFLAGS = \
        -DNMLIBDIR=\"$(nmlibdir)\" \
        -DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_LIB \
        -DNM_VERSION_MAX_ALLOWED=NM_VERSION_NEXT_STABLE \
-       $(GLIB_CFLAGS)
+       $(GLIB_CFLAGS) \
+       $(CODE_COVERAGE_CFLAGS)
 
 noinst_LTLIBRARIES = libnm-core.la
 
@@ -35,6 +38,9 @@ libnm_core_la_LIBADD =                        \
        $(GLIB_LIBS)                    \
        $(UUID_LIBS)
 
+libnm_core_la_LDFLAGS =                        \
+       $(CODE_COVERAGE_LDFLAGS)
+
 if WITH_GNUTLS
 AM_CPPFLAGS += $(GNUTLS_CFLAGS)
 libnm_core_la_SOURCES += crypto_gnutls.c
index 9512ee5..082ebc9 100644 (file)
@@ -283,4 +283,16 @@ void     _nm_setting_vlan_get_priorities (NMSettingVlan *setting,
 
 /***********************************************************/
 
+typedef enum {
+       NM_BOND_OPTION_TYPE_INT,
+       NM_BOND_OPTION_TYPE_STRING,
+       NM_BOND_OPTION_TYPE_BOTH,
+       NM_BOND_OPTION_TYPE_IP,
+       NM_BOND_OPTION_TYPE_MAC,
+       NM_BOND_OPTION_TYPE_IFNAME,
+} NMBondOptionType;
+
+NMBondOptionType
+_nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name);
+
 #endif
index 4d40de0..e008878 100644 (file)
@@ -32,6 +32,7 @@
 #include "nm-utils-private.h"
 #include "nm-connection-private.h"
 #include "nm-setting-infiniband.h"
+#include "nm-core-internal.h"
 
 /**
  * SECTION:nm-setting-bond
@@ -57,14 +58,6 @@ enum {
        LAST_PROP
 };
 
-enum {
-       TYPE_INT,
-       TYPE_STR,
-       TYPE_BOTH,
-       TYPE_IP,
-       TYPE_IFNAME,
-};
-
 typedef struct {
        const char *opt;
        const char *val;
@@ -75,28 +68,40 @@ typedef struct {
 } BondDefault;
 
 static const BondDefault defaults[] = {
-       { NM_SETTING_BOND_OPTION_MODE,             "balance-rr", TYPE_BOTH, 0, 6,
+       { NM_SETTING_BOND_OPTION_MODE,             "balance-rr", NM_BOND_OPTION_TYPE_BOTH, 0, 6,
          { "balance-rr", "active-backup", "balance-xor", "broadcast", "802.3ad", "balance-tlb", "balance-alb", NULL } },
-       { NM_SETTING_BOND_OPTION_MIIMON,           "100",        TYPE_INT, 0, G_MAXINT },
-       { NM_SETTING_BOND_OPTION_DOWNDELAY,        "0",          TYPE_INT, 0, G_MAXINT },
-       { NM_SETTING_BOND_OPTION_UPDELAY,          "0",          TYPE_INT, 0, G_MAXINT },
-       { NM_SETTING_BOND_OPTION_ARP_INTERVAL,     "0",          TYPE_INT, 0, G_MAXINT },
-       { NM_SETTING_BOND_OPTION_ARP_IP_TARGET,    "",           TYPE_IP },
-       { NM_SETTING_BOND_OPTION_ARP_VALIDATE,     "0",          TYPE_BOTH, 0, 3,
+       { NM_SETTING_BOND_OPTION_MIIMON,           "100",        NM_BOND_OPTION_TYPE_INT, 0, G_MAXINT },
+       { NM_SETTING_BOND_OPTION_DOWNDELAY,        "0",          NM_BOND_OPTION_TYPE_INT, 0, G_MAXINT },
+       { NM_SETTING_BOND_OPTION_UPDELAY,          "0",          NM_BOND_OPTION_TYPE_INT, 0, G_MAXINT },
+       { NM_SETTING_BOND_OPTION_ARP_INTERVAL,     "0",          NM_BOND_OPTION_TYPE_INT, 0, G_MAXINT },
+       { NM_SETTING_BOND_OPTION_ARP_IP_TARGET,    "",           NM_BOND_OPTION_TYPE_IP },
+       { NM_SETTING_BOND_OPTION_ARP_VALIDATE,     "none",       NM_BOND_OPTION_TYPE_BOTH, 0, 3,
          { "none", "active", "backup", "all", NULL } },
-       { NM_SETTING_BOND_OPTION_PRIMARY,          "",           TYPE_IFNAME },
-       { NM_SETTING_BOND_OPTION_PRIMARY_RESELECT, "0",          TYPE_BOTH, 0, 2,
+       { NM_SETTING_BOND_OPTION_PRIMARY,          "",           NM_BOND_OPTION_TYPE_IFNAME },
+       { NM_SETTING_BOND_OPTION_PRIMARY_RESELECT, "always",     NM_BOND_OPTION_TYPE_BOTH, 0, 2,
          { "always", "better", "failure", NULL } },
-       { NM_SETTING_BOND_OPTION_FAIL_OVER_MAC,    "0",          TYPE_BOTH, 0, 2,
+       { NM_SETTING_BOND_OPTION_FAIL_OVER_MAC,    "none",       NM_BOND_OPTION_TYPE_BOTH, 0, 2,
          { "none", "active", "follow", NULL } },
-       { NM_SETTING_BOND_OPTION_USE_CARRIER,      "1",          TYPE_INT, 0, 1 },
-       { NM_SETTING_BOND_OPTION_AD_SELECT,        "0",          TYPE_BOTH, 0, 2,
+       { NM_SETTING_BOND_OPTION_USE_CARRIER,      "1",          NM_BOND_OPTION_TYPE_INT, 0, 1 },
+       { NM_SETTING_BOND_OPTION_AD_SELECT,        "stable",     NM_BOND_OPTION_TYPE_BOTH, 0, 2,
          { "stable", "bandwidth", "count", NULL } },
-       { NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY, "0",          TYPE_BOTH, 0, 2,
+       { NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY, "layer2",     NM_BOND_OPTION_TYPE_BOTH, 0, 2,
          { "layer2", "layer3+4", "layer2+3", NULL } },
-       { NM_SETTING_BOND_OPTION_RESEND_IGMP,      "1",          TYPE_INT, 0, 255 },
-       { NM_SETTING_BOND_OPTION_LACP_RATE,        "0",          TYPE_BOTH, 0, 1,
+       { NM_SETTING_BOND_OPTION_RESEND_IGMP,      "1",          NM_BOND_OPTION_TYPE_INT, 0, 255 },
+       { NM_SETTING_BOND_OPTION_LACP_RATE,        "slow",       NM_BOND_OPTION_TYPE_BOTH, 0, 1,
          { "slow", "fast", NULL } },
+       { NM_SETTING_BOND_OPTION_ACTIVE_SLAVE,     "",           NM_BOND_OPTION_TYPE_IFNAME },
+       { NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO,"65535",      NM_BOND_OPTION_TYPE_INT, 1, 65535 },
+       { NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM,  "",           NM_BOND_OPTION_TYPE_MAC },
+       { NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY, "0",          NM_BOND_OPTION_TYPE_INT, 0, 1023},
+       { NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE,"0",          NM_BOND_OPTION_TYPE_INT, 0, 1},
+       { NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS,  "any",        NM_BOND_OPTION_TYPE_BOTH, 0, 1, {"any", "all"}},
+       { NM_SETTING_BOND_OPTION_MIN_LINKS,        "0",          NM_BOND_OPTION_TYPE_INT, 0, G_MAXINT },
+       { NM_SETTING_BOND_OPTION_NUM_GRAT_ARP,     "1",          NM_BOND_OPTION_TYPE_INT, 0, 255 },
+       { NM_SETTING_BOND_OPTION_NUM_UNSOL_NA,     "1",          NM_BOND_OPTION_TYPE_INT, 0, 255 },
+       { NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE,"1",          NM_BOND_OPTION_TYPE_INT, 0, 65535 },
+       { NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB,   "1",          NM_BOND_OPTION_TYPE_INT, 0, 1 },
+       { NM_SETTING_BOND_OPTION_LP_INTERVAL,      "1",          NM_BOND_OPTION_TYPE_INT, 1, G_MAXINT },
 };
 
 /**
@@ -268,16 +273,18 @@ nm_setting_bond_validate_option (const char *name,
                        if (value == NULL)
                                return TRUE;
                        switch (defaults[i].opt_type) {
-                       case TYPE_INT:
+                       case NM_BOND_OPTION_TYPE_INT:
                                return validate_int (name, value, &defaults[i]);
-                       case TYPE_STR:
+                       case NM_BOND_OPTION_TYPE_STRING:
                                return validate_list (name, value, &defaults[i]);
-                       case TYPE_BOTH:
+                       case NM_BOND_OPTION_TYPE_BOTH:
                                return (   validate_int (name, value, &defaults[i])
                                        || validate_list (name, value, &defaults[i]));
-                       case TYPE_IP:
+                       case NM_BOND_OPTION_TYPE_IP:
                                return validate_ip (name, value);
-                       case TYPE_IFNAME:
+                       case NM_BOND_OPTION_TYPE_MAC:
+                               return nm_utils_hwaddr_valid (value, ETH_ALEN);
+                       case NM_BOND_OPTION_TYPE_IFNAME:
                                return validate_ifname (name, value);
                        }
                        return FALSE;
@@ -433,6 +440,29 @@ nm_setting_bond_get_option_default (NMSettingBond *setting, const char *name)
        g_assert_not_reached ();
 }
 
+/**
+ * nm_setting_bond_get_option_type:
+ * @setting: the #NMSettingBond
+ * @name: the name of the option
+ *
+ * Returns: the type of the bond option.
+ **/
+NMBondOptionType
+_nm_setting_bond_get_option_type (NMSettingBond *setting, const char *name)
+{
+       guint i;
+
+       g_return_val_if_fail (NM_IS_SETTING_BOND (setting), NM_BOND_OPTION_TYPE_INT);
+       g_return_val_if_fail (nm_setting_bond_validate_option (name, NULL), NM_BOND_OPTION_TYPE_INT);
+
+       for (i = 0; i < G_N_ELEMENTS (defaults); i++) {
+               if (nm_streq0 (defaults[i].opt, name))
+                       return defaults[i].opt_type;
+       }
+       /* Any option that passes nm_setting_bond_validate_option() should also be found in defaults */
+       g_assert_not_reached ();
+}
+
 static gboolean
 verify (NMSetting *setting, NMConnection *connection, GError **error)
 {
@@ -440,6 +470,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
        GHashTableIter iter;
        const char *key, *value;
        int mode, miimon = 0, arp_interval = 0;
+       int num_grat_arp = -1, num_unsol_na = -1;
        const char *mode_orig, *mode_new;
        const char *arp_ip_target = NULL;
        const char *lacp_rate;
@@ -464,6 +495,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
        value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
        if (value)
                arp_interval = atoi (value);
+       value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
+       if (value)
+               num_grat_arp = atoi (value);
+       value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
+       if (value)
+               num_unsol_na = atoi (value);
 
        /* Can only set one of miimon and arp_interval */
        if (miimon > 0 && arp_interval > 0) {
@@ -474,11 +511,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                             NM_SETTING_BOND_OPTION_MIIMON,
                             NM_SETTING_BOND_OPTION_ARP_INTERVAL);
                g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
+               return FALSE;
        }
 
        /* Verify bond mode */
-       mode_orig = value = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_MODE);
-       if (!value) {
+       mode_orig = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_MODE);
+       if (!mode_orig) {
                g_set_error (error,
                             NM_CONNECTION_ERROR,
                             NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -487,7 +525,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
                return FALSE;
        }
-       mode = nm_utils_bond_mode_string_to_int (value);
+       mode = nm_utils_bond_mode_string_to_int (mode_orig);
        if (mode == -1) {
                g_set_error (error,
                             NM_CONNECTION_ERROR,
@@ -497,24 +535,24 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
                return FALSE;
        }
-       mode_new = value = nm_utils_bond_mode_int_to_string (mode);
+       mode_new = nm_utils_bond_mode_int_to_string (mode);
 
        /* Make sure mode is compatible with other settings */
-       if (   strcmp (value, "balance-alb") == 0
-           || strcmp (value, "balance-tlb") == 0) {
+       if (   strcmp (mode_new, "balance-alb") == 0
+           || strcmp (mode_new, "balance-tlb") == 0) {
                if (arp_interval > 0) {
                        g_set_error (error,
                                     NM_CONNECTION_ERROR,
                                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
                                     _("'%s=%s' is incompatible with '%s > 0'"),
-                                    NM_SETTING_BOND_OPTION_MODE, value, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
+                                    NM_SETTING_BOND_OPTION_MODE, mode_new, NM_SETTING_BOND_OPTION_ARP_INTERVAL);
                        g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
                        return FALSE;
                }
        }
 
        primary = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_PRIMARY);
-       if (strcmp (value, "active-backup") == 0) {
+       if (strcmp (mode_new, "active-backup") == 0) {
                if (primary && !nm_utils_iface_valid_name (primary)) {
                        g_set_error (error,
                                     NM_CONNECTION_ERROR,
@@ -538,12 +576,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
        }
 
        if (nm_connection_get_setting_infiniband (connection)) {
-               if (strcmp (value, "active-backup") != 0) {
+               if (strcmp (mode_new, "active-backup") != 0) {
                        g_set_error (error,
                                     NM_CONNECTION_ERROR,
                                     NM_CONNECTION_ERROR_INVALID_PROPERTY,
                                     _("'%s=%s' is not a valid configuration for '%s'"),
-                                    NM_SETTING_BOND_OPTION_MODE, value, NM_SETTING_INFINIBAND_SETTING_NAME);
+                                    NM_SETTING_BOND_OPTION_MODE, mode_new, NM_SETTING_INFINIBAND_SETTING_NAME);
                        g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
                        return FALSE;
                }
@@ -629,8 +667,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 
        lacp_rate = g_hash_table_lookup (priv->options, NM_SETTING_BOND_OPTION_LACP_RATE);
        if (   lacp_rate
-           && (g_strcmp0 (value, "802.3ad") != 0 && g_strcmp0 (value, "4") != 0)
-           && (strcmp (lacp_rate, "slow") != 0 && strcmp (lacp_rate, "0") != 0)) {
+           && g_strcmp0 (mode_new, "802.3ad")
+           && strcmp (lacp_rate, "slow") != 0
+           && strcmp (lacp_rate, "0") != 0) {
                g_set_error (error,
                             NM_CONNECTION_ERROR,
                             NM_CONNECTION_ERROR_INVALID_PROPERTY,
@@ -640,6 +679,18 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
                return FALSE;
        }
 
+       if (   (num_grat_arp != -1 && num_unsol_na != -1)
+           && (num_grat_arp != num_unsol_na)) {
+               g_set_error (error,
+                            NM_CONNECTION_ERROR,
+                            NM_CONNECTION_ERROR_INVALID_PROPERTY,
+                            _("'%s' and '%s' cannot have different values"),
+                            NM_SETTING_BOND_OPTION_NUM_GRAT_ARP,
+                            NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
+               g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_OPTIONS);
+               return FALSE;
+       }
+
        if (!_nm_connection_verify_required_interface_name (connection, error))
                return FALSE;
 
@@ -658,6 +709,63 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
        return TRUE;
 }
 
+static gboolean
+options_hash_match (NMSettingBond *s_bond, GHashTable *options1, GHashTable *options2)
+{
+       GHashTableIter iter;
+       const char *key, *value, *value2;
+
+       g_hash_table_iter_init (&iter, options1);
+       while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) {
+               value2 = g_hash_table_lookup (options2, key);
+
+               if (!value2) {
+                       if (nm_streq (key, "num_grat_arp"))
+                               value2 = g_hash_table_lookup (options2, "num_unsol_na");
+                       else if (nm_streq (key, "num_unsol_na"))
+                               value2 = g_hash_table_lookup (options2, "num_grat_arp");
+               }
+
+               if (value2) {
+                       if (nm_streq (value, value2))
+                               continue;
+               } else {
+                       if (nm_streq (value, nm_setting_bond_get_option_default (s_bond, key)))
+                               continue;
+               }
+
+               return FALSE;
+       }
+
+       return TRUE;
+}
+
+static gboolean
+options_equal (NMSettingBond *s_bond, GHashTable *options1, GHashTable *options2)
+{
+       return    options_hash_match (s_bond, options1, options2)
+              && options_hash_match (s_bond, options2, options1);
+}
+
+static gboolean
+compare_property (NMSetting *setting,
+                  NMSetting *other,
+                  const GParamSpec *prop_spec,
+                  NMSettingCompareFlags flags)
+{
+       NMSettingClass *parent_class;
+
+       if (nm_streq0 (prop_spec->name, NM_SETTING_BOND_OPTIONS)) {
+               return options_equal (NM_SETTING_BOND (setting),
+                                     NM_SETTING_BOND_GET_PRIVATE (setting)->options,
+                                     NM_SETTING_BOND_GET_PRIVATE (other)->options);
+       }
+
+       /* Otherwise chain up to parent to handle generic compare */
+       parent_class = NM_SETTING_CLASS (nm_setting_bond_parent_class);
+       return parent_class->compare_property (setting, other, prop_spec, flags);
+}
+
 static void
 nm_setting_bond_init (NMSettingBond *setting)
 {
@@ -721,10 +829,11 @@ nm_setting_bond_class_init (NMSettingBondClass *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;
+       object_class->set_property     = set_property;
+       object_class->get_property     = get_property;
+       object_class->finalize         = finalize;
+       parent_class->verify           = verify;
+       parent_class->compare_property = compare_property;
 
        /* Properties */
        /**
index 055801a..98fcc6d 100644 (file)
@@ -42,21 +42,33 @@ G_BEGIN_DECLS
 #define NM_SETTING_BOND_OPTIONS "options"
 
 /* Valid options for the 'options' property */
-#define NM_SETTING_BOND_OPTION_MODE             "mode"
-#define NM_SETTING_BOND_OPTION_MIIMON           "miimon"
-#define NM_SETTING_BOND_OPTION_DOWNDELAY        "downdelay"
-#define NM_SETTING_BOND_OPTION_UPDELAY          "updelay"
-#define NM_SETTING_BOND_OPTION_ARP_INTERVAL     "arp_interval"
-#define NM_SETTING_BOND_OPTION_ARP_IP_TARGET    "arp_ip_target"
-#define NM_SETTING_BOND_OPTION_ARP_VALIDATE     "arp_validate"
-#define NM_SETTING_BOND_OPTION_PRIMARY          "primary"
-#define NM_SETTING_BOND_OPTION_PRIMARY_RESELECT "primary_reselect"
-#define NM_SETTING_BOND_OPTION_FAIL_OVER_MAC    "fail_over_mac"
-#define NM_SETTING_BOND_OPTION_USE_CARRIER      "use_carrier"
-#define NM_SETTING_BOND_OPTION_AD_SELECT        "ad_select"
-#define NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY "xmit_hash_policy"
-#define NM_SETTING_BOND_OPTION_RESEND_IGMP      "resend_igmp"
-#define NM_SETTING_BOND_OPTION_LACP_RATE        "lacp_rate"
+#define NM_SETTING_BOND_OPTION_MODE              "mode"
+#define NM_SETTING_BOND_OPTION_MIIMON            "miimon"
+#define NM_SETTING_BOND_OPTION_DOWNDELAY         "downdelay"
+#define NM_SETTING_BOND_OPTION_UPDELAY           "updelay"
+#define NM_SETTING_BOND_OPTION_ARP_INTERVAL      "arp_interval"
+#define NM_SETTING_BOND_OPTION_ARP_IP_TARGET     "arp_ip_target"
+#define NM_SETTING_BOND_OPTION_ARP_VALIDATE      "arp_validate"
+#define NM_SETTING_BOND_OPTION_PRIMARY           "primary"
+#define NM_SETTING_BOND_OPTION_PRIMARY_RESELECT  "primary_reselect"
+#define NM_SETTING_BOND_OPTION_FAIL_OVER_MAC     "fail_over_mac"
+#define NM_SETTING_BOND_OPTION_USE_CARRIER       "use_carrier"
+#define NM_SETTING_BOND_OPTION_AD_SELECT         "ad_select"
+#define NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY  "xmit_hash_policy"
+#define NM_SETTING_BOND_OPTION_RESEND_IGMP       "resend_igmp"
+#define NM_SETTING_BOND_OPTION_LACP_RATE         "lacp_rate"
+#define NM_SETTING_BOND_OPTION_ACTIVE_SLAVE      "active_slave"
+#define NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO "ad_actor_sys_prio"
+#define NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM   "ad_actor_system"
+#define NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY  "ad_user_port_key"
+#define NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE "all_slaves_active"
+#define NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS   "arp_all_targets"
+#define NM_SETTING_BOND_OPTION_MIN_LINKS         "min_links"
+#define NM_SETTING_BOND_OPTION_NUM_GRAT_ARP      "num_grat_arp"
+#define NM_SETTING_BOND_OPTION_NUM_UNSOL_NA      "num_unsol_na"
+#define NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE "packets_per_slave"
+#define NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB    "tlb_dynamic_lb"
+#define NM_SETTING_BOND_OPTION_LP_INTERVAL       "lp_interval"
 
 struct _NMSettingBond {
        NMSetting parent;
index 92c816f..14a0eb6 100644 (file)
@@ -28,6 +28,7 @@ noinst_PROGRAMS =             \
        test-keyfile            \
        test-secrets            \
        test-setting-8021x      \
+       test-setting-bond       \
        test-setting-dcb        \
        test-settings-defaults
 
diff --git a/libnm-core/tests/test-setting-bond.c b/libnm-core/tests/test-setting-bond.c
new file mode 100644 (file)
index 0000000..a6afa1c
--- /dev/null
@@ -0,0 +1,198 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright 2016 Red Hat, Inc.
+ */
+
+#include "nm-default.h"
+
+#include "nm-utils.h"
+#include "nm-setting-bond.h"
+#include "nm-connection.h"
+#include "nm-simple-connection.h"
+#include "nm-setting-connection.h"
+#include "nm-errors.h"
+
+#include "nm-test-utils.h"
+
+static void
+create_bond_connection (NMConnection **con, NMSettingBond **s_bond)
+{
+       NMSettingConnection *s_con;
+
+       g_assert (con);
+       g_assert (s_bond);
+
+       *con = nmtst_create_minimal_connection ("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);
+
+       *s_bond = (NMSettingBond *) nm_setting_bond_new ();
+       g_assert (*s_bond);
+
+       nm_connection_add_setting (*con, NM_SETTING (*s_bond));
+}
+
+#define test_verify_options(exp, ...) \
+       G_STMT_START { \
+               const char *__opts[] = { __VA_ARGS__ , NULL }; \
+               \
+               _test_verify_options (__opts, exp); \
+       } G_STMT_END
+
+static void
+_test_verify_options (const char **options, gboolean expected_result)
+{
+       gs_unref_object NMConnection *con = NULL;
+       NMSettingBond *s_bond;
+       GError *error = NULL;
+       gboolean success;
+       const char **option;
+
+       create_bond_connection (&con, &s_bond);
+
+       for (option = options; option[0] && option[1]; 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,
+                                                       NM_CONNECTION_ERROR_INVALID_PROPERTY);
+       }
+}
+
+static void
+test_verify (void)
+{
+       test_verify_options (TRUE,
+                            "mode", "3",
+                            "arp_interval", "0");
+       test_verify_options (FALSE,
+                            /* arp_interval not supported in balance-alb mode */
+                            "mode", "balance-alb",
+                            "arp_interval", "1",
+                            "arp_ip_target", "1.2.3.4");
+       test_verify_options (FALSE,
+                            /* arp_ip_target requires arp_interval */
+                            "mode", "balance-rr",
+                            "arp_ip_target", "1.2.3.4");
+       test_verify_options (TRUE,
+                            "mode", "balance-rr",
+                            "arp_interval", "1",
+                            "arp_ip_target", "1.2.3.4");
+       test_verify_options (FALSE,
+                            /* num_grat_arp, num_unsol_na cannot be different */
+                            "mode", "balance-rr",
+                            "num_grat_arp", "3",
+                            "num_unsol_na", "4");
+       test_verify_options (TRUE,
+                            "mode", "balance-rr",
+                            "num_grat_arp", "5",
+                            "num_unsol_na", "5");
+       test_verify_options (TRUE,
+                            "mode", "active-backup",
+                            "primary", "eth0");
+       test_verify_options (FALSE,
+                            /* primary requires mode=active-backup */
+                            "mode", "802.3ad",
+                            "primary", "eth0");
+       test_verify_options (TRUE,
+                            "mode", "802.3ad",
+                            "lacp_rate", "fast");
+       test_verify_options (FALSE,
+                            /* lacp_rate=fast requires mode=802.3ad */
+                            "mode", "balance-rr",
+                            "lacp_rate", "fast");
+       test_verify_options (TRUE,
+                            "mode", "802.3ad",
+                            "ad_actor_system", "ae:00:11:33:44:55");
+}
+
+static void
+test_compare_options (gboolean exp_res, const char **opts1, const char **opts2)
+{
+       gs_unref_object NMSettingBond *s_bond1 = NULL, *s_bond2 = NULL;
+       const char **p;
+
+       s_bond1 = (NMSettingBond *) nm_setting_bond_new ();
+       g_assert (s_bond1);
+       s_bond2 = (NMSettingBond *) nm_setting_bond_new ();
+       g_assert (s_bond2);
+
+       for (p = opts1; p[0] && p[1]; p += 2)
+               g_assert (nm_setting_bond_add_option (s_bond1, p[0], p[1]));
+
+       for (p = opts2; p[0] && p[1]; p += 2)
+               g_assert (nm_setting_bond_add_option (s_bond2, p[0], p[1]));
+
+       g_assert_cmpint (nm_setting_compare ((NMSetting *) s_bond1,
+                                            (NMSetting *) s_bond2,
+                                            NM_SETTING_COMPARE_FLAG_EXACT),
+                        ==,
+                        exp_res);
+}
+
+static void
+test_compare (void)
+{
+       test_compare_options (TRUE,
+                             ((const char *[]){ "mode", "balance-rr", "miimon", "1", NULL }),
+                             ((const char *[]){ "mode", "balance-rr", "miimon", "1", NULL }));
+       test_compare_options (FALSE,
+                             ((const char *[]){ "mode", "balance-rr", "miimon", "1", NULL }),
+                             ((const char *[]){ "mode", "balance-rr", "miimon", "2", NULL }));
+
+       /* ignore default values */
+       test_compare_options (TRUE,
+                             ((const char *[]){ "miimon", "1", NULL }),
+                             ((const char *[]){ "miimon", "1", "updelay", "0", NULL }));
+
+       /* special handling of num_grat_arp, num_unsol_na */
+       test_compare_options (FALSE,
+                             ((const char *[]){ "num_grat_arp", "2", NULL }),
+                             ((const char *[]){ "num_grat_arp", "1", NULL }));
+       test_compare_options (TRUE,
+                             ((const char *[]){ "num_grat_arp", "3", NULL }),
+                             ((const char *[]){ "num_unsol_na", "3", NULL }));
+       test_compare_options (TRUE,
+                             ((const char *[]){ "num_grat_arp", "4", NULL }),
+                             ((const char *[]){ "num_unsol_na", "4", "num_grat_arp", "4", NULL }));
+}
+
+#define TPATH "/libnm/settings/bond/"
+
+NMTST_DEFINE ();
+
+int
+main (int argc, char **argv)
+{
+       nmtst_init (&argc, &argv, TRUE);
+
+       g_test_add_func (TPATH "verify", test_verify);
+       g_test_add_func (TPATH "compare", test_compare);
+
+       return g_test_run ();
+}
index 80cdccb..adf159d 100644 (file)
@@ -1,5 +1,7 @@
 include $(GLIB_MAKEFILE)
 
+@GNOME_CODE_COVERAGE_RULES@
+
 SUBDIRS = . tests
 
 AM_CPPFLAGS = \
@@ -15,7 +17,8 @@ AM_CPPFLAGS = \
        -DNM_VERSION_MAX_ALLOWED=NM_VERSION_NEXT_STABLE \
        $(GLIB_CFLAGS) \
        $(GUDEV_CFLAGS) \
-       -DNMRUNDIR=\"$(nmrundir)\"
+       -DNMRUNDIR=\"$(nmrundir)\" \
+       $(CODE_COVERAGE_CFLAGS)
 
 include $(top_srcdir)/libnm-core/Makefile.libnm-core
 
@@ -139,6 +142,9 @@ libnm_la_LIBADD = \
        $(UUID_LIBS) \
        $(GUDEV_LIBS)
 
+libnm_la_LDFLAGS = \
+       $(CODE_COVERAGE_LDFLAGS)
+
 SYMBOL_VIS_FILE=$(srcdir)/libnm.ver
 
 libnm_la_LDFLAGS = -Wl,--version-script=$(SYMBOL_VIS_FILE) \
index b88b251..6ae29b9 100644 (file)
@@ -160,16 +160,23 @@ update_connection (NMDevice *device, NMConnection *connection)
        while (options && *options) {
                gs_free char *value = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, *options);
                const char *defvalue = nm_setting_bond_get_option_default (s_bond, *options);
+               char *p;
 
-               if (value && !ignore_if_zero (*options, value) && (g_strcmp0 (value, defvalue) != 0)) {
+               if (_nm_setting_bond_get_option_type (s_bond, *options) == NM_BOND_OPTION_TYPE_BOTH) {
+                       p = strchr (value, ' ');
+                       if (p)
+                               *p = '\0';
+               }
+
+               if (   value
+                   && value[0]
+                   && !ignore_if_zero (*options, value)
+                   && !nm_streq0 (value, defvalue)) {
                        /* Replace " " with "," for arp_ip_targets from the kernel */
                        if (strcmp (*options, "arp_ip_target") == 0) {
-                               char *p = value;
-
-                               while (p && *p) {
+                               for (p = value; *p; p++) {
                                        if (*p == ' ')
                                                *p = ',';
-                                       p++;
                                }
                        }
 
@@ -285,7 +292,7 @@ apply_bonding_config (NMDevice *device)
        set_bond_attr (device, "mode", mode);
 
        /* arp_interval not compatible with ALB, TLB */
-       if (g_strcmp0 (mode, "balance-alb") == 0 || g_strcmp0 (mode, "balance-tlb") == 0)
+       if (NM_IN_STRSET (mode, "balance-alb", "balance-tlb"))
                set_arp_interval = FALSE;
 
        if (set_arp_interval) {
@@ -299,18 +306,17 @@ apply_bonding_config (NMDevice *device)
        value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_VALIDATE);
        /* arp_validate > 0 only valid in active-backup mode */
        if (   value
-           && g_strcmp0 (value, "0") != 0
-           && g_strcmp0 (value, "none") != 0
-           && g_strcmp0 (mode, "active-backup") == 0)
+           && !nm_streq (value, "0")
+           && !nm_streq (value, "none")
+           && nm_streq (mode, "active-backup"))
                set_bond_attr (device, "arp_validate", value);
        else
                set_bond_attr (device, "arp_validate", "0");
 
-       if (   g_strcmp0 (mode, "active-backup") == 0
-           || g_strcmp0 (mode, "balance-alb") == 0
-           || g_strcmp0 (mode, "balance-tlb") == 0) {
+       if (NM_IN_STRSET (mode, "active-backup", "balance-alb", "balance-tlb")) {
                value = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_PRIMARY);
                set_bond_attr (device, "primary", value ? value : "");
+               set_simple_option (device, "lp_internal", s_bond, NM_SETTING_BOND_OPTION_LP_INTERVAL);
        }
 
        /* Clear ARP targets */
@@ -328,15 +334,31 @@ apply_bonding_config (NMDevice *device)
        set_simple_option (device, "ad_select", s_bond, NM_SETTING_BOND_OPTION_AD_SELECT);
        set_simple_option (device, "xmit_hash_policy", s_bond, NM_SETTING_BOND_OPTION_XMIT_HASH_POLICY);
        set_simple_option (device, "resend_igmp", s_bond, NM_SETTING_BOND_OPTION_RESEND_IGMP);
+       set_simple_option (device, "active_slave", s_bond, NM_SETTING_BOND_OPTION_ACTIVE_SLAVE);
+       set_simple_option (device, "all_slaves_active", s_bond, NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE);
+       set_simple_option (device, "num_grat_arp", s_bond, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP);
+       set_simple_option (device, "num_unsol_na", s_bond, NM_SETTING_BOND_OPTION_NUM_UNSOL_NA);
 
-       if (   g_strcmp0 (mode, "4") == 0
-           || g_strcmp0 (mode, "802.3ad") == 0)
+       if (nm_streq (mode, "802.3ad")) {
                set_simple_option (device, "lacp_rate", s_bond, NM_SETTING_BOND_OPTION_LACP_RATE);
+               set_simple_option (device, "ad_actor_sys_prio", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO);
+               set_simple_option (device, "ad_actor_system", s_bond, NM_SETTING_BOND_OPTION_AD_ACTOR_SYSTEM);
+               set_simple_option (device, "ad_user_port_key", s_bond, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY);
+               set_simple_option (device, "min_links", s_bond, NM_SETTING_BOND_OPTION_MIN_LINKS);
+       }
+
+       if (nm_streq (mode, "active-backup"))
+               set_simple_option (device, "arp_all_targets", s_bond, NM_SETTING_BOND_OPTION_ARP_ALL_TARGETS);
+
+       if (nm_streq (mode, "balance-rr"))
+               set_simple_option (device, "packets_per_slave", s_bond, NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE);
+
+       if (nm_streq (mode, "balance-tlb"))
+               set_simple_option (device, "tlb_dynamic_lb", s_bond, NM_SETTING_BOND_OPTION_TLB_DYNAMIC_LB);
 
        return NM_ACT_STAGE_RETURN_SUCCESS;
 }
 
-
 static NMActStageReturn
 act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason)
 {