netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters
@ 2015-05-09  7:01 Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, David Miller

This is a resubmit of Mahesh's last 3 bonding patches from this series
(http://marc.info/?l=linux-netdev&m=142432864626179&w=2) with one
additional kernel patch which adds the netlink bits. I have noted any
modifications I did to the original patches just above my signoff line.
Patch 5 is the iproute2 support for these bonding options. All patches
were coded against the net-next branch of their respective projects.

v2:
  * rebased
  * only send these new parameters via netlink when bond is in mode 4
  * fixed ad_actor_sys_prio to be 0xFFFF by default even when the bond
    is initially created in mode 0 and switched to mode 4

v3:
  * reverted changes to bond_option_ad_actor_system_set() from v1 in Mahesh's
    patch "bonding: Allow userspace to set actors' macaddr in an AD-system."
    Instead implementing all setting in the option specific set function as
    Nik suggested.

Kernel series:

Andy Gospodarek (1):
  bonding: add netlink support for sys prio, actor sys mac, and port
    key

Mahesh Bandewar (3):
  bonding: Allow userspace to set actors' system_priority in AD system
  bonding: Allow userspace to set actors' macaddr in an AD-system.
  bonding: Implement user key part of port_key in an AD system.

 Documentation/networking/bonding.txt |   84 +++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_3ad.c       |   26 ++++++----
 drivers/net/bonding/bond_main.c      |   23 +++++++++
 drivers/net/bonding/bond_netlink.c   |   50 +++++++++++++++++++
 drivers/net/bonding/bond_options.c   |   91 ++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_procfs.c    |    8 +++
 drivers/net/bonding/bond_sysfs.c     |   46 +++++++++++++++++
 include/net/bond_options.h           |    3 ++
 include/net/bonding.h                |    3 ++
 include/uapi/linux/if_link.h         |    3 ++
 10 files changed, 328 insertions(+), 9 deletions(-)

iproute2 series:

Jonathan Toppins (1):
      iplink_bond: add support for ad_actor and port_key options

 ip/iplink_bond.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH linux v3 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
@ 2015-05-09  7:01 ` Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system Jonathan Toppins
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, David Miller
  Cc: Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

This patch allows user to randomize the system-priority in an ad-system.
The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user
does not specify this value, the system defaults to 0xFFFF, which is
what it was before this patch.

Following example code could set the value -
    # modprobe bonding mode=4
    # sys_prio=$(( 1 + RANDOM + RANDOM ))
    # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_sys_prio
    # echo +eth1 > /sys/class/net/bond0/bonding/slaves
    ...
    # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
[jt: * fixed up style issues reported by checkpatch
     * changed how the default value is set in bond_check_params(), this
       makes the default consistent between what gets set for a new bond
       and what the default is claimed to be in the bonding options.]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 v2:
    * rebased
    * aligned the default value to be the same for new bonds and what is
      specified in bonding options table as the default value for the
      parameter. Previously the "default" for new bonds was zero which is
      not the default specified in the options table. Noted fixup in my
      changelog notes above.
 v3:
    * no change

 Documentation/networking/bonding.txt |    9 +++++++++
 drivers/net/bonding/bond_3ad.c       |    5 ++++-
 drivers/net/bonding/bond_main.c      |   12 ++++++++++++
 drivers/net/bonding/bond_options.c   |   28 +++++++++++++++++++++++++++-
 drivers/net/bonding/bond_procfs.c    |    2 ++
 drivers/net/bonding/bond_sysfs.c     |   15 +++++++++++++++
 include/net/bond_options.h           |    1 +
 include/net/bonding.h                |    1 +
 8 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 83bf498..3494611 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -178,6 +178,15 @@ active_slave
 	active slave, or the empty string if there is no active slave or
 	the current mode does not use an active slave.
 
+ad_actor_sys_prio
+
+	In an AD system, this specifies the system priority. The allowed range
+	is 1 - 65535. If the value is not specified, it takes 65535 as the
+	default value.
+
+	This parameter has effect only in 802.3ad mode and is available through
+	SysFs interface.
+
 ad_select
 
 	Specifies the 802.3ad aggregation selection logic to use.  The
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index fbd54f0..4c003bc 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1908,7 +1908,8 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 
 		BOND_AD_INFO(bond).aggregator_identifier = 0;
 
-		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
+		BOND_AD_INFO(bond).system.sys_priority =
+			bond->params.ad_actor_sys_prio;
 		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
 
 		/* initialize how many times this module is called in one
@@ -1959,6 +1960,8 @@ void bond_3ad_bind_slave(struct slave *slave)
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
 		/* actor system is the bond's system */
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
+		port->actor_system_priority =
+		    BOND_AD_INFO(bond).system.sys_priority;
 		/* tx timer(to verify that no more than MAX_TX_IN_SECOND
 		 * lacpdu's are sent in one second)
 		 */
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d5fe5d5..5f2f28f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4140,6 +4140,7 @@ static int bond_check_params(struct bond_params *params)
 	struct bond_opt_value newval;
 	const struct bond_opt_value *valptr;
 	int arp_all_targets_value;
+	u16 ad_actor_sys_prio = 0;
 
 	/* Convert string parameters. */
 	if (mode) {
@@ -4434,6 +4435,16 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	bond_opt_initstr(&newval, "default");
+	valptr = bond_opt_parse(
+			bond_opt_get(BOND_OPT_AD_ACTOR_SYS_PRIO),
+				     &newval);
+	if (!valptr) {
+		pr_err("Error: No ad_actor_sys_prio default value");
+		return -EINVAL;
+	}
+	ad_actor_sys_prio = valptr->value;
+
 	if (lp_interval == 0) {
 		pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
 			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
@@ -4462,6 +4473,7 @@ static int bond_check_params(struct bond_params *params)
 	params->lp_interval = lp_interval;
 	params->packets_per_slave = packets_per_slave;
 	params->tlb_dynamic_lb = 1; /* Default value */
+	params->ad_actor_sys_prio = ad_actor_sys_prio;
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 4df2894..d2b47e5 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -70,6 +70,8 @@ static int bond_option_slaves_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
 static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
+static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
+					     const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -186,6 +188,12 @@ static const struct bond_opt_value bond_tlb_dynamic_lb_tbl[] = {
 	{ NULL,  -1, 0}
 };
 
+static const struct bond_opt_value bond_ad_actor_sys_prio_tbl[] = {
+	{ "minval",  1,     BOND_VALFLAG_MIN},
+	{ "maxval",  65535, BOND_VALFLAG_MAX | BOND_VALFLAG_DEFAULT},
+	{ NULL,      -1,    0},
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -379,7 +387,15 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.values = bond_tlb_dynamic_lb_tbl,
 		.flags = BOND_OPTFLAG_IFDOWN,
 		.set = bond_option_tlb_dynamic_lb_set,
-	}
+	},
+	[BOND_OPT_AD_ACTOR_SYS_PRIO] = {
+		.id = BOND_OPT_AD_ACTOR_SYS_PRIO,
+		.name = "ad_actor_sys_prio",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.values = bond_ad_actor_sys_prio_tbl,
+		.set = bond_option_ad_actor_sys_prio_set,
+	},
 };
 
 /* Searches for an option by name */
@@ -1349,3 +1365,13 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 
 	return 0;
 }
+
+static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
+					     const struct bond_opt_value *newval)
+{
+	netdev_info(bond->dev, "Setting ad_actor_sys_prio to (%llu)\n",
+		    newval->value);
+
+	bond->params.ad_actor_sys_prio = newval->value;
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index b20b35ac..1136929 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -135,6 +135,8 @@ static void bond_info_show_master(struct seq_file *seq)
 					  bond->params.ad_select);
 		seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
 			   optval->string);
+		seq_printf(seq, "System priority: %d\n",
+			   BOND_AD_INFO(bond).system.sys_priority);
 
 		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 7e9e151..4a76266 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -692,6 +692,20 @@ static ssize_t bonding_show_packets_per_slave(struct device *d,
 static DEVICE_ATTR(packets_per_slave, S_IRUGO | S_IWUSR,
 		   bonding_show_packets_per_slave, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_actor_sys_prio(struct device *d,
+					      struct device_attribute *attr,
+					      char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%hu\n", bond->params.ad_actor_sys_prio);
+
+	return 0;
+}
+static DEVICE_ATTR(ad_actor_sys_prio, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_actor_sys_prio, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -725,6 +739,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_lp_interval.attr,
 	&dev_attr_packets_per_slave.attr,
 	&dev_attr_tlb_dynamic_lb.attr,
+	&dev_attr_ad_actor_sys_prio.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index ea6546d..894002a 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -63,6 +63,7 @@ enum {
 	BOND_OPT_LP_INTERVAL,
 	BOND_OPT_SLAVES,
 	BOND_OPT_TLB_DYNAMIC_LB,
+	BOND_OPT_AD_ACTOR_SYS_PRIO,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 78ed135..405cf87 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -136,6 +136,7 @@ struct bond_params {
 	int packets_per_slave;
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
+	u16 ad_actor_sys_prio;
 };
 
 struct bond_parm_tbl {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH linux v3 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system.
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
@ 2015-05-09  7:01 ` Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 3/4] bonding: Implement user key part of port_key in an AD system Jonathan Toppins
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, David Miller
  Cc: Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

In an AD system, the communication between actor and partner is the
business between these two entities. In the current setup anyone on the
same L2 can "guess" the LACPDU contents and then possibly send the
spoofed LACPDUs and trick the partner causing connectivity issues for
the AD system. This patch allows to use a random mac-address obscuring
it's identity making it harder for someone in the L2 is do the same thing.

This patch allows user-space to choose the mac-address for the AD-system.
This mac-address can not be NULL or a Multicast. If the mac-address is set
from user-space; kernel will honor it and will not overwrite it. In the
absence (value from user space); the logic will default to using the
masters' mac as the mac-address for the AD-system.

It can be set using example code below -

   # modprobe bonding mode=4
   # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
                    $(( (RANDOM & 0xFE) | 0x02 )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )) \
                    $(( RANDOM & 0xFF )))
   # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system
   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
   ...
   # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
[jt: fixed up style issues reported by checkpatch]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 v2:
    * rebased
 v3:
    * reverted changes to bond_option_ad_actor_system_set() from v1

 Documentation/networking/bonding.txt |   12 ++++++++++++
 drivers/net/bonding/bond_3ad.c       |    7 ++++++-
 drivers/net/bonding/bond_main.c      |    1 +
 drivers/net/bonding/bond_options.c   |   27 +++++++++++++++++++++++++++
 drivers/net/bonding/bond_procfs.c    |    6 ++++++
 drivers/net/bonding/bond_sysfs.c     |   16 ++++++++++++++++
 include/net/bond_options.h           |    1 +
 include/net/bonding.h                |    1 +
 8 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 3494611..2c197b6 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -187,6 +187,18 @@ ad_actor_sys_prio
 	This parameter has effect only in 802.3ad mode and is available through
 	SysFs interface.
 
+ad_actor_system
+
+	In an AD system, this specifies the mac-address for the actor in
+	protocol packet exchanges (LACPDUs). The value cannot be NULL or
+	multicast. It is preferred to have the local-admin bit set for this
+	mac but driver does not enforce it. If the value is not given then
+	system defaults to using the masters' mac address as actors' system
+	address.
+
+	This parameter has effect only in 802.3ad mode and is available through
+	SysFs interface.
+
 ad_select
 
 	Specifies the 802.3ad aggregation selection logic to use.  The
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 4c003bc..012f7bc 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1910,7 +1910,12 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 
 		BOND_AD_INFO(bond).system.sys_priority =
 			bond->params.ad_actor_sys_prio;
-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
+		if (is_zero_ether_addr(bond->params.ad_actor_system))
+			BOND_AD_INFO(bond).system.sys_mac_addr =
+			    *((struct mac_addr *)bond->dev->dev_addr);
+		else
+			BOND_AD_INFO(bond).system.sys_mac_addr =
+			    *((struct mac_addr *)bond->params.ad_actor_system);
 
 		/* initialize how many times this module is called in one
 		 * second (should be about every 100ms)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5f2f28f..a4e2f27 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4474,6 +4474,7 @@ static int bond_check_params(struct bond_params *params)
 	params->packets_per_slave = packets_per_slave;
 	params->tlb_dynamic_lb = 1; /* Default value */
 	params->ad_actor_sys_prio = ad_actor_sys_prio;
+	eth_zero_addr(params->ad_actor_system);
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index d2b47e5..cdcef21 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -72,6 +72,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 				  const struct bond_opt_value *newval);
 static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
 					     const struct bond_opt_value *newval);
+static int bond_option_ad_actor_system_set(struct bonding *bond,
+					   const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -396,6 +398,13 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.values = bond_ad_actor_sys_prio_tbl,
 		.set = bond_option_ad_actor_sys_prio_set,
 	},
+	[BOND_OPT_AD_ACTOR_SYSTEM] = {
+		.id = BOND_OPT_AD_ACTOR_SYSTEM,
+		.name = "ad_actor_system",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
+		.set = bond_option_ad_actor_system_set,
+	},
 };
 
 /* Searches for an option by name */
@@ -1375,3 +1384,21 @@ static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
 	bond->params.ad_actor_sys_prio = newval->value;
 	return 0;
 }
+
+static int bond_option_ad_actor_system_set(struct bonding *bond,
+					   const struct bond_opt_value *newval)
+{
+	u8 macaddr[ETH_ALEN];
+	int i;
+
+	i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+		   &macaddr[0], &macaddr[1], &macaddr[2],
+		   &macaddr[3], &macaddr[4], &macaddr[5]);
+	if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
+		netdev_err(bond->dev, "Invalid MAC address.\n");
+		return -EINVAL;
+	}
+
+	ether_addr_copy(bond->params.ad_actor_system, macaddr);
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 1136929..e7f3047 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -137,6 +137,8 @@ static void bond_info_show_master(struct seq_file *seq)
 			   optval->string);
 		seq_printf(seq, "System priority: %d\n",
 			   BOND_AD_INFO(bond).system.sys_priority);
+		seq_printf(seq, "System MAC address: %pM\n",
+			   &BOND_AD_INFO(bond).system.sys_mac_addr);
 
 		if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 			seq_printf(seq, "bond %s has no active aggregator\n",
@@ -200,6 +202,8 @@ static void bond_info_show_slave(struct seq_file *seq,
 			seq_puts(seq, "details actor lacp pdu:\n");
 			seq_printf(seq, "    system priority: %d\n",
 				   port->actor_system_priority);
+			seq_printf(seq, "    system mac address: %pM\n",
+				   &port->actor_system);
 			seq_printf(seq, "    port key: %d\n",
 				   port->actor_oper_port_key);
 			seq_printf(seq, "    port priority: %d\n",
@@ -212,6 +216,8 @@ static void bond_info_show_slave(struct seq_file *seq,
 			seq_puts(seq, "details partner lacp pdu:\n");
 			seq_printf(seq, "    system priority: %d\n",
 				   port->partner_oper.system_priority);
+			seq_printf(seq, "    system mac address: %pM\n",
+				   &port->partner_oper.system);
 			seq_printf(seq, "    oper key: %d\n",
 				   port->partner_oper.key);
 			seq_printf(seq, "    port priority: %d\n",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 4a76266..09fefa5 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -706,6 +706,21 @@ static ssize_t bonding_show_ad_actor_sys_prio(struct device *d,
 static DEVICE_ATTR(ad_actor_sys_prio, S_IRUGO | S_IWUSR,
 		   bonding_show_ad_actor_sys_prio, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_actor_system(struct device *d,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%pM\n", bond->params.ad_actor_system);
+
+	return 0;
+}
+
+static DEVICE_ATTR(ad_actor_system, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_actor_system, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -740,6 +755,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_packets_per_slave.attr,
 	&dev_attr_tlb_dynamic_lb.attr,
 	&dev_attr_ad_actor_sys_prio.attr,
+	&dev_attr_ad_actor_system.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 894002a..eeeefa1 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -64,6 +64,7 @@ enum {
 	BOND_OPT_SLAVES,
 	BOND_OPT_TLB_DYNAMIC_LB,
 	BOND_OPT_AD_ACTOR_SYS_PRIO,
+	BOND_OPT_AD_ACTOR_SYSTEM,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 405cf87..650f386 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -137,6 +137,7 @@ struct bond_params {
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
 	u16 ad_actor_sys_prio;
+	u8 ad_actor_system[ETH_ALEN];
 };
 
 struct bond_parm_tbl {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH linux v3 net-next 3/4] bonding: Implement user key part of port_key in an AD system.
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system Jonathan Toppins
@ 2015-05-09  7:01 ` Jonathan Toppins
  2015-05-09  7:01 ` [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, David Miller
  Cc: Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

The port key has three components - user-key, speed-part, and duplex-part.
The LSBit is for the duplex-part, next 5 bits are for the speed while the
remaining 10 bits are the user defined key bits. Get these 10 bits
from the user-space (through the SysFs interface) and use it to form the
admin port-key. Allowed range for the user-key is 0 - 1023 (10 bits). If
it is not provided then use zero for the user-key-bits (default).

It can set using following example code -

   # modprobe bonding mode=4
   # usr_port_key=$(( RANDOM & 0x3FF ))
   # echo $usr_port_key > /sys/class/net/bond0/bonding/ad_user_port_key
   # echo +eth1 > /sys/class/net/bond0/bonding/slaves
   ...
   # ip link set bond0 up

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
[jt: * fixed up style issues reported by checkpatch
     * fixed up context from change in ad_actor_sys_prio patch]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 v2:
    * rebased
    * fixed up context from change in ad_actor_sys_prio patch
 v3:
    * no change

 Documentation/networking/bonding.txt |   63 ++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_3ad.c       |   14 ++++----
 drivers/net/bonding/bond_main.c      |   10 ++++++
 drivers/net/bonding/bond_options.c   |   26 ++++++++++++++
 drivers/net/bonding/bond_sysfs.c     |   15 ++++++++
 include/net/bond_options.h           |    1 +
 include/net/bonding.h                |    1 +
 7 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 2c197b6..334b49e 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -51,6 +51,7 @@ Table of Contents
 3.4	Configuring Bonding Manually via Sysfs
 3.5	Configuration with Interfaces Support
 3.6	Overriding Configuration for Special Cases
+3.7 Configuring LACP for 802.3ad mode in a more secure way
 
 4. Querying Bonding Configuration
 4.1	Bonding Configuration
@@ -241,6 +242,21 @@ ad_select
 
 	This option was added in bonding version 3.4.0.
 
+ad_user_port_key
+
+	In an AD system, the port-key has three parts as shown below -
+
+	   Bits   Use
+	   00     Duplex
+	   01-05  Speed
+	   06-15  User-defined
+
+	This defines the upper 10 bits of the port key. The values can be
+	from 0 - 1023. If not given, the system defaults to 0.
+
+	This parameter has effect only in 802.3ad mode and is available through
+	SysFs interface.
+
 all_slaves_active
 
 	Specifies that duplicate frames (received on inactive ports) should be
@@ -1643,6 +1659,53 @@ output port selection.
 This feature first appeared in bonding driver version 3.7.0 and support for
 output slave selection was limited to round-robin and active-backup modes.
 
+3.7 Configuring LACP for 802.3ad mode in a more secure way
+----------------------------------------------------------
+
+When using 802.3ad bonding mode, the Actor (host) and Partner (switch)
+exchange LACPDUs.  These LACPDUs cannot be sniffed, because they are
+destined to link local mac addresses (which switches/bridges are not
+supposed to forward).  However, most of the values are easily predictable
+or are simply the machine's MAC address (which is trivially known to all
+other hosts in the same L2).  This implies that other machines in the L2
+domain can spoof LACPDU packets from other hosts to the switch and potentially
+cause mayhem by joining (from the point of view of the switch) another
+machine's aggregate, thus receiving a portion of that hosts incoming
+traffic and / or spoofing traffic from that machine themselves (potentially
+even successfully terminating some portion of flows). Though this is not
+a likely scenario, one could avoid this possibility by simply configuring
+few bonding parameters:
+
+   (a) ad_actor_system : You can set a random mac-address that can be used for
+       these LACPDU exchanges. The value can not be either NULL or Multicast.
+       Also it's preferable to set the local-admin bit. Following shell code
+       generates a random mac-address as described above.
+
+       # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \
+                                $(( (RANDOM & 0xFE) | 0x02 )) \
+                                $(( RANDOM & 0xFF )) \
+                                $(( RANDOM & 0xFF )) \
+                                $(( RANDOM & 0xFF )) \
+                                $(( RANDOM & 0xFF )) \
+                                $(( RANDOM & 0xFF )))
+       # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system
+
+   (b) ad_actor_sys_prio : Randomize the system priority. The default value
+       is 65535, but system can take the value from 1 - 65535. Following shell
+       code generates random priority and sets it.
+
+       # sys_prio=$(( 1 + RANDOM + RANDOM ))
+       # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_sys_prio
+
+   (c) ad_user_port_key : Use the user portion of the port-key. The default
+       keeps this empty. These are the upper 10 bits of the port-key and value
+       ranges from 0 - 1023. Following shell code generates these 10 bits and
+       sets it.
+
+       # usr_port_key=$(( RANDOM & 0x3FF ))
+       # echo $usr_port_key > /sys/class/net/bond0/bonding/ad_user_port_key
+
+
 4 Querying Bonding Configuration
 =================================
 
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 012f7bc..7fde4d5 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -75,10 +75,10 @@
 /* Port Key definitions
  * key is determined according to the link speed, duplex and
  * user key (which is yet not supported)
- * --------------------------------------------------------------
- * Port key :	| User key	| Speed		| Duplex	|
- * --------------------------------------------------------------
- * 16		  6		  1		  0
+ *           --------------------------------------------------------------
+ * Port key  | User key (10 bits)           | Speed (5 bits)      | Duplex|
+ *           --------------------------------------------------------------
+ *           |15                           6|5                   1|0
  */
 #define  AD_DUPLEX_KEY_MASKS    0x1
 #define  AD_SPEED_KEY_MASKS     0x3E
@@ -1951,10 +1951,10 @@ void bond_3ad_bind_slave(struct slave *slave)
 
 		port->slave = slave;
 		port->actor_port_number = SLAVE_AD_INFO(slave)->id;
-		/* key is determined according to the link speed, duplex and user key(which
-		 * is yet not supported)
+		/* key is determined according to the link speed, duplex and
+		 * user key
 		 */
-		port->actor_admin_port_key = 0;
+		port->actor_admin_port_key = bond->params.ad_user_port_key << 6;
 		port->actor_admin_port_key |= __get_duplex(port);
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a4e2f27..2ee13be 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4141,6 +4141,7 @@ static int bond_check_params(struct bond_params *params)
 	const struct bond_opt_value *valptr;
 	int arp_all_targets_value;
 	u16 ad_actor_sys_prio = 0;
+	u16 ad_user_port_key = 0;
 
 	/* Convert string parameters. */
 	if (mode) {
@@ -4445,6 +4446,14 @@ static int bond_check_params(struct bond_params *params)
 	}
 	ad_actor_sys_prio = valptr->value;
 
+	valptr = bond_opt_parse(bond_opt_get(BOND_OPT_AD_USER_PORT_KEY),
+				&newval);
+	if (!valptr) {
+		pr_err("Error: No ad_user_port_key default value");
+		return -EINVAL;
+	}
+	ad_user_port_key = valptr->value;
+
 	if (lp_interval == 0) {
 		pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
 			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
@@ -4475,6 +4484,7 @@ static int bond_check_params(struct bond_params *params)
 	params->tlb_dynamic_lb = 1; /* Default value */
 	params->ad_actor_sys_prio = ad_actor_sys_prio;
 	eth_zero_addr(params->ad_actor_system);
+	params->ad_user_port_key = ad_user_port_key;
 	if (packets_per_slave > 0) {
 		params->reciprocal_packets_per_slave =
 			reciprocal_value(packets_per_slave);
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index cdcef21..c85da05 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -74,6 +74,8 @@ static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
 					     const struct bond_opt_value *newval);
 static int bond_option_ad_actor_system_set(struct bonding *bond,
 					   const struct bond_opt_value *newval);
+static int bond_option_ad_user_port_key_set(struct bonding *bond,
+					    const struct bond_opt_value *newval);
 
 
 static const struct bond_opt_value bond_mode_tbl[] = {
@@ -196,6 +198,12 @@ static const struct bond_opt_value bond_ad_actor_sys_prio_tbl[] = {
 	{ NULL,      -1,    0},
 };
 
+static const struct bond_opt_value bond_ad_user_port_key_tbl[] = {
+	{ "minval",  0,     BOND_VALFLAG_MIN | BOND_VALFLAG_DEFAULT},
+	{ "maxval",  1023,  BOND_VALFLAG_MAX},
+	{ NULL,      -1,    0},
+};
+
 static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 	[BOND_OPT_MODE] = {
 		.id = BOND_OPT_MODE,
@@ -405,6 +413,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
 		.flags = BOND_OPTFLAG_RAWVAL | BOND_OPTFLAG_IFDOWN,
 		.set = bond_option_ad_actor_system_set,
 	},
+	[BOND_OPT_AD_USER_PORT_KEY] = {
+		.id = BOND_OPT_AD_USER_PORT_KEY,
+		.name = "ad_user_port_key",
+		.unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
+		.flags = BOND_OPTFLAG_IFDOWN,
+		.values = bond_ad_user_port_key_tbl,
+		.set = bond_option_ad_user_port_key_set,
+	}
 };
 
 /* Searches for an option by name */
@@ -1402,3 +1418,13 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 	ether_addr_copy(bond->params.ad_actor_system, macaddr);
 	return 0;
 }
+
+static int bond_option_ad_user_port_key_set(struct bonding *bond,
+					    const struct bond_opt_value *newval)
+{
+	netdev_info(bond->dev, "Setting ad_user_port_key to (%llu)\n",
+		    newval->value);
+
+	bond->params.ad_user_port_key = newval->value;
+	return 0;
+}
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 09fefa5..143a2ab 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -721,6 +721,20 @@ static ssize_t bonding_show_ad_actor_system(struct device *d,
 static DEVICE_ATTR(ad_actor_system, S_IRUGO | S_IWUSR,
 		   bonding_show_ad_actor_system, bonding_sysfs_store_option);
 
+static ssize_t bonding_show_ad_user_port_key(struct device *d,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	if (BOND_MODE(bond) == BOND_MODE_8023AD)
+		return sprintf(buf, "%hu\n", bond->params.ad_user_port_key);
+
+	return 0;
+}
+static DEVICE_ATTR(ad_user_port_key, S_IRUGO | S_IWUSR,
+		   bonding_show_ad_user_port_key, bonding_sysfs_store_option);
+
 static struct attribute *per_bond_attrs[] = {
 	&dev_attr_slaves.attr,
 	&dev_attr_mode.attr,
@@ -756,6 +770,7 @@ static struct attribute *per_bond_attrs[] = {
 	&dev_attr_tlb_dynamic_lb.attr,
 	&dev_attr_ad_actor_sys_prio.attr,
 	&dev_attr_ad_actor_system.attr,
+	&dev_attr_ad_user_port_key.attr,
 	NULL,
 };
 
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index eeeefa1..c28aca2 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -65,6 +65,7 @@ enum {
 	BOND_OPT_TLB_DYNAMIC_LB,
 	BOND_OPT_AD_ACTOR_SYS_PRIO,
 	BOND_OPT_AD_ACTOR_SYSTEM,
+	BOND_OPT_AD_USER_PORT_KEY,
 	BOND_OPT_LAST
 };
 
diff --git a/include/net/bonding.h b/include/net/bonding.h
index 650f386..20defc0 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -137,6 +137,7 @@ struct bond_params {
 	int tlb_dynamic_lb;
 	struct reciprocal_value reciprocal_packets_per_slave;
 	u16 ad_actor_sys_prio;
+	u16 ad_user_port_key;
 	u8 ad_actor_system[ETH_ALEN];
 };
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
                   ` (2 preceding siblings ...)
  2015-05-09  7:01 ` [PATCH linux v3 net-next 3/4] bonding: Implement user key part of port_key in an AD system Jonathan Toppins
@ 2015-05-09  7:01 ` Jonathan Toppins
  2015-05-10  7:54   ` Nikolay Aleksandrov
  2015-05-09  7:01 ` [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
  2015-05-11 15:00 ` [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters David Miller
  5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, David Miller

From: Andy Gospodarek <gospo@cumulusnetworks.com>

Adds netlink support for the following bonding options:
* BOND_OPT_AD_ACTOR_SYS_PRIO
* BOND_OPT_AD_ACTOR_SYSTEM
* BOND_OPT_AD_USER_PORT_KEY

When setting the actor system mac address we assume the netlink message
contains a binary mac and not a string representation of a mac.

Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>
[jt: completed the setting side of the netlink attributes]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 v2:
    * rebased
 v3:
    * removed parenthesis from around values in the netdev_info calls in
      bond_option_ad_actor_sys_prio_set() and
      bond_option_ad_user_port_key_set()
    * fixed up bond_option_ad_actor_system_set() to support handling both
      string and value setting as Nik suggested

 drivers/net/bonding/bond_netlink.c |   50 ++++++++++++++++++++++++++++++++++++
 drivers/net/bonding/bond_options.c |   30 +++++++++++++++-------
 include/uapi/linux/if_link.h       |    3 +++
 3 files changed, 74 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 7b11243..f7015eb 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -94,6 +94,10 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
 	[IFLA_BOND_AD_LACP_RATE]	= { .type = NLA_U8 },
 	[IFLA_BOND_AD_SELECT]		= { .type = NLA_U8 },
 	[IFLA_BOND_AD_INFO]		= { .type = NLA_NESTED },
+	[IFLA_BOND_AD_ACTOR_SYS_PRIO]	= { .type = NLA_U16 },
+	[IFLA_BOND_AD_USER_PORT_KEY]	= { .type = NLA_U16 },
+	[IFLA_BOND_AD_ACTOR_SYSTEM]	= { .type = NLA_BINARY,
+					    .len  = ETH_ALEN },
 };
 
 static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
@@ -379,6 +383,36 @@ static int bond_changelink(struct net_device *bond_dev,
 		if (err)
 			return err;
 	}
+	if (data[IFLA_BOND_AD_ACTOR_SYS_PRIO]) {
+		int actor_sys_prio =
+			nla_get_u16(data[IFLA_BOND_AD_ACTOR_SYS_PRIO]);
+
+		bond_opt_initval(&newval, actor_sys_prio);
+		err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYS_PRIO, &newval);
+		if (err)
+			return err;
+	}
+
+	if (data[IFLA_BOND_AD_USER_PORT_KEY]) {
+		int port_key =
+			nla_get_u16(data[IFLA_BOND_AD_USER_PORT_KEY]);
+
+		bond_opt_initval(&newval, port_key);
+		err = __bond_opt_set(bond, BOND_OPT_AD_USER_PORT_KEY, &newval);
+		if (err)
+			return err;
+	}
+
+	if (data[IFLA_BOND_AD_ACTOR_SYSTEM]) {
+		if (nla_len(data[IFLA_BOND_AD_ACTOR_SYSTEM]) != ETH_ALEN)
+			return -EINVAL;
+
+		bond_opt_initval(&newval,
+				 nla_get_be64(data[IFLA_BOND_AD_ACTOR_SYSTEM]));
+		err = __bond_opt_set(bond, BOND_OPT_AD_ACTOR_SYSTEM, &newval);
+		if (err)
+			return err;
+	}
 	return 0;
 }
 
@@ -426,6 +460,9 @@ static size_t bond_get_size(const struct net_device *bond_dev)
 		nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_INFO_ACTOR_KEY */
 		nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_INFO_PARTNER_KEY*/
 		nla_total_size(ETH_ALEN) +    /* IFLA_BOND_AD_INFO_PARTNER_MAC*/
+		nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_ACTOR_SYS_PRIO */
+		nla_total_size(sizeof(u16)) + /* IFLA_BOND_AD_USER_PORT_KEY */
+		nla_total_size(ETH_ALEN) + /* IFLA_BOND_AD_ACTOR_SYSTEM */
 		0;
 }
 
@@ -551,6 +588,19 @@ static int bond_fill_info(struct sk_buff *skb,
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
 		struct ad_info info;
 
+		if (nla_put_u16(skb, IFLA_BOND_AD_ACTOR_SYS_PRIO,
+				bond->params.ad_actor_sys_prio))
+			goto nla_put_failure;
+
+		if (nla_put_u16(skb, IFLA_BOND_AD_USER_PORT_KEY,
+				bond->params.ad_user_port_key))
+			goto nla_put_failure;
+
+		if (nla_put(skb, IFLA_BOND_AD_ACTOR_SYSTEM,
+			    sizeof(bond->params.ad_actor_system),
+			    &bond->params.ad_actor_system))
+			goto nla_put_failure;
+
 		if (!bond_3ad_get_active_agg_info(bond, &info)) {
 			struct nlattr *nest;
 
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index c85da05..9a32bbd77 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1394,7 +1394,7 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
 					     const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ad_actor_sys_prio to (%llu)\n",
+	netdev_info(bond->dev, "Setting ad_actor_sys_prio to %llu\n",
 		    newval->value);
 
 	bond->params.ad_actor_sys_prio = newval->value;
@@ -1405,24 +1405,36 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 					   const struct bond_opt_value *newval)
 {
 	u8 macaddr[ETH_ALEN];
+	u8 *mac;
 	int i;
 
-	i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
-		   &macaddr[0], &macaddr[1], &macaddr[2],
-		   &macaddr[3], &macaddr[4], &macaddr[5]);
-	if (i != ETH_ALEN || !is_valid_ether_addr(macaddr)) {
-		netdev_err(bond->dev, "Invalid MAC address.\n");
-		return -EINVAL;
+	if (newval->string) {
+		i = sscanf(newval->string, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+			   &macaddr[0], &macaddr[1], &macaddr[2],
+			   &macaddr[3], &macaddr[4], &macaddr[5]);
+		if (i != ETH_ALEN)
+			goto err;
+		mac = macaddr;
+	} else {
+		mac = (u8 *)&newval->value;
 	}
 
-	ether_addr_copy(bond->params.ad_actor_system, macaddr);
+	if (!is_valid_ether_addr(mac))
+		goto err;
+
+	netdev_info(bond->dev, "Setting ad_actor_system to %pM\n", mac);
+	ether_addr_copy(bond->params.ad_actor_system, mac);
 	return 0;
+
+err:
+	netdev_err(bond->dev, "Invalid MAC address.\n");
+	return -EINVAL;
 }
 
 static int bond_option_ad_user_port_key_set(struct bonding *bond,
 					    const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ad_user_port_key to (%llu)\n",
+	netdev_info(bond->dev, "Setting ad_user_port_key to %llu\n",
 		    newval->value);
 
 	bond->params.ad_user_port_key = newval->value;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d9cd192..6d6e502 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -417,6 +417,9 @@ enum {
 	IFLA_BOND_AD_LACP_RATE,
 	IFLA_BOND_AD_SELECT,
 	IFLA_BOND_AD_INFO,
+	IFLA_BOND_AD_ACTOR_SYS_PRIO,
+	IFLA_BOND_AD_USER_PORT_KEY,
+	IFLA_BOND_AD_ACTOR_SYSTEM,
 	__IFLA_BOND_MAX,
 };
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
                   ` (3 preceding siblings ...)
  2015-05-09  7:01 ` [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
@ 2015-05-09  7:01 ` Jonathan Toppins
  2015-05-21 22:27   ` Stephen Hemminger
  2015-05-11 15:00 ` [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters David Miller
  5 siblings, 1 reply; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-09  7:01 UTC (permalink / raw)
  To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm,
	razor, Stephen Hemminger

This adds support for setting and displaying the following bonding
options:
* ad_user_port_key
* ad_actor_sys_prio
* ad_actor_system

Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
 v2:
    * rebased
 v3:
    * fixed a checkpatch line length warning

 ip/iplink_bond.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index a573f92..2a9783e 100644
--- a/ip/iplink_bond.c
+++ b/ip/iplink_bond.c
@@ -135,6 +135,9 @@ static void print_explain(FILE *f)
 		"                [ packets_per_slave PACKETS_PER_SLAVE ]\n"
 		"                [ lacp_rate LACP_RATE ]\n"
 		"                [ ad_select AD_SELECT ]\n"
+		"                [ ad_user_port_key PORTKEY ]\n"
+		"                [ ad_actor_sys_prio SYSPRIO ]\n"
+		"                [ ad_actor_system LLADDR ]\n"
 		"\n"
 		"BONDMODE := balance-rr|active-backup|balance-xor|broadcast|802.3ad|balance-tlb|balance-alb\n"
 		"ARP_VALIDATE := none|active|backup|all\n"
@@ -158,6 +161,7 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 	__u8 mode, use_carrier, primary_reselect, fail_over_mac;
 	__u8 xmit_hash_policy, num_peer_notif, all_slaves_active;
 	__u8 lacp_rate, ad_select;
+	__u16 ad_user_port_key, ad_actor_sys_prio;
 	__u32 miimon, updelay, downdelay, arp_interval, arp_validate;
 	__u32 arp_all_targets, resend_igmp, min_links, lp_interval;
 	__u32 packets_per_slave;
@@ -344,6 +348,32 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
 			}
 			ad_select = get_index(ad_select_tbl, *argv);
 			addattr8(n, 1024, IFLA_BOND_AD_SELECT, ad_select);
+		} else if (matches(*argv, "ad_user_port_key") == 0) {
+			NEXT_ARG();
+			if (get_u16(&ad_user_port_key, *argv, 0)) {
+				invarg("invalid ad_user_port_key", *argv);
+				return -1;
+			}
+			addattr16(n, 1024, IFLA_BOND_AD_USER_PORT_KEY,
+				  ad_user_port_key);
+		} else if (matches(*argv, "ad_actor_sys_prio") == 0) {
+			NEXT_ARG();
+			if (get_u16(&ad_actor_sys_prio, *argv, 0)) {
+				invarg("invalid ad_actor_sys_prio", *argv);
+				return -1;
+			}
+			addattr16(n, 1024, IFLA_BOND_AD_ACTOR_SYS_PRIO,
+				  ad_actor_sys_prio);
+		} else if (matches(*argv, "ad_actor_system") == 0) {
+			int len;
+			char abuf[32];
+
+			NEXT_ARG();
+			len = ll_addr_a2n(abuf, sizeof(abuf), *argv);
+			if (len < 0)
+				return -1;
+			addattr_l(n, 1024, IFLA_BOND_AD_ACTOR_SYSTEM,
+				  abuf, len);
 		} else if (matches(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -534,6 +564,25 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 				ll_addr_n2a(p, ETH_ALEN, 0, b, sizeof(b)));
 		}
 	}
+
+	if (tb[IFLA_BOND_AD_ACTOR_SYS_PRIO]) {
+		fprintf(f, "ad_actor_sys_prio %u ",
+			rta_getattr_u16(tb[IFLA_BOND_AD_ACTOR_SYS_PRIO]));
+	}
+
+	if (tb[IFLA_BOND_AD_USER_PORT_KEY]) {
+		fprintf(f, "ad_user_port_key %u ",
+			rta_getattr_u16(tb[IFLA_BOND_AD_USER_PORT_KEY]));
+	}
+
+	if (tb[IFLA_BOND_AD_ACTOR_SYSTEM]) {
+		/* We assume the l2 address is an Ethernet MAC address */
+		SPRINT_BUF(b1);
+		fprintf(f, "ad_actor_system %s ",
+			ll_addr_n2a(RTA_DATA(tb[IFLA_BOND_AD_ACTOR_SYSTEM]),
+				    RTA_PAYLOAD(tb[IFLA_BOND_AD_ACTOR_SYSTEM]),
+				    1 /*ARPHDR_ETHER*/, b1, sizeof(b1)));
+	}
 }
 
 static void bond_print_help(struct link_util *lu, int argc, char **argv,
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
  2015-05-09  7:01 ` [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
@ 2015-05-10  7:54   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Aleksandrov @ 2015-05-10  7:54 UTC (permalink / raw)
  To: Jonathan Toppins, netdev, Jay Vosburgh, Veaceslav Falico,
	Andy Gospodarek, shm, David Miller

On 09/05/15 09:01, Jonathan Toppins wrote:
> From: Andy Gospodarek <gospo@cumulusnetworks.com>
>
> Adds netlink support for the following bonding options:
> * BOND_OPT_AD_ACTOR_SYS_PRIO
> * BOND_OPT_AD_ACTOR_SYSTEM
> * BOND_OPT_AD_USER_PORT_KEY
>
> When setting the actor system mac address we assume the netlink message
> contains a binary mac and not a string representation of a mac.
>
> Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>
> [jt: completed the setting side of the netlink attributes]
> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
> ---
>  v2:
>     * rebased
>  v3:
>     * removed parenthesis from around values in the netdev_info calls in
>       bond_option_ad_actor_sys_prio_set() and
>       bond_option_ad_user_port_key_set()
>     * fixed up bond_option_ad_actor_system_set() to support handling both
>       string and value setting as Nik suggested
>
>  drivers/net/bonding/bond_netlink.c |   50 ++++++++++++++++++++++++++++++++++++
>  drivers/net/bonding/bond_options.c |   30 +++++++++++++++-------
>  include/uapi/linux/if_link.h       |    3 +++
>  3 files changed, 74 insertions(+), 9 deletions(-)
>
>

Thank you for re-working this, it actually looks better than I thought it
would. I'm traveling right now and can't test it, but I went over the patch
and it looks good to me.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters
  2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
                   ` (4 preceding siblings ...)
  2015-05-09  7:01 ` [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
@ 2015-05-11 15:00 ` David Miller
  2015-05-11 15:50   ` Jonathan Toppins
  5 siblings, 1 reply; 10+ messages in thread
From: David Miller @ 2015-05-11 15:00 UTC (permalink / raw)
  To: jtoppins; +Cc: netdev, j.vosburgh, vfalico, gospo, shm, razor

From: Jonathan Toppins <jtoppins@cumulusnetworks.com>
Date: Sat,  9 May 2015 00:01:54 -0700

> This is a resubmit of Mahesh's last 3 bonding patches from this series
> (http://marc.info/?l=linux-netdev&m=142432864626179&w=2) with one
> additional kernel patch which adds the netlink bits. I have noted any
> modifications I did to the original patches just above my signoff line.
> Patch 5 is the iproute2 support for these bonding options. All patches
> were coded against the net-next branch of their respective projects.
> 
> v2:
>   * rebased
>   * only send these new parameters via netlink when bond is in mode 4
>   * fixed ad_actor_sys_prio to be 0xFFFF by default even when the bond
>     is initially created in mode 0 and switched to mode 4
> 
> v3:
>   * reverted changes to bond_option_ad_actor_system_set() from v1 in Mahesh's
>     patch "bonding: Allow userspace to set actors' macaddr in an AD-system."
>     Instead implementing all setting in the option specific set function as
>     Nik suggested.

Series applied, thanks.

There are actually 4 patches in this series, so this subject should have
said "0/4" instead of "0/5" :-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters
  2015-05-11 15:00 ` [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters David Miller
@ 2015-05-11 15:50   ` Jonathan Toppins
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-11 15:50 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, j.vosburgh, vfalico, gospo, shm, razor

On 5/11/15 11:00 AM, David Miller wrote:
> From: Jonathan Toppins <jtoppins@cumulusnetworks.com>
> Date: Sat,  9 May 2015 00:01:54 -0700
>
>> This is a resubmit of Mahesh's last 3 bonding patches from this series
>> (http://marc.info/?l=linux-netdev&m=142432864626179&w=2) with one
>> additional kernel patch which adds the netlink bits. I have noted any
>> modifications I did to the original patches just above my signoff line.
>> Patch 5 is the iproute2 support for these bonding options. All patches
>> were coded against the net-next branch of their respective projects.
>>
>> v2:
>>    * rebased
>>    * only send these new parameters via netlink when bond is in mode 4
>>    * fixed ad_actor_sys_prio to be 0xFFFF by default even when the bond
>>      is initially created in mode 0 and switched to mode 4
>>
>> v3:
>>    * reverted changes to bond_option_ad_actor_system_set() from v1 in Mahesh's
>>      patch "bonding: Allow userspace to set actors' macaddr in an AD-system."
>>      Instead implementing all setting in the option specific set function as
>>      Nik suggested.
>
> Series applied, thanks.
>
> There are actually 4 patches in this series, so this subject should have
> said "0/4" instead of "0/5" :-)
>

Thanks David, noted for next time, apologies for confusion.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options
  2015-05-09  7:01 ` [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
@ 2015-05-21 22:27   ` Stephen Hemminger
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2015-05-21 22:27 UTC (permalink / raw)
  To: Jonathan Toppins
  Cc: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, shm, razor

On Sat,  9 May 2015 00:01:59 -0700
Jonathan Toppins <jtoppins@cumulusnetworks.com> wrote:

> This adds support for setting and displaying the following bonding
> options:
> * ad_user_port_key
> * ad_actor_sys_prio
> * ad_actor_system
> 
> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>

Applied to net-next branch

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-05-21 22:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-09  7:01 [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
2015-05-09  7:01 ` [PATCH linux v3 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
2015-05-09  7:01 ` [PATCH linux v3 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system Jonathan Toppins
2015-05-09  7:01 ` [PATCH linux v3 net-next 3/4] bonding: Implement user key part of port_key in an AD system Jonathan Toppins
2015-05-09  7:01 ` [PATCH linux v3 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
2015-05-10  7:54   ` Nikolay Aleksandrov
2015-05-09  7:01 ` [PATCH iproute2 v3 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
2015-05-21 22:27   ` Stephen Hemminger
2015-05-11 15:00 ` [PATCH linux v3 net-next 0/5] add netlink support for new lacp bonding parameters David Miller
2015-05-11 15:50   ` Jonathan Toppins

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).