[PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[Amir Noam]

This patch makes bonding create the /proc/net/bonding directory only
if it exists (and destroy it only if it's empty).

Synchronization with the NETDEV_CHANGENAME event is achieved through
the RTNL semaphore.

The patch applies on 2.4.23-pre8, after applying the patch that
restores backward compatibility sent by Shmulik on 2003/10/12.

Amir


diff -Narup a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c	Thu Oct 23 14:47:31 2003
+++ b/drivers/net/bonding/bond_main.c	Thu Oct 23 17:04:52 2003
@@ -3574,6 +3574,62 @@ static void bond_destroy_proc_info(struc
 		bond->bond_proc_file = NULL;
 	}
 }
+
+/* Create the bonding directory under /proc/net, if doesn't exist yet.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_create_proc_dir(void)
+{
+	int len = strlen(DRV_NAME);
+
+	for (bond_proc_dir = proc_net->subdir; bond_proc_dir;
+	     bond_proc_dir = bond_proc_dir->next) {
+		if ((bond_proc_dir->namelen == len) &&
+		    !memcmp(bond_proc_dir->name, DRV_NAME, len)) {
+			break;
+		}
+	}
+
+	if (!bond_proc_dir) {
+		bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
+		if (bond_proc_dir) {
+			bond_proc_dir->owner = THIS_MODULE;
+		} else {
+			printk(KERN_WARNING DRV_NAME
+				": Warning: cannot create /proc/net/%s\n",
+				DRV_NAME);
+		}
+	}
+}
+
+/* Destroy the bonding directory under /proc/net, if empty.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_destroy_proc_dir(void)
+{
+	struct proc_dir_entry *de;
+
+	if (!bond_proc_dir) {
+		return;
+	}
+
+	/* verify that the /proc dir is empty */
+	for (de = bond_proc_dir->subdir; de; de = de->next) {
+		/* ignore . and .. */
+		if (*(de->name) != '.') {
+			break;
+		}
+	}
+
+	if (de) {
+		if (bond_proc_dir->owner == THIS_MODULE) {
+			bond_proc_dir->owner = NULL;
+		}
+	} else {
+		remove_proc_entry(DRV_NAME, proc_net);
+		bond_proc_dir = NULL;
+	}
+}
 #endif /* CONFIG_PROC_FS */
 
 /*
@@ -3829,6 +3885,9 @@ static struct notifier_block bond_netdev
 	.notifier_call = bond_netdev_event,
 };
 
+/* De-initialize device specific data.
+ * Caller must hold rtnl_lock.
+ */
 static inline void bond_deinit(struct net_device *dev)
 {
 	struct bonding *bond = dev->priv;
@@ -3840,6 +3899,9 @@ static inline void bond_deinit(struct ne
 #endif
 }
 
+/* Unregister and free all bond devices.
+ * Caller must hold rtnl_lock.
+ */
 static void bond_free_all(void)
 {
 	struct bonding *bond, *nxt;
@@ -3847,16 +3909,13 @@ static void bond_free_all(void)
 	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
 		struct net_device *dev = bond->device;
 
-		unregister_netdev(dev);
+		unregister_netdevice(dev);
 		bond_deinit(dev);
 		free_netdev(dev);
 	}
 
 #ifdef CONFIG_PROC_FS
-	if (bond_proc_dir) {
-		remove_proc_entry(DRV_NAME, proc_net);
-		bond_proc_dir = NULL;
-	}
+	bond_destroy_proc_dir();
 #endif
 }
 
@@ -4234,18 +4293,12 @@ static int __init bonding_init(void)
 		primary = NULL;
 	}
 
+	rtnl_lock();
+
 #ifdef CONFIG_PROC_FS
-	bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
-	if (bond_proc_dir == NULL)  {
-		printk(KERN_WARNING
-		       "bonding_init(): can not create /proc/net/" DRV_NAME);
-	} else {
-		bond_proc_dir->owner = THIS_MODULE;
-	}
+	bond_create_proc_dir();
 #endif
 
-	rtnl_lock();
-
 	err = 0;
 	for (no = 0; no < max_bonds; no++) {
 		struct net_device *dev;
@@ -4288,18 +4341,21 @@ static int __init bonding_init(void)
 	return 0;
 
 out_err:
-	rtnl_unlock();
-
 	/* free and unregister all bonds that were successfully added */
 	bond_free_all();
 
+	rtnl_unlock();
+
 	return err;
 }
 
 static void __exit bonding_exit(void)
 {
 	unregister_netdevice_notifier(&bond_netdev_notifier);
+
+	rtnl_lock();
 	bond_free_all();
+	rtnl_unlock();
 }
 
 module_init(bonding_init);

Re: [Bonding-devel] [PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[Jay Vosburgh]

>This patch makes bonding create the /proc/net/bonding directory only
>if it exists (and destroy it only if it's empty).
>
>Synchronization with the NETDEV_CHANGENAME event is achieved through
>the RTNL semaphore.

	I've managed to break this running some scripts/programs to
simultaneously insmod/rmmod bonding, change the interface names, and
cat /proc/net/bonding/bondX files.  After the scripts run for a while,
the system ends up in a state where /proc/net/bonding/ is always
empty, no matter how many instances of bonding are loaded.

	Removing all instances of bonding also removes
/proc/net/bonding/ from /proc/net ("ls /proc/net" does not show it),
but "ls /proc/net/bonding" will succeed, showing an empty directory.
A subsequent insmod of bonding will create an always-empty
/proc/net/bonding (which again shows up in "ls /proc/net").  This
seems to be bad.

	I'm running this test on a 2 way 933, 640MB memory running
2.4.23-pre9 with the patch appiled.  I've reproduced this result twice
by running all three scripts simultaneously.  I haven't yet determined
what the problem is, but I'm presuming there's a race in the patched
code somewhere.  My test programs (3 scripts and a C program) are
appended.  When the "readproc-random" script stops printing "success"
messages, the error has occured.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

insmod-random.sh:
#!/bin/sh

isucc=0
rsucc=0
while /bin/true; do
	insert=${RANDOM};
	insert=`expr ${insert} % 2`
	num=${RANDOM};
	num=`expr ${num} % 10`;

	if [ ${insert} -eq 1 ]; then
		insmod -o bonding${num} bonding mode=0 > /dev/null 2>&1
		if [ $? -eq 0 ]; then
		    isucc=`expr ${isucc} + 1`;
		    echo "insmod success " ${isucc};
		fi
	else
		rmmod bonding${num} > /dev/null 2>&1
		if [ $? -eq 0 ]; then
		    rsucc=`expr ${rsucc} + 1`;
		    echo "rmmod  success " ${rsucc};
		fi
	fi
done

readproc-random.sh:
#!/bin/sh

succ=0
while /bin/true; do
	num=`expr ${RANDOM} % 10`;

	cat /proc/net/bonding/bond${num} > /dev/null 2>&1
	if [ $? -eq 0 ]; then
		succ=`expr ${succ} + 1`
		echo "success " ${succ};
	fi

done

sifname-random.sh:
#!/bin/sh

succ=0;
while /bin/true; do
	num1=`expr ${RANDOM} % 10`;
	num2=`expr ${RANDOM} % 10`;
	if [ ${num1} -eq ${num2} ]; then
		continue;
	fi

	./sifname bond${num1} bond${num2} > /dev/null 2>&1
	if [ $? -eq 0]; then
		succ=`expr ${succ} + 1`
		echo "success " ${succ};
	fi

done

sifname.c:
#include <stdio.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <net/if.h>
#include <linux/sockios.h>
#include <sys/ioctl.h>

int
main(int argc, char **argv)
{
	int rv, s;
	struct ifreq ifr;

	if (argc != 3) {
		fprintf(stderr, "usage: %s oldifname newifname\n", argv[0]);
		exit(1);
	}

	s = socket(PF_INET, SOCK_STREAM, 0);
	if (-1 == s) {
		perror("socket");
		exit(1);
	}

	memset(&ifr, 0, sizeof(ifr));
	memmove(&ifr.ifr_name, argv[1], strlen(argv[1]));
	memmove(&ifr.ifr_newname, argv[2], strlen(argv[2]));

	rv = ioctl(s, SIOCSIFNAME, &ifr);
	if (-1 == rv) {
		perror("ioctl(SIOCSIFNAME)");
		exit(1);
	}

	return 0;
}

Re: [Bonding-devel] [PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[David S. Miller]

I think it's not worthwhile trying to break procfs handling
wrt. any kind of network devices in 2.4.x, there are lots
of known far reaching problems in this area (none of which
are bonding specific) that require large intrusive changes
to fix and therefore are not likely to be cured in the 2.4.x
tree.

Simply stated, references to procfs files do not create the
necessary references to the network devices they are created
for and therefore anything stressing this sort of thing
is going to explode.

We fixed all of this in 2.6.x

Re: [Bonding-devel] [PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[Jay Vosburgh]

>I think it's not worthwhile trying to break procfs handling
>wrt. any kind of network devices in 2.4.x, there are lots
>of known far reaching problems in this area (none of which
>are bonding specific) that require large intrusive changes
>to fix and therefore are not likely to be cured in the 2.4.x
>tree.
>
>Simply stated, references to procfs files do not create the
>necessary references to the network devices they are created
>for and therefore anything stressing this sort of thing
>is going to explode.

	Well, never mind, then!

	Jeff, can you apply Amir's patch, as it seems to work just
fine except for this ignorable problem?  I'll append the patch below
for your convenience.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com


From: Amir Noam <amir.noam@intel.com>
To: Jay Vosburgh <fubar@us.ibm.com>, jgarzik@pobox.com
Subject: [PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir
Date: Thu, 23 Oct 2003 17:39:42 +0200

This patch makes bonding create the /proc/net/bonding directory only
if it exists (and destroy it only if it's empty).

Synchronization with the NETDEV_CHANGENAME event is achieved through
the RTNL semaphore.

The patch applies on 2.4.23-pre8, after applying the patch that
restores backward compatibility sent by Shmulik on 2003/10/12.

Amir


diff -Narup a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c	Thu Oct 23 14:47:31 2003
+++ b/drivers/net/bonding/bond_main.c	Thu Oct 23 17:04:52 2003
@@ -3574,6 +3574,62 @@ static void bond_destroy_proc_info(struc
 		bond->bond_proc_file = NULL;
 	}
 }
+
+/* Create the bonding directory under /proc/net, if doesn't exist yet.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_create_proc_dir(void)
+{
+	int len = strlen(DRV_NAME);
+
+	for (bond_proc_dir = proc_net->subdir; bond_proc_dir;
+	     bond_proc_dir = bond_proc_dir->next) {
+		if ((bond_proc_dir->namelen == len) &&
+		    !memcmp(bond_proc_dir->name, DRV_NAME, len)) {
+			break;
+		}
+	}
+
+	if (!bond_proc_dir) {
+		bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
+		if (bond_proc_dir) {
+			bond_proc_dir->owner = THIS_MODULE;
+		} else {
+			printk(KERN_WARNING DRV_NAME
+				": Warning: cannot create /proc/net/%s\n",
+				DRV_NAME);
+		}
+	}
+}
+
+/* Destroy the bonding directory under /proc/net, if empty.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_destroy_proc_dir(void)
+{
+	struct proc_dir_entry *de;
+
+	if (!bond_proc_dir) {
+		return;
+	}
+
+	/* verify that the /proc dir is empty */
+	for (de = bond_proc_dir->subdir; de; de = de->next) {
+		/* ignore . and .. */
+		if (*(de->name) != '.') {
+			break;
+		}
+	}
+
+	if (de) {
+		if (bond_proc_dir->owner == THIS_MODULE) {
+			bond_proc_dir->owner = NULL;
+		}
+	} else {
+		remove_proc_entry(DRV_NAME, proc_net);
+		bond_proc_dir = NULL;
+	}
+}
 #endif /* CONFIG_PROC_FS */
 
 /*
@@ -3829,6 +3885,9 @@ static struct notifier_block bond_netdev
 	.notifier_call = bond_netdev_event,
 };
 
+/* De-initialize device specific data.
+ * Caller must hold rtnl_lock.
+ */
 static inline void bond_deinit(struct net_device *dev)
 {
 	struct bonding *bond = dev->priv;
@@ -3840,6 +3899,9 @@ static inline void bond_deinit(struct ne
 #endif
 }
 
+/* Unregister and free all bond devices.
+ * Caller must hold rtnl_lock.
+ */
 static void bond_free_all(void)
 {
 	struct bonding *bond, *nxt;
@@ -3847,16 +3909,13 @@ static void bond_free_all(void)
 	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
 		struct net_device *dev = bond->device;
 
-		unregister_netdev(dev);
+		unregister_netdevice(dev);
 		bond_deinit(dev);
 		free_netdev(dev);
 	}
 
 #ifdef CONFIG_PROC_FS
-	if (bond_proc_dir) {
-		remove_proc_entry(DRV_NAME, proc_net);
-		bond_proc_dir = NULL;
-	}
+	bond_destroy_proc_dir();
 #endif
 }
 
@@ -4234,18 +4293,12 @@ static int __init bonding_init(void)
 		primary = NULL;
 	}
 
+	rtnl_lock();
+
 #ifdef CONFIG_PROC_FS
-	bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
-	if (bond_proc_dir == NULL)  {
-		printk(KERN_WARNING
-		       "bonding_init(): can not create /proc/net/" DRV_NAME);
-	} else {
-		bond_proc_dir->owner = THIS_MODULE;
-	}
+	bond_create_proc_dir();
 #endif
 
-	rtnl_lock();
-
 	err = 0;
 	for (no = 0; no < max_bonds; no++) {
 		struct net_device *dev;
@@ -4288,18 +4341,21 @@ static int __init bonding_init(void)
 	return 0;
 
 out_err:
-	rtnl_unlock();
-
 	/* free and unregister all bonds that were successfully added */
 	bond_free_all();
 
+	rtnl_unlock();
+
 	return err;
 }
 
 static void __exit bonding_exit(void)
 {
 	unregister_netdevice_notifier(&bond_netdev_notifier);
+
+	rtnl_lock();
 	bond_free_all();
+	rtnl_unlock();
 }
 
 module_init(bonding_init);

Re: [Bonding-devel] [PATCH] [bonding 2.4] fix creating/destroying the /proc/net/bonding dir

[Amir Noam]

On Friday 31 October 2003 10:10 pm, Jay Vosburgh wrote:
>         Jeff, can you apply Amir's patch, as it seems to work
> just fine except for this ignorable problem?  I'll append the
> patch below for your convenience.

That patch also applies to 2.6 (after the "restore backward
compatibility" patch) with an offset of one line per chunk.

I've appended it here again, against 2.6, just in case you want it to
apply cleanly with no warnings.

Amir


diff -Naurp a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c	Sun Nov  2 15:06:08 2003
+++ b/drivers/net/bonding/bond_main.c	Sun Nov  2 15:06:18 2003
@@ -3573,6 +3573,62 @@ static void bond_destroy_proc_info(struc
 		bond->bond_proc_file = NULL;
 	}
 }
+
+/* Create the bonding directory under /proc/net, if doesn't exist yet.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_create_proc_dir(void)
+{
+	int len = strlen(DRV_NAME);
+
+	for (bond_proc_dir = proc_net->subdir; bond_proc_dir;
+	     bond_proc_dir = bond_proc_dir->next) {
+		if ((bond_proc_dir->namelen == len) &&
+		    !memcmp(bond_proc_dir->name, DRV_NAME, len)) {
+			break;
+		}
+	}
+
+	if (!bond_proc_dir) {
+		bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
+		if (bond_proc_dir) {
+			bond_proc_dir->owner = THIS_MODULE;
+		} else {
+			printk(KERN_WARNING DRV_NAME
+				": Warning: cannot create /proc/net/%s\n",
+				DRV_NAME);
+		}
+	}
+}
+
+/* Destroy the bonding directory under /proc/net, if empty.
+ * Caller must hold rtnl_lock.
+ */
+static void bond_destroy_proc_dir(void)
+{
+	struct proc_dir_entry *de;
+
+	if (!bond_proc_dir) {
+		return;
+	}
+
+	/* verify that the /proc dir is empty */
+	for (de = bond_proc_dir->subdir; de; de = de->next) {
+		/* ignore . and .. */
+		if (*(de->name) != '.') {
+			break;
+		}
+	}
+
+	if (de) {
+		if (bond_proc_dir->owner == THIS_MODULE) {
+			bond_proc_dir->owner = NULL;
+		}
+	} else {
+		remove_proc_entry(DRV_NAME, proc_net);
+		bond_proc_dir = NULL;
+	}
+}
 #endif /* CONFIG_PROC_FS */
 
 /*
@@ -3828,6 +3884,9 @@ static struct notifier_block bond_netdev
 	.notifier_call = bond_netdev_event,
 };
 
+/* De-initialize device specific data.
+ * Caller must hold rtnl_lock.
+ */
 static inline void bond_deinit(struct net_device *dev)
 {
 	struct bonding *bond = dev->priv;
@@ -3839,6 +3898,9 @@ static inline void bond_deinit(struct ne
 #endif
 }
 
+/* Unregister and free all bond devices.
+ * Caller must hold rtnl_lock.
+ */
 static void bond_free_all(void)
 {
 	struct bonding *bond, *nxt;
@@ -3846,16 +3908,13 @@ static void bond_free_all(void)
 	list_for_each_entry_safe(bond, nxt, &bond_dev_list, bond_list) {
 		struct net_device *dev = bond->device;
 
-		unregister_netdev(dev);
+		unregister_netdevice(dev);
 		bond_deinit(dev);
 		free_netdev(dev);
 	}
 
 #ifdef CONFIG_PROC_FS
-	if (bond_proc_dir) {
-		remove_proc_entry(DRV_NAME, proc_net);
-		bond_proc_dir = NULL;
-	}
+	bond_destroy_proc_dir();
 #endif
 }
 
@@ -4233,18 +4292,12 @@ static int __init bonding_init(void)
 		primary = NULL;
 	}
 
+	rtnl_lock();
+
 #ifdef CONFIG_PROC_FS
-	bond_proc_dir = proc_mkdir(DRV_NAME, proc_net);
-	if (bond_proc_dir == NULL)  {
-		printk(KERN_WARNING
-		       "bonding_init(): can not create /proc/net/" DRV_NAME);
-	} else {
-		bond_proc_dir->owner = THIS_MODULE;
-	}
+	bond_create_proc_dir();
 #endif
 
-	rtnl_lock();
-
 	err = 0;
 	for (no = 0; no < max_bonds; no++) {
 		struct net_device *dev;
@@ -4287,18 +4340,21 @@ static int __init bonding_init(void)
 	return 0;
 
 out_err:
-	rtnl_unlock();
-
 	/* free and unregister all bonds that were successfully added */
 	bond_free_all();
 
+	rtnl_unlock();
+
 	return err;
 }
 
 static void __exit bonding_exit(void)
 {
 	unregister_netdevice_notifier(&bond_netdev_notifier);
+
+	rtnl_lock();
 	bond_free_all();
+	rtnl_unlock();
 }
 
 module_init(bonding_init);