* [PATCH] bonding: add the sysfs interface to see RLB hash table
@ 2010-11-30 10:01 Taku Izumi
2010-11-30 10:10 ` Eric Dumazet
2010-11-30 22:08 ` Stephen Hemminger
0 siblings, 2 replies; 6+ messages in thread
From: Taku Izumi @ 2010-11-30 10:01 UTC (permalink / raw)
To: netdev@vger.kernel.org, Jay Vosburgh
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(+)
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,
};
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bonding: add the sysfs interface to see RLB hash table
2010-11-30 10:01 [PATCH] bonding: add the sysfs interface to see RLB hash table Taku Izumi
@ 2010-11-30 10:10 ` Eric Dumazet
2010-11-30 18:37 ` Jay Vosburgh
2010-12-01 5:03 ` Taku Izumi
2010-11-30 22:08 ` Stephen Hemminger
1 sibling, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2010-11-30 10:10 UTC (permalink / raw)
To: Taku Izumi; +Cc: netdev@vger.kernel.org, Jay Vosburgh
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);
> + }
> +
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bonding: add the sysfs interface to see RLB hash table
2010-11-30 10:10 ` Eric Dumazet
@ 2010-11-30 18:37 ` Jay Vosburgh
2010-12-01 5:09 ` Taku Izumi
2010-12-01 5:03 ` Taku Izumi
1 sibling, 1 reply; 6+ messages in thread
From: Jay Vosburgh @ 2010-11-30 18:37 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Taku Izumi, netdev@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bonding: add the sysfs interface to see RLB hash table
2010-11-30 10:01 [PATCH] bonding: add the sysfs interface to see RLB hash table Taku Izumi
2010-11-30 10:10 ` Eric Dumazet
@ 2010-11-30 22:08 ` Stephen Hemminger
1 sibling, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2010-11-30 22:08 UTC (permalink / raw)
To: Taku Izumi; +Cc: netdev@vger.kernel.org, Jay Vosburgh
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.
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bonding: add the sysfs interface to see RLB hash table
2010-11-30 10:10 ` Eric Dumazet
2010-11-30 18:37 ` Jay Vosburgh
@ 2010-12-01 5:03 ` Taku Izumi
1 sibling, 0 replies; 6+ messages in thread
From: Taku Izumi @ 2010-12-01 5:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev@vger.kernel.org, Jay Vosburgh
(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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bonding: add the sysfs interface to see RLB hash table
2010-11-30 18:37 ` Jay Vosburgh
@ 2010-12-01 5:09 ` Taku Izumi
0 siblings, 0 replies; 6+ messages in thread
From: Taku Izumi @ 2010-12-01 5:09 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Eric Dumazet, netdev@vger.kernel.org, shemminger
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-12-01 5:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-30 10:01 [PATCH] bonding: add the sysfs interface to see RLB hash table Taku Izumi
2010-11-30 10:10 ` Eric Dumazet
2010-11-30 18:37 ` Jay Vosburgh
2010-12-01 5:09 ` Taku Izumi
2010-12-01 5:03 ` Taku Izumi
2010-11-30 22:08 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).