[PATCH net-next 00/25] bonding: introduce new option API

[PATCH net-next 00/25] bonding: introduce new option API

[Nikolay Aleksandrov]

Hi,
This patchset's goal is to introduce a new option API which should be used
to properly describe the bonding options with their mode dependcies and
requirements. With this patchset applied we get centralized option
manipulation, automatic RTNL acquire per option setting, automatic option
range checking, mode dependcy checking and other various flags which are
described in detail in patch 01's commit message and comments.
Also the parameter passing is changed to use a specialized structure which
is initialized to a value depending on the needs.
The main exported functions are:
 __bond_opt_set() - set an option (RTNL should be acquired prior)
 bond_opt_init(val|str) - init a bond_opt_value struct for value or string
                          parameter passing
 bond_opt_tryset_rtnl() - function which tries to acquire rtnl, mainly used
                          for sysfs
 bond_opt_parse - used to parse or check for valid values
 bond_opt_get - retrieve a pointer to bond_option struct for some option
 bond_opt_get_val - retrieve a pointer to a bond_opt_value struct for
                    some value

The same functions are used to set an option via sysfs and netlink, just
the parameter that's passed is usually initialized in a different way.
The converted options have multiple style fixes, there're some longer
lines but they looked either ugly or were strings/pr_warnings, if you
think some line would be better broken just let me know :-) there're
also a few sscanf false-positive warnings.
I decided to keep the "unsuppmodes" way of mode dep checking since it's
straight forward, if we make a more general way for checking dependencies
it'll be easy to change it.

Future plans for this work include:
 - Automatic sysfs generation from the bond_opts[].
 - Use of the API in bond_check_params() and thus cleaning it up (this has
   actually started, I'll take care of the rest in a separate patch)
 - Clean up all option-unrelated files of option definitions and functions

I've tried to leave as much documentation as possible, if there's anything
unclear please let me know. One more thing, I haven't moved all
option-related functions from bonding.h to the new bond_options.h, this
will be done in a separate patch, it's in my todo list.

This patchset has been tested by setting each converted option via sysfs
and netlink to a couple of wrong values, a couple of correct values and
some random values, also for the opts that have flags they have been
tested as well.

Best regards,
 Nikolay Aleksandrov

CC: Andy Gospodarek <andy@greyhouse.net>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Veaceslav Falico <vfalico@redhat.com>
CC: Scott Feldman <sfeldma@cumulusnetworks.com>
CC: David S. Miller <davem@davemloft.net>

Nikolay Aleksandrov (25):
  bonding: add infrastructure for an option API
  bonding: convert mode setting to use the new option API
  bonding: convert packets_per_slave to use the new option API
  bonding: convert xmit_hash_policy to use the new option API
  bonding: convert arp_validate to use the new option API
  bonding: convert arp_all_targets to use the new option API
  bonding: convert fail_over_mac to use the new option API
  bonding: convert arp_interval to use the new option API
  bonding: convert arp_ip_target to use the new option API
  bonding: convert downdelay to use the new option API
  bonding: convert updelay to use the new option API
  bonding: convert lacp_rate to use the new option API
  bonding: convert min_links to use the new option API
  bonding: convert ad_select to use the new option API
  bonding: convert num_peer_notif to use the new option API
  bonding: convert miimon to use the new option API
  bonding: convert primary to use the new option API
  bonding: convert primary_reselect to use the new option API
  bonding: convert use_carrier to use the new option API
  bonding: convert active_slave to use the new option API
  bonding: convert queue_id to use the new option API
  bonding: convert all_slaves_active to use the new option API
  bonding: convert resend_igmp to use the new option API
  bonding: convert lp_interval to use the new option API
  bonding: convert slaves to use the new option API

 drivers/net/bonding/bond_main.c    |  179 +++---
 drivers/net/bonding/bond_netlink.c |   87 ++-
 drivers/net/bonding/bond_options.c | 1086 +++++++++++++++++++++++++++---------
 drivers/net/bonding/bond_options.h |  170 ++++++
 drivers/net/bonding/bond_procfs.c  |   25 +-
 drivers/net/bonding/bond_sysfs.c   |  519 +++--------------
 drivers/net/bonding/bonding.h      |   29 +-
 7 files changed, 1214 insertions(+), 881 deletions(-)
 create mode 100644 drivers/net/bonding/bond_options.h

-- 
1.8.4.2

[PATCH net-next 01/25] bonding: add infrastructure for an option API

[Nikolay Aleksandrov]

This patch adds the necessary basic infrastructure to support
centralized and unified option manipulation API for the bonding. The new
structure bond_option will be used to describe each option with its
dependencies on modes which will be checked automatically thus removing a
lot of duplicated code. Also automatic range checking is added for
some options. Currently the option setting function requires RTNL to
be acquired prior to calling it, since many options already rely on RTNL
it seemed like the best choice to protect all against common race
conditions.
In order to add an option the following steps need to be done:
1. Add an entry BOND_OPT_<option> to bond_options.h so it gets a unique id
   and a bit corresponding to the id
2. Add a bond_option entry to the bond_opts[] array in bond_options.c which
   describes the option, its dependencies and its manipulation function
3. Add code to export the option through sysfs and/or as a module parameter
   (the sysfs export will be made automatically in the future)

The options can have different flags set, currently the following are
supported:
BOND_OPTFLAG_NOSLAVES - require that the bond device has no slaves prior
                        to setting the option
BOND_OPTFLAG_IFDOWN - require that the bond device is down prior to
                      setting the option
BOND_OPTFLAG_RAWVAL - don't parse the value but return it raw for the
                      option to parse

There's a new value structure to describe different types of values
which can have the following flags:
BOND_VALFLAG_DEFAULT - marks the default option (permanent string alias
                       to this option is "default")
BOND_VALFLAG_MIN - the minimum value that this option can have
BOND_VALFLAG_MAX - the maximum value that this option can have

An example would be nice here, so if we have an option which can have
the values "off"(2), "special"(4, default) and supports a range, say
16 - 32, it should be defined as follows:
"off", 2,
"special", 4, BOND_VALFLAG_DEFAULT,
"rangemin", 16, BOND_VALFLAG_MIN,
"rangemax", 32, BOND_VALFLAG_MAX
So we have the valid intervals: [2, 2], [4, 4], [16, 32]
Also the valid strings: "off" = 2, "special" and "default" = 4
                        "rangemin" = 16, "rangemax" = 32

BOND_VALFLAG_(MIN|MAX) can be used to specify a valid range for an
option, if MIN is omitted then 0 is considered as a minimum. If an
exact match is found in the values[] table it will be returned,
otherwise the range is tried (if available).

The option parameter passing is done by using a special structure called
bond_opt_value which can take either a string or a value to parse. One
of the bond_opt_init(val|str) macros should be used depending on which
one does the user want to parse (string or value). Then a call to
__bond_opt_set should be done under RTNL.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_options.c | 272 +++++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_options.h | 100 ++++++++++++++
 drivers/net/bonding/bonding.h      |   1 +
 3 files changed, 373 insertions(+)
 create mode 100644 drivers/net/bonding/bond_options.h

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 945a666..edc2941 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -17,8 +17,280 @@
 #include <linux/rwlock.h>
 #include <linux/rcupdate.h>
 #include <linux/reciprocal_div.h>
+#include <linux/ctype.h>
 #include "bonding.h"
 
+static struct bond_option bond_opts[] = {
+	{ }
+};
+
+/* Searches for a value in opt's values[] table */
+struct bond_opt_value *bond_opt_get_val(unsigned int option, u64 val)
+{
+	struct bond_option *opt;
+	int i;
+
+	opt = bond_opt_get(option);
+	if (WARN_ON(!opt))
+		return NULL;
+	for (i = 0; opt->values && opt->values[i].string; i++)
+		if (opt->values[i].value == val)
+			return &opt->values[i];
+
+	return NULL;
+}
+
+/* Searches for a value in opt's values[] table which matches the flagmask */
+static struct bond_opt_value *bond_opt_get_flags(const struct bond_option *opt,
+						 u32 flagmask)
+{
+	int i;
+
+	for (i = 0; opt->values && opt->values[i].string; i++)
+		if (opt->values[i].flags & flagmask)
+			return &opt->values[i];
+
+	return NULL;
+}
+
+/* If maxval is missing then there's no range to check. In case minval is
+ * missing then it's considered to be 0.
+ */
+static bool bond_opt_check_range(const struct bond_option *opt, u64 val)
+{
+	struct bond_opt_value *minval, *maxval;
+
+	minval = bond_opt_get_flags(opt, BOND_VALFLAG_MIN);
+	maxval = bond_opt_get_flags(opt, BOND_VALFLAG_MAX);
+	if (!maxval || (minval && val < minval->value) || val > maxval->value)
+		return false;
+
+	return true;
+}
+
+/**
+ * bond_opt_parse - parse option value
+ * @opt: the option to parse against
+ * @val: value to parse
+ *
+ * This function tries to extract the value from @val and check if it's
+ * a possible match for the option and returns NULL if a match isn't found,
+ * or the struct_opt_value that matched. It also strips the new line from
+ * @val->string if it's present.
+ */
+struct bond_opt_value *bond_opt_parse(const struct bond_option *opt,
+				      struct bond_opt_value *val)
+{
+	char *p, valstr[BOND_OPT_MAX_NAMELEN + 1] = { 0, };
+	struct bond_opt_value *tbl, *ret = NULL;
+	bool checkval;
+	int i, rv;
+
+	/* No parsing if the option wants a raw val */
+	if (opt->flags & BOND_OPTFLAG_RAWVAL)
+		return val;
+
+	tbl = opt->values;
+	if (!tbl)
+		goto out;
+
+	/* ULLONG_MAX is used to bypass string processing */
+	checkval = val->value != ULLONG_MAX;
+	if (!checkval) {
+		if (!val->string)
+			goto out;
+		p = strchr(val->string, '\n');
+		if (p)
+			*p = '\0';
+		for (p = val->string; *p; p++)
+			if (!(isdigit(*p) || isspace(*p)))
+				break;
+		/* The following code extracts the string to match or the value
+		 * and sets checkval appropriately
+		 */
+		if (*p) {
+			rv = sscanf(val->string, "%32s", valstr);
+		} else {
+			rv = sscanf(val->string, "%llu", &val->value);
+			checkval = true;
+		}
+		if (!rv)
+			goto out;
+	}
+
+	for (i = 0; tbl[i].string; i++) {
+		/* Check for exact match */
+		if (checkval) {
+			if (val->value == tbl[i].value)
+				ret = &tbl[i];
+		} else {
+			if (!strcmp(valstr, "default") &&
+			    (tbl[i].flags & BOND_VALFLAG_DEFAULT))
+				ret = &tbl[i];
+
+			if (!strcmp(valstr, tbl[i].string))
+				ret = &tbl[i];
+		}
+		/* Found an exact match */
+		if (ret)
+			goto out;
+	}
+	/* Possible range match */
+	if (checkval && bond_opt_check_range(opt, val->value))
+		ret = val;
+out:
+	return ret;
+}
+
+/* Check opt's dependencies against bond mode and currently set options */
+static int bond_opt_check_deps(struct bonding *bond,
+			       const struct bond_option *opt)
+{
+	struct bond_params *params = &bond->params;
+
+	if (test_bit(params->mode, &opt->unsuppmodes))
+		return -EACCES;
+	if ((opt->flags & BOND_OPTFLAG_NOSLAVES) && bond_has_slaves(bond))
+		return -ENOTEMPTY;
+	if ((opt->flags & BOND_OPTFLAG_IFDOWN) && (bond->dev->flags & IFF_UP))
+		return -EBUSY;
+
+	return 0;
+}
+
+static void bond_opt_dep_print(struct bonding *bond,
+			       const struct bond_option *opt)
+{
+	struct bond_params *params;
+
+	params = &bond->params;
+	if (test_bit(params->mode, &opt->unsuppmodes))
+		pr_err("%s: option %s: mode dependency failed\n",
+		       bond->dev->name, opt->name);
+}
+
+static void bond_opt_error_interpret(struct bonding *bond,
+				     const struct bond_option *opt,
+				     int error, struct bond_opt_value *val)
+{
+	struct bond_opt_value *minval, *maxval;
+	char *p;
+
+	switch (error) {
+	case -EINVAL:
+		if (val) {
+			if (val->string) {
+				/* sometimes RAWVAL opts may have new lines */
+				p = strchr(val->string, '\n');
+				if (p)
+					*p = '\0';
+				pr_err("%s: option %s: invalid value (%s).\n",
+				       bond->dev->name, opt->name, val->string);
+			} else {
+				pr_err("%s: option %s: invalid value (%llu).\n",
+				       bond->dev->name, opt->name, val->value);
+			}
+		}
+		minval = bond_opt_get_flags(opt, BOND_VALFLAG_MIN);
+		maxval = bond_opt_get_flags(opt, BOND_VALFLAG_MAX);
+		if (!maxval)
+			break;
+		pr_err("%s: option %s: allowed values %llu - %llu.\n",
+		       bond->dev->name, opt->name, minval ? minval->value : 0,
+		       maxval->value);
+		break;
+	case -EACCES:
+		bond_opt_dep_print(bond, opt);
+		break;
+	case -ENOTEMPTY:
+		pr_err("%s: option %s: unable to set because the bond device has slaves.\n",
+		       bond->dev->name, opt->name);
+		break;
+	case -EBUSY:
+		pr_err("%s: option %s: unable to set because the bond device is up.\n",
+		       bond->dev->name, opt->name);
+		break;
+	default:
+		break;
+	}
+}
+
+/**
+ * __bond_opt_set - set a bonding option
+ * @bond: target bond device
+ * @option: option to set
+ * @val: value to set it to
+ *
+ * This function is used to change the bond's option value, it can be
+ * used for both enabling/changing an option and for disabling it. RTNL lock
+ * must be obtained before calling this function.
+ */
+int __bond_opt_set(struct bonding *bond,
+		   unsigned int option, struct bond_opt_value *val)
+{
+	struct bond_opt_value *retval = NULL;
+	const struct bond_option *opt;
+	int ret = -ENOENT;
+
+	ASSERT_RTNL();
+
+	opt = bond_opt_get(option);
+	if (WARN_ON(!val) || WARN_ON(!opt))
+		goto out;
+	ret = bond_opt_check_deps(bond, opt);
+	if (ret)
+		goto out;
+	retval = bond_opt_parse(opt, val);
+	if (!retval) {
+		ret = -EINVAL;
+		goto out;
+	}
+	ret = opt->set(bond, retval);
+out:
+	if (ret)
+		bond_opt_error_interpret(bond, opt, ret, val);
+
+	return ret;
+}
+
+/**
+ * bond_opt_tryset_rtnl - try to acquire rtnl and call __bond_opt_set
+ * @bond: target bond device
+ * @option: option to set
+ * @buf: value to set it to
+ *
+ * This function tries to acquire RTNL without blocking and if successful
+ * calls __bond_opt_set. It is mainly used for sysfs option manipulation.
+ */
+int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf)
+{
+	struct bond_opt_value optval;
+	int ret;
+
+	if (!rtnl_trylock())
+		return restart_syscall();
+	bond_opt_initstr(&optval, buf);
+	ret = __bond_opt_set(bond, option, &optval);
+	rtnl_unlock();
+
+	return ret;
+}
+
+/**
+ * bond_opt_get - get a pointer to an option
+ * @option: option for which to return a pointer
+ *
+ * This function checks if option is valid and if so returns a pointer
+ * to its entry in the bond_opts[] option array.
+ */
+struct bond_option *bond_opt_get(unsigned int option)
+{
+	if (!BOND_OPT_VALID(option))
+		return NULL;
+
+	return &bond_opts[option];
+}
+
 int bond_option_mode_set(struct bonding *bond, int mode)
 {
 	if (bond_parm_tbl_lookup(mode, bond_mode_tbl) < 0) {
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
new file mode 100644
index 0000000..e20f2eb
--- /dev/null
+++ b/drivers/net/bonding/bond_options.h
@@ -0,0 +1,100 @@
+/*
+ * drivers/net/bond/bond_options.h - bonding options
+ * Copyright (c) 2013 Nikolay Aleksandrov <nikolay@redhat.com>
+ *
+ * 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 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _BOND_OPTIONS_H
+#define _BOND_OPTIONS_H
+
+#define BOND_OPT_MAX_NAMELEN 32
+#define BOND_OPT_VALID(opt) ((opt) < BOND_OPT_LAST)
+#define BOND_MODE_ALL_EX(x) (~(x))
+
+/* Option flags:
+ * BOND_OPTFLAG_NOSLAVES - check if the bond device is empty before setting
+ * BOND_OPTFLAG_IFDOWN - check if the bond device is down before setting
+ * BOND_OPTFLAG_RAWVAL - the option parses the value itself
+ */
+enum {
+	BOND_OPTFLAG_NOSLAVES	= BIT(0),
+	BOND_OPTFLAG_IFDOWN	= BIT(1),
+	BOND_OPTFLAG_RAWVAL	= BIT(2)
+};
+
+/* Value type flags:
+ * BOND_VALFLAG_DEFAULT - mark the value as default
+ * BOND_VALFLAG_(MIN|MAX) - mark the value as min/max
+ */
+enum {
+	BOND_VALFLAG_DEFAULT	= BIT(0),
+	BOND_VALFLAG_MIN	= BIT(1),
+	BOND_VALFLAG_MAX	= BIT(2)
+};
+
+/* Option IDs, their bit positions correspond to their IDs */
+enum {
+	BOND_OPT_LAST
+};
+
+/* This structure is used for storing option values and for passing option
+ * values when changing an option. The logic when used as an arg is as follows:
+ * - if string != NULL -> parse it, if the opt is RAW type then return it, else
+ *   return the parse result
+ * - if string == NULL -> parse value
+ */
+struct bond_opt_value {
+	char *string;
+	u64 value;
+	u32 flags;
+};
+
+struct bonding;
+
+struct bond_option {
+	int id;
+	char *name;
+	char *desc;
+	u32 flags;
+
+	/* unsuppmodes is used to denote modes in which the option isn't
+	 * supported.
+	 */
+	unsigned long unsuppmodes;
+	/* supported values which this option can have, can be a subset of
+	 * BOND_OPTVAL_RANGE's value range
+	 */
+	struct bond_opt_value *values;
+
+	int (*set)(struct bonding *bond, struct bond_opt_value *val);
+};
+
+int __bond_opt_set(struct bonding *bond, unsigned int option,
+		   struct bond_opt_value *val);
+int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
+struct bond_opt_value *bond_opt_parse(const struct bond_option *opt,
+				      struct bond_opt_value *val);
+struct bond_option *bond_opt_get(unsigned int option);
+struct bond_opt_value *bond_opt_get_val(unsigned int option, u64 val);
+
+/* This helper is used to initialize a bond_opt_value structure for parameter
+ * passing. There should be either a valid string or value, but not both.
+ * When value is ULLONG_MAX then string will be used.
+ */
+static inline void __bond_opt_init(struct bond_opt_value *optval,
+				   char *string, u64 value)
+{
+	memset(optval, 0, sizeof(*optval));
+	optval->value = ULLONG_MAX;
+	if (value == ULLONG_MAX)
+		optval->string = string;
+	else
+		optval->value = value;
+}
+#define bond_opt_initval(optval, value) __bond_opt_init(optval, NULL, value)
+#define bond_opt_initstr(optval, str) __bond_opt_init(optval, str, ULLONG_MAX)
+#endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 8a935f8..92ee557 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -25,6 +25,7 @@
 #include <linux/etherdevice.h>
 #include "bond_3ad.h"
 #include "bond_alb.h"
+#include "bond_options.h"
 
 #define DRV_VERSION	"3.7.1"
 #define DRV_RELDATE	"April 27, 2011"
-- 
1.8.4.2

[PATCH net-next 02/25] bonding: convert mode setting to use the new option API

[Nikolay Aleksandrov]

This patch makes the bond's mode setting use the new option API and
adds support for dependency printing which relies on having an entry for
the mode option in the bond_opts[] array.
Also add the ability to print the mode name when mode dependency fails
and fix some trivial/style errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 21 +++++----------
 drivers/net/bonding/bond_netlink.c |  4 ++-
 drivers/net/bonding/bond_options.c | 53 +++++++++++++++++++++-----------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 27 +++++--------------
 drivers/net/bonding/bonding.h      |  2 --
 6 files changed, 48 insertions(+), 62 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3220b48..25f236d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -215,17 +215,6 @@ const struct bond_parm_tbl bond_lacp_tbl[] = {
 {	NULL,		-1},
 };
 
-const struct bond_parm_tbl bond_mode_tbl[] = {
-{	"balance-rr",		BOND_MODE_ROUNDROBIN},
-{	"active-backup",	BOND_MODE_ACTIVEBACKUP},
-{	"balance-xor",		BOND_MODE_XOR},
-{	"broadcast",		BOND_MODE_BROADCAST},
-{	"802.3ad",		BOND_MODE_8023AD},
-{	"balance-tlb",		BOND_MODE_TLB},
-{	"balance-alb",		BOND_MODE_ALB},
-{	NULL,			-1},
-};
-
 const struct bond_parm_tbl xmit_hashtype_tbl[] = {
 {	"layer2",		BOND_XMIT_POLICY_LAYER2},
 {	"layer3+4",		BOND_XMIT_POLICY_LAYER34},
@@ -4026,18 +4015,20 @@ int bond_parse_parm(const char *buf, const struct bond_parm_tbl *tbl)
 static int bond_check_params(struct bond_params *params)
 {
 	int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
+	struct bond_opt_value newval, *valptr;
 	int arp_all_targets_value;
 
 	/*
 	 * Convert string parameters.
 	 */
 	if (mode) {
-		bond_mode = bond_parse_parm(mode, bond_mode_tbl);
-		if (bond_mode == -1) {
-			pr_err("Error: Invalid bonding mode \"%s\"\n",
-			       mode == NULL ? "NULL" : mode);
+		bond_opt_initstr(&newval, mode);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_MODE), &newval);
+		if (!valptr) {
+			pr_err("Error: Invalid bonding mode \"%s\"\n", mode);
 			return -EINVAL;
 		}
+		bond_mode = valptr->value;
 	}
 
 	if (xmit_hash_policy) {
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 21c6488..097cfdd 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -99,6 +99,7 @@ static int bond_changelink(struct net_device *bond_dev,
 			   struct nlattr *tb[], struct nlattr *data[])
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+	struct bond_opt_value newval;
 	int miimon = 0;
 	int err;
 
@@ -108,7 +109,8 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_MODE]) {
 		int mode = nla_get_u8(data[IFLA_BOND_MODE]);
 
-		err = bond_option_mode_set(bond, mode);
+		bond_opt_initval(&newval, mode);
+		err = __bond_opt_set(bond, BOND_OPT_MODE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index edc2941..0cbad5a 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -20,7 +20,26 @@
 #include <linux/ctype.h>
 #include "bonding.h"
 
+static struct bond_opt_value bond_mode_tbl[] = {
+	{ "balance-rr",    BOND_MODE_ROUNDROBIN,   BOND_VALFLAG_DEFAULT},
+	{ "active-backup", BOND_MODE_ACTIVEBACKUP, 0},
+	{ "balance-xor",   BOND_MODE_XOR,          0},
+	{ "broadcast",     BOND_MODE_BROADCAST,    0},
+	{ "802.3ad",       BOND_MODE_8023AD,       0},
+	{ "balance-tlb",   BOND_MODE_TLB,          0},
+	{ "balance-alb",   BOND_MODE_ALB,          0},
+	{ NULL,            -1,                     0},
+};
+
 static struct bond_option bond_opts[] = {
+	[BOND_OPT_MODE] = {
+		.id = BOND_OPT_MODE,
+		.name = "mode",
+		.desc = "bond device mode",
+		.flags = BOND_OPTFLAG_NOSLAVES | BOND_OPTFLAG_IFDOWN,
+		.values = bond_mode_tbl,
+		.set = bond_option_mode_set
+	},
 	{ }
 };
 
@@ -161,12 +180,15 @@ static int bond_opt_check_deps(struct bonding *bond,
 static void bond_opt_dep_print(struct bonding *bond,
 			       const struct bond_option *opt)
 {
+	struct bond_opt_value *modeval;
 	struct bond_params *params;
 
 	params = &bond->params;
+	modeval = bond_opt_get_val(BOND_OPT_MODE, params->mode);
 	if (test_bit(params->mode, &opt->unsuppmodes))
-		pr_err("%s: option %s: mode dependency failed\n",
-		       bond->dev->name, opt->name);
+		pr_err("%s: option %s: mode dependency failed, not supported in mode %s(%llu)\n",
+		       bond->dev->name, opt->name,
+		       modeval->string, modeval->value);
 }
 
 static void bond_opt_error_interpret(struct bonding *bond,
@@ -291,29 +313,11 @@ struct bond_option *bond_opt_get(unsigned int option)
 	return &bond_opts[option];
 }
 
-int bond_option_mode_set(struct bonding *bond, int mode)
+int bond_option_mode_set(struct bonding *bond, struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(mode, bond_mode_tbl) < 0) {
-		pr_err("%s: Ignoring invalid mode value %d.\n",
-		       bond->dev->name, mode);
-		return -EINVAL;
-	}
-
-	if (bond->dev->flags & IFF_UP) {
-		pr_err("%s: unable to update mode because interface is up.\n",
-		       bond->dev->name);
-		return -EPERM;
-	}
-
-	if (bond_has_slaves(bond)) {
-		pr_err("%s: unable to update mode because bond has slaves.\n",
-			bond->dev->name);
-		return -EPERM;
-	}
-
-	if (BOND_NO_USES_ARP(mode) && bond->params.arp_interval) {
+	if (BOND_NO_USES_ARP(newval->value) && bond->params.arp_interval) {
 		pr_info("%s: %s mode is incompatible with arp monitoring, start mii monitoring\n",
-			bond->dev->name, bond_mode_tbl[mode].modename);
+			bond->dev->name, newval->string);
 		/* disable arp monitoring */
 		bond->params.arp_interval = 0;
 		/* set miimon to default value */
@@ -324,7 +328,8 @@ int bond_option_mode_set(struct bonding *bond, int mode)
 
 	/* don't cache arp_validate between modes */
 	bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
-	bond->params.mode = mode;
+	bond->params.mode = newval->value;
+
 	return 0;
 }
 
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index e20f2eb..11e6c06 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -38,6 +38,7 @@ enum {
 
 /* Option IDs, their bit positions correspond to their IDs */
 enum {
+	BOND_OPT_MODE,
 	BOND_OPT_LAST
 };
 
@@ -97,4 +98,6 @@ static inline void __bond_opt_init(struct bond_opt_value *optval,
 }
 #define bond_opt_initval(optval, value) __bond_opt_init(optval, NULL, value)
 #define bond_opt_initstr(optval, str) __bond_opt_init(optval, str, ULLONG_MAX)
+
+int bond_option_mode_set(struct bonding *bond, struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 011f163..10ebd22 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -264,37 +264,24 @@ static ssize_t bonding_show_mode(struct device *d,
 				 struct device_attribute *attr, char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-			bond_mode_tbl[bond->params.mode].modename,
-			bond->params.mode);
+	val = bond_opt_get_val(BOND_OPT_MODE, bond->params.mode);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.mode);
 }
 
 static ssize_t bonding_store_mode(struct device *d,
 				  struct device_attribute *attr,
 				  const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	new_value = bond_parse_parm(buf, bond_mode_tbl);
-	if (new_value < 0)  {
-		pr_err("%s: Ignoring invalid mode value %.*s.\n",
-		       bond->dev->name, (int)strlen(buf) - 1, buf);
-		return -EINVAL;
-	}
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_mode_set(bond, new_value);
-	if (!ret) {
-		pr_info("%s: setting mode to %s (%d).\n",
-			bond->dev->name, bond_mode_tbl[new_value].modename,
-			new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_MODE, (char *)buf);
+	if (!ret)
 		ret = count;
-	}
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(mode, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 92ee557..3ff4df8 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -449,7 +449,6 @@ void bond_setup(struct net_device *bond_dev);
 unsigned int bond_get_num_tx_queues(void);
 int bond_netlink_init(void);
 void bond_netlink_fini(void);
-int bond_option_mode_set(struct bonding *bond, int mode);
 int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
 int bond_option_miimon_set(struct bonding *bond, int miimon);
 int bond_option_updelay_set(struct bonding *bond, int updelay);
@@ -560,7 +559,6 @@ static inline int bond_get_targets_ip(__be32 *targets, __be32 ip)
 /* exported from bond_main.c */
 extern int bond_net_id;
 extern const struct bond_parm_tbl bond_lacp_tbl[];
-extern const struct bond_parm_tbl bond_mode_tbl[];
 extern const struct bond_parm_tbl xmit_hashtype_tbl[];
 extern const struct bond_parm_tbl arp_validate_tbl[];
 extern const struct bond_parm_tbl arp_all_targets_tbl[];
-- 
1.8.4.2

[PATCH net-next 03/25] bonding: convert packets_per_slave to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so packets_per_slave would use the
new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  3 ++-
 drivers/net/bonding/bond_netlink.c |  4 ++--
 drivers/net/bonding/bond_options.c | 46 ++++++++++++++++++++------------------
 drivers/net/bonding/bond_options.h |  2 ++
 drivers/net/bonding/bond_sysfs.c   | 15 +++----------
 drivers/net/bonding/bonding.h      |  2 --
 6 files changed, 33 insertions(+), 39 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 25f236d..1ac55f1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4143,7 +4143,8 @@ static int bond_check_params(struct bond_params *params)
 		resend_igmp = BOND_DEFAULT_RESEND_IGMP;
 	}
 
-	if (packets_per_slave < 0 || packets_per_slave > USHRT_MAX) {
+	bond_opt_initval(&newval, packets_per_slave);
+	if (!bond_opt_parse(bond_opt_get(BOND_OPT_PACKETS_PER_SLAVE), &newval)) {
 		pr_warn("Warning: packets_per_slave (%d) should be between 0 and %u resetting to 1\n",
 			packets_per_slave, USHRT_MAX);
 		packets_per_slave = 1;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 097cfdd..554097b 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -288,8 +288,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int packets_per_slave =
 			nla_get_u32(data[IFLA_BOND_PACKETS_PER_SLAVE]);
 
-		err = bond_option_packets_per_slave_set(bond,
-							packets_per_slave);
+		bond_opt_initval(&newval, packets_per_slave);
+		err = __bond_opt_set(bond, BOND_OPT_PACKETS_PER_SLAVE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 0cbad5a..ce003ca 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -31,6 +31,12 @@ static struct bond_opt_value bond_mode_tbl[] = {
 	{ NULL,            -1,                     0},
 };
 
+static struct bond_opt_value bond_pps_tbl[] = {
+	{ "default", 1,         BOND_VALFLAG_DEFAULT},
+	{ "maxval",  USHRT_MAX, BOND_VALFLAG_MAX},
+	{ NULL,      -1,        0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -40,6 +46,14 @@ static struct bond_option bond_opts[] = {
 		.values = bond_mode_tbl,
 		.set = bond_option_mode_set
 	},
+	[BOND_OPT_PACKETS_PER_SLAVE] = {
+		.id = BOND_OPT_PACKETS_PER_SLAVE,
+		.name = "packets_per_slave",
+		.desc = "Packets to send per slave in RR mode",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_ROUNDROBIN)),
+		.values = bond_pps_tbl,
+		.set = bond_option_pps_set
+	},
 	{ }
 };
 
@@ -935,28 +949,6 @@ int bond_option_lp_interval_set(struct bonding *bond, int lp_interval)
 	return 0;
 }
 
-int bond_option_packets_per_slave_set(struct bonding *bond,
-				      int packets_per_slave)
-{
-	if (packets_per_slave < 0 || packets_per_slave > USHRT_MAX) {
-		pr_err("%s: packets_per_slave must be between 0 and %u\n",
-		       bond->dev->name, USHRT_MAX);
-		return -EINVAL;
-	}
-
-	if (bond->params.mode != BOND_MODE_ROUNDROBIN)
-		pr_warn("%s: Warning: packets_per_slave has effect only in balance-rr mode\n",
-			bond->dev->name);
-
-	if (packets_per_slave > 1)
-		bond->params.packets_per_slave =
-			reciprocal_value(packets_per_slave);
-	else
-		bond->params.packets_per_slave = packets_per_slave;
-
-	return 0;
-}
-
 int bond_option_lacp_rate_set(struct bonding *bond, int lacp_rate)
 {
 	if (bond_parm_tbl_lookup(lacp_rate, bond_lacp_tbl) < 0) {
@@ -1007,3 +999,13 @@ int bond_option_ad_select_set(struct bonding *bond, int ad_select)
 
 	return 0;
 }
+
+int bond_option_pps_set(struct bonding *bond, struct bond_opt_value *newval)
+{
+	if (newval->value > 1)
+		bond->params.packets_per_slave = reciprocal_value(newval->value);
+	else
+		bond->params.packets_per_slave = newval->value;
+
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 11e6c06..f859076 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -39,6 +39,7 @@ enum {
 /* Option IDs, their bit positions correspond to their IDs */
 enum {
 	BOND_OPT_MODE,
+	BOND_OPT_PACKETS_PER_SLAVE,
 	BOND_OPT_LAST
 };
 
@@ -100,4 +101,5 @@ static inline void __bond_opt_init(struct bond_opt_value *optval,
 #define bond_opt_initstr(optval, str) __bond_opt_init(optval, str, ULLONG_MAX)
 
 int bond_option_mode_set(struct bonding *bond, struct bond_opt_value *newval);
+int bond_option_pps_set(struct bonding *bond, struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 10ebd22..fbf5174 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1373,22 +1373,13 @@ static ssize_t bonding_store_packets_per_slave(struct device *d,
 					       const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no packets_per_slave value specified.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_packets_per_slave_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_PACKETS_PER_SLAVE,
+				   (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 3ff4df8..feecc4e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -473,8 +473,6 @@ int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
 int bond_option_min_links_set(struct bonding *bond, int min_links);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
-int bond_option_packets_per_slave_set(struct bonding *bond,
-				      int packets_per_slave);
 int bond_option_lacp_rate_set(struct bonding *bond, int lacp_rate);
 int bond_option_ad_select_set(struct bonding *bond, int ad_select);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
-- 
1.8.4.2

[PATCH net-next 04/25] bonding: convert xmit_hash_policy to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so xmit_hash_policy would use the
new bonding option API. Also fix some trivial/style errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 18 +++++-------------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 32 +++++++++++++++++++++-----------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_procfs.c  |  6 ++++--
 drivers/net/bonding/bond_sysfs.c   | 23 ++++++-----------------
 drivers/net/bonding/bonding.h      |  2 --
 7 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1ac55f1..283afa1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -215,15 +215,6 @@ const struct bond_parm_tbl bond_lacp_tbl[] = {
 {	NULL,		-1},
 };
 
-const struct bond_parm_tbl xmit_hashtype_tbl[] = {
-{	"layer2",		BOND_XMIT_POLICY_LAYER2},
-{	"layer3+4",		BOND_XMIT_POLICY_LAYER34},
-{	"layer2+3",		BOND_XMIT_POLICY_LAYER23},
-{	"encap2+3",		BOND_XMIT_POLICY_ENCAP23},
-{	"encap3+4",		BOND_XMIT_POLICY_ENCAP34},
-{	NULL,			-1},
-};
-
 const struct bond_parm_tbl arp_all_targets_tbl[] = {
 {	"any",			BOND_ARP_TARGETS_ANY},
 {	"all",			BOND_ARP_TARGETS_ALL},
@@ -4037,14 +4028,15 @@ static int bond_check_params(struct bond_params *params)
 			pr_info("xmit_hash_policy param is irrelevant in mode %s\n",
 			       bond_mode_name(bond_mode));
 		} else {
-			xmit_hashtype = bond_parse_parm(xmit_hash_policy,
-							xmit_hashtype_tbl);
-			if (xmit_hashtype == -1) {
+			bond_opt_initstr(&newval, xmit_hash_policy);
+			valptr = bond_opt_parse(bond_opt_get(BOND_OPT_XMIT_HASH),
+						&newval);
+			if (!valptr) {
 				pr_err("Error: Invalid xmit_hash_policy \"%s\"\n",
-				       xmit_hash_policy == NULL ? "NULL" :
 				       xmit_hash_policy);
 				return -EINVAL;
 			}
+			xmit_hashtype = valptr->value;
 		}
 	}
 
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 554097b..bf3e8ea 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -239,7 +239,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int xmit_hash_policy =
 			nla_get_u8(data[IFLA_BOND_XMIT_HASH_POLICY]);
 
-		err = bond_option_xmit_hash_policy_set(bond, xmit_hash_policy);
+		bond_opt_initval(&newval, xmit_hash_policy);
+		err = __bond_opt_set(bond, BOND_OPT_XMIT_HASH, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index ce003ca..ae5beba 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -37,6 +37,15 @@ static struct bond_opt_value bond_pps_tbl[] = {
 	{ NULL,      -1,        0},
 };
 
+static struct bond_opt_value bond_xmit_hashtype_tbl[] = {
+	{ "layer2",   BOND_XMIT_POLICY_LAYER2, BOND_VALFLAG_DEFAULT},
+	{ "layer3+4", BOND_XMIT_POLICY_LAYER34, 0},
+	{ "layer2+3", BOND_XMIT_POLICY_LAYER23, 0},
+	{ "encap2+3", BOND_XMIT_POLICY_ENCAP23, 0},
+	{ "encap3+4", BOND_XMIT_POLICY_ENCAP34, 0},
+	{ NULL,       -1,                       0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -54,6 +63,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_pps_tbl,
 		.set = bond_option_pps_set
 	},
+	[BOND_OPT_XMIT_HASH] = {
+		.id = BOND_OPT_XMIT_HASH,
+		.name = "xmit_hash_policy",
+		.desc = "balance-xor and 802.3ad hashing method",
+		.values = bond_xmit_hashtype_tbl,
+		.set = bond_option_xmit_hash_policy_set
+	},
 	{ }
 };
 
@@ -861,18 +877,12 @@ int bond_option_fail_over_mac_set(struct bonding *bond, int fail_over_mac)
 	return 0;
 }
 
-int bond_option_xmit_hash_policy_set(struct bonding *bond, int xmit_hash_policy)
+int bond_option_xmit_hash_policy_set(struct bonding *bond,
+				     struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(xmit_hash_policy, xmit_hashtype_tbl) < 0) {
-		pr_err("%s: Ignoring invalid xmit_hash_policy value %d.\n",
-		       bond->dev->name, xmit_hash_policy);
-		return -EINVAL;
-	}
-
-	bond->params.xmit_policy = xmit_hash_policy;
-	pr_info("%s: setting xmit hash policy to %s (%d).\n",
-		bond->dev->name,
-		xmit_hashtype_tbl[xmit_hash_policy].modename, xmit_hash_policy);
+	pr_info("%s: setting xmit hash policy to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.xmit_policy = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index f859076..2578f1b 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -40,6 +40,7 @@ enum {
 enum {
 	BOND_OPT_MODE,
 	BOND_OPT_PACKETS_PER_SLAVE,
+	BOND_OPT_XMIT_HASH,
 	BOND_OPT_LAST
 };
 
@@ -102,4 +103,6 @@ static inline void __bond_opt_init(struct bond_opt_value *optval,
 
 int bond_option_mode_set(struct bonding *bond, struct bond_opt_value *newval);
 int bond_option_pps_set(struct bonding *bond, struct bond_opt_value *newval);
+int bond_option_xmit_hash_policy_set(struct bonding *bond,
+				     struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 8515b344..edb7c18 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -65,6 +65,7 @@ static void bond_info_seq_stop(struct seq_file *seq, void *v)
 static void bond_info_show_master(struct seq_file *seq)
 {
 	struct bonding *bond = seq->private;
+	struct bond_opt_value *optval;
 	struct slave *curr;
 	int i;
 
@@ -84,9 +85,10 @@ static void bond_info_show_master(struct seq_file *seq)
 
 	if (bond->params.mode == BOND_MODE_XOR ||
 		bond->params.mode == BOND_MODE_8023AD) {
+		optval = bond_opt_get_val(BOND_OPT_XMIT_HASH,
+					  bond->params.xmit_policy);
 		seq_printf(seq, "Transmit Hash Policy: %s (%d)\n",
-			xmit_hashtype_tbl[bond->params.xmit_policy].modename,
-			bond->params.xmit_policy);
+			   optval->string, bond->params.xmit_policy);
 	}
 
 	if (USES_PRIMARY(bond->params.mode)) {
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index fbf5174..1967038 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -295,35 +295,24 @@ static ssize_t bonding_show_xmit_hash(struct device *d,
 				      char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-		       xmit_hashtype_tbl[bond->params.xmit_policy].modename,
-		       bond->params.xmit_policy);
+	val = bond_opt_get_val(BOND_OPT_XMIT_HASH, bond->params.xmit_policy);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.xmit_policy);
 }
 
 static ssize_t bonding_store_xmit_hash(struct device *d,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	new_value = bond_parse_parm(buf, xmit_hashtype_tbl);
-	if (new_value < 0)  {
-		pr_err("%s: Ignoring invalid xmit hash policy value %.*s.\n",
-		       bond->dev->name,
-		       (int)strlen(buf) - 1, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_xmit_hash_policy_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_XMIT_HASH, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(xmit_hash_policy, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index feecc4e..62431df 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -465,8 +465,6 @@ int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     int primary_reselect);
 int bond_option_fail_over_mac_set(struct bonding *bond, int fail_over_mac);
-int bond_option_xmit_hash_policy_set(struct bonding *bond,
-				     int xmit_hash_policy);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
 int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif);
 int bond_option_all_slaves_active_set(struct bonding *bond,
-- 
1.8.4.2

[PATCH net-next 05/25] bonding: convert arp_validate to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so arp_validate would use the
new bonding option API. Also fix some trivial/style errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 27 +++++++++++--------------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 40 +++++++++++++++++++++-----------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 23 +++++++---------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 45 insertions(+), 52 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 283afa1..81948d5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -221,14 +221,6 @@ const struct bond_parm_tbl arp_all_targets_tbl[] = {
 {	NULL,			-1},
 };
 
-const struct bond_parm_tbl arp_validate_tbl[] = {
-{	"none",			BOND_ARP_VALIDATE_NONE},
-{	"active",		BOND_ARP_VALIDATE_ACTIVE},
-{	"backup",		BOND_ARP_VALIDATE_BACKUP},
-{	"all",			BOND_ARP_VALIDATE_ALL},
-{	NULL,			-1},
-};
-
 const struct bond_parm_tbl fail_over_mac_tbl[] = {
 {	"none",			BOND_FOM_NONE},
 {	"active",		BOND_FOM_ACTIVE},
@@ -4222,15 +4214,18 @@ static int bond_check_params(struct bond_params *params)
 			return -EINVAL;
 		}
 
-		arp_validate_value = bond_parse_parm(arp_validate,
-						     arp_validate_tbl);
-		if (arp_validate_value == -1) {
+		bond_opt_initstr(&newval, arp_validate);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_VALIDATE),
+					&newval);
+		if (!valptr) {
 			pr_err("Error: invalid arp_validate \"%s\"\n",
-			       arp_validate == NULL ? "NULL" : arp_validate);
+			       arp_validate);
 			return -EINVAL;
 		}
-	} else
+		arp_validate_value = valptr->value;
+	} else {
 		arp_validate_value = 0;
+	}
 
 	arp_all_targets_value = 0;
 	if (arp_all_targets) {
@@ -4247,10 +4242,10 @@ static int bond_check_params(struct bond_params *params)
 	if (miimon) {
 		pr_info("MII link monitoring set to %d ms\n", miimon);
 	} else if (arp_interval) {
+		valptr = bond_opt_get_val(BOND_OPT_ARP_VALIDATE,
+					  arp_validate_value);
 		pr_info("ARP monitoring set to %d ms, validate %s, with %d target(s):",
-			arp_interval,
-			arp_validate_tbl[arp_validate_value].modename,
-			arp_ip_count);
+			arp_interval, valptr->string, arp_ip_count);
 
 		for (i = 0; i < arp_ip_count; i++)
 			pr_info(" %s", arp_ip_target[i]);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index bf3e8ea..d5bcf29 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -194,7 +194,8 @@ static int bond_changelink(struct net_device *bond_dev,
 			return -EINVAL;
 		}
 
-		err = bond_option_arp_validate_set(bond, arp_validate);
+		bond_opt_initval(&newval, arp_validate);
+		err = __bond_opt_set(bond, BOND_OPT_ARP_VALIDATE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index ae5beba..f082b6f 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -46,6 +46,14 @@ static struct bond_opt_value bond_xmit_hashtype_tbl[] = {
 	{ NULL,       -1,                       0},
 };
 
+static struct bond_opt_value bond_arp_validate_tbl[] = {
+	{ "none",   BOND_ARP_VALIDATE_NONE,   BOND_VALFLAG_DEFAULT},
+	{ "active", BOND_ARP_VALIDATE_ACTIVE, 0},
+	{ "backup", BOND_ARP_VALIDATE_BACKUP, 0},
+	{ "all",    BOND_ARP_VALIDATE_ALL,    0},
+	{ NULL,     -1,                       0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -70,6 +78,14 @@ static struct bond_option bond_opts[] = {
 		.values = bond_xmit_hashtype_tbl,
 		.set = bond_option_xmit_hash_policy_set
 	},
+	[BOND_OPT_ARP_VALIDATE] = {
+		.id = BOND_OPT_ARP_VALIDATE,
+		.name = "arp_validate",
+		.desc = "validate src/dst of ARP probes",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_ACTIVEBACKUP)),
+		.values = bond_arp_validate_tbl,
+		.set = bond_option_arp_validate_set
+	},
 	{ }
 };
 
@@ -735,31 +751,19 @@ int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
 	return ret;
 }
 
-int bond_option_arp_validate_set(struct bonding *bond, int arp_validate)
+int bond_option_arp_validate_set(struct bonding *bond,
+				 struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(arp_validate, arp_validate_tbl) < 0) {
-		pr_err("%s: Ignoring invalid arp_validate value %d.\n",
-		       bond->dev->name, arp_validate);
-		return -EINVAL;
-	}
-
-	if (bond->params.mode != BOND_MODE_ACTIVEBACKUP) {
-		pr_err("%s: arp_validate only supported in active-backup mode.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	pr_info("%s: setting arp_validate to %s (%d).\n",
-		bond->dev->name, arp_validate_tbl[arp_validate].modename,
-		arp_validate);
+	pr_info("%s: setting arp_validate to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
 
 	if (bond->dev->flags & IFF_UP) {
-		if (!arp_validate)
+		if (!newval->value)
 			bond->recv_probe = NULL;
 		else if (bond->params.arp_interval)
 			bond->recv_probe = bond_arp_rcv;
 	}
-	bond->params.arp_validate = arp_validate;
+	bond->params.arp_validate = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 2578f1b..15c6c01 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -41,6 +41,7 @@ enum {
 	BOND_OPT_MODE,
 	BOND_OPT_PACKETS_PER_SLAVE,
 	BOND_OPT_XMIT_HASH,
+	BOND_OPT_ARP_VALIDATE,
 	BOND_OPT_LAST
 };
 
@@ -105,4 +106,6 @@ int bond_option_mode_set(struct bonding *bond, struct bond_opt_value *newval);
 int bond_option_pps_set(struct bonding *bond, struct bond_opt_value *newval);
 int bond_option_xmit_hash_policy_set(struct bonding *bond,
 				     struct bond_opt_value *newval);
+int bond_option_arp_validate_set(struct bonding *bond,
+				 struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 1967038..592bb6d 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -326,10 +326,12 @@ static ssize_t bonding_show_arp_validate(struct device *d,
 					 char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-		       arp_validate_tbl[bond->params.arp_validate].modename,
-		       bond->params.arp_validate);
+	val = bond_opt_get_val(BOND_OPT_ARP_VALIDATE,
+			       bond->params.arp_validate);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.arp_validate);
 }
 
 static ssize_t bonding_store_arp_validate(struct device *d,
@@ -337,23 +339,12 @@ static ssize_t bonding_store_arp_validate(struct device *d,
 					  const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	new_value = bond_parse_parm(buf, arp_validate_tbl);
-	if (new_value < 0) {
-		pr_err("%s: Ignoring invalid arp_validate value %s\n",
-		       bond->dev->name, buf);
-		return -EINVAL;
-	}
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_arp_validate_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ARP_VALIDATE, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
-
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 62431df..339c8cd 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -459,7 +459,6 @@ int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
 				   int count);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_arp_validate_set(struct bonding *bond, int arp_validate);
 int bond_option_arp_all_targets_set(struct bonding *bond, int arp_all_targets);
 int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
-- 
1.8.4.2

[PATCH net-next 06/25] bonding: convert arp_all_targets to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so arp_all_targets would use the
new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 16 ++++++----------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 30 ++++++++++++++++++------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 24 +++++++-----------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 81948d5..1ad837b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -215,12 +215,6 @@ const struct bond_parm_tbl bond_lacp_tbl[] = {
 {	NULL,		-1},
 };
 
-const struct bond_parm_tbl arp_all_targets_tbl[] = {
-{	"any",			BOND_ARP_TARGETS_ANY},
-{	"all",			BOND_ARP_TARGETS_ALL},
-{	NULL,			-1},
-};
-
 const struct bond_parm_tbl fail_over_mac_tbl[] = {
 {	"none",			BOND_FOM_NONE},
 {	"active",		BOND_FOM_ACTIVE},
@@ -4229,13 +4223,15 @@ static int bond_check_params(struct bond_params *params)
 
 	arp_all_targets_value = 0;
 	if (arp_all_targets) {
-		arp_all_targets_value = bond_parse_parm(arp_all_targets,
-							arp_all_targets_tbl);
-
-		if (arp_all_targets_value == -1) {
+		bond_opt_initstr(&newval, arp_all_targets);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
+					&newval);
+		if (!valptr) {
 			pr_err("Error: invalid arp_all_targets_value \"%s\"\n",
 			       arp_all_targets);
 			arp_all_targets_value = 0;
+		} else {
+			arp_all_targets_value = valptr->value;
 		}
 	}
 
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index d5bcf29..37dce38 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -203,7 +203,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int arp_all_targets =
 			nla_get_u32(data[IFLA_BOND_ARP_ALL_TARGETS]);
 
-		err = bond_option_arp_all_targets_set(bond, arp_all_targets);
+		bond_opt_initval(&newval, arp_all_targets);
+		err = __bond_opt_set(bond, BOND_OPT_ARP_ALL_TARGETS, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index f082b6f..d993639 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -54,6 +54,12 @@ static struct bond_opt_value bond_arp_validate_tbl[] = {
 	{ NULL,     -1,                       0},
 };
 
+static struct bond_opt_value bond_arp_all_targets_tbl[] = {
+	{ "any", BOND_ARP_TARGETS_ANY, BOND_VALFLAG_DEFAULT},
+	{ "all", BOND_ARP_TARGETS_ALL, 0},
+	{ NULL,  -1,                   0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -86,6 +92,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_arp_validate_tbl,
 		.set = bond_option_arp_validate_set
 	},
+	[BOND_OPT_ARP_ALL_TARGETS] = {
+		.id = BOND_OPT_ARP_ALL_TARGETS,
+		.name = "arp_all_targets",
+		.desc = "fail on any/all arp targets timeout",
+		.values = bond_arp_all_targets_tbl,
+		.set = bond_option_arp_all_targets_set
+	},
 	{ }
 };
 
@@ -768,19 +781,12 @@ int bond_option_arp_validate_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_arp_all_targets_set(struct bonding *bond, int arp_all_targets)
+int bond_option_arp_all_targets_set(struct bonding *bond,
+				    struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(arp_all_targets, arp_all_targets_tbl) < 0) {
-		pr_err("%s: Ignoring invalid arp_all_targets value %d.\n",
-		       bond->dev->name, arp_all_targets);
-		return -EINVAL;
-	}
-
-	pr_info("%s: setting arp_all_targets to %s (%d).\n",
-		bond->dev->name, arp_all_targets_tbl[arp_all_targets].modename,
-		arp_all_targets);
-
-	bond->params.arp_all_targets = arp_all_targets;
+	pr_info("%s: setting arp_all_targets to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.arp_all_targets = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 15c6c01..88f8c17 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -42,6 +42,7 @@ enum {
 	BOND_OPT_PACKETS_PER_SLAVE,
 	BOND_OPT_XMIT_HASH,
 	BOND_OPT_ARP_VALIDATE,
+	BOND_OPT_ARP_ALL_TARGETS,
 	BOND_OPT_LAST
 };
 
@@ -108,4 +109,6 @@ int bond_option_xmit_hash_policy_set(struct bonding *bond,
 				     struct bond_opt_value *newval);
 int bond_option_arp_validate_set(struct bonding *bond,
 				 struct bond_opt_value *newval);
+int bond_option_arp_all_targets_set(struct bonding *bond,
+				    struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 592bb6d..500f7df 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -358,10 +358,12 @@ static ssize_t bonding_show_arp_all_targets(struct device *d,
 					 char *buf)
 {
 	struct bonding *bond = to_bond(d);
-	int value = bond->params.arp_all_targets;
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n", arp_all_targets_tbl[value].modename,
-		       value);
+	val = bond_opt_get_val(BOND_OPT_ARP_ALL_TARGETS,
+			       bond->params.arp_all_targets);
+	return sprintf(buf, "%s %d\n",
+		       val->string, bond->params.arp_all_targets);
 }
 
 static ssize_t bonding_store_arp_all_targets(struct device *d,
@@ -369,24 +371,12 @@ static ssize_t bonding_store_arp_all_targets(struct device *d,
 					  const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	new_value = bond_parse_parm(buf, arp_all_targets_tbl);
-	if (new_value < 0) {
-		pr_err("%s: Ignoring invalid arp_all_targets value %s\n",
-		       bond->dev->name, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_arp_all_targets_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ARP_ALL_TARGETS, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
-
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 339c8cd..e240b2f 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -459,7 +459,6 @@ int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
 				   int count);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_arp_all_targets_set(struct bonding *bond, int arp_all_targets);
 int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     int primary_reselect);
-- 
1.8.4.2

[PATCH net-next 07/25] bonding: convert fail_over_mac to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so fail_over_mac would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 18 ++++++------------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 37 ++++++++++++++++++++-----------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_procfs.c  |  8 +++++---
 drivers/net/bonding/bond_sysfs.c   | 23 +++++++----------------
 drivers/net/bonding/bonding.h      |  1 -
 7 files changed, 43 insertions(+), 50 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 1ad837b..f4da76e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -215,13 +215,6 @@ const struct bond_parm_tbl bond_lacp_tbl[] = {
 {	NULL,		-1},
 };
 
-const struct bond_parm_tbl fail_over_mac_tbl[] = {
-{	"none",			BOND_FOM_NONE},
-{	"active",		BOND_FOM_ACTIVE},
-{	"follow",		BOND_FOM_FOLLOW},
-{	NULL,			-1},
-};
-
 const struct bond_parm_tbl pri_reselect_tbl[] = {
 {	"always",		BOND_PRI_RESELECT_ALWAYS},
 {	"better",		BOND_PRI_RESELECT_BETTER},
@@ -4278,14 +4271,15 @@ static int bond_check_params(struct bond_params *params)
 	}
 
 	if (fail_over_mac) {
-		fail_over_mac_value = bond_parse_parm(fail_over_mac,
-						      fail_over_mac_tbl);
-		if (fail_over_mac_value == -1) {
+		bond_opt_initstr(&newval, fail_over_mac);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_FAIL_OVER_MAC),
+					&newval);
+		if (!valptr) {
 			pr_err("Error: invalid fail_over_mac \"%s\"\n",
-			       arp_validate == NULL ? "NULL" : arp_validate);
+			       fail_over_mac);
 			return -EINVAL;
 		}
-
+		fail_over_mac_value = valptr->value;
 		if (bond_mode != BOND_MODE_ACTIVEBACKUP)
 			pr_warning("Warning: fail_over_mac only affects active-backup mode.\n");
 	} else {
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 37dce38..a1b72ac 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -233,7 +233,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int fail_over_mac =
 			nla_get_u8(data[IFLA_BOND_FAIL_OVER_MAC]);
 
-		err = bond_option_fail_over_mac_set(bond, fail_over_mac);
+		bond_opt_initval(&newval, fail_over_mac);
+		err = __bond_opt_set(bond, BOND_OPT_FAIL_OVER_MAC, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index d993639..7e082e0 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -60,6 +60,13 @@ static struct bond_opt_value bond_arp_all_targets_tbl[] = {
 	{ NULL,  -1,                   0},
 };
 
+static struct bond_opt_value bond_fail_over_mac_tbl[] = {
+	{ "none",   BOND_FOM_NONE,   BOND_VALFLAG_DEFAULT},
+	{ "active", BOND_FOM_ACTIVE, 0},
+	{ "follow", BOND_FOM_FOLLOW, 0},
+	{ NULL,     -1,              0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -99,6 +106,14 @@ static struct bond_option bond_opts[] = {
 		.values = bond_arp_all_targets_tbl,
 		.set = bond_option_arp_all_targets_set
 	},
+	[BOND_OPT_FAIL_OVER_MAC] = {
+		.id = BOND_OPT_FAIL_OVER_MAC,
+		.name = "fail_over_mac",
+		.desc = "For active-backup, do not set all slaves to the same MAC",
+		.flags = BOND_OPTFLAG_NOSLAVES,
+		.values = bond_fail_over_mac_tbl,
+		.set = bond_option_fail_over_mac_set
+	},
 	{ }
 };
 
@@ -865,24 +880,12 @@ int bond_option_primary_reselect_set(struct bonding *bond, int primary_reselect)
 	return 0;
 }
 
-int bond_option_fail_over_mac_set(struct bonding *bond, int fail_over_mac)
+int bond_option_fail_over_mac_set(struct bonding *bond,
+				  struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(fail_over_mac, fail_over_mac_tbl) < 0) {
-		pr_err("%s: Ignoring invalid fail_over_mac value %d.\n",
-		       bond->dev->name, fail_over_mac);
-		return -EINVAL;
-	}
-
-	if (bond_has_slaves(bond)) {
-		pr_err("%s: Can't alter fail_over_mac with slaves in bond.\n",
-		       bond->dev->name);
-		return -EPERM;
-	}
-
-	bond->params.fail_over_mac = fail_over_mac;
-	pr_info("%s: Setting fail_over_mac to %s (%d).\n",
-		bond->dev->name, fail_over_mac_tbl[fail_over_mac].modename,
-		fail_over_mac);
+	pr_info("%s: Setting fail_over_mac to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.fail_over_mac = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 88f8c17..e80a031 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -43,6 +43,7 @@ enum {
 	BOND_OPT_XMIT_HASH,
 	BOND_OPT_ARP_VALIDATE,
 	BOND_OPT_ARP_ALL_TARGETS,
+	BOND_OPT_FAIL_OVER_MAC,
 	BOND_OPT_LAST
 };
 
@@ -111,4 +112,6 @@ int bond_option_arp_validate_set(struct bonding *bond,
 				 struct bond_opt_value *newval);
 int bond_option_arp_all_targets_set(struct bonding *bond,
 				    struct bond_opt_value *newval);
+int bond_option_fail_over_mac_set(struct bonding *bond,
+				  struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index edb7c18..1a66310 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -77,9 +77,11 @@ static void bond_info_show_master(struct seq_file *seq)
 		   bond_mode_name(bond->params.mode));
 
 	if (bond->params.mode == BOND_MODE_ACTIVEBACKUP &&
-	    bond->params.fail_over_mac)
-		seq_printf(seq, " (fail_over_mac %s)",
-		   fail_over_mac_tbl[bond->params.fail_over_mac].modename);
+	    bond->params.fail_over_mac) {
+		optval = bond_opt_get_val(BOND_OPT_FAIL_OVER_MAC,
+					  bond->params.fail_over_mac);
+		seq_printf(seq, " (fail_over_mac %s)", optval->string);
+	}
 
 	seq_printf(seq, "\n");
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 500f7df..e300190 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -392,34 +392,25 @@ static ssize_t bonding_show_fail_over_mac(struct device *d,
 					  char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-		       fail_over_mac_tbl[bond->params.fail_over_mac].modename,
-		       bond->params.fail_over_mac);
+	val = bond_opt_get_val(BOND_OPT_FAIL_OVER_MAC,
+			       bond->params.fail_over_mac);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.fail_over_mac);
 }
 
 static ssize_t bonding_store_fail_over_mac(struct device *d,
 					   struct device_attribute *attr,
 					   const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	new_value = bond_parse_parm(buf, fail_over_mac_tbl);
-	if (new_value < 0) {
-		pr_err("%s: Ignoring invalid fail_over_mac value %s.\n",
-		       bond->dev->name, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_fail_over_mac_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_FAIL_OVER_MAC, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e240b2f..0550615 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -462,7 +462,6 @@ int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
 int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     int primary_reselect);
-int bond_option_fail_over_mac_set(struct bonding *bond, int fail_over_mac);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
 int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif);
 int bond_option_all_slaves_active_set(struct bonding *bond,
-- 
1.8.4.2

[PATCH net-next 08/25] bonding: convert arp_interval to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so arp_interval would use
the new bonding option API. The "default" definition has been removed as
it was 0.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  9 ++++-----
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 37 +++++++++++++++++++++----------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f4da76e..9de3d35 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -88,7 +88,6 @@
 
 /* monitor all links that often (in milliseconds). <=0 disables monitoring */
 #define BOND_LINK_MON_INTERV	0
-#define BOND_LINK_ARP_INTERV	0
 
 static int max_bonds	= BOND_DEFAULT_MAX_BONDS;
 static int tx_queues	= BOND_DEFAULT_TX_QUEUES;
@@ -104,7 +103,7 @@ static char *lacp_rate;
 static int min_links;
 static char *ad_select;
 static char *xmit_hash_policy;
-static int arp_interval = BOND_LINK_ARP_INTERV;
+static int arp_interval;
 static char *arp_ip_target[BOND_MAX_ARP_TARGETS];
 static char *arp_validate;
 static char *arp_all_targets;
@@ -4160,9 +4159,9 @@ static int bond_check_params(struct bond_params *params)
 	}
 
 	if (arp_interval < 0) {
-		pr_warning("Warning: arp_interval module parameter (%d) , not in range 0-%d, so it was reset to %d\n",
-			   arp_interval, INT_MAX, BOND_LINK_ARP_INTERV);
-		arp_interval = BOND_LINK_ARP_INTERV;
+		pr_warning("Warning: arp_interval module parameter (%d) , not in range 0-%d, so it was reset to 0\n",
+			   arp_interval, INT_MAX);
+		arp_interval = 0;
 	}
 
 	for (arp_ip_count = 0, i = 0;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index a1b72ac..c9264107 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -167,7 +167,8 @@ static int bond_changelink(struct net_device *bond_dev,
 			return -EINVAL;
 		}
 
-		err = bond_option_arp_interval_set(bond, arp_interval);
+		bond_opt_initval(&newval, arp_interval);
+		err = __bond_opt_set(bond, BOND_OPT_ARP_INTERVAL, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 7e082e0..c3eb617 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -67,6 +67,11 @@ static struct bond_opt_value bond_fail_over_mac_tbl[] = {
 	{ NULL,     -1,              0},
 };
 
+static struct bond_opt_value bond_intmax_tbl[] = {
+	{ "off",     0,       BOND_VALFLAG_DEFAULT},
+	{ "maxval",  INT_MAX, BOND_VALFLAG_MAX},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -114,6 +119,15 @@ static struct bond_option bond_opts[] = {
 		.values = bond_fail_over_mac_tbl,
 		.set = bond_option_fail_over_mac_set
 	},
+	[BOND_OPT_ARP_INTERVAL] = {
+		.id = BOND_OPT_ARP_INTERVAL,
+		.name = "arp_interval",
+		.desc = "arp interval in milliseconds",
+		.unsuppmodes = BIT(BOND_MODE_8023AD) | BIT(BOND_MODE_TLB) |
+			       BIT(BOND_MODE_ALB),
+		.values = bond_intmax_tbl,
+		.set = bond_option_arp_interval_set
+	},
 	{ }
 };
 
@@ -602,22 +616,13 @@ int bond_option_use_carrier_set(struct bonding *bond, int use_carrier)
 	return 0;
 }
 
-int bond_option_arp_interval_set(struct bonding *bond, int arp_interval)
+int bond_option_arp_interval_set(struct bonding *bond,
+				 struct bond_opt_value *newval)
 {
-	if (arp_interval < 0) {
-		pr_err("%s: Invalid arp_interval value %d not in range 0-%d; rejected.\n",
-		       bond->dev->name, arp_interval, INT_MAX);
-		return -EINVAL;
-	}
-	if (BOND_NO_USES_ARP(bond->params.mode)) {
-		pr_info("%s: ARP monitoring cannot be used with ALB/TLB/802.3ad. Only MII monitoring is supported on %s.\n",
-			bond->dev->name, bond->dev->name);
-		return -EINVAL;
-	}
-	pr_info("%s: Setting ARP monitoring interval to %d.\n",
-		bond->dev->name, arp_interval);
-	bond->params.arp_interval = arp_interval;
-	if (arp_interval) {
+	pr_info("%s: Setting ARP monitoring interval to %llu.\n",
+		bond->dev->name, newval->value);
+	bond->params.arp_interval = newval->value;
+	if (newval->value) {
 		if (bond->params.miimon) {
 			pr_info("%s: ARP monitoring cannot be used with MII monitoring. %s Disabling MII monitoring.\n",
 				bond->dev->name, bond->dev->name);
@@ -633,7 +638,7 @@ int bond_option_arp_interval_set(struct bonding *bond, int arp_interval)
 		 * timer will get fired off when the open function
 		 * is called.
 		 */
-		if (!arp_interval) {
+		if (!newval->value) {
 			if (bond->params.arp_validate)
 				bond->recv_probe = NULL;
 			cancel_delayed_work_sync(&bond->arp_work);
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index e80a031..f02b857 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -44,6 +44,7 @@ enum {
 	BOND_OPT_ARP_VALIDATE,
 	BOND_OPT_ARP_ALL_TARGETS,
 	BOND_OPT_FAIL_OVER_MAC,
+	BOND_OPT_ARP_INTERVAL,
 	BOND_OPT_LAST
 };
 
@@ -114,4 +115,6 @@ int bond_option_arp_all_targets_set(struct bonding *bond,
 				    struct bond_opt_value *newval);
 int bond_option_fail_over_mac_set(struct bonding *bond,
 				  struct bond_opt_value *newval);
+int bond_option_arp_interval_set(struct bonding *bond,
+				 struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index e300190..414d713 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -437,22 +437,12 @@ static ssize_t bonding_store_arp_interval(struct device *d,
 					  const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no arp_interval value specified.\n",
-		bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_arp_interval_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ARP_INTERVAL, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(arp_interval, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 0550615..af065af 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -454,7 +454,6 @@ int bond_option_miimon_set(struct bonding *bond, int miimon);
 int bond_option_updelay_set(struct bonding *bond, int updelay);
 int bond_option_downdelay_set(struct bonding *bond, int downdelay);
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
-int bond_option_arp_interval_set(struct bonding *bond, int arp_interval);
 int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
 				   int count);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
-- 
1.8.4.2

[PATCH net-next 09/25] bonding: convert arp_ip_target to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so arp_ip_target would use
the new bonding option API. This option is an exception because of
the way it's currently implemented that's why its netlink code is
a bit different from the other options to keep the functionality as
before and at the same time to have a single set function.

This patch also fixes a few stylistic errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c | 15 ++++++++----
 drivers/net/bonding/bond_options.c | 48 +++++++++++++++++++++++++++-----------
 drivers/net/bonding/bond_options.h |  4 ++++
 drivers/net/bonding/bond_sysfs.c   | 25 ++++----------------
 drivers/net/bonding/bonding.h      |  2 --
 5 files changed, 53 insertions(+), 41 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index c9264107..3aab730 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -173,16 +173,23 @@ static int bond_changelink(struct net_device *bond_dev,
 			return err;
 	}
 	if (data[IFLA_BOND_ARP_IP_TARGET]) {
-		__be32 targets[BOND_MAX_ARP_TARGETS] = { 0, };
 		struct nlattr *attr;
 		int i = 0, rem;
 
+		bond_option_arp_ip_targets_clear(bond);
 		nla_for_each_nested(attr, data[IFLA_BOND_ARP_IP_TARGET], rem) {
 			__be32 target = nla_get_be32(attr);
-			targets[i++] = target;
-		}
 
-		err = bond_option_arp_ip_targets_set(bond, targets, i);
+			bond_opt_initval(&newval, target);
+			err = __bond_opt_set(bond, BOND_OPT_ARP_TARGETS,
+					     &newval);
+			if (err)
+				break;
+			i++;
+		}
+		if (i == 0 && bond->params.arp_interval)
+			pr_warn("%s: removing last arp target with arp_interval on\n",
+				bond->dev->name);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index c3eb617..5f6b064 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -18,6 +18,7 @@
 #include <linux/rcupdate.h>
 #include <linux/reciprocal_div.h>
 #include <linux/ctype.h>
+#include <linux/inet.h>
 #include "bonding.h"
 
 static struct bond_opt_value bond_mode_tbl[] = {
@@ -128,6 +129,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_arp_interval_set
 	},
+	[BOND_OPT_ARP_TARGETS] = {
+		.id = BOND_OPT_ARP_TARGETS,
+		.name = "arp_ip_target",
+		.desc = "arp targets in n.n.n.n form",
+		.flags = BOND_OPTFLAG_RAWVAL,
+		.set = bond_option_arp_ip_targets_set
+	},
 	{ }
 };
 
@@ -758,29 +766,41 @@ int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 	return 0;
 }
 
-int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
-				   int count)
+void bond_option_arp_ip_targets_clear(struct bonding *bond)
 {
-	int i, ret = 0;
+	int i;
 
 	/* not to race with bond_arp_rcv */
 	write_lock_bh(&bond->lock);
-
-	/* clear table */
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
 		_bond_options_arp_ip_target_set(bond, i, 0, 0);
+	write_unlock_bh(&bond->lock);
+}
 
-	if (count == 0 && bond->params.arp_interval)
-		pr_warn("%s: removing last arp target with arp_interval on\n",
-			bond->dev->name);
-
-	for (i = 0; i < count; i++) {
-		ret = _bond_option_arp_ip_target_add(bond, targets[i]);
-		if (ret)
-			break;
+int bond_option_arp_ip_targets_set(struct bonding *bond,
+				   struct bond_opt_value *newval)
+{
+	int ret = -EPERM;
+	__be32 target;
+
+	if (newval->string) {
+		if (!in4_pton(newval->string+1, -1, (u8 *)&target, -1, NULL)) {
+			pr_err("%s: invalid ARP target %pI4 specified\n",
+			       bond->dev->name, &target);
+			return ret;
+		}
+		if (newval->string[0] == '+')
+			ret = bond_option_arp_ip_target_add(bond, target);
+		else if (newval->string[0] == '-')
+			ret = bond_option_arp_ip_target_rem(bond, target);
+		else
+			pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n",
+			       bond->dev->name);
+	} else {
+		target = newval->value;
+		ret = bond_option_arp_ip_target_add(bond, target);
 	}
 
-	write_unlock_bh(&bond->lock);
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index f02b857..da35148 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -45,6 +45,7 @@ enum {
 	BOND_OPT_ARP_ALL_TARGETS,
 	BOND_OPT_FAIL_OVER_MAC,
 	BOND_OPT_ARP_INTERVAL,
+	BOND_OPT_ARP_TARGETS,
 	BOND_OPT_LAST
 };
 
@@ -117,4 +118,7 @@ int bond_option_fail_over_mac_set(struct bonding *bond,
 				  struct bond_opt_value *newval);
 int bond_option_arp_interval_set(struct bonding *bond,
 				 struct bond_opt_value *newval);
+int bond_option_arp_ip_targets_set(struct bonding *bond,
+				   struct bond_opt_value *newval);
+void bond_option_arp_ip_targets_clear(struct bonding *bond);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 414d713..14cc9fe 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -455,8 +455,8 @@ static ssize_t bonding_show_arp_targets(struct device *d,
 					struct device_attribute *attr,
 					char *buf)
 {
-	int i, res = 0;
 	struct bonding *bond = to_bond(d);
+	int i, res = 0;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
 		if (bond->params.arp_targets[i])
@@ -465,6 +465,7 @@ static ssize_t bonding_show_arp_targets(struct device *d,
 	}
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
+
 	return res;
 }
 
@@ -473,30 +474,12 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 					 const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	__be32 target;
-	int ret = -EPERM;
-
-	if (!in4_pton(buf + 1, -1, (u8 *)&target, -1, NULL)) {
-		pr_err("%s: invalid ARP target %pI4 specified\n",
-		       bond->dev->name, &target);
-		return -EPERM;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	if (buf[0] == '+')
-		ret = bond_option_arp_ip_target_add(bond, target);
-	else if (buf[0] == '-')
-		ret = bond_option_arp_ip_target_rem(bond, target);
-	else
-		pr_err("no command found in arp_ip_targets file for bond %s. Use +<addr> or -<addr>.\n",
-		       bond->dev->name);
+	int ret;
 
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ARP_TARGETS, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(arp_ip_target, S_IRUGO | S_IWUSR , bonding_show_arp_targets, bonding_store_arp_targets);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index af065af..85a32e3 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -454,8 +454,6 @@ int bond_option_miimon_set(struct bonding *bond, int miimon);
 int bond_option_updelay_set(struct bonding *bond, int updelay);
 int bond_option_downdelay_set(struct bonding *bond, int downdelay);
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
-int bond_option_arp_ip_targets_set(struct bonding *bond, __be32 *targets,
-				   int count);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
 int bond_option_primary_set(struct bonding *bond, const char *primary);
-- 
1.8.4.2

[PATCH net-next 10/25] bonding: convert downdelay to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so downdelay would use
the new bonding option API. Also some trivial style fixes.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 39 +++++++++++++++++++-------------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 13 ++-----------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 3aab730..089bb1f 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -147,7 +147,8 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_DOWNDELAY]) {
 		int downdelay = nla_get_u32(data[IFLA_BOND_DOWNDELAY]);
 
-		err = bond_option_downdelay_set(bond, downdelay);
+		bond_opt_initval(&newval, downdelay);
+		err = __bond_opt_set(bond, BOND_OPT_DOWNDELAY, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 5f6b064..f3449cf 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -136,6 +136,13 @@ static struct bond_option bond_opts[] = {
 		.flags = BOND_OPTFLAG_RAWVAL,
 		.set = bond_option_arp_ip_targets_set
 	},
+	[BOND_OPT_DOWNDELAY] = {
+		.id = BOND_OPT_DOWNDELAY,
+		.name = "downdelay",
+		.desc = "Delay before considering link down, in milliseconds",
+		.values = bond_intmax_tbl,
+		.set = bond_option_downdelay_set
+	},
 	{ }
 };
 
@@ -581,31 +588,25 @@ int bond_option_updelay_set(struct bonding *bond, int updelay)
 	return 0;
 }
 
-int bond_option_downdelay_set(struct bonding *bond, int downdelay)
+int bond_option_downdelay_set(struct bonding *bond,
+			      struct bond_opt_value *newval)
 {
-	if (!(bond->params.miimon)) {
+	if (!bond->params.miimon) {
 		pr_err("%s: Unable to set down delay as MII monitoring is disabled\n",
 		       bond->dev->name);
 		return -EPERM;
 	}
-
-	if (downdelay < 0) {
-		pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
-		       bond->dev->name, downdelay, 0, INT_MAX);
-		return -EINVAL;
-	} else {
-		if ((downdelay % bond->params.miimon) != 0) {
-			pr_warn("%s: Warning: down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
-				bond->dev->name, downdelay,
-				bond->params.miimon,
-				(downdelay / bond->params.miimon) *
-				bond->params.miimon);
-		}
-		bond->params.downdelay = downdelay / bond->params.miimon;
-		pr_info("%s: Setting down delay to %d.\n",
-			bond->dev->name,
-			bond->params.downdelay * bond->params.miimon);
+	if ((newval->value % bond->params.miimon) != 0) {
+		pr_warn("%s: Warning: down delay (%llu) is not a multiple of miimon (%d), delay rounded to %llu ms\n",
+			bond->dev->name, newval->value,
+			bond->params.miimon,
+			(newval->value / bond->params.miimon) *
+			bond->params.miimon);
 	}
+	bond->params.downdelay = newval->value / bond->params.miimon;
+	pr_info("%s: Setting down delay to %d.\n",
+		bond->dev->name,
+		bond->params.downdelay * bond->params.miimon);
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index da35148..5a3f405 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -46,6 +46,7 @@ enum {
 	BOND_OPT_FAIL_OVER_MAC,
 	BOND_OPT_ARP_INTERVAL,
 	BOND_OPT_ARP_TARGETS,
+	BOND_OPT_DOWNDELAY,
 	BOND_OPT_LAST
 };
 
@@ -121,4 +122,6 @@ int bond_option_arp_interval_set(struct bonding *bond,
 int bond_option_arp_ip_targets_set(struct bonding *bond,
 				   struct bond_opt_value *newval);
 void bond_option_arp_ip_targets_clear(struct bonding *bond);
+int bond_option_downdelay_set(struct bonding *bond,
+			      struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 14cc9fe..8eab0b9 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -502,22 +502,13 @@ static ssize_t bonding_store_downdelay(struct device *d,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no down delay value specified.\n", bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_downdelay_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_DOWNDELAY, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(downdelay, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 85a32e3..93854cc 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -452,7 +452,6 @@ void bond_netlink_fini(void);
 int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
 int bond_option_miimon_set(struct bonding *bond, int miimon);
 int bond_option_updelay_set(struct bonding *bond, int updelay);
-int bond_option_downdelay_set(struct bonding *bond, int downdelay);
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-- 
1.8.4.2

[PATCH net-next 11/25] bonding: convert updelay to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so updelay would use
the new bonding option API. Also some trivial style fixes.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 39 +++++++++++++++++++-------------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 27 insertions(+), 33 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 089bb1f..a40a2ec 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -140,7 +140,8 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_UPDELAY]) {
 		int updelay = nla_get_u32(data[IFLA_BOND_UPDELAY]);
 
-		err = bond_option_updelay_set(bond, updelay);
+		bond_opt_initval(&newval, updelay);
+		err = __bond_opt_set(bond, BOND_OPT_UPDELAY, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index f3449cf..abf6a0d 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -143,6 +143,14 @@ static struct bond_option bond_opts[] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_downdelay_set
 	},
+	[BOND_OPT_UPDELAY] = {
+		.id = BOND_OPT_UPDELAY,
+		.name = "updelay",
+		.desc = "Delay before considering link up, in milliseconds",
+		.values = bond_intmax_tbl,
+		.set = bond_option_updelay_set
+	},
+
 	{ }
 };
 
@@ -559,31 +567,24 @@ int bond_option_miimon_set(struct bonding *bond, int miimon)
 	return 0;
 }
 
-int bond_option_updelay_set(struct bonding *bond, int updelay)
+int bond_option_updelay_set(struct bonding *bond, struct bond_opt_value *newval)
 {
-	if (!(bond->params.miimon)) {
+	if (!bond->params.miimon) {
 		pr_err("%s: Unable to set up delay as MII monitoring is disabled\n",
 		       bond->dev->name);
 		return -EPERM;
 	}
-
-	if (updelay < 0) {
-		pr_err("%s: Invalid up delay value %d not in range %d-%d; rejected.\n",
-		       bond->dev->name, updelay, 0, INT_MAX);
-		return -EINVAL;
-	} else {
-		if ((updelay % bond->params.miimon) != 0) {
-			pr_warn("%s: Warning: up delay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
-				bond->dev->name, updelay,
-				bond->params.miimon,
-				(updelay / bond->params.miimon) *
-				bond->params.miimon);
-		}
-		bond->params.updelay = updelay / bond->params.miimon;
-		pr_info("%s: Setting up delay to %d.\n",
-			bond->dev->name,
-			bond->params.updelay * bond->params.miimon);
+	if ((newval->value % bond->params.miimon) != 0) {
+		pr_warn("%s: Warning: up delay (%llu) is not a multiple of miimon (%d), updelay rounded to %llu ms\n",
+			bond->dev->name, newval->value,
+			bond->params.miimon,
+			(newval->value / bond->params.miimon) *
+			bond->params.miimon);
 	}
+	bond->params.updelay = newval->value / bond->params.miimon;
+	pr_info("%s: Setting up delay to %d.\n",
+		bond->dev->name,
+		bond->params.updelay * bond->params.miimon);
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 5a3f405..594046c 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -47,6 +47,7 @@ enum {
 	BOND_OPT_ARP_INTERVAL,
 	BOND_OPT_ARP_TARGETS,
 	BOND_OPT_DOWNDELAY,
+	BOND_OPT_UPDELAY,
 	BOND_OPT_LAST
 };
 
@@ -124,4 +125,6 @@ int bond_option_arp_ip_targets_set(struct bonding *bond,
 void bond_option_arp_ip_targets_clear(struct bonding *bond);
 int bond_option_downdelay_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
+int bond_option_updelay_set(struct bonding *bond,
+			    struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 8eab0b9..7acc1bb 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -528,23 +528,13 @@ static ssize_t bonding_store_updelay(struct device *d,
 				     struct device_attribute *attr,
 				     const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no up delay value specified.\n",
-		bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_updelay_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_UPDELAY, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(updelay, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 93854cc..e8d4e11 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -451,7 +451,6 @@ int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
 int bond_option_miimon_set(struct bonding *bond, int miimon);
-int bond_option_updelay_set(struct bonding *bond, int updelay);
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-- 
1.8.4.2

[PATCH net-next 12/25] bonding: convert lacp_rate to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so lacp_rate would use
the new bonding option API. Also some trivial/style error fixes.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 15 ++++++-------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 44 +++++++++++++++++---------------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 22 ++++++-------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 37 insertions(+), 51 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9de3d35..dd0778c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -208,12 +208,6 @@ static int bond_mode	= BOND_MODE_ROUNDROBIN;
 static int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
 static int lacp_fast;
 
-const struct bond_parm_tbl bond_lacp_tbl[] = {
-{	"slow",		AD_LACP_SLOW},
-{	"fast",		AD_LACP_FAST},
-{	NULL,		-1},
-};
-
 const struct bond_parm_tbl pri_reselect_tbl[] = {
 {	"always",		BOND_PRI_RESELECT_ALWAYS},
 {	"better",		BOND_PRI_RESELECT_BETTER},
@@ -4023,12 +4017,15 @@ static int bond_check_params(struct bond_params *params)
 			pr_info("lacp_rate param is irrelevant in mode %s\n",
 				bond_mode_name(bond_mode));
 		} else {
-			lacp_fast = bond_parse_parm(lacp_rate, bond_lacp_tbl);
-			if (lacp_fast == -1) {
+			bond_opt_initstr(&newval, lacp_rate);
+			valptr = bond_opt_parse(bond_opt_get(BOND_OPT_LACP_RATE),
+						&newval);
+			if (!valptr) {
 				pr_err("Error: Invalid lacp rate \"%s\"\n",
-				       lacp_rate == NULL ? "NULL" : lacp_rate);
+				       lacp_rate);
 				return -EINVAL;
 			}
+			lacp_fast = valptr->value;
 		}
 	}
 
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index a40a2ec..7737a91 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -311,7 +311,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int lacp_rate =
 			nla_get_u8(data[IFLA_BOND_AD_LACP_RATE]);
 
-		err = bond_option_lacp_rate_set(bond, lacp_rate);
+		bond_opt_initval(&newval, lacp_rate);
+		err = __bond_opt_set(bond, BOND_OPT_LACP_RATE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index abf6a0d..01cbc7f 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -73,6 +73,12 @@ static struct bond_opt_value bond_intmax_tbl[] = {
 	{ "maxval",  INT_MAX, BOND_VALFLAG_MAX},
 };
 
+static struct bond_opt_value bond_lacp_rate_tbl[] = {
+	{ "slow", AD_LACP_SLOW, 0},
+	{ "fast", AD_LACP_FAST, 0},
+	{ NULL,   -1,           0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -150,7 +156,15 @@ static struct bond_option bond_opts[] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_updelay_set
 	},
-
+	[BOND_OPT_LACP_RATE] = {
+		.id = BOND_OPT_LACP_RATE,
+		.name = "lacp_rate",
+		.desc = "LACPDU tx rate to request from 802.3ad partner",
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.values = bond_lacp_rate_tbl,
+		.set = bond_option_lacp_rate_set
+	},
 	{ }
 };
 
@@ -999,31 +1013,13 @@ int bond_option_lp_interval_set(struct bonding *bond, int lp_interval)
 	return 0;
 }
 
-int bond_option_lacp_rate_set(struct bonding *bond, int lacp_rate)
+int bond_option_lacp_rate_set(struct bonding *bond,
+			      struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(lacp_rate, bond_lacp_tbl) < 0) {
-		pr_err("%s: Ignoring invalid LACP rate value %d.\n",
-		       bond->dev->name, lacp_rate);
-		return -EINVAL;
-	}
-
-	if (bond->dev->flags & IFF_UP) {
-		pr_err("%s: Unable to update LACP rate because interface is up.\n",
-		       bond->dev->name);
-		return -EPERM;
-	}
-
-	if (bond->params.mode != BOND_MODE_8023AD) {
-		pr_err("%s: Unable to update LACP rate because bond is not in 802.3ad mode.\n",
-		       bond->dev->name);
-		return -EPERM;
-	}
-
-	bond->params.lacp_fast = lacp_rate;
+	pr_info("%s: Setting LACP rate to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.lacp_fast = newval->value;
 	bond_3ad_update_lacp_rate(bond);
-	pr_info("%s: Setting LACP rate to %s (%d).\n",
-		bond->dev->name, bond_lacp_tbl[lacp_rate].modename,
-		lacp_rate);
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 594046c..7ee1a78 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -48,6 +48,7 @@ enum {
 	BOND_OPT_ARP_TARGETS,
 	BOND_OPT_DOWNDELAY,
 	BOND_OPT_UPDELAY,
+	BOND_OPT_LACP_RATE,
 	BOND_OPT_LAST
 };
 
@@ -127,4 +128,6 @@ int bond_option_downdelay_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
 int bond_option_updelay_set(struct bonding *bond,
 			    struct bond_opt_value *newval);
+int bond_option_lacp_rate_set(struct bonding *bond,
+			      struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 7acc1bb..98288dc 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -549,10 +549,11 @@ static ssize_t bonding_show_lacp(struct device *d,
 				 char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-		bond_lacp_tbl[bond->params.lacp_fast].modename,
-		bond->params.lacp_fast);
+	val = bond_opt_get_val(BOND_OPT_LACP_RATE, bond->params.lacp_fast);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.lacp_fast);
 }
 
 static ssize_t bonding_store_lacp(struct device *d,
@@ -560,23 +561,12 @@ static ssize_t bonding_store_lacp(struct device *d,
 				  const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	new_value = bond_parse_parm(buf, bond_lacp_tbl);
-	if (new_value < 0) {
-		pr_err("%s: Ignoring invalid LACP rate value %.*s.\n",
-		       bond->dev->name, (int)strlen(buf) - 1, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_lacp_rate_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_LACP_RATE, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e8d4e11..05f4aa0 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -463,7 +463,6 @@ int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
 int bond_option_min_links_set(struct bonding *bond, int min_links);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
-int bond_option_lacp_rate_set(struct bonding *bond, int lacp_rate);
 int bond_option_ad_select_set(struct bonding *bond, int ad_select);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
-- 
1.8.4.2

[PATCH net-next 13/25] bonding: convert min_links to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so min_links would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 16 ++++++++++++----
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 +-------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 7737a91..7eee1a0 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -286,7 +286,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int min_links =
 			nla_get_u32(data[IFLA_BOND_MIN_LINKS]);
 
-		err = bond_option_min_links_set(bond, min_links);
+		bond_opt_initval(&newval, min_links);
+		err = __bond_opt_set(bond, BOND_OPT_MINLINKS, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 01cbc7f..2c392e8 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -165,6 +165,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_lacp_rate_tbl,
 		.set = bond_option_lacp_rate_set
 	},
+	[BOND_OPT_MINLINKS] = {
+		.id = BOND_OPT_MINLINKS,
+		.name = "min_links",
+		.desc = "Minimum number of available links before turning on carrier",
+		.values = bond_intmax_tbl,
+		.set = bond_option_min_links_set
+	},
 	{ }
 };
 
@@ -991,11 +998,12 @@ int bond_option_all_slaves_active_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_min_links_set(struct bonding *bond, int min_links)
+int bond_option_min_links_set(struct bonding *bond,
+			      struct bond_opt_value *newval)
 {
-	pr_info("%s: Setting min links value to %u\n",
-		bond->dev->name, min_links);
-	bond->params.min_links = min_links;
+	pr_info("%s: Setting min links value to %llu\n",
+		bond->dev->name, newval->value);
+	bond->params.min_links = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 7ee1a78..cc8edef 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -49,6 +49,7 @@ enum {
 	BOND_OPT_DOWNDELAY,
 	BOND_OPT_UPDELAY,
 	BOND_OPT_LACP_RATE,
+	BOND_OPT_MINLINKS,
 	BOND_OPT_LAST
 };
 
@@ -130,4 +131,6 @@ int bond_option_updelay_set(struct bonding *bond,
 			    struct bond_opt_value *newval);
 int bond_option_lacp_rate_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
+int bond_option_min_links_set(struct bonding *bond,
+			      struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 98288dc..81e0581 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -587,23 +587,11 @@ static ssize_t bonding_store_min_links(struct device *d,
 {
 	struct bonding *bond = to_bond(d);
 	int ret;
-	unsigned int new_value;
 
-	ret = kstrtouint(buf, 0, &new_value);
-	if (ret < 0) {
-		pr_err("%s: Ignoring invalid min links value %s.\n",
-		       bond->dev->name, buf);
-		return ret;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_min_links_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_MINLINKS, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(min_links, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 05f4aa0..cb18727 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -461,7 +461,6 @@ int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
 int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif);
 int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
-int bond_option_min_links_set(struct bonding *bond, int min_links);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
 int bond_option_ad_select_set(struct bonding *bond, int ad_select);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
-- 
1.8.4.2

[PATCH net-next 14/25] bonding: convert ad_select to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so ad_select would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 21 +++++++--------------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 37 ++++++++++++++++++++-----------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_procfs.c  |  4 +++-
 drivers/net/bonding/bond_sysfs.c   | 22 ++++++----------------
 drivers/net/bonding/bonding.h      |  1 -
 7 files changed, 41 insertions(+), 50 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index dd0778c..7335adf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -215,13 +215,6 @@ const struct bond_parm_tbl pri_reselect_tbl[] = {
 {	NULL,			-1},
 };
 
-struct bond_parm_tbl ad_select_tbl[] = {
-{	"stable",	BOND_AD_STABLE},
-{	"bandwidth",	BOND_AD_BANDWIDTH},
-{	"count",	BOND_AD_COUNT},
-{	NULL,		-1},
-};
-
 /*-------------------------- Forward declarations ---------------------------*/
 
 static int bond_init(struct net_device *bond_dev);
@@ -4030,16 +4023,16 @@ static int bond_check_params(struct bond_params *params)
 	}
 
 	if (ad_select) {
-		params->ad_select = bond_parse_parm(ad_select, ad_select_tbl);
-		if (params->ad_select == -1) {
-			pr_err("Error: Invalid ad_select \"%s\"\n",
-			       ad_select == NULL ? "NULL" : ad_select);
+		bond_opt_initstr(&newval, lacp_rate);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_AD_SELECT),
+					&newval);
+		if (!valptr) {
+			pr_err("Error: Invalid ad_select \"%s\"\n", ad_select);
 			return -EINVAL;
 		}
-
-		if (bond_mode != BOND_MODE_8023AD) {
+		params->ad_select = valptr->value;
+		if (bond_mode != BOND_MODE_8023AD)
 			pr_warning("ad_select param only affects 802.3ad mode\n");
-		}
 	} else {
 		params->ad_select = BOND_AD_STABLE;
 	}
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 7eee1a0..f7be658 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -321,7 +321,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int ad_select =
 			nla_get_u8(data[IFLA_BOND_AD_SELECT]);
 
-		err = bond_option_ad_select_set(bond, ad_select);
+		bond_opt_initval(&newval, ad_select);
+		err = __bond_opt_set(bond, BOND_OPT_AD_SELECT, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 2c392e8..3deb9f3 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -79,6 +79,13 @@ static struct bond_opt_value bond_lacp_rate_tbl[] = {
 	{ NULL,   -1,           0},
 };
 
+static struct bond_opt_value bond_ad_select_tbl[] = {
+	{ "stable",    BOND_AD_STABLE,    BOND_VALFLAG_DEFAULT},
+	{ "bandwidth", BOND_AD_BANDWIDTH, 0},
+	{ "count",     BOND_AD_COUNT,     0},
+	{ NULL,        -1,                0},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -172,6 +179,14 @@ static struct bond_option bond_opts[] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_min_links_set
 	},
+	[BOND_OPT_AD_SELECT] = {
+		.id = BOND_OPT_AD_SELECT,
+		.name = "ad_select",
+		.desc = "803.ad aggregation selection logic",
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.values = bond_ad_select_tbl,
+		.set = bond_option_ad_select_set
+	},
 	{ }
 };
 
@@ -1032,24 +1047,12 @@ int bond_option_lacp_rate_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_ad_select_set(struct bonding *bond, int ad_select)
+int bond_option_ad_select_set(struct bonding *bond,
+			      struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(ad_select, ad_select_tbl) < 0) {
-		pr_err("%s: Ignoring invalid ad_select value %d.\n",
-		       bond->dev->name, ad_select);
-		return -EINVAL;
-	}
-
-	if (bond->dev->flags & IFF_UP) {
-		pr_err("%s: Unable to update ad_select because interface is up.\n",
-		       bond->dev->name);
-		return -EPERM;
-	}
-
-	bond->params.ad_select = ad_select;
-	pr_info("%s: Setting ad_select to %s (%d).\n",
-		bond->dev->name, ad_select_tbl[ad_select].modename,
-		ad_select);
+	pr_info("%s: Setting ad_select to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.ad_select = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index cc8edef..53df176 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -50,6 +50,7 @@ enum {
 	BOND_OPT_UPDELAY,
 	BOND_OPT_LACP_RATE,
 	BOND_OPT_MINLINKS,
+	BOND_OPT_AD_SELECT,
 	BOND_OPT_LAST
 };
 
@@ -133,4 +134,6 @@ int bond_option_lacp_rate_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
 int bond_option_min_links_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
+int bond_option_ad_select_set(struct bonding *bond,
+			      struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 1a66310..d28c3d7 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -140,8 +140,10 @@ static void bond_info_show_master(struct seq_file *seq)
 		seq_printf(seq, "LACP rate: %s\n",
 			   (bond->params.lacp_fast) ? "fast" : "slow");
 		seq_printf(seq, "Min links: %d\n", bond->params.min_links);
+		optval = bond_opt_get_val(BOND_OPT_AD_SELECT,
+					  bond->params.ad_select);
 		seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
-			   ad_select_tbl[bond->params.ad_select].modename);
+			   optval->string);
 
 		if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 			seq_printf(seq, "bond %s has no active aggregator\n",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 81e0581..e8081e8 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -602,10 +602,11 @@ static ssize_t bonding_show_ad_select(struct device *d,
 				      char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
 
-	return sprintf(buf, "%s %d\n",
-		ad_select_tbl[bond->params.ad_select].modename,
-		bond->params.ad_select);
+	val = bond_opt_get_val(BOND_OPT_AD_SELECT, bond->params.ad_select);
+
+	return sprintf(buf, "%s %d\n", val->string, bond->params.ad_select);
 }
 
 
@@ -613,24 +614,13 @@ static ssize_t bonding_store_ad_select(struct device *d,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	new_value = bond_parse_parm(buf, ad_select_tbl);
-	if (new_value < 0) {
-		pr_err("%s: Ignoring invalid ad_select value %.*s.\n",
-		       bond->dev->name, (int)strlen(buf) - 1, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_ad_select_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_AD_SELECT, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(ad_select, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index cb18727..923f612 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -462,7 +462,6 @@ int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif);
 int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
-int bond_option_ad_select_set(struct bonding *bond, int ad_select);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
 const char *bond_slave_link_status(s8 link);
-- 
1.8.4.2

[PATCH net-next 15/25] bonding: convert num_peer_notif to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so num_peer_notif would use
the new bonding option API.
When the auto-sysfs generation is done an alias should be added for
this option as there're currently 2 entries in sysfs for it.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 20 ++++++++++++++++++--
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 +-------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index f7be658..cbf1bb0 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -269,7 +269,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int num_peer_notif =
 			nla_get_u8(data[IFLA_BOND_NUM_PEER_NOTIF]);
 
-		err = bond_option_num_peer_notif_set(bond, num_peer_notif);
+		bond_opt_initval(&newval, num_peer_notif);
+		err = __bond_opt_set(bond, BOND_OPT_NUM_PEER_NOTIF, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 3deb9f3..4a10a93 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -86,6 +86,13 @@ static struct bond_opt_value bond_ad_select_tbl[] = {
 	{ NULL,        -1,                0},
 };
 
+static struct bond_opt_value bond_num_peer_notif_tbl[] = {
+	{ "off",     0,   0},
+	{ "maxval",  255, BOND_VALFLAG_MAX},
+	{ "default", 1,   BOND_VALFLAG_DEFAULT},
+	{ NULL,      -1,  0}
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -187,6 +194,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_ad_select_tbl,
 		.set = bond_option_ad_select_set
 	},
+	[BOND_OPT_NUM_PEER_NOTIF] = {
+		.id = BOND_OPT_NUM_PEER_NOTIF,
+		.name = "num_unsol_na",
+		.desc = "Number of peer notifications to send on failover event",
+		.values = bond_num_peer_notif_tbl,
+		.set = bond_option_num_peer_notif_set
+	},
 	{ }
 };
 
@@ -978,9 +992,11 @@ int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp)
 	return 0;
 }
 
-int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif)
+int bond_option_num_peer_notif_set(struct bonding *bond,
+				   struct bond_opt_value *newval)
 {
-	bond->params.num_peer_notif = num_peer_notif;
+	bond->params.num_peer_notif = newval->value;
+
 	return 0;
 }
 
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 53df176..014a359 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -51,6 +51,7 @@ enum {
 	BOND_OPT_LACP_RATE,
 	BOND_OPT_MINLINKS,
 	BOND_OPT_AD_SELECT,
+	BOND_OPT_NUM_PEER_NOTIF,
 	BOND_OPT_LAST
 };
 
@@ -136,4 +137,6 @@ int bond_option_min_links_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
 int bond_option_ad_select_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
+int bond_option_num_peer_notif_set(struct bonding *bond,
+				   struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index e8081e8..3c0ce6f 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -642,24 +642,12 @@ static ssize_t bonding_store_num_peer_notif(struct device *d,
 					    const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	u8 new_value;
 	int ret;
 
-	ret = kstrtou8(buf, 10, &new_value);
-	if (ret) {
-		pr_err("%s: invalid value %s specified.\n",
-		       bond->dev->name, buf);
-		return ret;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_num_peer_notif_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_NUM_PEER_NOTIF, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(num_grat_arp, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 923f612..52ec73f 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -458,7 +458,6 @@ int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     int primary_reselect);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
-int bond_option_num_peer_notif_set(struct bonding *bond, int num_peer_notif);
 int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
-- 
1.8.4.2

[PATCH net-next 16/25] bonding: convert miimon to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so miimon would use
the new bonding option API. The "default" definition has been removed as
it was 0.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  9 ++++-----
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 25 ++++++++++++++-----------
 drivers/net/bonding/bond_options.h |  2 ++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7335adf..5df75fc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -87,12 +87,11 @@
 /*---------------------------- Module parameters ----------------------------*/
 
 /* monitor all links that often (in milliseconds). <=0 disables monitoring */
-#define BOND_LINK_MON_INTERV	0
 
 static int max_bonds	= BOND_DEFAULT_MAX_BONDS;
 static int tx_queues	= BOND_DEFAULT_TX_QUEUES;
 static int num_peer_notif = 1;
-static int miimon	= BOND_LINK_MON_INTERV;
+static int miimon;
 static int updelay;
 static int downdelay;
 static int use_carrier	= 1;
@@ -4044,9 +4043,9 @@ static int bond_check_params(struct bond_params *params)
 	}
 
 	if (miimon < 0) {
-		pr_warning("Warning: miimon module parameter (%d), not in range 0-%d, so it was reset to %d\n",
-			   miimon, INT_MAX, BOND_LINK_MON_INTERV);
-		miimon = BOND_LINK_MON_INTERV;
+		pr_warning("Warning: miimon module parameter (%d), not in range 0-%d, so it was reset to 0\n",
+			   miimon, INT_MAX);
+		miimon = 0;
 	}
 
 	if (updelay < 0) {
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index cbf1bb0..d6d4aa7 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -133,7 +133,8 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_MIIMON]) {
 		miimon = nla_get_u32(data[IFLA_BOND_MIIMON]);
 
-		err = bond_option_miimon_set(bond, miimon);
+		bond_opt_initval(&newval, miimon);
+		err = __bond_opt_set(bond, BOND_OPT_MIIMON, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 4a10a93..b48c9b6 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -201,6 +201,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_num_peer_notif_tbl,
 		.set = bond_option_num_peer_notif_set
 	},
+	[BOND_OPT_MIIMON] = {
+		.id = BOND_OPT_MIIMON,
+		.name = "miimon",
+		.desc = "Link check interval in milliseconds",
+		.values = bond_intmax_tbl,
+		.set = bond_option_miimon_set
+	},
 	{ }
 };
 
@@ -576,16 +583,11 @@ int bond_option_active_slave_set(struct bonding *bond,
 	return ret;
 }
 
-int bond_option_miimon_set(struct bonding *bond, int miimon)
+int bond_option_miimon_set(struct bonding *bond, struct bond_opt_value *newval)
 {
-	if (miimon < 0) {
-		pr_err("%s: Invalid miimon value %d not in range %d-%d; rejected.\n",
-		       bond->dev->name, miimon, 0, INT_MAX);
-		return -EINVAL;
-	}
-	pr_info("%s: Setting MII monitoring interval to %d.\n",
-		bond->dev->name, miimon);
-	bond->params.miimon = miimon;
+	pr_info("%s: Setting MII monitoring interval to %llu.\n",
+		bond->dev->name, newval->value);
+	bond->params.miimon = newval->value;
 	if (bond->params.updelay)
 		pr_info("%s: Note: Updating updelay (to %d) since it is a multiple of the miimon value.\n",
 			bond->dev->name,
@@ -594,7 +596,7 @@ int bond_option_miimon_set(struct bonding *bond, int miimon)
 		pr_info("%s: Note: Updating downdelay (to %d) since it is a multiple of the miimon value.\n",
 			bond->dev->name,
 			bond->params.downdelay * bond->params.miimon);
-	if (miimon && bond->params.arp_interval) {
+	if (newval->value && bond->params.arp_interval) {
 		pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
 			bond->dev->name);
 		bond->params.arp_interval = 0;
@@ -607,13 +609,14 @@ int bond_option_miimon_set(struct bonding *bond, int miimon)
 		 * timer will get fired off when the open function
 		 * is called.
 		 */
-		if (!miimon) {
+		if (!newval->value) {
 			cancel_delayed_work_sync(&bond->mii_work);
 		} else {
 			cancel_delayed_work_sync(&bond->arp_work);
 			queue_delayed_work(bond->wq, &bond->mii_work, 0);
 		}
 	}
+
 	return 0;
 }
 
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 014a359..90f00c6 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -52,6 +52,7 @@ enum {
 	BOND_OPT_MINLINKS,
 	BOND_OPT_AD_SELECT,
 	BOND_OPT_NUM_PEER_NOTIF,
+	BOND_OPT_MIIMON,
 	BOND_OPT_LAST
 };
 
@@ -139,4 +140,5 @@ int bond_option_ad_select_set(struct bonding *bond,
 			      struct bond_opt_value *newval);
 int bond_option_num_peer_notif_set(struct bonding *bond,
 				   struct bond_opt_value *newval);
+int bond_option_miimon_set(struct bonding *bond, struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 3c0ce6f..029e323 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -674,23 +674,13 @@ static ssize_t bonding_store_miimon(struct device *d,
 				    struct device_attribute *attr,
 				    const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no miimon value specified.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_miimon_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_MIIMON, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(miimon, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 52ec73f..56d5510 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -450,7 +450,6 @@ unsigned int bond_get_num_tx_queues(void);
 int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
-int bond_option_miimon_set(struct bonding *bond, int miimon);
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-- 
1.8.4.2

[PATCH net-next 17/25] bonding: convert primary to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so primary would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 26 ++++++++++++++++----------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 11 +----------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index d6d4aa7..55f9aad 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -228,7 +228,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		if (dev)
 			primary = dev->name;
 
-		err = bond_option_primary_set(bond, primary);
+		bond_opt_initstr(&newval, primary);
+		err = __bond_opt_set(bond, BOND_OPT_PRIMARY, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index b48c9b6..e713abf 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -208,6 +208,16 @@ static struct bond_option bond_opts[] = {
 		.values = bond_intmax_tbl,
 		.set = bond_option_miimon_set
 	},
+	[BOND_OPT_PRIMARY] = {
+		.id = BOND_OPT_PRIMARY,
+		.name = "primary",
+		.desc = "Primary network device to use",
+		.flags = BOND_OPTFLAG_RAWVAL,
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_ACTIVEBACKUP) |
+						BIT(BOND_MODE_TLB) |
+						BIT(BOND_MODE_ALB)),
+		.set = bond_option_primary_set
+	},
 	{ }
 };
 
@@ -886,23 +896,19 @@ int bond_option_arp_all_targets_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_primary_set(struct bonding *bond, const char *primary)
+int bond_option_primary_set(struct bonding *bond, struct bond_opt_value *newval)
 {
+	char *p, *primary = newval->string;
 	struct list_head *iter;
 	struct slave *slave;
-	int err = 0;
 
 	block_netpoll_tx();
 	read_lock(&bond->lock);
 	write_lock_bh(&bond->curr_slave_lock);
 
-	if (!USES_PRIMARY(bond->params.mode)) {
-		pr_err("%s: Unable to set primary slave; %s is in mode %d\n",
-		       bond->dev->name, bond->dev->name, bond->params.mode);
-		err = -EINVAL;
-		goto out;
-	}
-
+	p = strchr(primary, '\n');
+	if (p)
+		*p = '\0';
 	/* check to see if we are clearing primary */
 	if (!strlen(primary)) {
 		pr_info("%s: Setting primary slave to None.\n",
@@ -935,7 +941,7 @@ out:
 	read_unlock(&bond->lock);
 	unblock_netpoll_tx();
 
-	return err;
+	return 0;
 }
 
 int bond_option_primary_reselect_set(struct bonding *bond, int primary_reselect)
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 90f00c6..6ff83103 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -53,6 +53,7 @@ enum {
 	BOND_OPT_AD_SELECT,
 	BOND_OPT_NUM_PEER_NOTIF,
 	BOND_OPT_MIIMON,
+	BOND_OPT_PRIMARY,
 	BOND_OPT_LAST
 };
 
@@ -141,4 +142,6 @@ int bond_option_ad_select_set(struct bonding *bond,
 int bond_option_num_peer_notif_set(struct bonding *bond,
 				   struct bond_opt_value *newval);
 int bond_option_miimon_set(struct bonding *bond, struct bond_opt_value *newval);
+int bond_option_primary_set(struct bonding *bond,
+			    struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 029e323..0d5248e 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -711,21 +711,12 @@ static ssize_t bonding_store_primary(struct device *d,
 				     const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	char ifname[IFNAMSIZ];
 	int ret;
 
-	sscanf(buf, "%15s", ifname); /* IFNAMSIZ */
-	if (ifname[0] == '\n')
-		ifname[0] = '\0';
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_primary_set(bond, ifname);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_PRIMARY, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(primary, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 56d5510..9afbe0a 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -453,7 +453,6 @@ int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_primary_set(struct bonding *bond, const char *primary);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     int primary_reselect);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
-- 
1.8.4.2

[PATCH net-next 18/25] bonding: convert primary_reselect to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so primary_reselect would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    | 18 ++++++------------
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 30 +++++++++++++++++++-----------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_procfs.c  |  7 +++++--
 drivers/net/bonding/bond_sysfs.c   | 24 ++++++++----------------
 drivers/net/bonding/bonding.h      |  2 --
 7 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5df75fc..fc2e2ef 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -207,13 +207,6 @@ static int bond_mode	= BOND_MODE_ROUNDROBIN;
 static int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
 static int lacp_fast;
 
-const struct bond_parm_tbl pri_reselect_tbl[] = {
-{	"always",		BOND_PRI_RESELECT_ALWAYS},
-{	"better",		BOND_PRI_RESELECT_BETTER},
-{	"failure",		BOND_PRI_RESELECT_FAILURE},
-{	NULL,			-1},
-};
-
 /*-------------------------- Forward declarations ---------------------------*/
 
 static int bond_init(struct net_device *bond_dev);
@@ -4246,14 +4239,15 @@ static int bond_check_params(struct bond_params *params)
 	}
 
 	if (primary && primary_reselect) {
-		primary_reselect_value = bond_parse_parm(primary_reselect,
-							 pri_reselect_tbl);
-		if (primary_reselect_value == -1) {
+		bond_opt_initstr(&newval, primary_reselect);
+		valptr = bond_opt_parse(bond_opt_get(BOND_OPT_PRIMARY_RESELECT),
+					&newval);
+		if (!valptr) {
 			pr_err("Error: Invalid primary_reselect \"%s\"\n",
-			       primary_reselect ==
-					NULL ? "NULL" : primary_reselect);
+			       primary_reselect);
 			return -EINVAL;
 		}
+		primary_reselect_value = valptr->value;
 	} else {
 		primary_reselect_value = BOND_PRI_RESELECT_ALWAYS;
 	}
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 55f9aad..367974b 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -237,7 +237,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int primary_reselect =
 			nla_get_u8(data[IFLA_BOND_PRIMARY_RESELECT]);
 
-		err = bond_option_primary_reselect_set(bond, primary_reselect);
+		bond_opt_initval(&newval, primary_reselect);
+		err = __bond_opt_set(bond, BOND_OPT_PRIMARY_RESELECT, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index e713abf..ebcf5aa 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -93,6 +93,13 @@ static struct bond_opt_value bond_num_peer_notif_tbl[] = {
 	{ NULL,      -1,  0}
 };
 
+static struct bond_opt_value bond_primary_reselect_tbl[] = {
+	{ "always",  BOND_PRI_RESELECT_ALWAYS,  BOND_VALFLAG_DEFAULT},
+	{ "better",  BOND_PRI_RESELECT_BETTER,  0},
+	{ "failure", BOND_PRI_RESELECT_FAILURE, 0},
+	{ NULL,      -1},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -218,6 +225,13 @@ static struct bond_option bond_opts[] = {
 						BIT(BOND_MODE_ALB)),
 		.set = bond_option_primary_set
 	},
+	[BOND_OPT_PRIMARY_RESELECT] = {
+		.id = BOND_OPT_PRIMARY_RESELECT,
+		.name = "primary_reselect",
+		.desc = "Reselect primary slave once it comes up",
+		.values = bond_primary_reselect_tbl,
+		.set = bond_option_primary_reselect_set
+	},
 	{ }
 };
 
@@ -944,18 +958,12 @@ out:
 	return 0;
 }
 
-int bond_option_primary_reselect_set(struct bonding *bond, int primary_reselect)
+int bond_option_primary_reselect_set(struct bonding *bond,
+				     struct bond_opt_value *newval)
 {
-	if (bond_parm_tbl_lookup(primary_reselect, pri_reselect_tbl) < 0) {
-		pr_err("%s: Ignoring invalid primary_reselect value %d.\n",
-		       bond->dev->name, primary_reselect);
-		return -EINVAL;
-	}
-
-	bond->params.primary_reselect = primary_reselect;
-	pr_info("%s: setting primary_reselect to %s (%d).\n",
-		bond->dev->name, pri_reselect_tbl[primary_reselect].modename,
-		primary_reselect);
+	pr_info("%s: setting primary_reselect to %s (%llu).\n",
+		bond->dev->name, newval->string, newval->value);
+	bond->params.primary_reselect = newval->value;
 
 	block_netpoll_tx();
 	write_lock_bh(&bond->curr_slave_lock);
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 6ff83103..1de315c 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -54,6 +54,7 @@ enum {
 	BOND_OPT_NUM_PEER_NOTIF,
 	BOND_OPT_MIIMON,
 	BOND_OPT_PRIMARY,
+	BOND_OPT_PRIMARY_RESELECT,
 	BOND_OPT_LAST
 };
 
@@ -144,4 +145,6 @@ int bond_option_num_peer_notif_set(struct bonding *bond,
 int bond_option_miimon_set(struct bonding *bond, struct bond_opt_value *newval);
 int bond_option_primary_set(struct bonding *bond,
 			    struct bond_opt_value *newval);
+int bond_option_primary_reselect_set(struct bonding *bond,
+				     struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index d28c3d7..3ac20e7 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -97,9 +97,12 @@ static void bond_info_show_master(struct seq_file *seq)
 		seq_printf(seq, "Primary Slave: %s",
 			   (bond->primary_slave) ?
 			   bond->primary_slave->dev->name : "None");
-		if (bond->primary_slave)
+		if (bond->primary_slave) {
+			optval = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT,
+						  bond->params.primary_reselect);
 			seq_printf(seq, " (primary_reselect %s)",
-		   pri_reselect_tbl[bond->params.primary_reselect].modename);
+				   optval->string);
+		}
 
 		seq_printf(seq, "\nCurrently Active Slave: %s\n",
 			   (curr) ? curr->dev->name : "None");
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 0d5248e..6056ac9 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -730,35 +730,27 @@ static ssize_t bonding_show_primary_reselect(struct device *d,
 					     char *buf)
 {
 	struct bonding *bond = to_bond(d);
+	struct bond_opt_value *val;
+
+	val = bond_opt_get_val(BOND_OPT_PRIMARY_RESELECT,
+			       bond->params.primary_reselect);
 
 	return sprintf(buf, "%s %d\n",
-		       pri_reselect_tbl[bond->params.primary_reselect].modename,
-		       bond->params.primary_reselect);
+		       val->string, bond->params.primary_reselect);
 }
 
 static ssize_t bonding_store_primary_reselect(struct device *d,
 					      struct device_attribute *attr,
 					      const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	new_value = bond_parse_parm(buf, pri_reselect_tbl);
-	if (new_value < 0)  {
-		pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
-		       bond->dev->name,
-		       (int) strlen(buf) - 1, buf);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_primary_reselect_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_PRIMARY_RESELECT,
+				   (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(primary_reselect, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 9afbe0a..532c4ce 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -453,8 +453,6 @@ int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_
 int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_primary_reselect_set(struct bonding *bond,
-				     int primary_reselect);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
 int bond_option_all_slaves_active_set(struct bonding *bond,
 				      int all_slaves_active);
-- 
1.8.4.2

[PATCH net-next 19/25] bonding: convert use_carrier to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so use_carrier would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 27 ++++++++++++++++++---------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 367974b..98c643d 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -157,7 +157,8 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_USE_CARRIER]) {
 		int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]);
 
-		err = bond_option_use_carrier_set(bond, use_carrier);
+		bond_opt_initval(&newval, use_carrier);
+		err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index ebcf5aa..87cc534 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -100,6 +100,12 @@ static struct bond_opt_value bond_primary_reselect_tbl[] = {
 	{ NULL,      -1},
 };
 
+static struct bond_opt_value bond_use_carrier_tbl[] = {
+	{ "off", 0,  0},
+	{ "on",  1,  BOND_VALFLAG_DEFAULT},
+	{ NULL,  -1, 0}
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -232,6 +238,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_primary_reselect_tbl,
 		.set = bond_option_primary_reselect_set
 	},
+	[BOND_OPT_USE_CARRIER] = {
+		.id = BOND_OPT_USE_CARRIER,
+		.name = "use_carrier",
+		.desc = "Use netif_carrier_ok (vs MII ioctls) in miimon",
+		.values = bond_use_carrier_tbl,
+		.set = bond_option_use_carrier_set
+	},
 	{ }
 };
 
@@ -689,16 +702,12 @@ int bond_option_downdelay_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_use_carrier_set(struct bonding *bond, int use_carrier)
+int bond_option_use_carrier_set(struct bonding *bond,
+				struct bond_opt_value *newval)
 {
-	if ((use_carrier == 0) || (use_carrier == 1)) {
-		bond->params.use_carrier = use_carrier;
-		pr_info("%s: Setting use_carrier to %d.\n",
-			bond->dev->name, use_carrier);
-	} else {
-		pr_info("%s: Ignoring invalid use_carrier value %d.\n",
-			bond->dev->name, use_carrier);
-	}
+	pr_info("%s: Setting use_carrier to %llu.\n",
+		bond->dev->name, newval->value);
+	bond->params.use_carrier = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 1de315c..229f5c1 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -55,6 +55,7 @@ enum {
 	BOND_OPT_MIIMON,
 	BOND_OPT_PRIMARY,
 	BOND_OPT_PRIMARY_RESELECT,
+	BOND_OPT_USE_CARRIER,
 	BOND_OPT_LAST
 };
 
@@ -147,4 +148,6 @@ int bond_option_primary_set(struct bonding *bond,
 			    struct bond_opt_value *newval);
 int bond_option_primary_reselect_set(struct bonding *bond,
 				     struct bond_opt_value *newval);
+int bond_option_use_carrier_set(struct bonding *bond,
+				struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 6056ac9..079dcbc 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -773,23 +773,13 @@ static ssize_t bonding_store_carrier(struct device *d,
 				     struct device_attribute *attr,
 				     const char *buf, size_t count)
 {
-	int new_value, ret;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no use_carrier value specified.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_use_carrier_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_USE_CARRIER, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(use_carrier, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 532c4ce..06fc79e 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -450,7 +450,6 @@ unsigned int bond_get_num_tx_queues(void);
 int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
-int bond_option_use_carrier_set(struct bonding *bond, int use_carrier);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
-- 
1.8.4.2

[PATCH net-next 20/25] bonding: convert active_slave to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so active_slave would use
the new bonding option API. Also some trivial/style fixes.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  4 +++-
 drivers/net/bonding/bond_netlink.c |  9 +++++----
 drivers/net/bonding/bond_options.c | 30 +++++++++++++++++++++++-------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 24 ++----------------------
 drivers/net/bonding/bonding.h      |  1 -
 6 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fc2e2ef..02ae65f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3124,6 +3124,7 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 	struct ifslave k_sinfo;
 	struct ifslave __user *u_sinfo = NULL;
 	struct mii_ioctl_data *mii = NULL;
+	struct bond_opt_value newval;
 	struct net *net;
 	int res = 0;
 
@@ -3219,7 +3220,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 		break;
 	case BOND_CHANGE_ACTIVE_OLD:
 	case SIOCBONDCHANGEACTIVE:
-		res = bond_option_active_slave_set(bond, slave_dev);
+		bond_opt_initstr(&newval, slave_dev->name);
+		res = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
 		break;
 	default:
 		res = -EOPNOTSUPP;
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 98c643d..bd26d1c 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -117,16 +117,17 @@ static int bond_changelink(struct net_device *bond_dev,
 	if (data[IFLA_BOND_ACTIVE_SLAVE]) {
 		int ifindex = nla_get_u32(data[IFLA_BOND_ACTIVE_SLAVE]);
 		struct net_device *slave_dev;
+		char *active_slave = "";
 
-		if (ifindex == 0) {
-			slave_dev = NULL;
-		} else {
+		if (ifindex != 0) {
 			slave_dev = __dev_get_by_index(dev_net(bond_dev),
 						       ifindex);
 			if (!slave_dev)
 				return -ENODEV;
+			active_slave = slave_dev->name;
 		}
-		err = bond_option_active_slave_set(bond, slave_dev);
+		bond_opt_initstr(&newval, active_slave);
+		err = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 87cc534..2cba72a 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -245,6 +245,16 @@ static struct bond_option bond_opts[] = {
 		.values = bond_use_carrier_tbl,
 		.set = bond_option_use_carrier_set
 	},
+	[BOND_OPT_ACTIVE_SLAVE] = {
+		.id = BOND_OPT_ACTIVE_SLAVE,
+		.name = "active_slave",
+		.desc = "Currently active slave",
+		.flags = BOND_OPTFLAG_RAWVAL,
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_ACTIVEBACKUP) |
+						BIT(BOND_MODE_TLB) |
+						BIT(BOND_MODE_ALB)),
+		.set = bond_option_active_slave_set
+	},
 	{ }
 };
 
@@ -557,10 +567,21 @@ struct net_device *bond_option_active_slave_get(struct bonding *bond)
 }
 
 int bond_option_active_slave_set(struct bonding *bond,
-				 struct net_device *slave_dev)
+				 struct bond_opt_value *newval)
 {
+	char ifname[IFNAMSIZ] = { 0, };
+	struct net_device *slave_dev;
 	int ret = 0;
 
+	sscanf(newval->string, "%15s", ifname); /* IFNAMSIZ */
+	if (!strlen(ifname) || newval->string[0] == '\n') {
+		slave_dev = NULL;
+	} else {
+		slave_dev = __dev_get_by_name(dev_net(bond->dev), ifname);
+		if (!slave_dev)
+			return -ENODEV;
+	}
+
 	if (slave_dev) {
 		if (!netif_is_bond_slave(slave_dev)) {
 			pr_err("Device %s is not bonding slave.\n",
@@ -575,12 +596,6 @@ int bond_option_active_slave_set(struct bonding *bond,
 		}
 	}
 
-	if (!USES_PRIMARY(bond->params.mode)) {
-		pr_err("%s: Unable to change active slave; %s is in mode %d\n",
-		       bond->dev->name, bond->dev->name, bond->params.mode);
-		return -EINVAL;
-	}
-
 	block_netpoll_tx();
 	write_lock_bh(&bond->curr_slave_lock);
 
@@ -617,6 +632,7 @@ int bond_option_active_slave_set(struct bonding *bond,
 
 	write_unlock_bh(&bond->curr_slave_lock);
 	unblock_netpoll_tx();
+
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 229f5c1..1f486a7 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -56,6 +56,7 @@ enum {
 	BOND_OPT_PRIMARY,
 	BOND_OPT_PRIMARY_RESELECT,
 	BOND_OPT_USE_CARRIER,
+	BOND_OPT_ACTIVE_SLAVE,
 	BOND_OPT_LAST
 };
 
@@ -150,4 +151,6 @@ int bond_option_primary_reselect_set(struct bonding *bond,
 				     struct bond_opt_value *newval);
 int bond_option_use_carrier_set(struct bonding *bond,
 				struct bond_opt_value *newval);
+int bond_option_active_slave_set(struct bonding *bond,
+				 struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 079dcbc..0728965 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -810,34 +810,14 @@ static ssize_t bonding_store_active_slave(struct device *d,
 					  struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
-	int ret;
 	struct bonding *bond = to_bond(d);
-	char ifname[IFNAMSIZ];
-	struct net_device *dev;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	sscanf(buf, "%15s", ifname); /* IFNAMSIZ */
-	if (!strlen(ifname) || buf[0] == '\n') {
-		dev = NULL;
-	} else {
-		dev = __dev_get_by_name(dev_net(bond->dev), ifname);
-		if (!dev) {
-			ret = -ENODEV;
-			goto out;
-		}
-	}
+	int ret;
 
-	ret = bond_option_active_slave_set(bond, dev);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ACTIVE_SLAVE, (char *)buf);
 	if (!ret)
 		ret = count;
 
- out:
-	rtnl_unlock();
-
 	return ret;
-
 }
 static DEVICE_ATTR(active_slave, S_IRUGO | S_IWUSR,
 		   bonding_show_active_slave, bonding_store_active_slave);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 06fc79e..014605d 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -449,7 +449,6 @@ void bond_setup(struct net_device *bond_dev);
 unsigned int bond_get_num_tx_queues(void);
 int bond_netlink_init(void);
 void bond_netlink_fini(void);
-int bond_option_active_slave_set(struct bonding *bond, struct net_device *slave_dev);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
-- 
1.8.4.2

[PATCH net-next 21/25] bonding: convert queue_id to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so queue_id would use
the new bonding option API. Also move it to its own set function in
bond_options.c and fix some style errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_options.c | 70 ++++++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_options.h |  3 ++
 drivers/net/bonding/bond_sysfs.c   | 65 +++--------------------------------
 3 files changed, 77 insertions(+), 61 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 2cba72a..3fc2730 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -255,6 +255,13 @@ static struct bond_option bond_opts[] = {
 						BIT(BOND_MODE_ALB)),
 		.set = bond_option_active_slave_set
 	},
+	[BOND_OPT_QUEUE_ID] = {
+		.id = BOND_OPT_QUEUE_ID,
+		.name = "queue_id",
+		.desc = "Set queue id of a slave",
+		.flags = BOND_OPTFLAG_RAWVAL,
+		.set = bond_option_queue_id_set
+	},
 	{ }
 };
 
@@ -1124,3 +1131,66 @@ int bond_option_pps_set(struct bonding *bond, struct bond_opt_value *newval)
 
 	return 0;
 }
+
+int bond_option_queue_id_set(struct bonding *bond,
+			     struct bond_opt_value *newval)
+{
+	struct slave *slave, *update_slave;
+	struct net_device *sdev;
+	struct list_head *iter;
+	char *delim;
+	int ret = 0;
+	u16 qid;
+
+	/* delim will point to queue id if successful */
+	delim = strchr(newval->string, ':');
+	if (!delim)
+		goto err_no_cmd;
+
+	/* Terminate string that points to device name and bump it
+	 * up one, so we can read the queue id there.
+	 */
+	*delim = '\0';
+	if (sscanf(++delim, "%hd\n", &qid) != 1)
+		goto err_no_cmd;
+
+	/* Check buffer length, valid ifname and queue id */
+	if (strlen(newval->string) > IFNAMSIZ ||
+	    !dev_valid_name(newval->string) ||
+	    qid > bond->dev->real_num_tx_queues)
+		goto err_no_cmd;
+
+	/* Get the pointer to that interface if it exists */
+	sdev = __dev_get_by_name(dev_net(bond->dev), newval->string);
+	if (!sdev)
+		goto err_no_cmd;
+
+	/* Search for thes slave and check for duplicate qids */
+	update_slave = NULL;
+	bond_for_each_slave(bond, slave, iter) {
+		if (sdev == slave->dev)
+			/* We don't need to check the matching
+			 * slave for dups, since we're overwriting it
+			 */
+			update_slave = slave;
+		else if (qid && qid == slave->queue_id) {
+			goto err_no_cmd;
+		}
+	}
+
+	if (!update_slave)
+		goto err_no_cmd;
+
+	/* Actually set the qids for the slave */
+	update_slave->queue_id = qid;
+
+out:
+	return ret;
+
+err_no_cmd:
+	pr_info("invalid input for queue_id set for %s.\n",
+		bond->dev->name);
+	ret = -EPERM;
+	goto out;
+
+}
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 1f486a7..396d504 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -57,6 +57,7 @@ enum {
 	BOND_OPT_PRIMARY_RESELECT,
 	BOND_OPT_USE_CARRIER,
 	BOND_OPT_ACTIVE_SLAVE,
+	BOND_OPT_QUEUE_ID,
 	BOND_OPT_LAST
 };
 
@@ -153,4 +154,6 @@ int bond_option_use_carrier_set(struct bonding *bond,
 				struct bond_opt_value *newval);
 int bond_option_active_slave_set(struct bonding *bond,
 				 struct bond_opt_value *newval);
+int bond_option_queue_id_set(struct bonding *bond,
+			     struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 0728965..2da37bd 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -986,72 +986,15 @@ static ssize_t bonding_store_queue_id(struct device *d,
 				      struct device_attribute *attr,
 				      const char *buffer, size_t count)
 {
-	struct slave *slave, *update_slave;
 	struct bonding *bond = to_bond(d);
-	struct list_head *iter;
-	u16 qid;
-	int ret = count;
-	char *delim;
-	struct net_device *sdev = NULL;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	/* delim will point to queue id if successful */
-	delim = strchr(buffer, ':');
-	if (!delim)
-		goto err_no_cmd;
-
-	/*
-	 * Terminate string that points to device name and bump it
-	 * up one, so we can read the queue id there.
-	 */
-	*delim = '\0';
-	if (sscanf(++delim, "%hd\n", &qid) != 1)
-		goto err_no_cmd;
-
-	/* Check buffer length, valid ifname and queue id */
-	if (strlen(buffer) > IFNAMSIZ ||
-	    !dev_valid_name(buffer) ||
-	    qid > bond->dev->real_num_tx_queues)
-		goto err_no_cmd;
-
-	/* Get the pointer to that interface if it exists */
-	sdev = __dev_get_by_name(dev_net(bond->dev), buffer);
-	if (!sdev)
-		goto err_no_cmd;
-
-	/* Search for thes slave and check for duplicate qids */
-	update_slave = NULL;
-	bond_for_each_slave(bond, slave, iter) {
-		if (sdev == slave->dev)
-			/*
-			 * We don't need to check the matching
-			 * slave for dups, since we're overwriting it
-			 */
-			update_slave = slave;
-		else if (qid && qid == slave->queue_id) {
-			goto err_no_cmd;
-		}
-	}
-
-	if (!update_slave)
-		goto err_no_cmd;
+	int ret;
 
-	/* Actually set the qids for the slave */
-	update_slave->queue_id = qid;
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_QUEUE_ID, (char *)buffer);
+	if (!ret)
+		ret = count;
 
-out:
-	rtnl_unlock();
 	return ret;
-
-err_no_cmd:
-	pr_info("invalid input for queue_id set for %s.\n",
-		bond->dev->name);
-	ret = -EPERM;
-	goto out;
 }
-
 static DEVICE_ATTR(queue_id, S_IRUGO | S_IWUSR, bonding_show_queue_id,
 		   bonding_store_queue_id);
 
-- 
1.8.4.2

[PATCH net-next 22/25] bonding: convert all_slaves_active to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so all_slaves_active would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  4 ++--
 drivers/net/bonding/bond_options.c | 29 +++++++++++++++++------------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 15 +++------------
 drivers/net/bonding/bonding.h      |  2 --
 5 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index bd26d1c..d0f22ff 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -283,8 +283,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int all_slaves_active =
 			nla_get_u8(data[IFLA_BOND_ALL_SLAVES_ACTIVE]);
 
-		err = bond_option_all_slaves_active_set(bond,
-							all_slaves_active);
+		bond_opt_initval(&newval, all_slaves_active);
+		err = __bond_opt_set(bond, BOND_OPT_ALL_SLAVES_ACTIVE, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 3fc2730..8309d51 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -106,6 +106,12 @@ static struct bond_opt_value bond_use_carrier_tbl[] = {
 	{ NULL,  -1, 0}
 };
 
+static struct bond_opt_value bond_all_slaves_active_tbl[] = {
+	{ "off", 0,  BOND_VALFLAG_DEFAULT},
+	{ "on",  1,  0},
+	{ NULL,  -1, 0}
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -262,6 +268,13 @@ static struct bond_option bond_opts[] = {
 		.flags = BOND_OPTFLAG_RAWVAL,
 		.set = bond_option_queue_id_set
 	},
+	[BOND_OPT_ALL_SLAVES_ACTIVE] = {
+		.id = BOND_OPT_ALL_SLAVES_ACTIVE,
+		.name = "all_slaves_active",
+		.desc = "Keep all frames received on an interface by setting active flag for all slaves",
+		.values = bond_all_slaves_active_tbl,
+		.set = bond_option_all_slaves_active_set
+	},
 	{ }
 };
 
@@ -1050,25 +1063,17 @@ int bond_option_num_peer_notif_set(struct bonding *bond,
 }
 
 int bond_option_all_slaves_active_set(struct bonding *bond,
-				      int all_slaves_active)
+				      struct bond_opt_value *newval)
 {
 	struct list_head *iter;
 	struct slave *slave;
 
-	if (all_slaves_active == bond->params.all_slaves_active)
+	if (newval->value == bond->params.all_slaves_active)
 		return 0;
-
-	if ((all_slaves_active == 0) || (all_slaves_active == 1)) {
-		bond->params.all_slaves_active = all_slaves_active;
-	} else {
-		pr_info("%s: Ignoring invalid all_slaves_active value %d.\n",
-			bond->dev->name, all_slaves_active);
-		return -EINVAL;
-	}
-
+	bond->params.all_slaves_active = newval->value;
 	bond_for_each_slave(bond, slave, iter) {
 		if (!bond_is_active_slave(slave)) {
-			if (all_slaves_active)
+			if (newval->value)
 				slave->inactive = 0;
 			else
 				slave->inactive = 1;
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 396d504..09ee8c8 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -58,6 +58,7 @@ enum {
 	BOND_OPT_USE_CARRIER,
 	BOND_OPT_ACTIVE_SLAVE,
 	BOND_OPT_QUEUE_ID,
+	BOND_OPT_ALL_SLAVES_ACTIVE,
 	BOND_OPT_LAST
 };
 
@@ -156,4 +157,6 @@ int bond_option_active_slave_set(struct bonding *bond,
 				 struct bond_opt_value *newval);
 int bond_option_queue_id_set(struct bonding *bond,
 			     struct bond_opt_value *newval);
+int bond_option_all_slaves_active_set(struct bonding *bond,
+				      struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 2da37bd..a403345 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1016,22 +1016,13 @@ static ssize_t bonding_store_slaves_active(struct device *d,
 					   const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no all_slaves_active value specified.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_all_slaves_active_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_ALL_SLAVES_ACTIVE,
+				   (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 static DEVICE_ATTR(all_slaves_active, S_IRUGO | S_IWUSR,
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 014605d..c021d54 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -452,8 +452,6 @@ void bond_netlink_fini(void);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
 int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
-int bond_option_all_slaves_active_set(struct bonding *bond,
-				      int all_slaves_active);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
-- 
1.8.4.2

[PATCH net-next 23/25] bonding: convert resend_igmp to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so resend_igmp would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 29 +++++++++++++++++++----------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index d0f22ff..e183c66 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -266,7 +266,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int resend_igmp =
 			nla_get_u32(data[IFLA_BOND_RESEND_IGMP]);
 
-		err = bond_option_resend_igmp_set(bond, resend_igmp);
+		bond_opt_initval(&newval, resend_igmp);
+		err = __bond_opt_set(bond, BOND_OPT_RESEND_IGMP, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 8309d51..17fbaaf 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -112,6 +112,13 @@ static struct bond_opt_value bond_all_slaves_active_tbl[] = {
 	{ NULL,  -1, 0}
 };
 
+static struct bond_opt_value bond_resend_igmp_tbl[] = {
+	{ "off",     0,   0},
+	{ "maxval",  255, BOND_VALFLAG_MAX},
+	{ "default", 1,   BOND_VALFLAG_DEFAULT},
+	{ NULL,      -1,  0}
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -275,6 +282,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_all_slaves_active_tbl,
 		.set = bond_option_all_slaves_active_set
 	},
+	[BOND_OPT_RESEND_IGMP] = {
+		.id = BOND_OPT_RESEND_IGMP,
+		.name = "resend_igmp",
+		.desc = "Number of IGMP membership reports to send on link failure",
+		.values = bond_resend_igmp_tbl,
+		.set = bond_option_resend_igmp_set
+	},
 	{ }
 };
 
@@ -1039,17 +1053,12 @@ int bond_option_xmit_hash_policy_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp)
+int bond_option_resend_igmp_set(struct bonding *bond,
+				struct bond_opt_value *newval)
 {
-	if (resend_igmp < 0 || resend_igmp > 255) {
-		pr_err("%s: Invalid resend_igmp value %d not in range 0-255; rejected.\n",
-		       bond->dev->name, resend_igmp);
-		return -EINVAL;
-	}
-
-	bond->params.resend_igmp = resend_igmp;
-	pr_info("%s: Setting resend_igmp to %d.\n",
-		bond->dev->name, resend_igmp);
+	pr_info("%s: Setting resend_igmp to %llu.\n",
+		bond->dev->name, newval->value);
+	bond->params.resend_igmp = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index 09ee8c8..f0c2cbb 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -59,6 +59,7 @@ enum {
 	BOND_OPT_ACTIVE_SLAVE,
 	BOND_OPT_QUEUE_ID,
 	BOND_OPT_ALL_SLAVES_ACTIVE,
+	BOND_OPT_RESEND_IGMP,
 	BOND_OPT_LAST
 };
 
@@ -159,4 +160,6 @@ int bond_option_queue_id_set(struct bonding *bond,
 			     struct bond_opt_value *newval);
 int bond_option_all_slaves_active_set(struct bonding *bond,
 				      struct bond_opt_value *newval);
+int bond_option_resend_igmp_set(struct bonding *bond,
+				struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index a403345..4fad7cb 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1044,23 +1044,13 @@ static ssize_t bonding_store_resend_igmp(struct device *d,
 					 struct device_attribute *attr,
 					 const char *buf, size_t count)
 {
-	int new_value, ret = count;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no resend_igmp value specified.\n",
-		       bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	ret = bond_option_resend_igmp_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_RESEND_IGMP, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index c021d54..00b5e91 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -451,7 +451,6 @@ int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_resend_igmp_set(struct bonding *bond, int resend_igmp);
 int bond_option_lp_interval_set(struct bonding *bond, int min_links);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
-- 
1.8.4.2

[PATCH net-next 24/25] bonding: convert lp_interval to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so lp_interval would use
the new bonding option API.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_netlink.c |  3 ++-
 drivers/net/bonding/bond_options.c | 23 +++++++++++++++--------
 drivers/net/bonding/bond_options.h |  3 +++
 drivers/net/bonding/bond_sysfs.c   | 14 ++------------
 drivers/net/bonding/bonding.h      |  1 -
 5 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index e183c66..50fbe3f 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -302,7 +302,8 @@ static int bond_changelink(struct net_device *bond_dev,
 		int lp_interval =
 			nla_get_u32(data[IFLA_BOND_LP_INTERVAL]);
 
-		err = bond_option_lp_interval_set(bond, lp_interval);
+		bond_opt_initval(&newval, lp_interval);
+		err = __bond_opt_set(bond, BOND_OPT_LP_INTERVAL, &newval);
 		if (err)
 			return err;
 	}
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 17fbaaf..b9e9bbe 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -119,6 +119,11 @@ static struct bond_opt_value bond_resend_igmp_tbl[] = {
 	{ NULL,      -1,  0}
 };
 
+static struct bond_opt_value bond_lp_interval_tbl[] = {
+	{ "minval",  1,       BOND_VALFLAG_MIN | BOND_VALFLAG_DEFAULT},
+	{ "maxval",  INT_MAX, BOND_VALFLAG_MAX},
+};
+
 static struct bond_option bond_opts[] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -289,6 +294,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_resend_igmp_tbl,
 		.set = bond_option_resend_igmp_set
 	},
+	[BOND_OPT_LP_INTERVAL] = {
+		.id = BOND_OPT_LP_INTERVAL,
+		.name = "lp_interval",
+		.desc = "The number of seconds between instances where the bonding driver sends learning packets to each slave's peer switch",
+		.values = bond_lp_interval_tbl,
+		.set = bond_option_lp_interval_set
+	},
 	{ }
 };
 
@@ -1102,15 +1114,10 @@ int bond_option_min_links_set(struct bonding *bond,
 	return 0;
 }
 
-int bond_option_lp_interval_set(struct bonding *bond, int lp_interval)
+int bond_option_lp_interval_set(struct bonding *bond,
+				struct bond_opt_value *newval)
 {
-	if (lp_interval <= 0) {
-		pr_err("%s: lp_interval must be between 1 and %d\n",
-		       bond->dev->name, INT_MAX);
-		return -EINVAL;
-	}
-
-	bond->params.lp_interval = lp_interval;
+	bond->params.lp_interval = newval->value;
 
 	return 0;
 }
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index f0c2cbb..eb3a773 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -60,6 +60,7 @@ enum {
 	BOND_OPT_QUEUE_ID,
 	BOND_OPT_ALL_SLAVES_ACTIVE,
 	BOND_OPT_RESEND_IGMP,
+	BOND_OPT_LP_INTERVAL,
 	BOND_OPT_LAST
 };
 
@@ -162,4 +163,6 @@ int bond_option_all_slaves_active_set(struct bonding *bond,
 				      struct bond_opt_value *newval);
 int bond_option_resend_igmp_set(struct bonding *bond,
 				struct bond_opt_value *newval);
+int bond_option_lp_interval_set(struct bonding *bond,
+				struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 4fad7cb..8667e55 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1071,22 +1071,12 @@ static ssize_t bonding_store_lp_interval(struct device *d,
 					 const char *buf, size_t count)
 {
 	struct bonding *bond = to_bond(d);
-	int new_value, ret;
-
-	if (sscanf(buf, "%d", &new_value) != 1) {
-		pr_err("%s: no lp interval value specified.\n",
-			bond->dev->name);
-		return -EINVAL;
-	}
-
-	if (!rtnl_trylock())
-		return restart_syscall();
+	int ret;
 
-	ret = bond_option_lp_interval_set(bond, new_value);
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_LP_INTERVAL, (char *)buf);
 	if (!ret)
 		ret = count;
 
-	rtnl_unlock();
 	return ret;
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 00b5e91..69fc177 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -451,7 +451,6 @@ int bond_netlink_init(void);
 void bond_netlink_fini(void);
 int bond_option_arp_ip_target_add(struct bonding *bond, __be32 target);
 int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target);
-int bond_option_lp_interval_set(struct bonding *bond, int min_links);
 struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
 struct net_device *bond_option_active_slave_get(struct bonding *bond);
 const char *bond_slave_link_status(s8 link);
-- 
1.8.4.2

[PATCH net-next 25/25] bonding: convert slaves to use the new option API

[Nikolay Aleksandrov]

This patch adds the necessary changes so slaves would use
the new bonding option API. Also move the option to its own set function
in bond_options.c and fix some style errors.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_options.c | 53 ++++++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_options.h |  2 ++
 drivers/net/bonding/bond_sysfs.c   | 51 +++---------------------------------
 3 files changed, 59 insertions(+), 47 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index b9e9bbe..c241601 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -301,6 +301,13 @@ static struct bond_option bond_opts[] = {
 		.values = bond_lp_interval_tbl,
 		.set = bond_option_lp_interval_set
 	},
+	[BOND_OPT_SLAVES] = {
+		.id = BOND_OPT_SLAVES,
+		.name = "slaves",
+		.desc = "Slave membership management",
+		.flags = BOND_OPTFLAG_RAWVAL,
+		.set = bond_option_slaves_set
+	},
 	{ }
 };
 
@@ -1215,3 +1222,49 @@ err_no_cmd:
 	goto out;
 
 }
+
+int bond_option_slaves_set(struct bonding *bond, struct bond_opt_value *newval)
+{
+	char command[IFNAMSIZ + 1] = { 0, };
+	struct net_device *dev;
+	char *ifname;
+	int ret;
+
+	sscanf(newval->string, "%16s", command); /* IFNAMSIZ*/
+	ifname = command + 1;
+	if ((strlen(command) <= 1) ||
+	    !dev_valid_name(ifname))
+		goto err_no_cmd;
+
+	dev = __dev_get_by_name(dev_net(bond->dev), ifname);
+	if (!dev) {
+		pr_info("%s: Interface %s does not exist!\n",
+			bond->dev->name, ifname);
+		ret = -ENODEV;
+		goto out;
+	}
+
+	switch (command[0]) {
+	case '+':
+		pr_info("%s: Adding slave %s.\n", bond->dev->name, dev->name);
+		ret = bond_enslave(bond->dev, dev);
+		break;
+
+	case '-':
+		pr_info("%s: Removing slave %s.\n", bond->dev->name, dev->name);
+		ret = bond_release(bond->dev, dev);
+		break;
+
+	default:
+		goto err_no_cmd;
+	}
+
+out:
+	return ret;
+
+err_no_cmd:
+	pr_err("no command found in slaves file for bond %s. Use +ifname or -ifname.\n",
+	       bond->dev->name);
+	ret = -EPERM;
+	goto out;
+}
diff --git a/drivers/net/bonding/bond_options.h b/drivers/net/bonding/bond_options.h
index eb3a773..433d37f 100644
--- a/drivers/net/bonding/bond_options.h
+++ b/drivers/net/bonding/bond_options.h
@@ -61,6 +61,7 @@ enum {
 	BOND_OPT_ALL_SLAVES_ACTIVE,
 	BOND_OPT_RESEND_IGMP,
 	BOND_OPT_LP_INTERVAL,
+	BOND_OPT_SLAVES,
 	BOND_OPT_LAST
 };
 
@@ -165,4 +166,5 @@ int bond_option_resend_igmp_set(struct bonding *bond,
 				struct bond_opt_value *newval);
 int bond_option_lp_interval_set(struct bonding *bond,
 				struct bond_opt_value *newval);
+int bond_option_slaves_set(struct bonding *bond, struct bond_opt_value *newval);
 #endif /* _BOND_OPTIONS_H */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 8667e55..ceacb12 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -201,58 +201,15 @@ static ssize_t bonding_store_slaves(struct device *d,
 				    struct device_attribute *attr,
 				    const char *buffer, size_t count)
 {
-	char command[IFNAMSIZ + 1] = { 0, };
-	char *ifname;
-	int res, ret = count;
-	struct net_device *dev;
 	struct bonding *bond = to_bond(d);
+	int ret;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
-	sscanf(buffer, "%16s", command); /* IFNAMSIZ*/
-	ifname = command + 1;
-	if ((strlen(command) <= 1) ||
-	    !dev_valid_name(ifname))
-		goto err_no_cmd;
-
-	dev = __dev_get_by_name(dev_net(bond->dev), ifname);
-	if (!dev) {
-		pr_info("%s: Interface %s does not exist!\n",
-			bond->dev->name, ifname);
-		ret = -ENODEV;
-		goto out;
-	}
-
-	switch (command[0]) {
-	case '+':
-		pr_info("%s: Adding slave %s.\n", bond->dev->name, dev->name);
-		res = bond_enslave(bond->dev, dev);
-		break;
-
-	case '-':
-		pr_info("%s: Removing slave %s.\n", bond->dev->name, dev->name);
-		res = bond_release(bond->dev, dev);
-		break;
-
-	default:
-		goto err_no_cmd;
-	}
-
-	if (res)
-		ret = res;
-	goto out;
-
-err_no_cmd:
-	pr_err("no command found in slaves file for bond %s. Use +ifname or -ifname.\n",
-	       bond->dev->name);
-	ret = -EPERM;
+	ret = bond_opt_tryset_rtnl(bond, BOND_OPT_SLAVES, (char *)buffer);
+	if (!ret)
+		ret = count;
 
-out:
-	rtnl_unlock();
 	return ret;
 }
-
 static DEVICE_ATTR(slaves, S_IRUGO | S_IWUSR, bonding_show_slaves,
 		   bonding_store_slaves);
 
-- 
1.8.4.2

Re: [PATCH net-next 00/25] bonding: introduce new option API

[Dan Williams]

On Tue, 2014-01-21 at 15:54 +0100, Nikolay Aleksandrov wrote:
> Hi,
> This patchset's goal is to introduce a new option API which should be used
> to properly describe the bonding options with their mode dependcies and
> requirements. With this patchset applied we get centralized option
> manipulation, automatic RTNL acquire per option setting, automatic option
> range checking, mode dependcy checking and other various flags which are
> described in detail in patch 01's commit message and comments.
> Also the parameter passing is changed to use a specialized structure which
> is initialized to a value depending on the needs.
> The main exported functions are:
>  __bond_opt_set() - set an option (RTNL should be acquired prior)
>  bond_opt_init(val|str) - init a bond_opt_value struct for value or string
>                           parameter passing
>  bond_opt_tryset_rtnl() - function which tries to acquire rtnl, mainly used
>                           for sysfs
>  bond_opt_parse - used to parse or check for valid values
>  bond_opt_get - retrieve a pointer to bond_option struct for some option
>  bond_opt_get_val - retrieve a pointer to a bond_opt_value struct for
>                     some value
> 
> The same functions are used to set an option via sysfs and netlink, just
> the parameter that's passed is usually initialized in a different way.
> The converted options have multiple style fixes, there're some longer
> lines but they looked either ugly or were strings/pr_warnings, if you
> think some line would be better broken just let me know :-) there're
> also a few sscanf false-positive warnings.
> I decided to keep the "unsuppmodes" way of mode dep checking since it's
> straight forward, if we make a more general way for checking dependencies
> it'll be easy to change it.
> 
> Future plans for this work include:
>  - Automatic sysfs generation from the bond_opts[].
>  - Use of the API in bond_check_params() and thus cleaning it up (this has
>    actually started, I'll take care of the rest in a separate patch)
>  - Clean up all option-unrelated files of option definitions and functions
> 
> I've tried to leave as much documentation as possible, if there's anything
> unclear please let me know. One more thing, I haven't moved all
> option-related functions from bonding.h to the new bond_options.h, this
> will be done in a separate patch, it's in my todo list.
> 
> This patchset has been tested by setting each converted option via sysfs
> and netlink to a couple of wrong values, a couple of correct values and
> some random values, also for the opts that have flags they have been
> tested as well.

Currently userspace has to encode a lot of the same logic the kernel has
for option validation, for example when creating a user interface for
this stuff, you have to know that miimon and arp are incompatible, and
that certain options are only relevant with certain bond modes, and it's
a mess.  And this also sometimes changes when new kernel options or
capabilities are added.

So, is there any good way to describe the valid value ranges or
capabilities for userspace?  One idea is to send a package of bond
options to the kernel and see if they validate before actually applying
them, though this only works when actually configuring the interface.
An additional idea would be to somehow describe the available options
(eg, value ranges for numeric values, list-of-strings for bond modes,
etc) that the kernel supports before any bonds are even created (thus
probably through netlink, not sysfs).

Thoughts?

Dan

> Best regards,
>  Nikolay Aleksandrov
> 
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Veaceslav Falico <vfalico@redhat.com>
> CC: Scott Feldman <sfeldma@cumulusnetworks.com>
> CC: David S. Miller <davem@davemloft.net>
> 
> Nikolay Aleksandrov (25):
>   bonding: add infrastructure for an option API
>   bonding: convert mode setting to use the new option API
>   bonding: convert packets_per_slave to use the new option API
>   bonding: convert xmit_hash_policy to use the new option API
>   bonding: convert arp_validate to use the new option API
>   bonding: convert arp_all_targets to use the new option API
>   bonding: convert fail_over_mac to use the new option API
>   bonding: convert arp_interval to use the new option API
>   bonding: convert arp_ip_target to use the new option API
>   bonding: convert downdelay to use the new option API
>   bonding: convert updelay to use the new option API
>   bonding: convert lacp_rate to use the new option API
>   bonding: convert min_links to use the new option API
>   bonding: convert ad_select to use the new option API
>   bonding: convert num_peer_notif to use the new option API
>   bonding: convert miimon to use the new option API
>   bonding: convert primary to use the new option API
>   bonding: convert primary_reselect to use the new option API
>   bonding: convert use_carrier to use the new option API
>   bonding: convert active_slave to use the new option API
>   bonding: convert queue_id to use the new option API
>   bonding: convert all_slaves_active to use the new option API
>   bonding: convert resend_igmp to use the new option API
>   bonding: convert lp_interval to use the new option API
>   bonding: convert slaves to use the new option API
> 
>  drivers/net/bonding/bond_main.c    |  179 +++---
>  drivers/net/bonding/bond_netlink.c |   87 ++-
>  drivers/net/bonding/bond_options.c | 1086 +++++++++++++++++++++++++++---------
>  drivers/net/bonding/bond_options.h |  170 ++++++
>  drivers/net/bonding/bond_procfs.c  |   25 +-
>  drivers/net/bonding/bond_sysfs.c   |  519 +++--------------
>  drivers/net/bonding/bonding.h      |   29 +-
>  7 files changed, 1214 insertions(+), 881 deletions(-)
>  create mode 100644 drivers/net/bonding/bond_options.h
> 

Re: [PATCH net-next 00/25] bonding: introduce new option API

[Nikolay Aleksandrov]

On 01/21/2014 04:59 PM, Dan Williams wrote:
> On Tue, 2014-01-21 at 15:54 +0100, Nikolay Aleksandrov wrote:
>> Hi,
>> This patchset's goal is to introduce a new option API which should be used
>> to properly describe the bonding options with their mode dependcies and
>> requirements. With this patchset applied we get centralized option
>> manipulation, automatic RTNL acquire per option setting, automatic option
>> range checking, mode dependcy checking and other various flags which are
>> described in detail in patch 01's commit message and comments.
>> Also the parameter passing is changed to use a specialized structure which
>> is initialized to a value depending on the needs.
>> The main exported functions are:
>>  __bond_opt_set() - set an option (RTNL should be acquired prior)
>>  bond_opt_init(val|str) - init a bond_opt_value struct for value or string
>>                           parameter passing
>>  bond_opt_tryset_rtnl() - function which tries to acquire rtnl, mainly used
>>                           for sysfs
>>  bond_opt_parse - used to parse or check for valid values
>>  bond_opt_get - retrieve a pointer to bond_option struct for some option
>>  bond_opt_get_val - retrieve a pointer to a bond_opt_value struct for
>>                     some value
>>
>> The same functions are used to set an option via sysfs and netlink, just
>> the parameter that's passed is usually initialized in a different way.
>> The converted options have multiple style fixes, there're some longer
>> lines but they looked either ugly or were strings/pr_warnings, if you
>> think some line would be better broken just let me know :-) there're
>> also a few sscanf false-positive warnings.
>> I decided to keep the "unsuppmodes" way of mode dep checking since it's
>> straight forward, if we make a more general way for checking dependencies
>> it'll be easy to change it.
>>
>> Future plans for this work include:
>>  - Automatic sysfs generation from the bond_opts[].
>>  - Use of the API in bond_check_params() and thus cleaning it up (this has
>>    actually started, I'll take care of the rest in a separate patch)
>>  - Clean up all option-unrelated files of option definitions and functions
>>
>> I've tried to leave as much documentation as possible, if there's anything
>> unclear please let me know. One more thing, I haven't moved all
>> option-related functions from bonding.h to the new bond_options.h, this
>> will be done in a separate patch, it's in my todo list.
>>
>> This patchset has been tested by setting each converted option via sysfs
>> and netlink to a couple of wrong values, a couple of correct values and
>> some random values, also for the opts that have flags they have been
>> tested as well.
> 
> Currently userspace has to encode a lot of the same logic the kernel has
> for option validation, for example when creating a user interface for
> this stuff, you have to know that miimon and arp are incompatible, and
> that certain options are only relevant with certain bond modes, and it's
> a mess.  And this also sometimes changes when new kernel options or
> capabilities are added.
> 
> So, is there any good way to describe the valid value ranges or
> capabilities for userspace?  One idea is to send a package of bond
> options to the kernel and see if they validate before actually applying
> them, though this only works when actually configuring the interface.
> An additional idea would be to somehow describe the available options
> (eg, value ranges for numeric values, list-of-strings for bond modes,
> etc) that the kernel supports before any bonds are even created (thus
> probably through netlink, not sysfs).
> 
> Thoughts?
> 
> Dan
> 
Hi Dan,
I noticed the same thing while looking at the new ip link code. With this
change though you can pass anything to the option and it'll get validated,
so even if you pass a wrong value - not a problem you'll get the
appropriate error, so I don't see a problem for ip link to accept any
input, maybe the problem comes from the dual-personality options (accepting
both string and integers), we can always switch to the way sysfs does it
with either passing the argument as a string so __bond_opt_set would parse
it or the other option would be to export the option value tables via some
interface so ip can do its own validation.

This is a nice task to add to the todo list, just have to agree on a
solution :-)
Thanks for bringing it up.

Cheers,
 Nik
>> Best regards,
>>  Nikolay Aleksandrov
>>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> CC: Jay Vosburgh <fubar@us.ibm.com>
>> CC: Veaceslav Falico <vfalico@redhat.com>
>> CC: Scott Feldman <sfeldma@cumulusnetworks.com>
>> CC: David S. Miller <davem@davemloft.net>
>>
>> Nikolay Aleksandrov (25):
>>   bonding: add infrastructure for an option API
>>   bonding: convert mode setting to use the new option API
>>   bonding: convert packets_per_slave to use the new option API
>>   bonding: convert xmit_hash_policy to use the new option API
>>   bonding: convert arp_validate to use the new option API
>>   bonding: convert arp_all_targets to use the new option API
>>   bonding: convert fail_over_mac to use the new option API
>>   bonding: convert arp_interval to use the new option API
>>   bonding: convert arp_ip_target to use the new option API
>>   bonding: convert downdelay to use the new option API
>>   bonding: convert updelay to use the new option API
>>   bonding: convert lacp_rate to use the new option API
>>   bonding: convert min_links to use the new option API
>>   bonding: convert ad_select to use the new option API
>>   bonding: convert num_peer_notif to use the new option API
>>   bonding: convert miimon to use the new option API
>>   bonding: convert primary to use the new option API
>>   bonding: convert primary_reselect to use the new option API
>>   bonding: convert use_carrier to use the new option API
>>   bonding: convert active_slave to use the new option API
>>   bonding: convert queue_id to use the new option API
>>   bonding: convert all_slaves_active to use the new option API
>>   bonding: convert resend_igmp to use the new option API
>>   bonding: convert lp_interval to use the new option API
>>   bonding: convert slaves to use the new option API
>>
>>  drivers/net/bonding/bond_main.c    |  179 +++---
>>  drivers/net/bonding/bond_netlink.c |   87 ++-
>>  drivers/net/bonding/bond_options.c | 1086 +++++++++++++++++++++++++++---------
>>  drivers/net/bonding/bond_options.h |  170 ++++++
>>  drivers/net/bonding/bond_procfs.c  |   25 +-
>>  drivers/net/bonding/bond_sysfs.c   |  519 +++--------------
>>  drivers/net/bonding/bonding.h      |   29 +-
>>  7 files changed, 1214 insertions(+), 881 deletions(-)
>>  create mode 100644 drivers/net/bonding/bond_options.h
>>
> 
> 

Re: [PATCH net-next 00/25] bonding: introduce new option API

[Scott Feldman]

On Jan 21, 2014, at 7:59 AM, Dan Williams <dcbw@redhat.com> wrote:

> On Tue, 2014-01-21 at 15:54 +0100, Nikolay Aleksandrov wrote:
>> Hi,
>> This patchset's goal is to introduce a new option API which should be used
>> to properly describe the bonding options with their mode dependcies and
>> requirements. With this patchset applied we get centralized option
>> manipulation, automatic RTNL acquire per option setting, automatic option
>> range checking, mode dependcy checking and other various flags which are
>> described in detail in patch 01's commit message and comments.
>> Also the parameter passing is changed to use a specialized structure which
>> is initialized to a value depending on the needs.
> 
> Currently userspace has to encode a lot of the same logic the kernel has
> for option validation, for example when creating a user interface for
> this stuff, you have to know that miimon and arp are incompatible, and
> that certain options are only relevant with certain bond modes, and it's
> a mess.  And this also sometimes changes when new kernel options or
> capabilities are added.

Note that if using the new netlink support for bonding via an updated ip link command, incompatible options will be caught and the command will fail.  For example, if you try to create a new bond and set both miimon and arp, the bond create will fail (and err msg logged).  Once the bond is created, again trying to set incompatible options with one command will fail in the same way.  If options are set one at a time on existing bond, then last set option takes precedence and may override earlier settings.

None of this is true if using sysfs.  So user-space tools should move to ip link (or netlink directly) rather than using sysfs for bond config.

> 
> So, is there any good way to describe the valid value ranges or
> capabilities for userspace?  One idea is to send a package of bond
> options to the kernel and see if they validate before actually applying
> them, though this only works when actually configuring the interface.
> An additional idea would be to somehow describe the available options
> (eg, value ranges for numeric values, list-of-strings for bond modes,
> etc) that the kernel supports before any bonds are even created (thus
> probably through netlink, not sysfs).
> 
> Thoughts?

Re: [PATCH net-next 00/25] bonding: introduce new option API

[Ding Tianhong]

On 2014/1/21 22:54, Nikolay Aleksandrov wrote:
> Hi,
> This patchset's goal is to introduce a new option API which should be used
> to properly describe the bonding options with their mode dependcies and
> requirements. With this patchset applied we get centralized option
> manipulation, automatic RTNL acquire per option setting, automatic option
> range checking, mode dependcy checking and other various flags which are
> described in detail in patch 01's commit message and comments.
> Also the parameter passing is changed to use a specialized structure which
> is initialized to a value depending on the needs.
> The main exported functions are:
>  __bond_opt_set() - set an option (RTNL should be acquired prior)
>  bond_opt_init(val|str) - init a bond_opt_value struct for value or string
>                           parameter passing
>  bond_opt_tryset_rtnl() - function which tries to acquire rtnl, mainly used
>                           for sysfs
>  bond_opt_parse - used to parse or check for valid values
>  bond_opt_get - retrieve a pointer to bond_option struct for some option
>  bond_opt_get_val - retrieve a pointer to a bond_opt_value struct for
>                     some value
> 
> The same functions are used to set an option via sysfs and netlink, just
> the parameter that's passed is usually initialized in a different way.
> The converted options have multiple style fixes, there're some longer
> lines but they looked either ugly or were strings/pr_warnings, if you
> think some line would be better broken just let me know :-) there're
> also a few sscanf false-positive warnings.
> I decided to keep the "unsuppmodes" way of mode dep checking since it's
> straight forward, if we make a more general way for checking dependencies
> it'll be easy to change it.
> 
> Future plans for this work include:
>  - Automatic sysfs generation from the bond_opts[].
>  - Use of the API in bond_check_params() and thus cleaning it up (this has
>    actually started, I'll take care of the rest in a separate patch)
>  - Clean up all option-unrelated files of option definitions and functions
> 
> I've tried to leave as much documentation as possible, if there's anything
> unclear please let me know. One more thing, I haven't moved all
> option-related functions from bonding.h to the new bond_options.h, this
> will be done in a separate patch, it's in my todo list.
> 
> This patchset has been tested by setting each converted option via sysfs
> and netlink to a couple of wrong values, a couple of correct values and
> some random values, also for the opts that have flags they have been
> tested as well.
> 
> Best regards,
>  Nikolay Aleksandrov
> 

cool work, I think I can applied and test them, but it is really a hard work.:)

Ding

> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Veaceslav Falico <vfalico@redhat.com>
> CC: Scott Feldman <sfeldma@cumulusnetworks.com>
> CC: David S. Miller <davem@davemloft.net>
> 
> Nikolay Aleksandrov (25):
>   bonding: add infrastructure for an option API
>   bonding: convert mode setting to use the new option API
>   bonding: convert packets_per_slave to use the new option API
>   bonding: convert xmit_hash_policy to use the new option API
>   bonding: convert arp_validate to use the new option API
>   bonding: convert arp_all_targets to use the new option API
>   bonding: convert fail_over_mac to use the new option API
>   bonding: convert arp_interval to use the new option API
>   bonding: convert arp_ip_target to use the new option API
>   bonding: convert downdelay to use the new option API
>   bonding: convert updelay to use the new option API
>   bonding: convert lacp_rate to use the new option API
>   bonding: convert min_links to use the new option API
>   bonding: convert ad_select to use the new option API
>   bonding: convert num_peer_notif to use the new option API
>   bonding: convert miimon to use the new option API
>   bonding: convert primary to use the new option API
>   bonding: convert primary_reselect to use the new option API
>   bonding: convert use_carrier to use the new option API
>   bonding: convert active_slave to use the new option API
>   bonding: convert queue_id to use the new option API
>   bonding: convert all_slaves_active to use the new option API
>   bonding: convert resend_igmp to use the new option API
>   bonding: convert lp_interval to use the new option API
>   bonding: convert slaves to use the new option API
> 
>  drivers/net/bonding/bond_main.c    |  179 +++---
>  drivers/net/bonding/bond_netlink.c |   87 ++-
>  drivers/net/bonding/bond_options.c | 1086 +++++++++++++++++++++++++++---------
>  drivers/net/bonding/bond_options.h |  170 ++++++
>  drivers/net/bonding/bond_procfs.c  |   25 +-
>  drivers/net/bonding/bond_sysfs.c   |  519 +++--------------
>  drivers/net/bonding/bonding.h      |   29 +-
>  7 files changed, 1214 insertions(+), 881 deletions(-)
>  create mode 100644 drivers/net/bonding/bond_options.h
> 

Re: [PATCH net-next 03/25] bonding: convert packets_per_slave to use the new option API

[Hannes Frederic Sowa]

Hi Nikolay!

On Tue, Jan 21, 2014 at 03:54:52PM +0100, Nikolay Aleksandrov wrote:
> This patch adds the necessary changes so packets_per_slave would use the
> new bonding option API.

Just want to warn you that because of the reciproal_divide merge in net-next
there will be some conflicts with this patch in net-next.

I actually looked to rebase Daniel and my series on your patchset, but now
David pulled ours first.

Greetings,

  Hannes

Re: [PATCH net-next 03/25] bonding: convert packets_per_slave to use the new option API

[Nikolay Aleksandrov]

On 01/22/2014 08:25 AM, Hannes Frederic Sowa wrote:
> Hi Nikolay!
> 
> On Tue, Jan 21, 2014 at 03:54:52PM +0100, Nikolay Aleksandrov wrote:
>> This patch adds the necessary changes so packets_per_slave would use the
>> new bonding option API.
> 
> Just want to warn you that because of the reciproal_divide merge in net-next
> there will be some conflicts with this patch in net-next.
> 
> I actually looked to rebase Daniel and my series on your patchset, but now
> David pulled ours first.
> 
> Greetings,
> 
>   Hannes
> 
Hi Hannes,
Thanks for the heads up, I just synced net-next with your changes and indeed
there're a few conflicts.
I'll rebase on top of them and post a v2, shouldn't be a problem :-)

Cheers,
 Nik

Re: [PATCH net-next 00/25] bonding: introduce new option API

[Nikolay Aleksandrov]

On 01/22/2014 03:01 AM, Ding Tianhong wrote:
> On 2014/1/21 22:54, Nikolay Aleksandrov wrote:
>> Hi,
>> This patchset's goal is to introduce a new option API which should be used
>> to properly describe the bonding options with their mode dependcies and
>> requirements. With this patchset applied we get centralized option
>> manipulation, automatic RTNL acquire per option setting, automatic option
>> range checking, mode dependcy checking and other various flags which are
>> described in detail in patch 01's commit message and comments.
>> Also the parameter passing is changed to use a specialized structure which
>> is initialized to a value depending on the needs.
>> The main exported functions are:
>>  __bond_opt_set() - set an option (RTNL should be acquired prior)
>>  bond_opt_init(val|str) - init a bond_opt_value struct for value or string
>>                           parameter passing
>>  bond_opt_tryset_rtnl() - function which tries to acquire rtnl, mainly used
>>                           for sysfs
>>  bond_opt_parse - used to parse or check for valid values
>>  bond_opt_get - retrieve a pointer to bond_option struct for some option
>>  bond_opt_get_val - retrieve a pointer to a bond_opt_value struct for
>>                     some value
>>
>> The same functions are used to set an option via sysfs and netlink, just
>> the parameter that's passed is usually initialized in a different way.
>> The converted options have multiple style fixes, there're some longer
>> lines but they looked either ugly or were strings/pr_warnings, if you
>> think some line would be better broken just let me know :-) there're
>> also a few sscanf false-positive warnings.
>> I decided to keep the "unsuppmodes" way of mode dep checking since it's
>> straight forward, if we make a more general way for checking dependencies
>> it'll be easy to change it.
>>
>> Future plans for this work include:
>>  - Automatic sysfs generation from the bond_opts[].
>>  - Use of the API in bond_check_params() and thus cleaning it up (this has
>>    actually started, I'll take care of the rest in a separate patch)
>>  - Clean up all option-unrelated files of option definitions and functions
>>
>> I've tried to leave as much documentation as possible, if there's anything
>> unclear please let me know. One more thing, I haven't moved all
>> option-related functions from bonding.h to the new bond_options.h, this
>> will be done in a separate patch, it's in my todo list.
>>
>> This patchset has been tested by setting each converted option via sysfs
>> and netlink to a couple of wrong values, a couple of correct values and
>> some random values, also for the opts that have flags they have been
>> tested as well.
>>
>> Best regards,
>>  Nikolay Aleksandrov
>>
> 
> cool work, I think I can applied and test them, but it is really a hard work.:)
> 
> Ding
> 

Thanks Ding, any testing would be appreciated!
I'm about to post a v2 on top of net-next with the reciprocal_divide changes
that got in while waiting, it should apply cleanly now.

>> CC: Andy Gospodarek <andy@greyhouse.net>
>> CC: Jay Vosburgh <fubar@us.ibm.com>
>> CC: Veaceslav Falico <vfalico@redhat.com>
>> CC: Scott Feldman <sfeldma@cumulusnetworks.com>
>> CC: David S. Miller <davem@davemloft.net>
>>
>> Nikolay Aleksandrov (25):
>>   bonding: add infrastructure for an option API
>>   bonding: convert mode setting to use the new option API
>>   bonding: convert packets_per_slave to use the new option API
>>   bonding: convert xmit_hash_policy to use the new option API
>>   bonding: convert arp_validate to use the new option API
>>   bonding: convert arp_all_targets to use the new option API
>>   bonding: convert fail_over_mac to use the new option API
>>   bonding: convert arp_interval to use the new option API
>>   bonding: convert arp_ip_target to use the new option API
>>   bonding: convert downdelay to use the new option API
>>   bonding: convert updelay to use the new option API
>>   bonding: convert lacp_rate to use the new option API
>>   bonding: convert min_links to use the new option API
>>   bonding: convert ad_select to use the new option API
>>   bonding: convert num_peer_notif to use the new option API
>>   bonding: convert miimon to use the new option API
>>   bonding: convert primary to use the new option API
>>   bonding: convert primary_reselect to use the new option API
>>   bonding: convert use_carrier to use the new option API
>>   bonding: convert active_slave to use the new option API
>>   bonding: convert queue_id to use the new option API
>>   bonding: convert all_slaves_active to use the new option API
>>   bonding: convert resend_igmp to use the new option API
>>   bonding: convert lp_interval to use the new option API
>>   bonding: convert slaves to use the new option API
>>
>>  drivers/net/bonding/bond_main.c    |  179 +++---
>>  drivers/net/bonding/bond_netlink.c |   87 ++-
>>  drivers/net/bonding/bond_options.c | 1086 +++++++++++++++++++++++++++---------
>>  drivers/net/bonding/bond_options.h |  170 ++++++
>>  drivers/net/bonding/bond_procfs.c  |   25 +-
>>  drivers/net/bonding/bond_sysfs.c   |  519 +++--------------
>>  drivers/net/bonding/bonding.h      |   29 +-
>>  7 files changed, 1214 insertions(+), 881 deletions(-)
>>  create mode 100644 drivers/net/bonding/bond_options.h
>>
> 
>