| Message ID | 4CF4CB85.4010708@jp.fujitsu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Le mardi 30 novembre 2010 à 19:01 +0900, Taku Izumi a écrit : > This patch provides the sysfs interface to see RLB hash table > like the following: > > # cat /sys/class/net/bond0/bonding/rlb_hash_table > > SourceIP DestinationIP Destination MAC DEV > 10.124.196.205 10.124.196. 81 00:19:99:XX:XX:XX eth3 > 10.124.196.205 10.124.196.222 00:0a:79:XX:XX:XX eth0 > 10.124.196.205 10.124.196. 75 00:15:17:XX:XX:XX eth4 > 10.124.196.205 10.124.196. 1 00:21:d8:XX:XX:XX eth3 > 10.124.196.205 10.124.196.205 ff:ff:ff:ff:ff:ff eth0 > why spaces in IP addresses ? > > This is helpful to check if the receive load balancing works as expected. > > Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> > > --- > drivers/net/bonding/bond_sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > Index: net-next/drivers/net/bonding/bond_sysfs.c > =================================================================== > --- net-next.orig/drivers/net/bonding/bond_sysfs.c > +++ net-next/drivers/net/bonding/bond_sysfs.c > @@ -43,6 +43,7 @@ > #include <linux/nsproxy.h> > > #include "bonding.h" > +#include "bond_alb.h" > > #define to_dev(obj) container_of(obj, struct device, kobj) > #define to_bond(cd) ((struct bonding *)(netdev_priv(to_net_dev(cd)))) > @@ -1643,6 +1644,60 @@ out: > static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR, > bonding_show_resend_igmp, bonding_store_resend_igmp); > > +/* > + * Show RLB hash table > + */ > +#define RLB_NULL_INDEX 0xffffffff > +static ssize_t bonding_show_rlb_hashtable(struct device *d, > + struct device_attribute *attr, > + char *buf) > +{ > + int count = 0; > + struct bonding *bond = to_bond(d); > + struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); > + struct rlb_client_info *client_info; > + u32 hash_index; > + > + if (bond->params.mode != BOND_MODE_ALB) > + return count; > + > + count += sprintf(buf + count, "SourceIP " > + "DestinationIP Destination MAC DEV\n"); > + > + spin_lock_bh(&(BOND_ALB_INFO(bond).rx_hashtbl_lock)); > + > + hash_index = bond_info->rx_hashtbl_head; > + for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) { > + client_info = &(bond_info->rx_hashtbl[hash_index]); > + > + count += sprintf(buf + count, > + "%3d.%3d.%3d.%3d %3d.%3d.%3d.%3d " > + "%02x:%02x:%02x:%02x:%02x:%02x %s\n", Oh well, I guess you dont read Joe patches on netdev ;) Please take a look at %pI4 and %pM sprintf(buf + count, "%pI4 %pI4 %pM %s\n", ...) > + client_info->ip_src & 0xff, > + (client_info->ip_src >> 8) & 0xff, > + (client_info->ip_src >> 16) & 0xff, > + (client_info->ip_src >> 24) & 0xff, > + client_info->ip_dst & 0xff, > + (client_info->ip_dst >> 8) & 0xff, > + (client_info->ip_dst >> 16) & 0xff, > + (client_info->ip_dst >> 24) & 0xff, > + client_info->mac_dst[0], > + client_info->mac_dst[1], > + client_info->mac_dst[2], > + client_info->mac_dst[3], > + client_info->mac_dst[4], > + client_info->mac_dst[5], > + client_info->slave->dev->name); > + } > + -- 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
Eric Dumazet <eric.dumazet@gmail.com> wrote: >Le mardi 30 novembre 2010 à 19:01 +0900, Taku Izumi a écrit : >> This patch provides the sysfs interface to see RLB hash table >> like the following: >> >> # cat /sys/class/net/bond0/bonding/rlb_hash_table >> >> SourceIP DestinationIP Destination MAC DEV >> 10.124.196.205 10.124.196. 81 00:19:99:XX:XX:XX eth3 >> 10.124.196.205 10.124.196.222 00:0a:79:XX:XX:XX eth0 >> 10.124.196.205 10.124.196. 75 00:15:17:XX:XX:XX eth4 >> 10.124.196.205 10.124.196. 1 00:21:d8:XX:XX:XX eth3 >> 10.124.196.205 10.124.196.205 ff:ff:ff:ff:ff:ff eth0 I'm reasonably sure something like this isn't going to be acceptable in sysfs (it's much too large). In the proc file that bonding already uses, this type of information isn't unreasonable, but I don't think that is the best place for this, for two reasons. First, the table may have up to 256 entries. Therefore, a sufficiently populated table will easily overrun the one page of space available to a sysfs show function or a proc seq_printf (per iteration), so it will have to handle that. The current code in bonding to do its proc file already iterates over the slaves; adding another iteration loop to handle this table seems overly complicated. A well populated table would also make the current proc file's output rather verbose, particularly if the TLB table is added later. Second, it would have to hold the hash table spin lock, which may provide an easy way to mess with bonding (user space doing "while 1 cat rlb_hash_table > /dev/null"). Therefore, I'd suggest this go into debugfs somewhere, perhaps a /sys/kernel/debug/bonding/rlb_hash_table (perhaps with a tlb_hash_table as the logical pairing for the TX side), readable only by root. Alternatively, if there are objections to using debufs, a new file in /proc/net/bonding could be used, although that seems cumbersome (because it would have to be named to avoid conflicts, e.g., /proc/net/bonding/bond0_rlb_hash_table). >why spaces in IP addresses ? > >> >> This is helpful to check if the receive load balancing works as expected. >> >> Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> >> >> --- >> drivers/net/bonding/bond_sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) >> >> Index: net-next/drivers/net/bonding/bond_sysfs.c >> =================================================================== >> --- net-next.orig/drivers/net/bonding/bond_sysfs.c >> +++ net-next/drivers/net/bonding/bond_sysfs.c >> @@ -43,6 +43,7 @@ >> #include <linux/nsproxy.h> >> >> #include "bonding.h" >> +#include "bond_alb.h" >> >> #define to_dev(obj) container_of(obj, struct device, kobj) >> #define to_bond(cd) ((struct bonding *)(netdev_priv(to_net_dev(cd)))) >> @@ -1643,6 +1644,60 @@ out: >> static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR, >> bonding_show_resend_igmp, bonding_store_resend_igmp); >> >> +/* >> + * Show RLB hash table >> + */ >> +#define RLB_NULL_INDEX 0xffffffff >> +static ssize_t bonding_show_rlb_hashtable(struct device *d, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + int count = 0; >> + struct bonding *bond = to_bond(d); >> + struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); >> + struct rlb_client_info *client_info; >> + u32 hash_index; >> + >> + if (bond->params.mode != BOND_MODE_ALB) >> + return count; >> + >> + count += sprintf(buf + count, "SourceIP " >> + "DestinationIP Destination MAC DEV\n"); >> + >> + spin_lock_bh(&(BOND_ALB_INFO(bond).rx_hashtbl_lock)); >> + >> + hash_index = bond_info->rx_hashtbl_head; >> + for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) { >> + client_info = &(bond_info->rx_hashtbl[hash_index]); >> + >> + count += sprintf(buf + count, >> + "%3d.%3d.%3d.%3d %3d.%3d.%3d.%3d " >> + "%02x:%02x:%02x:%02x:%02x:%02x %s\n", > > >Oh well, I guess you dont read Joe patches on netdev ;) > >Please take a look at %pI4 and %pM Agreed. -J >sprintf(buf + count, "%pI4 %pI4 %pM %s\n", ...) > > >> + client_info->ip_src & 0xff, >> + (client_info->ip_src >> 8) & 0xff, >> + (client_info->ip_src >> 16) & 0xff, >> + (client_info->ip_src >> 24) & 0xff, >> + client_info->ip_dst & 0xff, >> + (client_info->ip_dst >> 8) & 0xff, >> + (client_info->ip_dst >> 16) & 0xff, >> + (client_info->ip_dst >> 24) & 0xff, >> + client_info->mac_dst[0], >> + client_info->mac_dst[1], >> + client_info->mac_dst[2], >> + client_info->mac_dst[3], >> + client_info->mac_dst[4], >> + client_info->mac_dst[5], >> + client_info->slave->dev->name); >> + } >> + > --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- 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
On Tue, 30 Nov 2010 19:01:41 +0900 Taku Izumi <izumi.taku@jp.fujitsu.com> wrote: > # cat /sys/class/net/bond0/bonding/rlb_hash_table > > SourceIP DestinationIP Destination MAC DEV > 10.124.196.205 10.124.196. 81 00:19:99:XX:XX:XX eth3 > 10.124.196.205 10.124.196.222 00:0a:79:XX:XX:XX eth0 > 10.124.196.205 10.124.196. 75 00:15:17:XX:XX:XX eth4 > 10.124.196.205 10.124.196. 1 00:21:d8:XX:XX:XX eth3 > 10.124.196.205 10.124.196.205 ff:ff:ff:ff:ff:ff eth0 This violates the one value per file convention of sysfs. It belongs in /proc. Unfortunately there already is a /proc/net/bonding/bond0 but it is a file not a directory.
(2010/11/30 19:10), Eric Dumazet wrote: > Le mardi 30 novembre 2010 à 19:01 +0900, Taku Izumi a écrit : >> + hash_index = bond_info->rx_hashtbl_head; >> + for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) { >> + client_info =&(bond_info->rx_hashtbl[hash_index]); >> + >> + count += sprintf(buf + count, >> + "%3d.%3d.%3d.%3d %3d.%3d.%3d.%3d " >> + "%02x:%02x:%02x:%02x:%02x:%02x %s\n", > > > Oh well, I guess you dont read Joe patches on netdev ;) > > Please take a look at %pI4 and %pM > > sprintf(buf + count, "%pI4 %pI4 %pM %s\n", ...) Thank you for your advice. I've become a little wiser.. Taku Izumi <izumi.taku@jp.fujitsu.com> -- 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
Dear Jay Volburgh and Stephen Hemminger: (2010/12/01 3:37), Jay Vosburgh wrote: > Eric Dumazet<eric.dumazet@gmail.com> wrote: > >> Le mardi 30 novembre 2010 à 19:01 +0900, Taku Izumi a écrit : >>> This patch provides the sysfs interface to see RLB hash table >>> like the following: >>> >>> # cat /sys/class/net/bond0/bonding/rlb_hash_table >>> >>> SourceIP DestinationIP Destination MAC DEV >>> 10.124.196.205 10.124.196. 81 00:19:99:XX:XX:XX eth3 >>> 10.124.196.205 10.124.196.222 00:0a:79:XX:XX:XX eth0 >>> 10.124.196.205 10.124.196. 75 00:15:17:XX:XX:XX eth4 >>> 10.124.196.205 10.124.196. 1 00:21:d8:XX:XX:XX eth3 >>> 10.124.196.205 10.124.196.205 ff:ff:ff:ff:ff:ff eth0 > > I'm reasonably sure something like this isn't going to be > acceptable in sysfs (it's much too large). > > In the proc file that bonding already uses, this type of > information isn't unreasonable, but I don't think that is the best place > for this, for two reasons. > > First, the table may have up to 256 entries. Therefore, a > sufficiently populated table will easily overrun the one page of space > available to a sysfs show function or a proc seq_printf (per iteration), > so it will have to handle that. The current code in bonding to do its > proc file already iterates over the slaves; adding another iteration > loop to handle this table seems overly complicated. A well populated > table would also make the current proc file's output rather verbose, > particularly if the TLB table is added later. > > Second, it would have to hold the hash table spin lock, which > may provide an easy way to mess with bonding (user space doing "while 1 > cat rlb_hash_table> /dev/null"). > > Therefore, I'd suggest this go into debugfs somewhere, perhaps a > /sys/kernel/debug/bonding/rlb_hash_table (perhaps with a tlb_hash_table > as the logical pairing for the TX side), readable only by root. > > Alternatively, if there are objections to using debufs, a new > file in /proc/net/bonding could be used, although that seems cumbersome > (because it would have to be named to avoid conflicts, e.g., > /proc/net/bonding/bond0_rlb_hash_table). > I understand the sysfs is not the proper place. I have no objection to using debugfs. I'll try to rewrite my patch. Taku Izumi <izumi.taku@jp.fujitsu.com> -- 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
Index: net-next/drivers/net/bonding/bond_sysfs.c =================================================================== --- net-next.orig/drivers/net/bonding/bond_sysfs.c +++ net-next/drivers/net/bonding/bond_sysfs.c @@ -43,6 +43,7 @@ #include <linux/nsproxy.h> #include "bonding.h" +#include "bond_alb.h" #define to_dev(obj) container_of(obj, struct device, kobj) #define to_bond(cd) ((struct bonding *)(netdev_priv(to_net_dev(cd)))) @@ -1643,6 +1644,60 @@ out: static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR, bonding_show_resend_igmp, bonding_store_resend_igmp); +/* + * Show RLB hash table + */ +#define RLB_NULL_INDEX 0xffffffff +static ssize_t bonding_show_rlb_hashtable(struct device *d, + struct device_attribute *attr, + char *buf) +{ + int count = 0; + struct bonding *bond = to_bond(d); + struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond)); + struct rlb_client_info *client_info; + u32 hash_index; + + if (bond->params.mode != BOND_MODE_ALB) + return count; + + count += sprintf(buf + count, "SourceIP " + "DestinationIP Destination MAC DEV\n"); + + spin_lock_bh(&(BOND_ALB_INFO(bond).rx_hashtbl_lock)); + + hash_index = bond_info->rx_hashtbl_head; + for (; hash_index != RLB_NULL_INDEX; hash_index = client_info->next) { + client_info = &(bond_info->rx_hashtbl[hash_index]); + + count += sprintf(buf + count, + "%3d.%3d.%3d.%3d %3d.%3d.%3d.%3d " + "%02x:%02x:%02x:%02x:%02x:%02x %s\n", + client_info->ip_src & 0xff, + (client_info->ip_src >> 8) & 0xff, + (client_info->ip_src >> 16) & 0xff, + (client_info->ip_src >> 24) & 0xff, + client_info->ip_dst & 0xff, + (client_info->ip_dst >> 8) & 0xff, + (client_info->ip_dst >> 16) & 0xff, + (client_info->ip_dst >> 24) & 0xff, + client_info->mac_dst[0], + client_info->mac_dst[1], + client_info->mac_dst[2], + client_info->mac_dst[3], + client_info->mac_dst[4], + client_info->mac_dst[5], + client_info->slave->dev->name); + } + + spin_unlock_bh(&(BOND_ALB_INFO(bond).rx_hashtbl_lock)); + + return count; + +} +static DEVICE_ATTR(rlb_hash_table, S_IRUGO, bonding_show_rlb_hashtable, NULL); + + static struct attribute *per_bond_attrs[] = { &dev_attr_slaves.attr, &dev_attr_mode.attr, @@ -1671,6 +1726,7 @@ static struct attribute *per_bond_attrs[ &dev_attr_queue_id.attr, &dev_attr_all_slaves_active.attr, &dev_attr_resend_igmp.attr, + &dev_attr_rlb_hash_table.attr, NULL, };
This patch provides the sysfs interface to see RLB hash table like the following: # cat /sys/class/net/bond0/bonding/rlb_hash_table SourceIP DestinationIP Destination MAC DEV 10.124.196.205 10.124.196. 81 00:19:99:XX:XX:XX eth3 10.124.196.205 10.124.196.222 00:0a:79:XX:XX:XX eth0 10.124.196.205 10.124.196. 75 00:15:17:XX:XX:XX eth4 10.124.196.205 10.124.196. 1 00:21:d8:XX:XX:XX eth3 10.124.196.205 10.124.196.205 ff:ff:ff:ff:ff:ff eth0 This is helpful to check if the receive load balancing works as expected. Signed-off-by: Taku Izumi <izumi.taku@jp.fujitsu.com> --- drivers/net/bonding/bond_sysfs.c | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) -- 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