[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <52D79298.3010608@huawei.com>
Date: Thu, 16 Jan 2014 16:04:40 +0800
From: Ding Tianhong <dingtianhong@...wei.com>
To: Scott Feldman <sfeldma@...ulusnetworks.com>, <vfalico@...hat.com>,
<fubar@...ibm.com>, <andy@...yhouse.net>
CC: <netdev@...r.kernel.org>, <roopa@...ulusnetworks.com>,
<shm@...ulusnetworks.com>
Subject: Re: [PATCH net-next 1/2] bonding: add sysfs /slave dir for bond slave
devices.
On 2014/1/16 13:54, Scott Feldman wrote:
> Add sub-directory under /sys/class/net/<interface>/slave with
> read-only attributes for slave. Directory only appears when
> <interface> is a slave.
>
> $ tree /sys/class/net/eth2/slave/
> /sys/class/net/eth2/slave/
> ├── ad_aggregator_id
> ├── link_failure_count
> ├── mii_status
> ├── perm_hwaddr
> ├── queue_id
> └── state
>
> $ cat /sys/class/net/eth2/slave/*
> 2
> 0
> up
> 40:02:10:ef:06:01
> 0
> active
>
> Signed-off-by: Scott Feldman <sfeldma@...ulusnetworks.com>
> ---
> drivers/net/bonding/Makefile | 2
> drivers/net/bonding/bond_main.c | 27 ++++++
> drivers/net/bonding/bond_procfs.c | 12 ---
> drivers/net/bonding/bond_sysfs_slave.c | 142 ++++++++++++++++++++++++++++++++
> drivers/net/bonding/bonding.h | 4 +
> 5 files changed, 174 insertions(+), 13 deletions(-)
> create mode 100644 drivers/net/bonding/bond_sysfs_slave.c
>
> diff --git a/drivers/net/bonding/Makefile b/drivers/net/bonding/Makefile
> index 5a5d720..6f4e808 100644
> --- a/drivers/net/bonding/Makefile
> +++ b/drivers/net/bonding/Makefile
> @@ -4,7 +4,7 @@
>
> obj-$(CONFIG_BONDING) += bonding.o
>
> -bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_debugfs.o bond_netlink.o bond_options.o
> +bonding-objs := bond_main.o bond_3ad.o bond_alb.o bond_sysfs.o bond_sysfs_slave.o bond_debugfs.o bond_netlink.o bond_options.o
>
> proc-$(CONFIG_PROC_FS) += bond_procfs.o
> bonding-objs += $(proc-y)
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index f2fe6cb..4f1adae 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -466,6 +466,22 @@ static void bond_update_speed_duplex(struct slave *slave)
> return;
> }
>
> +const char *bond_slave_link_status(s8 link)
> +{
> + switch (link) {
> + case BOND_LINK_UP:
> + return "up";
> + case BOND_LINK_FAIL:
> + return "going down";
> + case BOND_LINK_DOWN:
> + return "down";
> + case BOND_LINK_BACK:
> + return "going back";
> + default:
> + return "unknown";
> + }
> +}
> +
> /*
> * if <dev> supports MII link status reporting, check its link status.
> *
> @@ -1576,6 +1592,12 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> goto err_unregister;
> }
>
> + res = bond_sysfs_slave_add(new_slave);
> + if (res) {
> + pr_debug("Error %d calling bond_sysfs_slave_add\n", res);
> + goto err_upper_unlink;
> + }
> +
> bond->slave_cnt++;
> bond_compute_features(bond);
> bond_set_carrier(bond);
> @@ -1595,6 +1617,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
> return 0;
>
> /* Undo stages on error */
> +err_upper_unlink:
> + bond_upper_dev_unlink(bond_dev, slave_dev);
> +
> err_unregister:
> netdev_rx_handler_unregister(slave_dev);
>
> @@ -1687,6 +1712,8 @@ static int __bond_release_one(struct net_device *bond_dev,
> /* release the slave from its bond */
> bond->slave_cnt--;
>
> + bond_sysfs_slave_del(slave);
> +
> bond_upper_dev_unlink(bond_dev, slave_dev);
> /* unregister rx_handler early so bond_handle_frame wouldn't be called
> * for this slave anymore.
> diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
> index fb868d6..8515b344 100644
> --- a/drivers/net/bonding/bond_procfs.c
> +++ b/drivers/net/bonding/bond_procfs.c
> @@ -159,18 +159,6 @@ static void bond_info_show_master(struct seq_file *seq)
> }
> }
>
> -static const char *bond_slave_link_status(s8 link)
> -{
> - static const char * const status[] = {
> - [BOND_LINK_UP] = "up",
> - [BOND_LINK_FAIL] = "going down",
> - [BOND_LINK_DOWN] = "down",
> - [BOND_LINK_BACK] = "going back",
> - };
> -
> - return status[link];
> -}
> -
> static void bond_info_show_slave(struct seq_file *seq,
> const struct slave *slave)
> {
> diff --git a/drivers/net/bonding/bond_sysfs_slave.c b/drivers/net/bonding/bond_sysfs_slave.c
> new file mode 100644
> index 0000000..28390af
> --- /dev/null
> +++ b/drivers/net/bonding/bond_sysfs_slave.c
> @@ -0,0 +1,142 @@
> +/* Sysfs attributes of bond slaves
> + *
> + * Copyright (c) 2014 Scott Feldman <sfeldma@...ulusnetworks.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#include <linux/capability.h>
> +#include <linux/kernel.h>
> +#include <linux/netdevice.h>
> +
> +#include "bonding.h"
> +
> +struct slave_attribute {
> + struct attribute attr;
> + ssize_t (*show)(struct slave *, char *);
> +};
> +
> +#define SLAVE_ATTR(_name, _mode, _show) \
> +const struct slave_attribute slave_attr_##_name = { \
> + .attr = {.name = __stringify(_name), \
> + .mode = _mode }, \
> + .show = _show, \
> +};
> +#define SLAVE_ATTR_RO(_name) \
> + SLAVE_ATTR(_name, S_IRUGO, _name##_show)
> +
> +static ssize_t state_show(struct slave *slave, char *buf)
> +{
> + switch (bond_slave_state(slave)) {
> + case BOND_STATE_ACTIVE:
> + return sprintf(buf, "active\n");
> + case BOND_STATE_BACKUP:
> + return sprintf(buf, "backup\n");
> + default:
> + return sprintf(buf, "UNKONWN\n");
> + }
> +}
> +static SLAVE_ATTR_RO(state);
> +
> +static ssize_t mii_status_show(struct slave *slave, char *buf)
> +{
> + return sprintf(buf, "%s\n", bond_slave_link_status(slave->link));
> +}
> +static SLAVE_ATTR_RO(mii_status);
> +
> +static ssize_t link_failure_count_show(struct slave *slave, char *buf)
> +{
> + return sprintf(buf, "%d\n", slave->link_failure_count);
> +}
> +static SLAVE_ATTR_RO(link_failure_count);
> +
> +static ssize_t perm_hwaddr_show(struct slave *slave, char *buf)
> +{
> + return sprintf(buf, "%pM\n", slave->perm_hwaddr);
> +}
> +static SLAVE_ATTR_RO(perm_hwaddr);
> +
> +static ssize_t queue_id_show(struct slave *slave, char *buf)
> +{
> + return sprintf(buf, "%d\n", slave->queue_id);
> +}
> +static SLAVE_ATTR_RO(queue_id);
> +
> +static ssize_t ad_aggregator_id_show(struct slave *slave, char *buf)
> +{
> + const struct aggregator *agg;
> +
> + if (slave->bond->params.mode == BOND_MODE_8023AD) {
> + agg = SLAVE_AD_INFO(slave).port.aggregator;
> + if (agg)
> + return sprintf(buf, "%d\n",
> + agg->aggregator_identifier);
> + }
> +
> + return sprintf(buf, "N/A\n");
> +}
> +static SLAVE_ATTR_RO(ad_aggregator_id);
> +
> +static const struct slave_attribute *slave_attrs[] = {
> + &slave_attr_state,
> + &slave_attr_mii_status,
> + &slave_attr_link_failure_count,
> + &slave_attr_perm_hwaddr,
> + &slave_attr_queue_id,
> + &slave_attr_ad_aggregator_id,
> + NULL
> +};
> +
> +#define to_slave_attr(_at) container_of(_at, struct slave_attribute, attr)
> +#define to_slave(obj) container_of(obj, struct slave, kobj)
> +
> +static ssize_t slave_show(struct kobject *kobj,
> + struct attribute *attr, char *buf)
> +{
> + struct slave_attribute *slave_attr = to_slave_attr(attr);
> + struct slave *slave = to_slave(kobj);
> +
> + return slave_attr->show(slave, buf);
> +}
> +
> +const struct sysfs_ops slave_sysfs_ops = {
> + .show = slave_show,
> +};
> +
> +static struct kobj_type slave_ktype = {
> +#ifdef CONFIG_SYSFS
> + .sysfs_ops = &slave_sysfs_ops,
> +#endif
> +};
> +
> +int bond_sysfs_slave_add(struct slave *slave)
> +{
> + const struct slave_attribute **a;
> + int err;
> +
> + err = kobject_init_and_add(&slave->kobj, &slave_ktype,
> + &(slave->dev->dev.kobj), "slave");
> + if (err)
> + return err;
> +
> + for (a = slave_attrs; *a; ++a) {
> + err = sysfs_create_file(&slave->kobj, &((*a)->attr));
> + if (err)
> + return err;
> + }
> +
I think if add rollback path if create failed, it will be better.
Regards
Ding
> + return 0;
> +}
> +
> +void bond_sysfs_slave_del(struct slave *slave)
> +{
> + const struct slave_attribute **a;
> +
> + for (a = slave_attrs; *a; ++a)
> + sysfs_remove_file(&slave->kobj, &((*a)->attr));
> +
> + kobject_del(&slave->kobj);
> +}
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 955dc48..309757d 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -203,6 +203,7 @@ struct slave {
> #ifdef CONFIG_NET_POLL_CONTROLLER
> struct netpoll *np;
> #endif
> + struct kobject kobj;
> };
>
> /*
> @@ -421,6 +422,8 @@ int bond_create(struct net *net, const char *name);
> int bond_create_sysfs(struct bond_net *net);
> void bond_destroy_sysfs(struct bond_net *net);
> void bond_prepare_sysfs_group(struct bonding *bond);
> +int bond_sysfs_slave_add(struct slave *slave);
> +void bond_sysfs_slave_del(struct slave *slave);
> int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev);
> int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
> int bond_xmit_hash(struct bonding *bond, struct sk_buff *skb, int count);
> @@ -469,6 +472,7 @@ int bond_option_lacp_rate_set(struct bonding *bond, int lacp_rate);
> int bond_option_ad_select_set(struct bonding *bond, int ad_select);
> struct net_device *bond_option_active_slave_get_rcu(struct bonding *bond);
> struct net_device *bond_option_active_slave_get(struct bonding *bond);
> +const char *bond_slave_link_status(s8 link);
>
> struct bond_net {
> struct net * net; /* Associated network namespace */
>
> --
> 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
>
> .
>
--
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