| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20131228214031.GE2447@minipsycho>
Date: Sat, 28 Dec 2013 22:40:31 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Scott Feldman <sfeldma@...ulusnetworks.com>
Cc: vfalico@...hat.com, fubar@...ibm.com, andy@...yhouse.net,
netdev@...r.kernel.org, roopa@...ulusnetworks.com,
shm@...ulusnetworks.com
Subject: Re: [PATCH net-next 4/4] bonding: add active_slaves attribute
Sat, Dec 28, 2013 at 08:16:03AM CET, sfeldma@...ulusnetworks.com wrote:
>Add active_slaves list to sysfs and netlink interfaces.
I do not like this one bit. This information should be exported per-slave.
Not by master device.
I think we should ass sysfs and netlink entries for slaves similar to
how bridge does it for its ports.
Jiri
>
>Signed-off-by: Scott Feldman <sfeldma@...ulusnetworks.com>
>---
> drivers/net/bonding/bond_netlink.c | 34 ++++++++++++++++++++++++++++++-
> drivers/net/bonding/bond_sysfs.c | 39 ++++++++++++++++++++++++++++++++++++
> include/uapi/linux/if_link.h | 1 +
> 3 files changed, 73 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 1b4013b..b111da7 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -46,6 +46,7 @@ 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_ACTIVE_SLAVES] = { .type = NLA_NESTED },
> };
>
> static int bond_validate(struct nlattr *tb[], struct nlattr *data[])
>@@ -288,6 +289,21 @@ static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
>
> static size_t bond_get_size(const struct net_device *bond_dev)
> {
>+ struct bonding *bond = netdev_priv(bond_dev);
>+
>+ int active_slave_count(struct bonding *bond)
>+ {
>+ struct list_head *iter;
>+ struct slave *slave;
>+ int count = 0;
>+
>+ bond_for_each_slave(bond, slave, iter)
>+ if (bond_is_active_slave(slave))
>+ count++;
>+
>+ return count;
>+ }
>+
Not nice. Function nesting in general...
> return nla_total_size(sizeof(u8)) + /* IFLA_BOND_MODE */
> nla_total_size(sizeof(u32)) + /* IFLA_BOND_ACTIVE_SLAVE */
> nla_total_size(sizeof(u32)) + /* IFLA_BOND_MIIMON */
>@@ -317,6 +333,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*/
>+ /* IFLA_BOND_ACTIVE_SLAVES */
>+ nla_total_size(sizeof(struct nlattr)) +
>+ nla_total_size(sizeof(u32)) * active_slave_count(bond) +
> 0;
> }
>
>@@ -325,7 +344,9 @@ static int bond_fill_info(struct sk_buff *skb,
> {
> struct bonding *bond = netdev_priv(bond_dev);
> struct net_device *slave_dev = bond_option_active_slave_get(bond);
>- struct nlattr *targets;
>+ struct nlattr *targets, *active_slaves;
>+ struct list_head *iter;
>+ struct slave *slave;
> unsigned int packets_per_slave;
> int i, targets_added;
>
>@@ -461,6 +482,17 @@ static int bond_fill_info(struct sk_buff *skb,
> }
> }
>
>+ active_slaves = nla_nest_start(skb, IFLA_BOND_ACTIVE_SLAVES);
>+ if (!active_slaves)
>+ goto nla_put_failure;
>+
>+ i = 0;
>+ bond_for_each_slave(bond, slave, iter)
>+ if (bond_is_active_slave(slave))
>+ nla_put_u32(skb, i++, slave->dev->ifindex);
>+
>+ nla_nest_end(skb, active_slaves);
>+
> return 0;
>
> nla_put_failure:
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 9a1ea4a..ad976eb 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -257,6 +257,44 @@ static DEVICE_ATTR(slaves, S_IRUGO | S_IWUSR, bonding_show_slaves,
> bonding_store_slaves);
>
> /*
>+ * Show the active slaves in the current bond.
>+ */
>+static ssize_t bonding_show_active_slaves(struct device *d,
>+ struct device_attribute *attr,
>+ char *buf)
>+{
>+ struct bonding *bond = to_bond(d);
>+ struct list_head *iter;
>+ struct slave *slave;
>+ int res = 0;
>+
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>+
>+ bond_for_each_slave(bond, slave, iter) {
>+ if (!bond_is_active_slave(slave))
>+ continue;
>+ if (res > (PAGE_SIZE - IFNAMSIZ)) {
>+ /* not enough space for another interface name */
>+ if ((PAGE_SIZE - res) > 10)
>+ res = PAGE_SIZE - 10;
>+ res += sprintf(buf + res, "++more++ ");
>+ break;
>+ }
>+ res += sprintf(buf + res, "%s ", slave->dev->name);
>+ }
>+
>+ rtnl_unlock();
>+
>+ if (res)
>+ buf[res-1] = '\n'; /* eat the leftover space */
>+
>+ return res;
>+}
>+
>+static DEVICE_ATTR(active_slaves, S_IRUGO, bonding_show_active_slaves, NULL);
>+
>+/*
> * Show and set the bonding mode. The bond interface must be down to
> * change the mode.
> */
>@@ -1441,6 +1479,7 @@ static struct attribute *per_bond_attrs[] = {
> &dev_attr_min_links.attr,
> &dev_attr_lp_interval.attr,
> &dev_attr_packets_per_slave.attr,
>+ &dev_attr_active_slaves.attr,
> NULL,
> };
>
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index 3e6bd3c..29d1c66 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -352,6 +352,7 @@ enum {
> IFLA_BOND_AD_LACP_RATE,
> IFLA_BOND_AD_SELECT,
> IFLA_BOND_AD_INFO,
>+ IFLA_BOND_ACTIVE_SLAVES,
> __IFLA_BOND_MAX,
> };
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists