cleanup netif handling and netif-dev.d/ events

cleanup netif handling and netif-dev.d/ events

[Kay Sievers]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

Here we supress the dev.d/ execution if we didn't change a network
interface's name with a rule. This should solve the issue of two
running dhclients for the same interface, cause the
/etc/dev.d/net/hotplug.dev script that fakes the hotplug event runs
with every udevstart for every interface and fakes a second identical
hotplug event on bootup.

With this patch netif interfaces are no longer stored in the udevdb.
It is not needed, cause we don't have permissions or symlinks :) and
all information is available in sysfs.

This patch also moves the dev_d execution calls out of the
udev_add/udev_remove. As with the former api-cleanup-patch we have
all processed data in one udev struct and can place the execution
calls where needed.

Thanks,
Kay

[-- Attachment #2: udev-dev.d-cleanup-01.patch --]
[-- Type: text/plain, Size: 8606 bytes --]

diff -Nru a/dev_d.c b/dev_d.c
--- a/dev_d.c	2004-10-18 19:48:33 +02:00
+++ b/dev_d.c	2004-10-18 19:48:33 +02:00
@@ -62,7 +62,7 @@
 
 		execv(name, argv);
 		dbg("exec of child failed");
-		exit(1);
+		_exit(1);
 	case -1:
 		dbg("fork of child failed");
 		break;
@@ -80,42 +80,35 @@
  * 	subsystem/
  * 	default/
  */
-void dev_d_send(struct udevice *udev)
+void dev_d_execute(struct udevice *udev)
 {
-	char dirname[256];
-	char env_devname[NAME_SIZE];
-	char *devname;
+	char dirname[PATH_MAX];
+	char devname[NAME_SIZE];
 	char *temp;
 
+	/* skip if UDEV_NO_DEVD is set */
 	if (udev_dev_d == 0)
 		return;
 
-	memset(env_devname, 0x00, sizeof(env_devname));
-	if (udev->type == 'b' || udev->type == 'c') {
-		strfieldcpy(env_devname, udev_root);
-		strfieldcat(env_devname, udev->name);
-	} else if (udev->type == 'n') {
-		strfieldcpy(env_devname, udev->name);
-		setenv("DEVPATH", udev->devpath, 1);
-	}
-	setenv("DEVNAME", env_devname, 1);
-	dbg("DEVNAME='%s'", env_devname);
-
-	devname = strdup(udev->name);
-	if (!devname) {
-		dbg("out of memory");
+	/* skip if udev did nothing, like unchanged netif or no "dev" file */
+	if (udev->devname[0] == '\0')
 		return;
-	}
+
+	/* add the node name or the netif name to the environment */
+	setenv("DEVNAME", udev->devname, 1);
+	dbg("DEVNAME='%s'", udev->devname);
+
+	strfieldcpy(devname, udev->name);
 
 	/* Chop the device name up into pieces based on '/' */
 	temp = strchr(devname, '/');
 	while (temp != NULL) {
-		*temp = 0x00;
+		temp[0] = '\0';
 		strcpy(dirname, DEVD_DIR);
 		strfieldcat(dirname, devname);
 		call_foreach_file(run_program, dirname, DEVD_SUFFIX);
 
-		*temp = '/';
+		temp[0] = '/';
 		++temp;
 		temp = strchr(temp, '/');
 	}
diff -Nru a/udev.c b/udev.c
--- a/udev.c	2004-10-18 19:48:33 +02:00
+++ b/udev.c	2004-10-18 19:48:33 +02:00
@@ -210,10 +210,20 @@
 
 		/* name, create node, store in db */
 		retval = udev_add_device(&udev, class_dev);
+
+		/* run scripts */
+		dev_d_execute(&udev);
+
+		sysfs_close_class_device(class_dev);
 		break;
 	case REMOVE:
 		dbg("udev remove");
+
+		/* get node from db, delete it*/
 		retval = udev_remove_device(&udev);
+
+		/* run scripts */
+		dev_d_execute(&udev);
 	}
 
 	udevdb_exit();
diff -Nru a/udev.h b/udev.h
--- a/udev.h	2004-10-18 19:48:33 +02:00
+++ b/udev.h	2004-10-18 19:48:33 +02:00
@@ -66,6 +66,7 @@
 	char program_result[NAME_SIZE];
 	char kernel_number[NAME_SIZE];
 	char kernel_name[NAME_SIZE];
+	char devname[NAME_SIZE];
 	int test_run;
 };
 
@@ -74,7 +75,7 @@
 extern void udev_init_config(void);
 extern int udev_start(void);
 extern int parse_get_pair(char **orig_string, char **left, char **right);
-extern void dev_d_send(struct udevice *udev);
+extern void dev_d_execute(struct udevice *udev);
 
 extern char **main_argv;
 extern char **main_envp;
diff -Nru a/udev_add.c b/udev_add.c
--- a/udev_add.c	2004-10-18 19:48:33 +02:00
+++ b/udev_add.c	2004-10-18 19:48:33 +02:00
@@ -355,7 +355,7 @@
 		retval = get_major_minor(class_dev, udev);
 		if (retval != 0) {
 			dbg("no dev-file found, do nothing");
-			goto close;
+			return 0;
 		}
 	}
 
@@ -365,45 +365,44 @@
 	dbg("adding name='%s'", udev->name);
 
 	selinux_init();
-	switch (udev->type) {
-	case 'b':
-	case 'c':
+
+	if (udev->type == 'b' || udev->type == 'c') {
 		retval = create_node(udev);
 		if (retval != 0)
 			goto exit;
-		if ((!udev->test_run) && (udevdb_add_dev(udev) != 0))
-			dbg("udevdb_add_dev failed, but we are going to try "
-			    "to create the node anyway. But remove might not "
-			    "work properly for this device.");
 
-		dev_d_send(udev);
-		break;
+		if (udevdb_add_dev(udev) != 0)
+			dbg("udevdb_add_dev failed, but we create the node anyway, "
+			    "remove might not work for custom names");
+
+		/* use full path to the environment */
+		snprintf(udev->devname, NAME_SIZE-1, "%s%s", udev_root, udev->name);
 
-	case 'n':
+	} else if (udev->type == 'n') {
+		/* look if we want to change the name of the netif */
 		if (strcmp(udev->name, udev->kernel_name) != 0) {
 			retval = rename_net_if(udev);
 			if (retval != 0)
 				goto exit;
-			/* netif's are keyed with the configured name, cause
-			 * the original kernel name sleeps with the fishes
+
+			/* we've changed the name, now fake the devpath,
+			 * cause original kernel name sleeps with the fishes
+			 * and we don't get any event from the kernel now
 			 */
 			pos = strrchr(udev->devpath, '/');
 			if (pos != NULL) {
 				pos[1] = '\0';
 				strfieldcat(udev->devpath, udev->name);
+				setenv("DEVPATH", udev->devpath, 1);
 			}
-		}
-		if ((!udev->test_run) && (udevdb_add_dev(udev) != 0))
-			dbg("udevdb_add_dev failed");
 
-		dev_d_send(udev);
-		break;
+			/* use netif name for the environment */
+			strfieldcpy(udev->devname, udev->name);
+		}
 	}
 
 exit:
 	selinux_restore();
-close:
-	sysfs_close_class_device(class_dev);
 
 	return retval;
 }
diff -Nru a/udev_remove.c b/udev_remove.c
--- a/udev_remove.c	2004-10-18 19:48:33 +02:00
+++ b/udev_remove.c	2004-10-18 19:48:33 +02:00
@@ -36,7 +36,7 @@
 #include "namedev.h"
 #include "udevdb.h"
 
-static int delete_path(char *path)
+static int delete_path(const char *path)
 {
 	char *pos;
 	int retval;
@@ -168,14 +168,15 @@
 int udev_remove_device(struct udevice *udev)
 {
 	struct udevice db_dev;
-	char *temp;
+	const char *temp;
 	int retval;
 
-	memset(&db_dev, 0x00, sizeof(struct udevice));
+	if (udev->type != 'b' && udev->type != 'c')
+		return 0;
 
 	retval = udevdb_get_dev(udev->devpath, &db_dev);
 	if (retval == 0) {
-		/* get stored values in our device */
+		/* copy over the stored values to our device */
 		memcpy(udev, &db_dev, UDEVICE_DB_LEN);
 	} else {
 		/* fall back to kernel name */
@@ -185,15 +186,12 @@
 		strfieldcpy(udev->name, &temp[1]);
 		dbg("'%s' not found in database, falling back on default name", udev->name);
 	}
-	dbg("remove name='%s'", udev->name);
 
-	dev_d_send(udev);
+	dbg("remove name='%s'", udev->name);
 	udevdb_delete_dev(udev->devpath);
 
-	if (udev->type == 'b' || udev->type == 'c')
-		retval = delete_node(udev);
-	else
-		retval = 0;
+	/* use full path to the environment */
+	snprintf(udev->devname, NAME_SIZE-1, "%s%s", udev_root, udev->name);
 
-	return retval;
+	return delete_node(udev);
 }
diff -Nru a/udevdb.c b/udevdb.c
--- a/udevdb.c	2004-10-18 19:48:33 +02:00
+++ b/udevdb.c	2004-10-18 19:48:33 +02:00
@@ -49,6 +49,9 @@
 	TDB_DATA key, data;
 	char keystr[SYSFS_PATH_MAX];
 
+	if (udev->test_run)
+		return 0;
+
 	if (udevdb == NULL)
 		return -1;
 
diff -Nru a/udevstart.c b/udevstart.c
--- a/udevstart.c	2004-10-18 19:48:33 +02:00
+++ b/udevstart.c	2004-10-18 19:48:33 +02:00
@@ -103,7 +103,7 @@
 	setenv("DEVPATH", devpath, 1);
 	setenv("ACTION", "add", 1);
 
-	snprintf(path, SYSFS_PATH_MAX, "%s%s", sysfs_path, devpath);
+	snprintf(path, SYSFS_PATH_MAX-1, "%s%s", sysfs_path, devpath);
 	class_dev = sysfs_open_class_device_path(path);
 	if (class_dev == NULL) {
 		dbg ("sysfs_open_class_device_path failed");
@@ -111,8 +111,14 @@
 	}
 
 	udev_set_values(&udev, devpath, subsystem);
+	udev_add_device(&udev, class_dev);
 
-	return udev_add_device(&udev, class_dev);
+	/* run scripts */
+	dev_d_execute(&udev);
+
+	sysfs_close_class_device(class_dev);
+
+	return 0;
 }
 
 static void exec_list(struct list_head *device_list)
diff -Nru a/udevtest.c b/udevtest.c
--- a/udevtest.c	2004-10-18 19:48:33 +02:00
+++ b/udevtest.c	2004-10-18 19:48:33 +02:00
@@ -69,12 +69,9 @@
 
 	if (argv[1] == NULL) {
 		info("udevinfo expects the DEVPATH of the sysfs device as a argument");
-		goto exit;
+		return 1;
 	}
 
-	/* initialize our configuration */
-	udev_init_config();
-
 	/* remove sysfs_path if given */
 	if (strncmp(argv[1], sysfs_path, strlen(sysfs_path)) == 0)
 		devpath = argv[1] + strlen(sysfs_path);
@@ -93,9 +90,12 @@
 	/* we only care about class devices and block stuff */
 	if (!strstr(devpath, "class") && !strstr(devpath, "block")) {
 		dbg("not a block or class device");
-		goto exit;
+		return 2;
 	}
 
+	/* initialize our configuration */
+	udev_init_config();
+
 	/* initialize the naming deamon */
 	namedev_init();
 
@@ -104,7 +104,6 @@
 
 	/* fill in values and test_run flag*/
 	udev_set_values(&udev, devpath, subsystem);
-	udev.test_run = 1;
 
 	/* open the device */
 	snprintf(path, SYSFS_PATH_MAX, "%s%s", sysfs_path, udev.devpath);
@@ -114,9 +113,11 @@
 	else
 		dbg("opened class_dev->name='%s'", class_dev->name);
 
-	/* simulate node creation with fake flag */
+	/* simulate node creation with test flag */
+	udev.test_run = 1;
 	udev_add_device(&udev, class_dev);
 
-exit:
+	sysfs_close_class_device(class_dev);
+
 	return 0;
 }

Re: cleanup netif handling and netif-dev.d/ events

[Greg KH]

On Mon, Oct 18, 2004 at 08:17:13PM +0200, Kay Sievers wrote:
> Here we supress the dev.d/ execution if we didn't change a network
> interface's name with a rule. This should solve the issue of two
> running dhclients for the same interface, cause the
> /etc/dev.d/net/hotplug.dev script that fakes the hotplug event runs
> with every udevstart for every interface and fakes a second identical
> hotplug event on bootup.

Huh?  /etc/dev.d/net/hotplug.dev should contain the following three
lines at the beginning to prevent this:
	# See if we really did rename this device.
	if [ "$INTERFACE" = "$DEVNAME" ]; then
		exit 0
	fi

right?  What distro doesn't do this?

Do we want to hardcode this logic inside udev itself now?

> With this patch netif interfaces are no longer stored in the udevdb.
> It is not needed, cause we don't have permissions or symlinks :) and
> all information is available in sysfs.

That's good.

> This patch also moves the dev_d execution calls out of the
> udev_add/udev_remove. As with the former api-cleanup-patch we have
> all processed data in one udev struct and can place the execution
> calls where needed.

Very nice.

Applied, thanks.

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: cleanup netif handling and netif-dev.d/ events

[Kay Sievers]

On Mon, Oct 18, 2004 at 11:28:06AM -0700, Greg KH wrote:
> On Mon, Oct 18, 2004 at 08:17:13PM +0200, Kay Sievers wrote:
> > Here we supress the dev.d/ execution if we didn't change a network
> > interface's name with a rule. This should solve the issue of two
> > running dhclients for the same interface, cause the
> > /etc/dev.d/net/hotplug.dev script that fakes the hotplug event runs
> > with every udevstart for every interface and fakes a second identical
> > hotplug event on bootup.
> 
> Huh?  /etc/dev.d/net/hotplug.dev should contain the following three
> lines at the beginning to prevent this:
> 	# See if we really did rename this device.
> 	if [ "$INTERFACE" = "$DEVNAME" ]; then
> 		exit 0
> 	fi

I don't think that $INTERFACE is available to the dev.d/ scripts.
I will look into the hotplug "rename" event, if the uevent patches
reached the mainline kernel :)

> right?  What distro doesn't do this?
> 
> Do we want to hardcode this logic inside udev itself now?

No, if we don't create a node, or don't rename a netif, we just don't
call dev.d/ now.

Kay


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: cleanup netif handling and netif-dev.d/ events

[Greg KH]

On Mon, Oct 18, 2004 at 08:44:23PM +0200, Kay Sievers wrote:
> On Mon, Oct 18, 2004 at 11:28:06AM -0700, Greg KH wrote:
> > On Mon, Oct 18, 2004 at 08:17:13PM +0200, Kay Sievers wrote:
> > > Here we supress the dev.d/ execution if we didn't change a network
> > > interface's name with a rule. This should solve the issue of two
> > > running dhclients for the same interface, cause the
> > > /etc/dev.d/net/hotplug.dev script that fakes the hotplug event runs
> > > with every udevstart for every interface and fakes a second identical
> > > hotplug event on bootup.
> > 
> > Huh?  /etc/dev.d/net/hotplug.dev should contain the following three
> > lines at the beginning to prevent this:
> > 	# See if we really did rename this device.
> > 	if [ "$INTERFACE" = "$DEVNAME" ]; then
> > 		exit 0
> > 	fi
> 
> I don't think that $INTERFACE is available to the dev.d/ scripts.
> I will look into the hotplug "rename" event, if the uevent patches
> reached the mainline kernel :)

Heh, you mean this always is false.  bah, so much for checking...

> > right?  What distro doesn't do this?
> > 
> > Do we want to hardcode this logic inside udev itself now?
> 
> No, if we don't create a node, or don't rename a netif, we just don't
> call dev.d/ now.

Ok, that's a good thing to do.

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel