* [PATCH linux v1 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system
2015-05-01 17:24 [PATCH linux v1 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
@ 2015-05-01 17:24 ` Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system Jonathan Toppins
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-01 17:24 UTC (permalink / raw)
To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
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]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
Documentation/networking/bonding.txt | 9 +++++++++
drivers/net/bonding/bond_3ad.c | 5 ++++-
drivers/net/bonding/bond_main.c | 14 ++++++++++++++
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, 73 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 3a10551..e5bf232 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,18 @@ static int bond_check_params(struct bond_params *params)
fail_over_mac_value = BOND_FOM_NONE;
}
+ if (bond_mode == BOND_MODE_8023AD) {
+ 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 +4475,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 v1 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system.
2015-05-01 17:24 [PATCH linux v1 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
@ 2015-05-01 17:24 ` Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 3/4] bonding: Implement user key part of port_key in an AD system Jonathan Toppins
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-01 17:24 UTC (permalink / raw)
To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
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, also changed
bond_option_ad_actor_system_set to assume a binary mac so it can
be reused in the netlink option set case]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
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 | 21 ++++++++++++++++++
drivers/net/bonding/bond_procfs.c | 6 ++++++
drivers/net/bonding/bond_sysfs.c | 39 ++++++++++++++++++++++++++++++++++
include/net/bond_options.h | 1 +
include/net/bonding.h | 1 +
8 files changed, 87 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 e5bf232..a0de085 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4476,6 +4476,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..978a46a 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,15 @@ 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)
+{
+ if (!is_valid_ether_addr(newval->string)) {
+ netdev_err(bond->dev, "Invalid MAC address.\n");
+ return -EINVAL;
+ }
+
+ ether_addr_copy(bond->params.ad_actor_system, newval->string);
+ 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..5e4c2ea 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -706,6 +706,44 @@ 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 ssize_t bonding_store_ad_actor_system(struct device *d,
+ struct device_attribute *attr,
+ const char *buffer, size_t count)
+{
+ struct bonding *bond = to_bond(d);
+ u8 macaddr[ETH_ALEN];
+ int ret;
+
+ ret = sscanf(buffer, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+ &macaddr[0], &macaddr[1], &macaddr[2],
+ &macaddr[3], &macaddr[4], &macaddr[5]);
+ if (ret != ETH_ALEN) {
+ netdev_err(bond->dev, "Invalid MAC address.\n");
+ return -EINVAL;
+ }
+
+ ret = bond_opt_tryset_rtnl(bond, BOND_OPT_AD_ACTOR_SYSTEM, macaddr);
+ if (!ret)
+ ret = count;
+
+ return ret;
+}
+
+static DEVICE_ATTR(ad_actor_system, S_IRUGO | S_IWUSR,
+ bonding_show_ad_actor_system, bonding_store_ad_actor_system);
+
static struct attribute *per_bond_attrs[] = {
&dev_attr_slaves.attr,
&dev_attr_mode.attr,
@@ -740,6 +778,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 v1 net-next 3/4] bonding: Implement user key part of port_key in an AD system.
2015-05-01 17:24 [PATCH linux v1 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 1/4] bonding: Allow userspace to set actors' system_priority in AD system Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 2/4] bonding: Allow userspace to set actors' macaddr in an AD-system Jonathan Toppins
@ 2015-05-01 17:24 ` Jonathan Toppins
2015-05-01 17:24 ` [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
2015-05-01 17:24 ` [PATCH iproute2 v1 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
4 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-01 17:24 UTC (permalink / raw)
To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
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]
Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
---
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 a0de085..45f1b29 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)
return -EINVAL;
}
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) {
@@ -4477,6 +4486,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 978a46a..dccc432 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 */
@@ -1396,3 +1412,13 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
ether_addr_copy(bond->params.ad_actor_system, newval->string);
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 5e4c2ea..00109dc 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -744,6 +744,20 @@ static ssize_t bonding_store_ad_actor_system(struct device *d,
static DEVICE_ATTR(ad_actor_system, S_IRUGO | S_IWUSR,
bonding_show_ad_actor_system, bonding_store_ad_actor_system);
+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,
@@ -779,6 +793,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 v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
2015-05-01 17:24 [PATCH linux v1 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
` (2 preceding siblings ...)
2015-05-01 17:24 ` [PATCH linux v1 net-next 3/4] bonding: Implement user key part of port_key in an AD system Jonathan Toppins
@ 2015-05-01 17:24 ` Jonathan Toppins
2015-05-01 23:15 ` Mahesh Bandewar
2015-05-01 17:24 ` [PATCH iproute2 v1 net-next] iplink_bond: add support for ad_actor and port_key options Jonathan Toppins
4 siblings, 1 reply; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-01 17:24 UTC (permalink / raw)
To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
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>
---
drivers/net/bonding/bond_netlink.c | 50 ++++++++++++++++++++++++++++++++++++
include/uapi/linux/if_link.h | 3 +++
2 files changed, 53 insertions(+)
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 7b11243..d98f770 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 = MAX_ADDR_LEN },
};
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_initstr(&newval,
+ nla_data(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;
}
@@ -548,6 +585,19 @@ static int bond_fill_info(struct sk_buff *skb,
bond->params.ad_select))
goto nla_put_failure;
+ 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_MODE(bond) == BOND_MODE_8023AD) {
struct ad_info info;
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* Re: [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
2015-05-01 17:24 ` [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
@ 2015-05-01 23:15 ` Mahesh Bandewar
2015-05-04 20:48 ` Jonathan Toppins
0 siblings, 1 reply; 10+ messages in thread
From: Mahesh Bandewar @ 2015-05-01 23:15 UTC (permalink / raw)
To: Jonathan Toppins
Cc: linux-netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller
On Fri, May 1, 2015 at 10:24 AM, Jonathan Toppins
<jtoppins@cumulusnetworks.com> 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>
> ---
> drivers/net/bonding/bond_netlink.c | 50 ++++++++++++++++++++++++++++++++++++
> include/uapi/linux/if_link.h | 3 +++
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> index 7b11243..d98f770 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 = MAX_ADDR_LEN },
> };
>
> 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_initstr(&newval,
> + nla_data(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;
> }
>
> @@ -548,6 +585,19 @@ static int bond_fill_info(struct sk_buff *skb,
> bond->params.ad_select))
> goto nla_put_failure;
>
> + 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;
> +
I think this does not make sense for MODE != 8023AD. Shouldn't this be
inside next block which is for the 802.3ad mode?
> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> struct ad_info info;
>
> 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,
Even though this is available / stored in bond->param, I feel that
these belong to IFLA_BOND_AD_INFO_, no?
> __IFLA_BOND_MAX,
> };
>
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
2015-05-01 23:15 ` Mahesh Bandewar
@ 2015-05-04 20:48 ` Jonathan Toppins
2015-05-06 1:07 ` Jonathan Toppins
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-04 20:48 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: linux-netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller
On 5/1/15 7:15 PM, Mahesh Bandewar wrote:
>>
>> @@ -548,6 +585,19 @@ static int bond_fill_info(struct sk_buff *skb,
>> bond->params.ad_select))
>> goto nla_put_failure;
>>
>> + 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;
>> +
> I think this does not make sense for MODE != 8023AD. Shouldn't this be
> inside next block which is for the 802.3ad mode?
>
>> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>> struct ad_info info;
>>
>> 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,
> Even though this is available / stored in bond->param, I feel that
> these belong to IFLA_BOND_AD_INFO_, no?
Can see it fitting in there. Not sure of the history of the AD_INFO_*
object. Evaluating implementation, will respond tomorrow with conclusion.
>
>> __IFLA_BOND_MAX,
>> };
>>
>> --
>> 1.7.10.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
2015-05-04 20:48 ` Jonathan Toppins
@ 2015-05-06 1:07 ` Jonathan Toppins
2015-05-06 7:07 ` Mahesh Bandewar
0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-06 1:07 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: linux-netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller, Stephen Hemminger
On 5/4/15 4:48 PM, Jonathan Toppins wrote:
> On 5/1/15 7:15 PM, Mahesh Bandewar wrote:
>>>
>>> @@ -548,6 +585,19 @@ static int bond_fill_info(struct sk_buff *skb,
>>> bond->params.ad_select))
>>> goto nla_put_failure;
>>>
>>> + 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;
>>> +
>> I think this does not make sense for MODE != 8023AD. Shouldn't this be
>> inside next block which is for the 802.3ad mode?
Agreed, the kernel should filter attributes not useful for that mode.
Will move the sending of these attributes to be inside the mode check below.
>>
>>> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>>> struct ad_info info;
>>>
>>> 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,
>> Even though this is available / stored in bond->param, I feel that
>> these belong to IFLA_BOND_AD_INFO_, no?
>
> Can see it fitting in there. Not sure of the history of the AD_INFO_*
> object. Evaluating implementation, will respond tomorrow with conclusion.
So I did complete a rough implementation of this in both the kernel and
iproute2 [1][2].
After having implemented the example code it became clear that we should
continue to divide based on write permissions. That being all current
bonding attributes that are writable exist in "IFLA_BOND_*" and all
"IFLA_BOND_AD_INFO*" attributes are read-only. By dividing this way it
makes consumers of the API (f.e. iprotue2) pretty straight forward to
implement in both the write and read cases. Also all IFLA_BOND_AD_INFO
attributes I do not see getting converted to writable, as they are
generated as a result of lacp negotiation with input from these new ad_
attributes, agree?
In the implementation code for iproute2 [2] a "submode" for defining
ad_info attributes was needed to keep the parsing simple. This felt
awkward because all other attributes just need to be listed, with no
special submode beyond the link type.
Summary, will send a v2 with the current implementation and change
bond_fill_info() to only send these new ad attributes if the bond is in
mode 4, will not modify lacp_rate or ad_select at this time. Will post
v2 tomorrow if no strong disagreements.
Thanks!
[1] https://github.com/jtoppins/net-next/tree/ad_actor-patches-v2
[2] https://github.com/jtoppins/iproute2/tree/ad_actor-patches-v2
>
>>
>>> __IFLA_BOND_MAX,
>>> };
>>>
>>> --
>>> 1.7.10.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key
2015-05-06 1:07 ` Jonathan Toppins
@ 2015-05-06 7:07 ` Mahesh Bandewar
0 siblings, 0 replies; 10+ messages in thread
From: Mahesh Bandewar @ 2015-05-06 7:07 UTC (permalink / raw)
To: Jonathan Toppins
Cc: linux-netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
David Miller, Stephen Hemminger
On Tue, May 5, 2015 at 6:07 PM, Jonathan Toppins
<jtoppins@cumulusnetworks.com> wrote:
> On 5/4/15 4:48 PM, Jonathan Toppins wrote:
>>
>> On 5/1/15 7:15 PM, Mahesh Bandewar wrote:
>>>>
>>>>
>>>> @@ -548,6 +585,19 @@ static int bond_fill_info(struct sk_buff *skb,
>>>> bond->params.ad_select))
>>>> goto nla_put_failure;
>>>>
>>>> + 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;
>>>> +
>>>
>>> I think this does not make sense for MODE != 8023AD. Shouldn't this be
>>> inside next block which is for the 802.3ad mode?
>
>
> Agreed, the kernel should filter attributes not useful for that mode. Will
> move the sending of these attributes to be inside the mode check below.
>
>>>
>>>> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
>>>> struct ad_info info;
>>>>
>>>> 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,
>>>
>>> Even though this is available / stored in bond->param, I feel that
>>> these belong to IFLA_BOND_AD_INFO_, no?
>>
>>
>> Can see it fitting in there. Not sure of the history of the AD_INFO_*
>> object. Evaluating implementation, will respond tomorrow with conclusion.
>
>
> So I did complete a rough implementation of this in both the kernel and
> iproute2 [1][2].
>
> After having implemented the example code it became clear that we should
> continue to divide based on write permissions. That being all current
> bonding attributes that are writable exist in "IFLA_BOND_*" and all
> "IFLA_BOND_AD_INFO*" attributes are read-only. By dividing this way it makes
> consumers of the API (f.e. iprotue2) pretty straight forward to implement in
> both the write and read cases. Also all IFLA_BOND_AD_INFO attributes I do
> not see getting converted to writable, as they are generated as a result of
> lacp negotiation with input from these new ad_ attributes, agree?
>
So far what we had was only LACP negotiated values but now that we
have variables that we are setting and are used for LACP negotiations
makes it little different.
I'm fine if the overall command structure makes sense and the
read-values are kept separate from write-values as you have mentioned
and is easier to parse.
> In the implementation code for iproute2 [2] a "submode" for defining ad_info
> attributes was needed to keep the parsing simple. This felt awkward because
> all other attributes just need to be listed, with no special submode beyond
> the link type.
>
> Summary, will send a v2 with the current implementation and change
> bond_fill_info() to only send these new ad attributes if the bond is in mode
> 4, will not modify lacp_rate or ad_select at this time. Will post v2
> tomorrow if no strong disagreements.
>
> Thanks!
>
> [1] https://github.com/jtoppins/net-next/tree/ad_actor-patches-v2
> [2] https://github.com/jtoppins/iproute2/tree/ad_actor-patches-v2
>
>>
>>>
>>>> __IFLA_BOND_MAX,
>>>> };
>>>>
>>>> --
>>>> 1.7.10.4
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH iproute2 v1 net-next] iplink_bond: add support for ad_actor and port_key options
2015-05-01 17:24 [PATCH linux v1 net-next 0/5] add netlink support for new lacp bonding parameters Jonathan Toppins
` (3 preceding siblings ...)
2015-05-01 17:24 ` [PATCH linux v1 net-next 4/4] bonding: add netlink support for sys prio, actor sys mac, and port key Jonathan Toppins
@ 2015-05-01 17:24 ` Jonathan Toppins
4 siblings, 0 replies; 10+ messages in thread
From: Jonathan Toppins @ 2015-05-01 17:24 UTC (permalink / raw)
To: netdev, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
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>
---
ip/iplink_bond.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
index a573f92..989e642 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,31 @@ 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 +563,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