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
| ||
|
Date: Fri, 11 Feb 2011 18:51:40 +0100 From: Jiri Pirko <jpirko@...hat.com> To: Jay Vosburgh <fubar@...ibm.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 Fri, Feb 11, 2011 at 06:19:50PM CET, fubar@...ibm.com wrote: >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. Well technically is not netlink code so givin it into *netlink* file would imho make no sense. > >>@@ -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
Powered by blists - more mailing lists