netdev - Re: [patch net-next-2.6 3/4] bond: implement slave management operations

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 11 Feb 2011 09:19:50 -0800
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Jiri Pirko <jpirko@...hat.com>
cc:	netdev@...r.kernel.org, davem@...emloft.net,
	shemminger@...ux-foundation.org, kaber@...sh.net
Subject: Re: [patch net-next-2.6 3/4] bond: implement slave management operations

Jiri Pirko <jpirko@...hat.com> wrote:

>Signed-off-by: Jiri Pirko <jpirko@...hat.com>
>---
> drivers/net/bonding/bond_main.c |   38 ++++++++++++++++++++++++++++++++++++++
> 1 files changed, 38 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 1df9f0e..f8e59f9 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c

	I think this would be better served by a new bond_netlink.c
file instead of cramming this into (the already huge) bond_main.c.  In
the long run, there will be a lot more netlink related code in bonding,
so I think it makes sense to give it a file of its own from the
beginning.

>@@ -4285,6 +4285,40 @@ unwind:
> 	return res;
> }
>
>+static int bond_add_slave(struct net_device *bond_dev,
>+			  struct net_device *slave_dev)
>+{
>+	return bond_enslave(bond_dev, slave_dev);
>+}
>+
>+static int bond_del_slave(struct net_device *bond_dev,
>+			  struct net_device *slave_dev)
>+{
>+	return bond_release(bond_dev, slave_dev);
>+}
>+
>+static int bond_get_slave_count(const struct net_device *bond_dev)
>+{
>+	struct bonding *bond = netdev_priv(bond_dev);
>+
>+	return bond->slave_cnt;
>+}
>+
>+static struct net_device *bond_get_slave(const struct net_device *bond_dev,
>+					 int slave_index)
>+{
>+	struct bonding *bond = netdev_priv(bond_dev);
>+	struct slave *slave;
>+	int i;
>+
>+	/* no need to hold bond->lock here, protected against writers by rtnl */
>+	bond_for_each_slave(bond, slave, i) {
>+		if (slave_index == i)
>+			return slave->dev;
>+	}
>+	return NULL;

	I think using the name "slave_index" for this variable is
confusing, since it isn't the ifindex of the slave.  This "index" is
used to iterate through the list of slaves, so perhaps "slave_num" or
"slave_position" is clearer.  The same comment applies to the equivalent
code for bridge.

	-J

>+}
>+
> static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev)
> {
> 	struct bonding *bond = netdev_priv(bond_dev);
>@@ -4657,6 +4691,10 @@ static const struct net_device_ops bond_netdev_ops = {
> 	.ndo_netpoll_cleanup	= bond_netpoll_cleanup,
> 	.ndo_poll_controller	= bond_poll_controller,
> #endif
>+	.ndo_add_slave		= bond_add_slave,
>+	.ndo_del_slave		= bond_del_slave,
>+	.ndo_get_slave_count	= bond_get_slave_count,
>+	.ndo_get_slave		= bond_get_slave,
> };
>
> static void bond_destructor(struct net_device *bond_dev)
>-- 
>1.7.3.4
>

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@...ibm.com
--
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