[PATCH] convering bonding driver /proc interface to seq_file

[PATCH] convering bonding driver /proc interface to seq_file

[Stephen Hemminger]

This patch converts /proc/net/bondXX/info to the seq_file interface;
patch is against 2.6.0-test3; but seq_file has been backported to latest
2.4 as well.

It also fixes a bug with multiple bond devices.  The original code
mushes all the bond device status information together.  In other words,
/proc/net/bond0/info gives the same output as /proc/net/bond1/info.

I fixed this in this version, by stuffing a link back to the bonding
data structure in the created proc_dir_entry; which is then used
to only pickup the slaves for that pseudo-device.

Don't think anyone ever used more that one device, because they
probably would have noticed this.

The name "/proc/net/bond0/info" is kind of unconventional, but now
changing that in a almost stable release would be bad.  Better choice
would be "/proc/net/bonding/bond0".

--- linux-2.5/drivers/net/bonding/bond_main.c	2003-08-20 11:19:42.000000000 -0700
+++ linux-2.5-net/drivers/net/bonding/bond_main.c	2003-08-21 16:42:57.130509618 -0700
@@ -402,6 +402,8 @@
 #include <linux/inetdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/skbuff.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
 #include <net/sock.h>
 #include <linux/rtnetlink.h>
 
@@ -545,14 +547,6 @@
 static int bond_release_all(struct net_device *master);
 static int bond_sethwaddr(struct net_device *master, struct net_device *slave);
 
-/*
- * bond_get_info is the interface into the /proc filesystem.  This is
- * a different interface than the BOND_INFO_QUERY ioctl.  That is done
- * through the generic networking ioctl interface, and bond_info_query
- * is the internal function which provides that information.
- */
-static int bond_get_info(char *buf, char **start, off_t offset, int length);
-
 /* Caller must hold bond->ptrlock for write */
 static inline struct slave*
 bond_assign_current_slave(struct bonding *bond,struct slave *newslave)
@@ -3327,133 +3321,195 @@
 	return stats;
 }
 
-static int bond_get_info(char *buf, char **start, off_t offset, int length)
-{
-	bonding_t *bond;
-	int len = 0;
-	off_t begin = 0;
-	u16 link;
-	slave_t *slave = NULL;
+#ifdef CONFIG_PROC_FS
 
-	len += sprintf(buf + len, "%s\n", version);
+#define BOND_PROC_HEADER ((void *)1)	/* magic pointer for header line */
+
+static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	bonding_t *bond = seq->private;
+	loff_t off = 0;
+	slave_t *slave;
 
 	read_lock(&dev_base_lock);
-	list_for_each_entry(bond, &bond_dev_list, bond_list) {
-		/*
-		 * This function locks the mutex, so we can't lock it until 
-		 * afterwards
-		 */
-		link = bond_check_mii_link(bond);
 
-		len += sprintf(buf + len, "Bonding Mode: %s\n",
-			       bond_mode_name());
+	if (*pos == 0)
+		return BOND_PROC_HEADER;
 
-		if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
-		    (bond_mode == BOND_MODE_TLB) ||
-		    (bond_mode == BOND_MODE_ALB)) {
-			read_lock_bh(&bond->lock);
-			read_lock(&bond->ptrlock);
-			if (bond->current_slave != NULL) {
-				len += sprintf(buf + len, 
-					"Currently Active Slave: %s\n", 
-					bond->current_slave->dev->name);
-			}
-			read_unlock(&bond->ptrlock);
-			read_unlock_bh(&bond->lock);
-		}
-
-		len += sprintf(buf + len, "MII Status: ");
-		len += sprintf(buf + len, 
-				link == BMSR_LSTATUS ? "up\n" : "down\n");
-		len += sprintf(buf + len, "MII Polling Interval (ms): %d\n", 
-				miimon);
-		len += sprintf(buf + len, "Up Delay (ms): %d\n", 
-				updelay * miimon);
-		len += sprintf(buf + len, "Down Delay (ms): %d\n", 
-				downdelay * miimon);
-		len += sprintf(buf + len, "Multicast Mode: %s\n",
-			       multicast_mode_name());
+	read_lock_bh(&bond->lock);
+	for (slave = bond->prev; slave != (slave_t *)bond; slave = slave->prev) {
+		if (++off == *pos)
+			return slave;
+	}
+	
+	return NULL;
+}
+
+static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	bonding_t *bond = seq->private;
+	slave_t *slave = v;
 
+	++*pos;
+	if (v == BOND_PROC_HEADER) {
 		read_lock_bh(&bond->lock);
+		slave = bond->prev;
+	} else {
+		slave = slave->prev;
+	}
+	return (slave == (slave_t *) bond) ? NULL : slave;
+}
 
-		if (bond_mode == BOND_MODE_8023AD) {
-			struct ad_info ad_info;
+static void bond_info_seq_stop(struct seq_file *seq, void *v)
+{
+	bonding_t *bond = seq->private;
+
+	if (v != BOND_PROC_HEADER)
+		read_unlock_bh(&bond->lock);
+	read_unlock(&dev_base_lock);
+}
 
-			len += sprintf(buf + len, "\n802.3ad info\n");
+static void bond_info_show_master(struct seq_file *seq, bonding_t *bond)
+{
+	u16 link = bond_check_mii_link(bond);
 
-			if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
-				len += sprintf(buf + len, "bond %s has no active aggregator\n", bond->device->name);
-			} else {
-				len += sprintf(buf + len, "Active Aggregator Info:\n");
+	seq_printf(seq, "Bonding Mode: %s\n",
+		   bond_mode_name());
 
-				len += sprintf(buf + len, "\tAggregator ID: %d\n", ad_info.aggregator_id);
-				len += sprintf(buf + len, "\tNumber of ports: %d\n", ad_info.ports);
-				len += sprintf(buf + len, "\tActor Key: %d\n", ad_info.actor_key);
-				len += sprintf(buf + len, "\tPartner Key: %d\n", ad_info.partner_key);
-				len += sprintf(buf + len, "\tPartner Mac Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
-					       ad_info.partner_system[0],
-					       ad_info.partner_system[1],
-					       ad_info.partner_system[2],
-					       ad_info.partner_system[3],
-					       ad_info.partner_system[4],
-					       ad_info.partner_system[5]);
-			}
+	if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
+	    (bond_mode == BOND_MODE_TLB) ||
+	    (bond_mode == BOND_MODE_ALB)) {
+		read_lock_bh(&bond->lock);
+		read_lock(&bond->ptrlock);
+		if (bond->current_slave != NULL) {
+			seq_printf(seq, 
+				   "Currently Active Slave: %s\n", 
+				   bond->current_slave->dev->name);
 		}
+		read_unlock(&bond->ptrlock);
+		read_unlock_bh(&bond->lock);
+	}
 
-		for (slave = bond->prev; slave != (slave_t *)bond; 
-		     slave = slave->prev) {
-			len += sprintf(buf + len, "\nSlave Interface: %s\n", slave->dev->name);
-
-			len += sprintf(buf + len, "MII Status: ");
-
-			len += sprintf(buf + len, 
-				slave->link == BOND_LINK_UP ? 
-				"up\n" : "down\n");
-			len += sprintf(buf + len, "Link Failure Count: %d\n", 
-				slave->link_failure_count);
-
-			if (app_abi_ver >= 1) {
-				len += sprintf(buf + len,
-					       "Permanent HW addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
-					       slave->perm_hwaddr[0],
-					       slave->perm_hwaddr[1],
-					       slave->perm_hwaddr[2],
-					       slave->perm_hwaddr[3],
-					       slave->perm_hwaddr[4],
-					       slave->perm_hwaddr[5]);
-			}
+	seq_printf(seq, "MII Status: ");
+	seq_printf(seq, 
+		   link == BMSR_LSTATUS ? "up\n" : "down\n");
+	seq_printf(seq, "MII Polling Interval (ms): %d\n", 
+		   miimon);
+	seq_printf(seq, "Up Delay (ms): %d\n", 
+		   updelay * miimon);
+	seq_printf(seq, "Down Delay (ms): %d\n", 
+		   downdelay * miimon);
+	seq_printf(seq, "Multicast Mode: %s\n",
+		   multicast_mode_name());
 
-			if (bond_mode == BOND_MODE_8023AD) {
-				struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
 
-				if (agg) {
-					len += sprintf(buf + len, "Aggregator ID: %d\n",
-						       agg->aggregator_identifier);
-				} else {
-					len += sprintf(buf + len, "Aggregator ID: N/A\n");
-				}
-			}
-		}
-		read_unlock_bh(&bond->lock);
+	if (bond_mode == BOND_MODE_8023AD) {
+		struct ad_info ad_info;
 
-		/*
-		 * Figure out the calcs for the /proc/net interface
-		 */
-		*start = buf + (offset - begin);
-		len -= (offset - begin);
-		if (len > length) {
-			len = length;
-		}
-		if (len < 0) {
-			len = 0;
+		seq_printf(seq, "\n802.3ad info\n");
+
+		if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
+			seq_printf(seq, "bond %s has no active aggregator\n", bond->device->name);
+		} else {
+			seq_printf(seq, "Active Aggregator Info:\n");
+
+			seq_printf(seq, "\tAggregator ID: %d\n", ad_info.aggregator_id);
+			seq_printf(seq, "\tNumber of ports: %d\n", ad_info.ports);
+			seq_printf(seq, "\tActor Key: %d\n", ad_info.actor_key);
+			seq_printf(seq, "\tPartner Key: %d\n", ad_info.partner_key);
+			seq_printf(seq, "\tPartner Mac Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
+				   ad_info.partner_system[0],
+				   ad_info.partner_system[1],
+				   ad_info.partner_system[2],
+				   ad_info.partner_system[3],
+				   ad_info.partner_system[4],
+				   ad_info.partner_system[5]);
 		}
+	}
+
+}
+
+static void bond_info_show_slave(struct seq_file *seq, const slave_t *slave)
+{
+
+	seq_printf(seq, "\nSlave Interface: %s\n", slave->dev->name);
 
+	seq_printf(seq, "MII Status: ");
+
+	seq_printf(seq, 
+		   slave->link == BOND_LINK_UP ? 
+		   "up\n" : "down\n");
+	seq_printf(seq, "Link Failure Count: %d\n", 
+		   slave->link_failure_count);
+
+	if (app_abi_ver >= 1) {
+		seq_printf(seq,
+			   "Permanent HW addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
+			   slave->perm_hwaddr[0],
+			   slave->perm_hwaddr[1],
+			   slave->perm_hwaddr[2],
+			   slave->perm_hwaddr[3],
+			   slave->perm_hwaddr[4],
+			   slave->perm_hwaddr[5]);
 	}
-	read_unlock(&dev_base_lock);
 
-	return len;
+	if (bond_mode == BOND_MODE_8023AD) {
+		const struct aggregator *agg
+			= SLAVE_AD_INFO(slave).port.aggregator;
+
+		if (agg) 
+			seq_printf(seq, "Aggregator ID: %d\n",
+				   agg->aggregator_identifier);
+		else
+			seq_printf(seq, "Aggregator ID: N/A\n");
+	}
+}
+
+static int bond_info_seq_show(struct seq_file *seq, void *v)
+{
+	if (v == BOND_PROC_HEADER) {
+		seq_printf(seq, "%s\n", version);
+		bond_info_show_master(seq, seq->private);
+	} else 
+		bond_info_show_slave(seq, v);
+	return 0;
 }
 
+
+static struct seq_operations bond_info_seq_ops = {
+	.start = bond_info_seq_start,
+	.next  = bond_info_seq_next,
+	.stop  = bond_info_seq_stop,
+	.show  = bond_info_seq_show,
+};
+
+static int bond_info_open(struct inode *inode, struct file *file)
+{
+	struct seq_file *seq;
+	struct proc_dir_entry *proc;
+	int rc;
+
+	rc = seq_open(file, &bond_info_seq_ops);
+	if (!rc) {
+		/* recover the pointer buried in proc_dir_entry data */
+		seq = file->private_data;
+		proc = PDE(inode);
+		seq->private = proc->data;
+	}
+	return rc;
+}
+
+static struct file_operations bond_info_fops = {
+	.owner	 = THIS_MODULE,
+	.open    = bond_info_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = seq_release,
+};
+
+
+#endif
+
 static int bond_event(struct notifier_block *this, unsigned long event, 
 			void *ptr)
 {
@@ -3560,15 +3616,15 @@
 	bond->bond_proc_dir->owner = THIS_MODULE;
 
 	bond->bond_proc_info_file = 
-		create_proc_info_entry("info", 0, bond->bond_proc_dir, 
-					bond_get_info);
+		create_proc_entry("info", 0, bond->bond_proc_dir);
 	if (bond->bond_proc_info_file == NULL) {
 		printk(KERN_ERR "%s: Cannot init /proc/net/%s/info\n", 
 			dev->name, dev->name);
 		remove_proc_entry(dev->name, proc_net);
 		return -ENOMEM;
 	}
-	bond->bond_proc_info_file->owner = THIS_MODULE;
+	bond->bond_proc_info_file->data = bond;
+	bond->bond_proc_info_file->proc_fops = &bond_info_fops;
 #endif /* CONFIG_PROC_FS */
 
 

Re: [PATCH] convering bonding driver /proc interface to seq_file

[James Morris]

On Thu, 21 Aug 2003, Stephen Hemminger wrote:

> The name "/proc/net/bond0/info" is kind of unconventional, but now
> changing that in a almost stable release would be bad.  Better choice
> would be "/proc/net/bonding/bond0".

I'm not sure it's too late to do this.


- James
-- 
James Morris
<jmorris@intercode.com.au>

Re: [PATCH] convering bonding driver /proc interface to seq_file

[Chad N. Tindel]

> > The name "/proc/net/bond0/info" is kind of unconventional, but now
> > changing that in a almost stable release would be bad.  Better choice
> > would be "/proc/net/bonding/bond0".
> 
> I'm not sure it's too late to do this.

It certainly would match what other drivers do.

Chad

[PATCH] change /proc/bond0/info to /proc/bonding/bond0

[Stephen Hemminger]

On Fri, 22 Aug 2003 10:17:07 +1000 (EST)
James Morris <jmorris@intercode.com.au> wrote:

> On Thu, 21 Aug 2003, Stephen Hemminger wrote:
> 
> > The name "/proc/net/bond0/info" is kind of unconventional, but now
> > changing that in a almost stable release would be bad.  Better choice
> > would be "/proc/net/bonding/bond0".
> 
> I'm not sure it's too late to do this.

Changes the name of the /proc entry on 2.6.0-test3 with my earlier
patch.  Also, documentation and comments are updated.

This version will work if the /proc creation fails; a warning is printed
but see no reason to prevent module from loading.

diff -Nru a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
--- a/Documentation/networking/bonding.txt	Fri Aug 22 13:00:24 2003
+++ b/Documentation/networking/bonding.txt	Fri Aug 22 13:00:24 2003
@@ -529,7 +529,7 @@
 ----------------------------
 The bonding driver information files reside in the /proc/net/bond* directories.
 
-Sample contents of /proc/net/bond0/info after the driver is loaded with 
+Sample contents of /proc/net/bonding/bond0 after the driver is loaded with 
 parameters of mode=0 and miimon=1000 is shown below.
  
         Bonding Mode: load balancing (round-robin)
diff -Nru a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c	Fri Aug 22 13:00:24 2003
+++ b/drivers/net/bonding/bond_main.c	Fri Aug 22 13:00:24 2003
@@ -119,7 +119,7 @@
  *
  * 2001/6/01 - Chad N. Tindel <ctindel at ieee dot org>
  *     - Added /proc support for getting bond and slave information.
- *       Information is in /proc/net/<bond device>/info. 
+ *       Information is in /proc/net/bonding/<bond device>
  *     - Changed the locking when calling bond_close to prevent deadlock.
  *
  * 2001/8/05 - Janice Girouard <girouard at us.ibm.com>
@@ -501,6 +501,7 @@
 };
 
 static LIST_HEAD(bond_dev_list);
+static struct proc_dir_entry *bond_proc_dir;
 
 MODULE_PARM(max_bonds, "i");
 MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
@@ -3607,27 +3608,21 @@
 	}
 
 #ifdef CONFIG_PROC_FS
-	bond->bond_proc_dir = proc_mkdir(dev->name, proc_net);
-	if (bond->bond_proc_dir == NULL) {
-		printk(KERN_ERR "%s: Cannot init /proc/net/%s/\n", 
-			dev->name, dev->name);
-		return -ENOMEM;
-	}
-	bond->bond_proc_dir->owner = THIS_MODULE;
-
-	bond->bond_proc_info_file = 
-		create_proc_entry("info", 0, bond->bond_proc_dir);
-	if (bond->bond_proc_info_file == NULL) {
-		printk(KERN_ERR "%s: Cannot init /proc/net/%s/info\n", 
-			dev->name, dev->name);
-		remove_proc_entry(dev->name, proc_net);
-		return -ENOMEM;
+	if (bond_proc_dir) {
+		bond->bond_proc_file = create_proc_entry(dev->name,
+							 S_IRUGO, 
+							 bond_proc_dir);
+		if (bond->bond_proc_file == NULL) {
+			printk(KERN_WARNING "%s: Cannot create "
+			       "/proc/net/bonding/%s\n", 
+			       dev->name, dev->name);
+		} else {
+			bond->bond_proc_file->data = bond;
+			bond->bond_proc_file->proc_fops = &bond_info_fops;
+		}
 	}
-	bond->bond_proc_info_file->data = bond;
-	bond->bond_proc_info_file->proc_fops = &bond_info_fops;
 #endif /* CONFIG_PROC_FS */
 
-
 	list_add_tail(&bond->bond_list, &bond_dev_list);
 	return 0;
 }
@@ -3908,6 +3903,15 @@
 
 	register_netdevice_notifier(&bond_netdev_notifier);
 
+#ifdef CONFIG_PROC_FS
+	bond_proc_dir = proc_mkdir("bonding", proc_net);
+	if (bond_proc_dir == NULL) 
+		printk(KERN_WARNING "bonding_init(): can not create "
+		       "/proc/net/bonding");
+	else 
+		bond_proc_dir->owner = THIS_MODULE;
+#endif
+
 	for (no = 0; no < max_bonds; no++) {
 		struct net_device *dev;
 		char name[IFNAMSIZ];
@@ -3943,13 +3947,15 @@
 		 
 	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
 		struct net_device *dev = bond->device;
-#ifdef CONFIG_PROC_FS
-		remove_proc_entry("info", bond->bond_proc_dir);
-		remove_proc_entry(dev->name, proc_net);
-#endif
+
+		if (bond_proc_dir)
+			remove_proc_entry(dev->name, bond_proc_dir);
 		unregister_netdev(dev);
 		free_netdev(dev);
 	}
+
+	if (bond_proc_dir)
+		remove_proc_entry("bonding", proc_net);
 }
 
 module_init(bonding_init);
diff -Nru a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
--- a/drivers/net/bonding/bonding.h	Fri Aug 22 13:00:24 2003
+++ b/drivers/net/bonding/bonding.h	Fri Aug 22 13:00:24 2003
@@ -101,8 +101,7 @@
 	struct timer_list arp_timer;
 	struct net_device_stats *stats;
 #ifdef CONFIG_PROC_FS
-	struct proc_dir_entry *bond_proc_dir;
-	struct proc_dir_entry *bond_proc_info_file;
+	struct proc_dir_entry *bond_proc_file;
 #endif /* CONFIG_PROC_FS */
 	struct list_head bond_list;
 	struct net_device *device;

RE: [PATCH] convering bonding driver /proc interface to seq_file

[Hen, Shmulik]

Guys,

We have already submitted a patch that does the exact same
thing (give back info only for the queried bond) that is
sitting in Jeff's net-drivers queue. It is a part of a whole
set of patches meant to get the 2.4 and 2.6 versions
synched (referred to as back porting patch set, or set #1).

The patch set is being held until Marcelo goes to 2.4.23-pre1.
Jeff also said something about reservation he might have about
that set, so we're still waiting.

Could everyone please hold off the bonding modifications for
only a short while until we get this sorted out? We have a
bunch of other patch sets in queue that add lots of new stuff
and they all depend on that first set being accepted.

If I'm asking for too much, let me know, but do it gently :)

-- 
| Shmulik Hen   Advanced Network Services  |
| Israel Design Center, Jerusalem          |
| LAN Access Division, Platform Networking |
| Intel Communications Group, Intel corp.  |

 

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@osdl.org]
> Sent: Friday, August 22, 2003 2:53 AM
> To: Chad Tindel; Jay Vosburgh; David S. Miller; Jeff Garzik
> Cc: bonding-devel@lists.sourceforge.net; netdev@oss.sgi.com
> Subject: [PATCH] convering bonding driver /proc interface to seq_file
> 
> 
> This patch converts /proc/net/bondXX/info to the seq_file interface;
> patch is against 2.6.0-test3; but seq_file has been 
> backported to latest
> 2.4 as well.
> 
> It also fixes a bug with multiple bond devices.  The original code
> mushes all the bond device status information together.  In 
> other words,
> /proc/net/bond0/info gives the same output as /proc/net/bond1/info.
> 
> I fixed this in this version, by stuffing a link back to the bonding
> data structure in the created proc_dir_entry; which is then used
> to only pickup the slaves for that pseudo-device.
> 
> Don't think anyone ever used more that one device, because they
> probably would have noticed this.
> 
> The name "/proc/net/bond0/info" is kind of unconventional, but now
> changing that in a almost stable release would be bad.  Better choice
> would be "/proc/net/bonding/bond0".
> 
> --- linux-2.5/drivers/net/bonding/bond_main.c	2003-08-20 
> 11:19:42.000000000 -0700
> +++ linux-2.5-net/drivers/net/bonding/bond_main.c	
> 2003-08-21 16:42:57.130509618 -0700
> @@ -402,6 +402,8 @@
>  #include <linux/inetdevice.h>
>  #include <linux/etherdevice.h>
>  #include <linux/skbuff.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
>  #include <net/sock.h>
>  #include <linux/rtnetlink.h>
>  
> @@ -545,14 +547,6 @@
>  static int bond_release_all(struct net_device *master);
>  static int bond_sethwaddr(struct net_device *master, struct 
> net_device *slave);
>  
> -/*
> - * bond_get_info is the interface into the /proc filesystem.  This is
> - * a different interface than the BOND_INFO_QUERY ioctl.  
> That is done
> - * through the generic networking ioctl interface, and 
> bond_info_query
> - * is the internal function which provides that information.
> - */
> -static int bond_get_info(char *buf, char **start, off_t 
> offset, int length);
> -
>  /* Caller must hold bond->ptrlock for write */
>  static inline struct slave*
>  bond_assign_current_slave(struct bonding *bond,struct slave 
> *newslave)
> @@ -3327,133 +3321,195 @@
>  	return stats;
>  }
>  
> -static int bond_get_info(char *buf, char **start, off_t 
> offset, int length)
> -{
> -	bonding_t *bond;
> -	int len = 0;
> -	off_t begin = 0;
> -	u16 link;
> -	slave_t *slave = NULL;
> +#ifdef CONFIG_PROC_FS
>  
> -	len += sprintf(buf + len, "%s\n", version);
> +#define BOND_PROC_HEADER ((void *)1)	/* magic pointer for 
> header line */
> +
> +static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
> +{
> +	bonding_t *bond = seq->private;
> +	loff_t off = 0;
> +	slave_t *slave;
>  
>  	read_lock(&dev_base_lock);
> -	list_for_each_entry(bond, &bond_dev_list, bond_list) {
> -		/*
> -		 * This function locks the mutex, so we can't 
> lock it until 
> -		 * afterwards
> -		 */
> -		link = bond_check_mii_link(bond);
>  
> -		len += sprintf(buf + len, "Bonding Mode: %s\n",
> -			       bond_mode_name());
> +	if (*pos == 0)
> +		return BOND_PROC_HEADER;
>  
> -		if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
> -		    (bond_mode == BOND_MODE_TLB) ||
> -		    (bond_mode == BOND_MODE_ALB)) {
> -			read_lock_bh(&bond->lock);
> -			read_lock(&bond->ptrlock);
> -			if (bond->current_slave != NULL) {
> -				len += sprintf(buf + len, 
> -					"Currently Active Slave: %s\n", 
> -					bond->current_slave->dev->name);
> -			}
> -			read_unlock(&bond->ptrlock);
> -			read_unlock_bh(&bond->lock);
> -		}
> -
> -		len += sprintf(buf + len, "MII Status: ");
> -		len += sprintf(buf + len, 
> -				link == BMSR_LSTATUS ? "up\n" : 
> "down\n");
> -		len += sprintf(buf + len, "MII Polling Interval 
> (ms): %d\n", 
> -				miimon);
> -		len += sprintf(buf + len, "Up Delay (ms): %d\n", 
> -				updelay * miimon);
> -		len += sprintf(buf + len, "Down Delay (ms): %d\n", 
> -				downdelay * miimon);
> -		len += sprintf(buf + len, "Multicast Mode: %s\n",
> -			       multicast_mode_name());
> +	read_lock_bh(&bond->lock);
> +	for (slave = bond->prev; slave != (slave_t *)bond; 
> slave = slave->prev) {
> +		if (++off == *pos)
> +			return slave;
> +	}
> +	
> +	return NULL;
> +}
> +
> +static void *bond_info_seq_next(struct seq_file *seq, void 
> *v, loff_t *pos)
> +{
> +	bonding_t *bond = seq->private;
> +	slave_t *slave = v;
>  
> +	++*pos;
> +	if (v == BOND_PROC_HEADER) {
>  		read_lock_bh(&bond->lock);
> +		slave = bond->prev;
> +	} else {
> +		slave = slave->prev;
> +	}
> +	return (slave == (slave_t *) bond) ? NULL : slave;
> +}
>  
> -		if (bond_mode == BOND_MODE_8023AD) {
> -			struct ad_info ad_info;
> +static void bond_info_seq_stop(struct seq_file *seq, void *v)
> +{
> +	bonding_t *bond = seq->private;
> +
> +	if (v != BOND_PROC_HEADER)
> +		read_unlock_bh(&bond->lock);
> +	read_unlock(&dev_base_lock);
> +}
>  
> -			len += sprintf(buf + len, "\n802.3ad info\n");
> +static void bond_info_show_master(struct seq_file *seq, 
> bonding_t *bond)
> +{
> +	u16 link = bond_check_mii_link(bond);
>  
> -			if (bond_3ad_get_active_agg_info(bond, 
> &ad_info)) {
> -				len += sprintf(buf + len, "bond 
> %s has no active aggregator\n", bond->device->name);
> -			} else {
> -				len += sprintf(buf + len, 
> "Active Aggregator Info:\n");
> +	seq_printf(seq, "Bonding Mode: %s\n",
> +		   bond_mode_name());
>  
> -				len += sprintf(buf + len, 
> "\tAggregator ID: %d\n", ad_info.aggregator_id);
> -				len += sprintf(buf + len, 
> "\tNumber of ports: %d\n", ad_info.ports);
> -				len += sprintf(buf + len, 
> "\tActor Key: %d\n", ad_info.actor_key);
> -				len += sprintf(buf + len, 
> "\tPartner Key: %d\n", ad_info.partner_key);
> -				len += sprintf(buf + len, 
> "\tPartner Mac Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -					       
ad_info.partner_system[0],
> -					       
ad_info.partner_system[1],
> -					       
ad_info.partner_system[2],
> -					       
ad_info.partner_system[3],
> -					       
ad_info.partner_system[4],
> -					       
ad_info.partner_system[5]);
> -			}
> +	if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
> +	    (bond_mode == BOND_MODE_TLB) ||
> +	    (bond_mode == BOND_MODE_ALB)) {
> +		read_lock_bh(&bond->lock);
> +		read_lock(&bond->ptrlock);
> +		if (bond->current_slave != NULL) {
> +			seq_printf(seq, 
> +				   "Currently Active Slave: %s\n", 
> +				   bond->current_slave->dev->name);
>  		}
> +		read_unlock(&bond->ptrlock);
> +		read_unlock_bh(&bond->lock);
> +	}
>  
> -		for (slave = bond->prev; slave != (slave_t *)bond; 
> -		     slave = slave->prev) {
> -			len += sprintf(buf + len, "\nSlave 
> Interface: %s\n", slave->dev->name);
> -
> -			len += sprintf(buf + len, "MII Status: ");
> -
> -			len += sprintf(buf + len, 
> -				slave->link == BOND_LINK_UP ? 
> -				"up\n" : "down\n");
> -			len += sprintf(buf + len, "Link Failure 
> Count: %d\n", 
> -				slave->link_failure_count);
> -
> -			if (app_abi_ver >= 1) {
> -				len += sprintf(buf + len,
> -					       "Permanent HW 
> addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
> -					       slave->perm_hwaddr[0],
> -					       slave->perm_hwaddr[1],
> -					       slave->perm_hwaddr[2],
> -					       slave->perm_hwaddr[3],
> -					       slave->perm_hwaddr[4],
> -					       slave->perm_hwaddr[5]);
> -			}
> +	seq_printf(seq, "MII Status: ");
> +	seq_printf(seq, 
> +		   link == BMSR_LSTATUS ? "up\n" : "down\n");
> +	seq_printf(seq, "MII Polling Interval (ms): %d\n", 
> +		   miimon);
> +	seq_printf(seq, "Up Delay (ms): %d\n", 
> +		   updelay * miimon);
> +	seq_printf(seq, "Down Delay (ms): %d\n", 
> +		   downdelay * miimon);
> +	seq_printf(seq, "Multicast Mode: %s\n",
> +		   multicast_mode_name());
>  
> -			if (bond_mode == BOND_MODE_8023AD) {
> -				struct aggregator *agg = 
> SLAVE_AD_INFO(slave).port.aggregator;
>  
> -				if (agg) {
> -					len += sprintf(buf + 
> len, "Aggregator ID: %d\n",
> -						       
> agg->aggregator_identifier);
> -				} else {
> -					len += sprintf(buf + 
> len, "Aggregator ID: N/A\n");
> -				}
> -			}
> -		}
> -		read_unlock_bh(&bond->lock);
> +	if (bond_mode == BOND_MODE_8023AD) {
> +		struct ad_info ad_info;
>  
> -		/*
> -		 * Figure out the calcs for the /proc/net interface
> -		 */
> -		*start = buf + (offset - begin);
> -		len -= (offset - begin);
> -		if (len > length) {
> -			len = length;
> -		}
> -		if (len < 0) {
> -			len = 0;
> +		seq_printf(seq, "\n802.3ad info\n");
> +
> +		if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> +			seq_printf(seq, "bond %s has no active 
> aggregator\n", bond->device->name);
> +		} else {
> +			seq_printf(seq, "Active Aggregator Info:\n");
> +
> +			seq_printf(seq, "\tAggregator ID: 
> %d\n", ad_info.aggregator_id);
> +			seq_printf(seq, "\tNumber of ports: 
> %d\n", ad_info.ports);
> +			seq_printf(seq, "\tActor Key: %d\n", 
> ad_info.actor_key);
> +			seq_printf(seq, "\tPartner Key: %d\n", 
> ad_info.partner_key);
> +			seq_printf(seq, "\tPartner Mac Address: 
> %02x:%02x:%02x:%02x:%02x:%02x\n",
> +				   ad_info.partner_system[0],
> +				   ad_info.partner_system[1],
> +				   ad_info.partner_system[2],
> +				   ad_info.partner_system[3],
> +				   ad_info.partner_system[4],
> +				   ad_info.partner_system[5]);
>  		}
> +	}
> +
> +}
> +
> +static void bond_info_show_slave(struct seq_file *seq, const 
> slave_t *slave)
> +{
> +
> +	seq_printf(seq, "\nSlave Interface: %s\n", slave->dev->name);
>  
> +	seq_printf(seq, "MII Status: ");
> +
> +	seq_printf(seq, 
> +		   slave->link == BOND_LINK_UP ? 
> +		   "up\n" : "down\n");
> +	seq_printf(seq, "Link Failure Count: %d\n", 
> +		   slave->link_failure_count);
> +
> +	if (app_abi_ver >= 1) {
> +		seq_printf(seq,
> +			   "Permanent HW addr: 
> %02x:%02x:%02x:%02x:%02x:%02x\n",
> +			   slave->perm_hwaddr[0],
> +			   slave->perm_hwaddr[1],
> +			   slave->perm_hwaddr[2],
> +			   slave->perm_hwaddr[3],
> +			   slave->perm_hwaddr[4],
> +			   slave->perm_hwaddr[5]);
>  	}
> -	read_unlock(&dev_base_lock);
>  
> -	return len;
> +	if (bond_mode == BOND_MODE_8023AD) {
> +		const struct aggregator *agg
> +			= SLAVE_AD_INFO(slave).port.aggregator;
> +
> +		if (agg) 
> +			seq_printf(seq, "Aggregator ID: %d\n",
> +				   agg->aggregator_identifier);
> +		else
> +			seq_printf(seq, "Aggregator ID: N/A\n");
> +	}
> +}
> +
> +static int bond_info_seq_show(struct seq_file *seq, void *v)
> +{
> +	if (v == BOND_PROC_HEADER) {
> +		seq_printf(seq, "%s\n", version);
> +		bond_info_show_master(seq, seq->private);
> +	} else 
> +		bond_info_show_slave(seq, v);
> +	return 0;
>  }
>  
> +
> +static struct seq_operations bond_info_seq_ops = {
> +	.start = bond_info_seq_start,
> +	.next  = bond_info_seq_next,
> +	.stop  = bond_info_seq_stop,
> +	.show  = bond_info_seq_show,
> +};
> +
> +static int bond_info_open(struct inode *inode, struct file *file)
> +{
> +	struct seq_file *seq;
> +	struct proc_dir_entry *proc;
> +	int rc;
> +
> +	rc = seq_open(file, &bond_info_seq_ops);
> +	if (!rc) {
> +		/* recover the pointer buried in proc_dir_entry data */
> +		seq = file->private_data;
> +		proc = PDE(inode);
> +		seq->private = proc->data;
> +	}
> +	return rc;
> +}
> +
> +static struct file_operations bond_info_fops = {
> +	.owner	 = THIS_MODULE,
> +	.open    = bond_info_open,
> +	.read    = seq_read,
> +	.llseek  = seq_lseek,
> +	.release = seq_release,
> +};
> +
> +
> +#endif
> +
>  static int bond_event(struct notifier_block *this, unsigned 
> long event, 
>  			void *ptr)
>  {
> @@ -3560,15 +3616,15 @@
>  	bond->bond_proc_dir->owner = THIS_MODULE;
>  
>  	bond->bond_proc_info_file = 
> -		create_proc_info_entry("info", 0, bond->bond_proc_dir, 
> -					bond_get_info);
> +		create_proc_entry("info", 0, bond->bond_proc_dir);
>  	if (bond->bond_proc_info_file == NULL) {
>  		printk(KERN_ERR "%s: Cannot init /proc/net/%s/info\n", 
>  			dev->name, dev->name);
>  		remove_proc_entry(dev->name, proc_net);
>  		return -ENOMEM;
>  	}
> -	bond->bond_proc_info_file->owner = THIS_MODULE;
> +	bond->bond_proc_info_file->data = bond;
> +	bond->bond_proc_info_file->proc_fops = &bond_info_fops;
>  #endif /* CONFIG_PROC_FS */
>  
>  
> 
> 

RE: [PATCH] convering bonding driver /proc interface to seq_file

[Hen, Shmulik]

There is one more reason for my plea that I didn't mention:

Currently the bonding version in 2.6 is way behind what's in
2.4. We are hoping that once the stuff in 2.4 is synched, we
could just copy everything from 2.4 to 2.6 as is, and once
that's done, continue regular development on 2.6 and back
port into 2.4 from time to time.

Also, instead of using a seq_file interface and using a
pointer to skip to the relevant part, we've used a different
proc creation interface that allows passing a pointer to bond
and thus retrieve just the relevant bond device's data.


	Shmulik.

> -----Original Message-----
> From: Hen, Shmulik 
> Sent: Saturday, August 23, 2003 4:35 PM
> To: 'Stephen Hemminger'; Chad Tindel; Jay Vosburgh; David S. Miller;
> Jeff Garzik
> Cc: bonding-devel@lists.sourceforge.net; netdev@oss.sgi.com
> Subject: RE: [PATCH] convering bonding driver /proc interface to
> seq_file
> 
> 
> Guys,
> 
> We have already submitted a patch that does the exact same
> thing (give back info only for the queried bond) that is
> sitting in Jeff's net-drivers queue. It is a part of a whole
> set of patches meant to get the 2.4 and 2.6 versions
> synched (referred to as back porting patch set, or set #1).
> 
> The patch set is being held until Marcelo goes to 2.4.23-pre1.
> Jeff also said something about reservation he might have about
> that set, so we're still waiting.
> 
> Could everyone please hold off the bonding modifications for
> only a short while until we get this sorted out? We have a
> bunch of other patch sets in queue that add lots of new stuff
> and they all depend on that first set being accepted.
> 
> If I'm asking for too much, let me know, but do it gently :)
> 
> -- 
> | Shmulik Hen   Advanced Network Services  |
> | Israel Design Center, Jerusalem          |
> | LAN Access Division, Platform Networking |
> | Intel Communications Group, Intel corp.  |
> 
>  
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@osdl.org]
> > Sent: Friday, August 22, 2003 2:53 AM
> > To: Chad Tindel; Jay Vosburgh; David S. Miller; Jeff Garzik
> > Cc: bonding-devel@lists.sourceforge.net; netdev@oss.sgi.com
> > Subject: [PATCH] convering bonding driver /proc interface 
> to seq_file
> > 
> > 
> > This patch converts /proc/net/bondXX/info to the seq_file interface;
> > patch is against 2.6.0-test3; but seq_file has been 
> > backported to latest
> > 2.4 as well.
> > 
> > It also fixes a bug with multiple bond devices.  The original code
> > mushes all the bond device status information together.  In 
> > other words,
> > /proc/net/bond0/info gives the same output as /proc/net/bond1/info.
> > 
> > I fixed this in this version, by stuffing a link back to the bonding
> > data structure in the created proc_dir_entry; which is then used
> > to only pickup the slaves for that pseudo-device.
> > 
> > Don't think anyone ever used more that one device, because they
> > probably would have noticed this.
> > 
> > The name "/proc/net/bond0/info" is kind of unconventional, but now
> > changing that in a almost stable release would be bad.  
> Better choice
> > would be "/proc/net/bonding/bond0".
> > 
> > --- linux-2.5/drivers/net/bonding/bond_main.c	2003-08-20 
> > 11:19:42.000000000 -0700
> > +++ linux-2.5-net/drivers/net/bonding/bond_main.c	
> > 2003-08-21 16:42:57.130509618 -0700
> > @@ -402,6 +402,8 @@
> >  #include <linux/inetdevice.h>
> >  #include <linux/etherdevice.h>
> >  #include <linux/skbuff.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/seq_file.h>
> >  #include <net/sock.h>
> >  #include <linux/rtnetlink.h>
> >  
> > @@ -545,14 +547,6 @@
> >  static int bond_release_all(struct net_device *master);
> >  static int bond_sethwaddr(struct net_device *master, struct 
> > net_device *slave);
> >  
> > -/*
> > - * bond_get_info is the interface into the /proc 
> filesystem.  This is
> > - * a different interface than the BOND_INFO_QUERY ioctl.  
> > That is done
> > - * through the generic networking ioctl interface, and 
> > bond_info_query
> > - * is the internal function which provides that information.
> > - */
> > -static int bond_get_info(char *buf, char **start, off_t 
> > offset, int length);
> > -
> >  /* Caller must hold bond->ptrlock for write */
> >  static inline struct slave*
> >  bond_assign_current_slave(struct bonding *bond,struct slave 
> > *newslave)
> > @@ -3327,133 +3321,195 @@
> >  	return stats;
> >  }
> >  
> > -static int bond_get_info(char *buf, char **start, off_t 
> > offset, int length)
> > -{
> > -	bonding_t *bond;
> > -	int len = 0;
> > -	off_t begin = 0;
> > -	u16 link;
> > -	slave_t *slave = NULL;
> > +#ifdef CONFIG_PROC_FS
> >  
> > -	len += sprintf(buf + len, "%s\n", version);
> > +#define BOND_PROC_HEADER ((void *)1)	/* magic pointer for 
> > header line */
> > +
> > +static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
> > +{
> > +	bonding_t *bond = seq->private;
> > +	loff_t off = 0;
> > +	slave_t *slave;
> >  
> >  	read_lock(&dev_base_lock);
> > -	list_for_each_entry(bond, &bond_dev_list, bond_list) {
> > -		/*
> > -		 * This function locks the mutex, so we can't 
> > lock it until 
> > -		 * afterwards
> > -		 */
> > -		link = bond_check_mii_link(bond);
> >  
> > -		len += sprintf(buf + len, "Bonding Mode: %s\n",
> > -			       bond_mode_name());
> > +	if (*pos == 0)
> > +		return BOND_PROC_HEADER;
> >  
> > -		if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
> > -		    (bond_mode == BOND_MODE_TLB) ||
> > -		    (bond_mode == BOND_MODE_ALB)) {
> > -			read_lock_bh(&bond->lock);
> > -			read_lock(&bond->ptrlock);
> > -			if (bond->current_slave != NULL) {
> > -				len += sprintf(buf + len, 
> > -					"Currently Active Slave: %s\n", 
> > -					bond->current_slave->dev->name);
> > -			}
> > -			read_unlock(&bond->ptrlock);
> > -			read_unlock_bh(&bond->lock);
> > -		}
> > -
> > -		len += sprintf(buf + len, "MII Status: ");
> > -		len += sprintf(buf + len, 
> > -				link == BMSR_LSTATUS ? "up\n" : 
> > "down\n");
> > -		len += sprintf(buf + len, "MII Polling Interval 
> > (ms): %d\n", 
> > -				miimon);
> > -		len += sprintf(buf + len, "Up Delay (ms): %d\n", 
> > -				updelay * miimon);
> > -		len += sprintf(buf + len, "Down Delay (ms): %d\n", 
> > -				downdelay * miimon);
> > -		len += sprintf(buf + len, "Multicast Mode: %s\n",
> > -			       multicast_mode_name());
> > +	read_lock_bh(&bond->lock);
> > +	for (slave = bond->prev; slave != (slave_t *)bond; 
> > slave = slave->prev) {
> > +		if (++off == *pos)
> > +			return slave;
> > +	}
> > +	
> > +	return NULL;
> > +}
> > +
> > +static void *bond_info_seq_next(struct seq_file *seq, void 
> > *v, loff_t *pos)
> > +{
> > +	bonding_t *bond = seq->private;
> > +	slave_t *slave = v;
> >  
> > +	++*pos;
> > +	if (v == BOND_PROC_HEADER) {
> >  		read_lock_bh(&bond->lock);
> > +		slave = bond->prev;
> > +	} else {
> > +		slave = slave->prev;
> > +	}
> > +	return (slave == (slave_t *) bond) ? NULL : slave;
> > +}
> >  
> > -		if (bond_mode == BOND_MODE_8023AD) {
> > -			struct ad_info ad_info;
> > +static void bond_info_seq_stop(struct seq_file *seq, void *v)
> > +{
> > +	bonding_t *bond = seq->private;
> > +
> > +	if (v != BOND_PROC_HEADER)
> > +		read_unlock_bh(&bond->lock);
> > +	read_unlock(&dev_base_lock);
> > +}
> >  
> > -			len += sprintf(buf + len, "\n802.3ad info\n");
> > +static void bond_info_show_master(struct seq_file *seq, 
> > bonding_t *bond)
> > +{
> > +	u16 link = bond_check_mii_link(bond);
> >  
> > -			if (bond_3ad_get_active_agg_info(bond, 
> > &ad_info)) {
> > -				len += sprintf(buf + len, "bond 
> > %s has no active aggregator\n", bond->device->name);
> > -			} else {
> > -				len += sprintf(buf + len, 
> > "Active Aggregator Info:\n");
> > +	seq_printf(seq, "Bonding Mode: %s\n",
> > +		   bond_mode_name());
> >  
> > -				len += sprintf(buf + len, 
> > "\tAggregator ID: %d\n", ad_info.aggregator_id);
> > -				len += sprintf(buf + len, 
> > "\tNumber of ports: %d\n", ad_info.ports);
> > -				len += sprintf(buf + len, 
> > "\tActor Key: %d\n", ad_info.actor_key);
> > -				len += sprintf(buf + len, 
> > "\tPartner Key: %d\n", ad_info.partner_key);
> > -				len += sprintf(buf + len, 
> > "\tPartner Mac Address: %02x:%02x:%02x:%02x:%02x:%02x\n",
> > -					       
> ad_info.partner_system[0],
> > -					       
> ad_info.partner_system[1],
> > -					       
> ad_info.partner_system[2],
> > -					       
> ad_info.partner_system[3],
> > -					       
> ad_info.partner_system[4],
> > -					       
> ad_info.partner_system[5]);
> > -			}
> > +	if ((bond_mode == BOND_MODE_ACTIVEBACKUP) ||
> > +	    (bond_mode == BOND_MODE_TLB) ||
> > +	    (bond_mode == BOND_MODE_ALB)) {
> > +		read_lock_bh(&bond->lock);
> > +		read_lock(&bond->ptrlock);
> > +		if (bond->current_slave != NULL) {
> > +			seq_printf(seq, 
> > +				   "Currently Active Slave: %s\n", 
> > +				   bond->current_slave->dev->name);
> >  		}
> > +		read_unlock(&bond->ptrlock);
> > +		read_unlock_bh(&bond->lock);
> > +	}
> >  
> > -		for (slave = bond->prev; slave != (slave_t *)bond; 
> > -		     slave = slave->prev) {
> > -			len += sprintf(buf + len, "\nSlave 
> > Interface: %s\n", slave->dev->name);
> > -
> > -			len += sprintf(buf + len, "MII Status: ");
> > -
> > -			len += sprintf(buf + len, 
> > -				slave->link == BOND_LINK_UP ? 
> > -				"up\n" : "down\n");
> > -			len += sprintf(buf + len, "Link Failure 
> > Count: %d\n", 
> > -				slave->link_failure_count);
> > -
> > -			if (app_abi_ver >= 1) {
> > -				len += sprintf(buf + len,
> > -					       "Permanent HW 
> > addr: %02x:%02x:%02x:%02x:%02x:%02x\n",
> > -					       slave->perm_hwaddr[0],
> > -					       slave->perm_hwaddr[1],
> > -					       slave->perm_hwaddr[2],
> > -					       slave->perm_hwaddr[3],
> > -					       slave->perm_hwaddr[4],
> > -					       slave->perm_hwaddr[5]);
> > -			}
> > +	seq_printf(seq, "MII Status: ");
> > +	seq_printf(seq, 
> > +		   link == BMSR_LSTATUS ? "up\n" : "down\n");
> > +	seq_printf(seq, "MII Polling Interval (ms): %d\n", 
> > +		   miimon);
> > +	seq_printf(seq, "Up Delay (ms): %d\n", 
> > +		   updelay * miimon);
> > +	seq_printf(seq, "Down Delay (ms): %d\n", 
> > +		   downdelay * miimon);
> > +	seq_printf(seq, "Multicast Mode: %s\n",
> > +		   multicast_mode_name());
> >  
> > -			if (bond_mode == BOND_MODE_8023AD) {
> > -				struct aggregator *agg = 
> > SLAVE_AD_INFO(slave).port.aggregator;
> >  
> > -				if (agg) {
> > -					len += sprintf(buf + 
> > len, "Aggregator ID: %d\n",
> > -						       
> > agg->aggregator_identifier);
> > -				} else {
> > -					len += sprintf(buf + 
> > len, "Aggregator ID: N/A\n");
> > -				}
> > -			}
> > -		}
> > -		read_unlock_bh(&bond->lock);
> > +	if (bond_mode == BOND_MODE_8023AD) {
> > +		struct ad_info ad_info;
> >  
> > -		/*
> > -		 * Figure out the calcs for the /proc/net interface
> > -		 */
> > -		*start = buf + (offset - begin);
> > -		len -= (offset - begin);
> > -		if (len > length) {
> > -			len = length;
> > -		}
> > -		if (len < 0) {
> > -			len = 0;
> > +		seq_printf(seq, "\n802.3ad info\n");
> > +
> > +		if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
> > +			seq_printf(seq, "bond %s has no active 
> > aggregator\n", bond->device->name);
> > +		} else {
> > +			seq_printf(seq, "Active Aggregator Info:\n");
> > +
> > +			seq_printf(seq, "\tAggregator ID: 
> > %d\n", ad_info.aggregator_id);
> > +			seq_printf(seq, "\tNumber of ports: 
> > %d\n", ad_info.ports);
> > +			seq_printf(seq, "\tActor Key: %d\n", 
> > ad_info.actor_key);
> > +			seq_printf(seq, "\tPartner Key: %d\n", 
> > ad_info.partner_key);
> > +			seq_printf(seq, "\tPartner Mac Address: 
> > %02x:%02x:%02x:%02x:%02x:%02x\n",
> > +				   ad_info.partner_system[0],
> > +				   ad_info.partner_system[1],
> > +				   ad_info.partner_system[2],
> > +				   ad_info.partner_system[3],
> > +				   ad_info.partner_system[4],
> > +				   ad_info.partner_system[5]);
> >  		}
> > +	}
> > +
> > +}
> > +
> > +static void bond_info_show_slave(struct seq_file *seq, const 
> > slave_t *slave)
> > +{
> > +
> > +	seq_printf(seq, "\nSlave Interface: %s\n", slave->dev->name);
> >  
> > +	seq_printf(seq, "MII Status: ");
> > +
> > +	seq_printf(seq, 
> > +		   slave->link == BOND_LINK_UP ? 
> > +		   "up\n" : "down\n");
> > +	seq_printf(seq, "Link Failure Count: %d\n", 
> > +		   slave->link_failure_count);
> > +
> > +	if (app_abi_ver >= 1) {
> > +		seq_printf(seq,
> > +			   "Permanent HW addr: 
> > %02x:%02x:%02x:%02x:%02x:%02x\n",
> > +			   slave->perm_hwaddr[0],
> > +			   slave->perm_hwaddr[1],
> > +			   slave->perm_hwaddr[2],
> > +			   slave->perm_hwaddr[3],
> > +			   slave->perm_hwaddr[4],
> > +			   slave->perm_hwaddr[5]);
> >  	}
> > -	read_unlock(&dev_base_lock);
> >  
> > -	return len;
> > +	if (bond_mode == BOND_MODE_8023AD) {
> > +		const struct aggregator *agg
> > +			= SLAVE_AD_INFO(slave).port.aggregator;
> > +
> > +		if (agg) 
> > +			seq_printf(seq, "Aggregator ID: %d\n",
> > +				   agg->aggregator_identifier);
> > +		else
> > +			seq_printf(seq, "Aggregator ID: N/A\n");
> > +	}
> > +}
> > +
> > +static int bond_info_seq_show(struct seq_file *seq, void *v)
> > +{
> > +	if (v == BOND_PROC_HEADER) {
> > +		seq_printf(seq, "%s\n", version);
> > +		bond_info_show_master(seq, seq->private);
> > +	} else 
> > +		bond_info_show_slave(seq, v);
> > +	return 0;
> >  }
> >  
> > +
> > +static struct seq_operations bond_info_seq_ops = {
> > +	.start = bond_info_seq_start,
> > +	.next  = bond_info_seq_next,
> > +	.stop  = bond_info_seq_stop,
> > +	.show  = bond_info_seq_show,
> > +};
> > +
> > +static int bond_info_open(struct inode *inode, struct file *file)
> > +{
> > +	struct seq_file *seq;
> > +	struct proc_dir_entry *proc;
> > +	int rc;
> > +
> > +	rc = seq_open(file, &bond_info_seq_ops);
> > +	if (!rc) {
> > +		/* recover the pointer buried in proc_dir_entry data */
> > +		seq = file->private_data;
> > +		proc = PDE(inode);
> > +		seq->private = proc->data;
> > +	}
> > +	return rc;
> > +}
> > +
> > +static struct file_operations bond_info_fops = {
> > +	.owner	 = THIS_MODULE,
> > +	.open    = bond_info_open,
> > +	.read    = seq_read,
> > +	.llseek  = seq_lseek,
> > +	.release = seq_release,
> > +};
> > +
> > +
> > +#endif
> > +
> >  static int bond_event(struct notifier_block *this, unsigned 
> > long event, 
> >  			void *ptr)
> >  {
> > @@ -3560,15 +3616,15 @@
> >  	bond->bond_proc_dir->owner = THIS_MODULE;
> >  
> >  	bond->bond_proc_info_file = 
> > -		create_proc_info_entry("info", 0, bond->bond_proc_dir, 
> > -					bond_get_info);
> > +		create_proc_entry("info", 0, bond->bond_proc_dir);
> >  	if (bond->bond_proc_info_file == NULL) {
> >  		printk(KERN_ERR "%s: Cannot init /proc/net/%s/info\n", 
> >  			dev->name, dev->name);
> >  		remove_proc_entry(dev->name, proc_net);
> >  		return -ENOMEM;
> >  	}
> > -	bond->bond_proc_info_file->owner = THIS_MODULE;
> > +	bond->bond_proc_info_file->data = bond;
> > +	bond->bond_proc_info_file->proc_fops = &bond_info_fops;
> >  #endif /* CONFIG_PROC_FS */
> >  
> >  
> > 
> > 
> 

RE: [PATCH] convering bonding driver /proc interface to seq_file

[James Morris]

On Sat, 23 Aug 2003, Hen, Shmulik wrote:

> Also, instead of using a seq_file interface and using a
> pointer to skip to the relevant part, we've used a different
> proc creation interface that allows passing a pointer to bond
> and thus retrieve just the relevant bond device's data.

Can you provide a pointer to this code?

seq_file is the standard and preferred way to do this kind of thing.


- James
-- 
James Morris
<jmorris@intercode.com.au>

Re: [PATCH] convering bonding driver /proc interface to seq_file

[Shmulik Hen]

On Sunday 24 August 2003 06:15 pm, James Morris wrote:
>
> Can you provide a pointer to this code?
>

Please see my email from August 8th with subject:
"[SET 1][PATCH 3/6][bonding] backport 2.6 changes to 2.4"
sent to netdev@oss.sgi.com and bonding-devel@lists.sourceforge.net.

> seq_file is the standard and preferred way to do this kind of
> thing.

Yes, we know. We wanted to convert to seq_file too, but as it turns 
out, in order to create a separate proc file for each bond device you 
need to use the PDE(inode) macro which doesn't exist in 2.4 yet. 
Since we wanted to maintain compatibility in the bonding module 
between 2.4 and 2.6 as much as possible, we figured that as a first 
step we'll start using bond_read_proc() instead of bond_get_info() to 
allow just that.

BTW, we really like the change from /proc/net/bond0/info to 
/proc/net/bonding/bond0. We thought we'll add that into our cleanup 
patch set that should come out once the back porting set is accepted.

-- 
| Shmulik Hen   Advanced Network Services  |
| Israel Design Center, Jerusalem          |
| LAN Access Division, Platform Networking |
| Intel Communications Group, Intel corp.  |

Re: [PATCH] convering bonding driver /proc interface to seq_file

[Stephen Hemminger]

On Mon, 25 Aug 2003 01:15:13 +1000 (EST)
James Morris <jmorris@intercode.com.au> wrote:

> On Sat, 23 Aug 2003, Hen, Shmulik wrote:
> 
> > Also, instead of using a seq_file interface and using a
> > pointer to skip to the relevant part, we've used a different
> > proc creation interface that allows passing a pointer to bond
> > and thus retrieve just the relevant bond device's data.
> 
> Can you provide a pointer to this code?
> 
> seq_file is the standard and preferred way to do this kind of thing.

Using a pointer with seq_file was really easy.

init...
	bond->bond_proc_file = create_proc_entry(dev->name,
						 S_IRUGO, 
						 bond_proc_dir);
	if (bond->bonc_proc_file) {
		bond->bond_proc_file->data = bond;
		bond->bond_proc_file->proc_fops = &bond_info_fops;
	}

open...
	rc = seq_open(file, &bond_info_seq_ops);
	if (!rc) {
		seq = file->private_data;
		seq->private = PDE(inode)->data;
	}

RE: [PATCH] convering bonding driver /proc interface to seq_file

[Hen, Shmulik]

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@osdl.org]
> Sent: Monday, August 25, 2003 6:45 PM
> To: James Morris
> Cc: Hen, Shmulik; Chad Tindel; Jay Vosburgh; David S. Miller; 
> Jeff Garzik; bonding-devel@lists.sourceforge.net; netdev@oss.sgi.com
> Subject: Re: [PATCH] convering bonding driver /proc interface 
> to seq_file
> 
> Using a pointer with seq_file was really easy.
[snip]
> 		seq->private = PDE(inode)->data;

Yes, but PDE(inode) does not exist in 2.4 yet, and we wanted
to achieve compatibility between bonding in 2.6 and 2.4 first
so we may then update 2.6 bonding which is too far behind
what's in 2.4 bonding. Once this is done it will be possible
to develop on 2.6, and back port into 2.4 from time to time.


	Shmulik.

Re: [PATCH] convering bonding driver /proc interface to seq_file

[Stephen Hemminger]

On Mon, 25 Aug 2003 21:43:24 +0300
"Hen, Shmulik" <shmulik.hen@intel.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@osdl.org]
> > Sent: Monday, August 25, 2003 6:45 PM
> > To: James Morris
> > Cc: Hen, Shmulik; Chad Tindel; Jay Vosburgh; David S. Miller; 
> > Jeff Garzik; bonding-devel@lists.sourceforge.net; netdev@oss.sgi.com
> > Subject: Re: [PATCH] convering bonding driver /proc interface 
> > to seq_file
> > 
> > Using a pointer with seq_file was really easy.
> [snip]
> > 		seq->private = PDE(inode)->data;
> 
> Yes, but PDE(inode) does not exist in 2.4 yet, and we wanted
> to achieve compatibility between bonding in 2.6 and 2.4 first
> so we may then update 2.6 bonding which is too far behind
> what's in 2.4 bonding. Once this is done it will be possible
> to develop on 2.6, and back port into 2.4 from time to time.
> 
> 
> 	Shmulik.

#ifndef PDE
#define PDE(i) ((struct proc_dir_entry *)(i)->u.generic_ip)
#endif

Re: [PATCH] convering bonding driver /proc interface to seq_file

[Shmulik Hen]

On Monday 25 August 2003 10:27 pm, Stephen Hemminger wrote:
>
> #ifndef PDE
> #define PDE(i) ((struct proc_dir_entry *)(i)->u.generic_ip)
> #endif

When you're right, you're right. We didn't know we could do that 
safely in 2.4.

How about the following suggestion:
I'll take it as my personal commitment to implement the fix you've 
suggested, along with that of James, but only after the 2.4<->2.6 
synchronization is complete.

After all, my word is my bond(ing) :)

-- 
| Shmulik Hen   Advanced Network Services  |
| Israel Design Center, Jerusalem          |
| LAN Access Division, Platform Networking |
| Intel Communications Group, Intel corp.  |