From 824dd17f86ff299c9350f8070bd1d1cac6f3f2ad Mon Sep 17 00:00:00 2001 From: Pawel Wieczorkiewicz Date: Wed, 10 Jun 2015 11:36:38 +0200 Subject: [PATCH 1/4] bonding: don't insist on active-backup mode when arp_validate=none (boo#919573) --- src/bonding.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bonding.c b/src/bonding.c index c73e6d4bb..3b569066a 100644 --- a/src/bonding.c +++ b/src/bonding.c @@ -251,6 +251,7 @@ ni_bonding_validate(const ni_bonding_t *bonding) switch (bonding->arpmon.validate) { case NI_BOND_ARP_VALIDATE_NONE: + break; case NI_BOND_ARP_VALIDATE_ACTIVE: case NI_BOND_ARP_VALIDATE_BACKUP: case NI_BOND_ARP_VALIDATE_ALL: From 1586e7bdb83f40103dda559555cf567fbc01e3c5 Mon Sep 17 00:00:00 2001 From: Pawel Wieczorkiewicz Date: Wed, 10 Jun 2015 11:49:06 +0200 Subject: [PATCH 2/4] bonding: reject arp monitoring in balance-tlb/-alb or 802.3ad mode --- src/bonding.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/bonding.c b/src/bonding.c index 3b569066a..63350812e 100644 --- a/src/bonding.c +++ b/src/bonding.c @@ -241,9 +241,14 @@ ni_bonding_validate(const ni_bonding_t *bonding) if (bonding->miimon.frequency > 0) return "invalid arp and mii monitoring option mix"; - if (bonding->mode == NI_BOND_MODE_BALANCE_TLB || - bonding->mode == NI_BOND_MODE_BALANCE_ALB) - return "invalid arp monitoring in balance-tlb/-alb mode"; + switch(bonding->mode) { + case NI_BOND_MODE_802_3AD: + case NI_BOND_MODE_BALANCE_TLB: + case NI_BOND_MODE_BALANCE_ALB: + return "invalid arp monitoring in balance-tlb/-alb or 802.3ad mode"; + default: + break; + } if (bonding->arpmon.interval == 0 || bonding->arpmon.interval > INT_MAX) From 0df2f0c08b1ca1ed0099e68d5106ed565356cd77 Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Wed, 10 Jun 2015 12:56:22 +0200 Subject: [PATCH 3/4] bonding: reparse settings after setting them Older bonding drivers support arp_validate only in active-backup mode and cause a failure on attempts to set even the default arp_validate=none. Reparse and update actual bonding options before and after setting them to better catch option deps. --- include/wicked/bonding.h | 2 +- src/bonding.c | 9 +++++++-- src/ifconfig.c | 3 +++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/include/wicked/bonding.h b/include/wicked/bonding.h index ae03b90d1..70ca77267 100644 --- a/include/wicked/bonding.h +++ b/include/wicked/bonding.h @@ -114,7 +114,7 @@ extern ni_bool_t ni_bonding_set_option(ni_bonding_t *, const char *, const char extern int ni_bonding_parse_sysfs_attrs(const char *, ni_bonding_t *); extern int ni_bonding_write_sysfs_attrs(const char *ifname, const ni_bonding_t *cfg_bond, - const ni_bonding_t *cur_bond, + ni_bonding_t *cur_bond, ni_bool_t is_up, ni_bool_t has_slaves); extern ni_bool_t ni_bonding_is_valid_arp_ip_target(const char *); diff --git a/src/bonding.c b/src/bonding.c index 63350812e..63842c3cc 100644 --- a/src/bonding.c +++ b/src/bonding.c @@ -826,7 +826,7 @@ ni_bonding_parse_sysfs_attribute(ni_bonding_t *bonding, const char *attr, char * bonding->monitoring = NI_BOND_MONITOR_ARP; } else if (!strcmp(attr, "arp_ip_target")) { char *s, *p = NULL; - for (s = strtok_r(value, ",", &p); s; s = strtok_r(NULL, ",", &p)) { + for (s = strtok_r(value, " \t\n", &p); s; s = strtok_r(NULL, " \t\n", &p)) { if (ni_bonding_is_valid_arp_ip_target(s)) ni_string_array_append(&bonding->arpmon.targets, s); } @@ -1069,7 +1069,7 @@ ni_bonding_write_one_sysfs_attr(const char *ifname, const ni_bonding_t *bonding, * as well as in dependency of the bonding up/down state and slave count. */ int -ni_bonding_write_sysfs_attrs(const char *ifname, const ni_bonding_t *bonding, const ni_bonding_t *current, ni_bool_t is_up, ni_bool_t has_slaves) +ni_bonding_write_sysfs_attrs(const char *ifname, const ni_bonding_t *bonding, ni_bonding_t *current, ni_bool_t is_up, ni_bool_t has_slaves) { /* * option up/down slaves modes @@ -1131,6 +1131,7 @@ ni_bonding_write_sysfs_attrs(const char *ifname, const ni_bonding_t *bonding, co }; const struct attr_matrix *attrs; unsigned int i; + char *attrval = NULL; attrs = attr_matrix; for (i = 0; attrs[i].name; ++i) { @@ -1154,6 +1155,10 @@ ni_bonding_write_sysfs_attrs(const char *ifname, const ni_bonding_t *bonding, co !attrs[i].nofail) return -1; } + + if (!ni_sysfs_bonding_get_attr(ifname, attrs[i].name, &attrval) && attrval) + ni_bonding_parse_sysfs_attribute(current, attrs[i].name, attrval); + ni_string_free(&attrval); } return 0; diff --git a/src/ifconfig.c b/src/ifconfig.c index 680917e73..dd4dc6ab8 100644 --- a/src/ifconfig.c +++ b/src/ifconfig.c @@ -1369,6 +1369,7 @@ ni_system_bond_setup(ni_netconfig_t *nc, ni_netdev_t *dev, const ni_bonding_t *b */ ni_debug_ifconfig("%s: configuring bonding device (stage 0.%u.%u)", dev->name, is_up, has_slaves); + ni_bonding_parse_sysfs_attrs(dev->name, bond); if (ni_bonding_write_sysfs_attrs(dev->name, bond_cfg, bond, is_up, has_slaves) < 0) { ni_error("%s: cannot configure bonding device (stage 0.%u.%u)", @@ -1442,6 +1443,7 @@ ni_system_bond_setup(ni_netconfig_t *nc, ni_netdev_t *dev, const ni_bonding_t *b */ ni_debug_ifconfig("%s: configuring bonding device (stage 2.%u.%u)", dev->name, is_up, has_slaves); + ni_bonding_parse_sysfs_attrs(dev->name, bond); if (ni_bonding_write_sysfs_attrs(dev->name, bond_cfg, bond, is_up, has_slaves) < 0) { ni_error("%s: cannot configure bonding device (stage 2.%u.%u)", @@ -1453,6 +1455,7 @@ ni_system_bond_setup(ni_netconfig_t *nc, ni_netdev_t *dev, const ni_bonding_t *b dev->name); return -1; } + ni_bonding_parse_sysfs_attrs(dev->name, bond); return 0; } From 23013c6f5cd038fd16b2065a78cfedc4c348981b Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Thu, 11 Jun 2015 08:34:33 +0200 Subject: [PATCH 4/4] schema: address element names for bonding arpmon targets --- schema/bonding.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/schema/bonding.xml b/schema/bonding.xml index ff3f28788..90fb9715b 100644 --- a/schema/bonding.xml +++ b/schema/bonding.xml @@ -40,7 +40,7 @@ - +