[PATCH] udev: add rule based program execution

[PATCH] udev: add rule based program execution

[Kay Sievers]

This patch on top of the current bk-tree allows to specify programs to
be executed after the device handling. This is meant as a general
replacement for the dev.d/ logic which is known to be the main reason for
the boot-time delay on udev systems.

This patch introduces the HOTPLUG key. Every matching rule can add one
HOTPLUG key to a list of program to be executed after the node is
created/removed.

The HOTPLUG key can be used in any kind of rule:
  HOTPLUG="/sbin/program"
    will execute the program for every event that reaches udev. Just
    like /etc/dev.d/default/*.

  SUBSYSTEM="block", HOTPLUG="/sbin/program"
    will execute the program only for block device events.

  ACTION="remove", SUBSYSTEM="block", HOTPLUG="/sbin/program"
    will execute the program, if a block device is removed.

  ACTION="add, BUS="usb", KERNEL="video*", SYSFS{idProduct}="a511", HOTPLUG="/sbin/webcam-program"
    will execute the program only if my old webcam is connected.

To get this working, I changed the logic to read all rules and not to
stop at the first match. If a rule should be the last one to be applied
to a device it must use OPTION="last_rule". After the first rule that
assigns a NAME to a device, all later rules with a NAME key will be
ignored, so it should not change the current behavior too much.
The rules are also read on "remove" events now.

On my box I've removed all scripts from /etc/dev.d and replaced it with
following rules in the main rules file:

  ACTION="add", SUBSYSTEM="sound", HOTPLUG="/etc/udev/scripts/alsa.dev"
  ACTION="add", KERNEL="fd*", HOTPLUG="/etc/udev/scripts/MAKEDEV.dev"
  KERNEL="ttyUSB*", HOTPLUG="/etc/udev/scripts/test-serial-device"
  SUBSYSTEM="tty", OPTIONS="last_rule"
  HOTPLUG="/etc/udev/scripts/pam_console.dev"
  HOTPLUG="/etc/udev/sripts/log.dev"

This cuts down the execution-time of udevstart from ~10 seconds to ~2
seconds, cause the hundreds of tty events will not call any script.

Thanks,
Kay

=== udev.8.in 1.83 vs edited ==--- 1.83/udev.8.in	2005-03-26 16:11:01 +01:00
+++ edited/udev.8.in	2005-03-29 14:51:32 +02:00
@@ -111,6 +111,9 @@ Match the kernel device name.
 .B SUBSYSTEM
 Match the kernel subsystem name.
 .TP
+.B ACTION
+Match the kernel action name.
+.TP
 .B DRIVER
 Match the kernel driver name.
 .TP
@@ -170,6 +173,9 @@ distribution provided rules file.
 The permissions for the device node. Every specified value overwrites the
 compiled-in default value.
 .TP
+.B HOTPLUG
+Add a program to the list of programs to be executed for a specific device.
+.TP
 .B OPTIONS
 .B last_rule
 will be the last rule applied. No later rules will have any effect.
@@ -234,6 +240,9 @@ The node name of the parent device.
 .BI %s{ filename }
 The content of a sysfs attribute.
 .TP
+.B %r
+The udev_root value.
+.TP
 .B %e
 If a device node already exists with the name, the smallest positive
 decimal integer N is substituted such that the resulting name doesn't
@@ -344,6 +353,9 @@ config file.
 .B UDEV_LOG
 Overrides the log priority specified in the config file.
 .TP
+.B UDEV_HOTPLUG
+If set to "0", it disables the execution of programs added by rules.
+.TP
 .B UDEV_NO_DEVD
 The default behavior of
 .B udev
@@ -356,7 +368,6 @@ will skip this step.
 .nf
 /sbin/udev                           udev program
 /etc/udev/*                          udev config files
-/etc/hotplug.d/default/udev.hotplug  hotplug symlink to udev program
 /etc/dev.d/*                         programs invoked by udev
 .fi
 .SH "SEE ALSO"
=== udev.c 1.113 vs edited ==--- 1.113/udev.c	2005-03-29 03:25:52 +02:00
+++ edited/udev.c	2005-03-29 15:20:13 +02:00
@@ -157,6 +157,8 @@ int main(int argc, char *argv[], char *e
 	}
 
 	if (udev.type = DEV_BLOCK || udev.type = DEV_CLASS || udev.type = DEV_NET) {
+		udev_rules_init();
+
 		if (strcmp(action, "add") = 0) {
 			/* wait for sysfs and possibly add node */
 			dbg("udev add");
@@ -178,9 +180,6 @@ int main(int argc, char *argv[], char *e
 
 			wait_for_class_device(class_dev, &error);
 
-			/* init rules */
-			udev_rules_init();
-
 			/* name, create node, store in db */
 			retval = udev_add_device(&udev, class_dev);
 
@@ -195,8 +194,23 @@ int main(int argc, char *argv[], char *e
 				goto hotplug;
 			}
 
+			udev_rules_get_hotplug(&udev);
+			if (udev.ignore_device) {
+				dbg("device event will be ignored");
+				goto hotplug;
+			}
+
 			/* get node from db, remove db-entry, delete created node */
 			retval = udev_remove_device(&udev);
+		}
+
+		/* execute collected hotplug programs */
+		if (udev_hotplug && !list_empty(&udev.hotplug_list)) {
+			struct name_entry *name_loop;
+
+			dbg("executing hotplug program list");
+			list_for_each_entry(name_loop, &udev.hotplug_list, node)
+				execute_command(name_loop->name, udev.subsystem);
 		}
 
 		/* run dev.d/ scripts if we created/deleted a node or changed a netif name */
=== udev.h 1.93 vs edited ==--- 1.93/udev.h	2005-03-26 16:11:01 +01:00
+++ edited/udev.h	2005-03-29 15:15:52 +02:00
@@ -58,6 +58,7 @@ enum device_type {
 struct udevice {
 	char devpath[PATH_SIZE];
 	char subsystem[NAME_SIZE];
+	char action[NAME_SIZE];
 
 	enum device_type type;
 	char name[PATH_SIZE];
@@ -67,9 +68,11 @@ struct udevice {
 	char group[USER_SIZE];
 	mode_t mode;
 	dev_t devt;
+	struct list_head hotplug_list;
 
 	char tmp_node[PATH_SIZE];
 	int partitions;
+	int ignore_device;
 	int ignore_remove;
 	int config_line;
 	char config_file[PATH_SIZE];
@@ -93,6 +96,7 @@ extern char udev_db_path[PATH_SIZE];
 extern char udev_config_filename[PATH_SIZE];
 extern char udev_rules_filename[PATH_SIZE];
 extern int udev_log_priority;
+extern int udev_hotplug;
 extern int udev_dev_d;
 extern int udev_hotplug_d;
 
=== udev_add.c 1.105 vs edited ==--- 1.105/udev_add.c	2005-03-26 16:11:01 +01:00
+++ edited/udev_add.c	2005-03-29 15:22:52 +02:00
@@ -276,8 +276,11 @@ int udev_add_device(struct udevice *udev
 		}
 	}
 
-	if (udev_rules_get_name(udev, class_dev) != 0)
+	udev_rules_get_name(udev, class_dev);
+	if (udev->ignore_device) {
+		dbg("device event will be ignored");
 		return 0;
+	}
 
 	dbg("adding name='%s'", udev->name);
 
=== udev_config.c 1.38 vs edited ==--- 1.38/udev_config.c	2005-03-27 04:39:11 +02:00
+++ edited/udev_config.c	2005-03-29 14:51:32 +02:00
@@ -44,6 +44,7 @@ char udev_db_path[PATH_SIZE];
 char udev_config_filename[PATH_SIZE];
 char udev_rules_filename[PATH_SIZE];
 int udev_log_priority;
+int udev_hotplug;
 int udev_dev_d;
 int udev_hotplug_d;
 
@@ -217,9 +218,15 @@ void udev_init_config(void)
 	strcpy(udev_config_filename, UDEV_CONFIG_FILE);
 	strcpy(udev_rules_filename, UDEV_RULES_FILE);
 	udev_log_priority = LOG_ERR;
+	udev_hotplug = 1;
 	udev_dev_d = 1;
 	udev_hotplug_d = 1;
 	sysfs_get_mnt_path(sysfs_path, sizeof(sysfs_path));
+
+	/* disable HOTPLUG program execution */
+	env = getenv("UDEV_HOTPLUG");
+	if (env && !string_is_true(env))
+		udev_hotplug = 0;
 
 	env = getenv("UDEV_NO_DEVD");
 	if (env && string_is_true(env))
=== udev_rules.c 1.213 vs edited ==--- 1.213/udev_rules.c	2005-03-29 03:25:52 +02:00
+++ edited/udev_rules.c	2005-03-29 15:16:47 +02:00
@@ -474,6 +474,21 @@ static int match_rule(struct udevice *ud
 {
 	struct sysfs_device *parent_device = sysfs_device;
 
+	if (rule->action_operation != KEY_OP_UNSET) {
+		dbg("check for " KEY_ACTION " rule->action='%s' udev->action='%s'",
+		    rule->action, udev->action);
+		if (strcmp_pattern(rule->action, udev->action) != 0) {
+			dbg(KEY_ACTION " is not matching");
+			if (rule->action_operation != KEY_OP_NOMATCH)
+				goto exit;
+		} else {
+			dbg(KEY_ACTION " matches");
+			if (rule->action_operation = KEY_OP_NOMATCH)
+				goto exit;
+		}
+		dbg(KEY_ACTION " key is true");
+	}
+
 	if (rule->kernel_operation != KEY_OP_UNSET) {
 		dbg("check for " KEY_KERNEL " rule->kernel='%s' udev_kernel_name='%s'",
 		    rule->kernel, udev->kernel_name);
@@ -716,12 +731,17 @@ int udev_rules_get_name(struct udevice *
 	list_for_each_entry(rule, &udev_rule_list, node) {
 		dbg("process rule");
 		if (match_rule(udev, rule, class_dev, sysfs_device) = 0) {
+			if (udev->name[0] != '\0' && rule->name[0] != '\0') {
+				dbg("node name already set, rule ignored");
+				continue;
+			}
 
 			/* apply options */
 			if (rule->ignore_device) {
 				info("configured rule in '%s[%i]' applied, '%s' is ignored",
 				     rule->config_file, rule->config_line, udev->kernel_name);
-				return -1;
+				udev->ignore_device = 1;
+				return 0;
 			}
 			if (rule->ignore_remove) {
 				udev->ignore_remove = 1;
@@ -773,7 +793,7 @@ int udev_rules_get_name(struct udevice *
 				name_list_add(&udev->symlink_list, pos, 0);
 			}
 
-			/* rule matches */
+			/* set name, later rules with name set will be ignored */
 			if (rule->name[0] != '\0') {
 				info("configured rule in '%s[%i]' applied, '%s' becomes '%s'",
 				     rule->config_file, rule->config_line, udev->kernel_name, rule->name);
@@ -786,20 +806,25 @@ int udev_rules_get_name(struct udevice *
 				if (udev->type != DEV_NET)
 					dbg("name, '%s' is going to have owner='%s', group='%s', mode=%#o partitions=%i",
 					    udev->name, udev->owner, udev->group, udev->mode, udev->partitions);
+			}
 
-				break;
+			if (rule->hotplug[0] != '\0') {
+				char program[PATH_SIZE];
+
+				strlcpy(program, rule->hotplug, sizeof(program));
+				apply_format(udev, program, sizeof(program), class_dev, sysfs_device);
+				dbg("add hotplug program '%s'", program);
+				name_list_add(&udev->hotplug_list, program, 0);
 			}
 
 			if (rule->last_rule) {
 				dbg("last rule to be applied");
 				break;
 			}
-
 		}
 	}
 
 	if (udev->name[0] = '\0') {
-		/* no rule matched, so we use the kernel name */
 		strlcpy(udev->name, udev->kernel_name, sizeof(udev->name));
 		info("no rule found, use kernel name '%s'", udev->name);
 	}
@@ -808,6 +833,119 @@ int udev_rules_get_name(struct udevice *
 		dbg("removing temporary device node");
 		unlink_secure(udev->tmp_node);
 		udev->tmp_node[0] = '\0';
+	}
+
+	return 0;
+}
+
+int udev_rules_get_hotplug(struct udevice *udev)
+{
+	struct udev_rule *rule;
+
+	/* look for a matching rule to apply */
+	list_for_each_entry(rule, &udev_rule_list, node) {
+		dbg("process rule");
+
+		if (rule->name[0] != '\0' || rule->symlink[0] != '\0' ||
+		    rule->mode != 0000 || rule->owner[0] != '\0' || rule->group[0] != '\0') {
+			dbg("skip rule that names a device");
+			continue;
+		}
+
+		if (rule->action_operation != KEY_OP_UNSET) {
+			dbg("check for " KEY_ACTION " rule->action='%s' udev->action='%s'",
+			    rule->action, udev->action);
+			if (strcmp_pattern(rule->action, udev->action) != 0) {
+				dbg(KEY_ACTION " is not matching");
+				if (rule->action_operation != KEY_OP_NOMATCH)
+					continue;
+			} else {
+				dbg(KEY_ACTION " matches");
+				if (rule->action_operation = KEY_OP_NOMATCH)
+					continue;
+			}
+			dbg(KEY_ACTION " key is true");
+		}
+
+		if (rule->kernel_operation != KEY_OP_UNSET) {
+			dbg("check for " KEY_KERNEL " rule->kernel='%s' udev->kernel_name='%s'",
+			    rule->kernel, udev->kernel_name);
+			if (strcmp_pattern(rule->kernel, udev->kernel_name) != 0) {
+				dbg(KEY_KERNEL " is not matching");
+				if (rule->kernel_operation != KEY_OP_NOMATCH)
+					continue;
+		} else {
+				dbg(KEY_KERNEL " matches");
+				if (rule->kernel_operation = KEY_OP_NOMATCH)
+					continue;
+			}
+			dbg(KEY_KERNEL " key is true");
+		}
+
+		if (rule->subsystem_operation != KEY_OP_UNSET) {
+			dbg("check for " KEY_SUBSYSTEM " rule->subsystem='%s' udev->subsystem='%s'",
+			    rule->subsystem, udev->subsystem);
+			if (strcmp_pattern(rule->subsystem, udev->subsystem) != 0) {
+				dbg(KEY_SUBSYSTEM " is not matching");
+				if (rule->subsystem_operation != KEY_OP_NOMATCH)
+					continue;
+			} else {
+				dbg(KEY_SUBSYSTEM " matches");
+				if (rule->subsystem_operation = KEY_OP_NOMATCH)
+					continue;
+			}
+			dbg(KEY_SUBSYSTEM " key is true");
+		}
+
+		if (rule->env_pair_count) {
+			int i;
+
+			dbg("check for " KEY_ENV " pairs");
+			for (i = 0; i < rule->env_pair_count; i++) {
+				struct key_pair *pair;
+				const char *value;
+
+				pair = &rule->env_pair[i];
+				value = getenv(pair->name);
+				if (!value) {
+					dbg(KEY_ENV "{'%s'} is not found", pair->name);
+					continue;
+				}
+				if (strcmp_pattern(pair->value, value) != 0) {
+					dbg(KEY_ENV "{'%s'} is not matching", pair->name);
+					if (pair->operation != KEY_OP_NOMATCH)
+						continue;
+				} else {
+					dbg(KEY_ENV "{'%s'} matches", pair->name);
+					if (pair->operation = KEY_OP_NOMATCH)
+						continue;
+				}
+			}
+			dbg(KEY_ENV " key is true");
+		}
+
+		/* rule matches */
+
+		if (rule->ignore_device) {
+			info("configured rule in '%s[%i]' applied, '%s' is ignored",
+			     rule->config_file, rule->config_line, udev->kernel_name);
+			udev->ignore_device = 1;
+			return 0;
+		}
+
+		if (rule->hotplug[0] != '\0') {
+			char program[PATH_SIZE];
+
+			strlcpy(program, rule->hotplug, sizeof(program));
+			apply_format(udev, program, sizeof(program), NULL, NULL);
+			dbg("add hotplug program '%s'", program);
+			name_list_add(&udev->hotplug_list, program, 0);
+		}
+
+		if (rule->last_rule) {
+			dbg("last rule to be applied");
+			break;
+		}
 	}
 
 	return 0;
=== udev_rules.h 1.51 vs edited ==--- 1.51/udev_rules.h	2005-03-28 04:20:05 +02:00
+++ edited/udev_rules.h	2005-03-29 14:51:32 +02:00
@@ -30,6 +30,7 @@
 
 #define KEY_KERNEL		"KERNEL"
 #define KEY_SUBSYSTEM		"SUBSYSTEM"
+#define KEY_ACTION		"ACTION"
 #define KEY_BUS			"BUS"
 #define KEY_ID			"ID"
 #define KEY_PROGRAM		"PROGRAM"
@@ -42,6 +43,7 @@
 #define KEY_OWNER		"OWNER"
 #define KEY_GROUP		"GROUP"
 #define KEY_MODE		"MODE"
+#define KEY_HOTPLUG		"HOTPLUG"
 #define KEY_OPTIONS		"OPTIONS"
 
 #define OPTION_LAST_RULE	"last_rule"
@@ -75,6 +77,8 @@ struct udev_rule {
 	enum key_operation kernel_operation;
 	char subsystem[NAME_SIZE];
 	enum key_operation subsystem_operation;
+	char action[NAME_SIZE];
+	enum key_operation action_operation;
 	char bus[NAME_SIZE];
 	enum key_operation bus_operation;
 	char id[NAME_SIZE];
@@ -95,6 +99,7 @@ struct udev_rule {
 	char owner[USER_SIZE];
 	char group[USER_SIZE];
 	mode_t mode;
+	char hotplug[PATH_SIZE];
 
 	int last_rule;
 	int ignore_device;
@@ -109,6 +114,7 @@ extern struct list_head udev_rule_list;
 
 extern int udev_rules_init(void);
 extern int udev_rules_get_name(struct udevice *udev, struct sysfs_class_device *class_dev);
+extern int udev_rules_get_hotplug(struct udevice *udev);
 extern void udev_rules_close(void);
 
 #endif
=== udev_rules_parse.c 1.67 vs edited ==--- 1.67/udev_rules_parse.c	2005-03-28 04:20:05 +02:00
+++ edited/udev_rules_parse.c	2005-03-29 14:51:33 +02:00
@@ -256,6 +256,13 @@ static int rules_parse(const char *filen
 				continue;
 			}
 
+			if (strcasecmp(key, KEY_ACTION) = 0) {
+				strlcpy(rule.action, value, sizeof(rule.action));
+				rule.action_operation = operation;
+				valid = 1;
+				continue;
+			}
+
 			if (strcasecmp(key, KEY_BUS) = 0) {
 				strlcpy(rule.bus, value, sizeof(rule.bus));
 				rule.bus_operation = operation;
@@ -375,6 +382,12 @@ static int rules_parse(const char *filen
 
 			if (strcasecmp(key, KEY_MODE) = 0) {
 				rule.mode = strtol(value, NULL, 8);
+				valid = 1;
+				continue;
+			}
+
+			if (strcasecmp(key, KEY_HOTPLUG) = 0) {
+				strlcpy(rule.hotplug, value, sizeof(rule.hotplug));
 				valid = 1;
 				continue;
 			}
=== udev_utils.c 1.43 vs edited ==--- 1.43/udev_utils.c	2005-03-29 03:25:52 +02:00
+++ edited/udev_utils.c	2005-03-29 14:51:33 +02:00
@@ -45,9 +45,13 @@ int udev_init_device(struct udevice *ude
 
 	memset(udev, 0x00, sizeof(struct udevice));
 	INIT_LIST_HEAD(&udev->symlink_list);
+	INIT_LIST_HEAD(&udev->hotplug_list);
 
 	if (subsystem)
 		strlcpy(udev->subsystem, subsystem, sizeof(udev->subsystem));
+
+	if (action)
+		strlcpy(udev->action, action, sizeof(udev->action));
 
 	if (devpath) {
 		strlcpy(udev->devpath, devpath, sizeof(udev->devpath));
=== udevstart.c 1.36 vs edited ==--- 1.36/udevstart.c	2005-03-29 03:25:52 +02:00
+++ edited/udevstart.c	2005-03-29 14:51:33 +02:00
@@ -126,6 +126,15 @@ static int add_device(const char *path, 
 	udev_init_device(&udev, devpath, subsystem, "add");
 	udev_add_device(&udev, class_dev);
 
+	/* execute collected hotplug programs */
+	if (udev_hotplug && !list_empty(&udev.hotplug_list)) {
+		struct name_entry *name_loop;
+
+		dbg("executing hotplug program list");
+		list_for_each_entry(name_loop, &udev.hotplug_list, node)
+			execute_command(name_loop->name, udev.subsystem);
+	}
+
 	/* run dev.d/ scripts if we created a node or changed a netif name */
 	if (udev_dev_d && udev.devname[0] != '\0') {
 		setenv("DEVNAME", udev.devname, 1);
=== test/udev-test.pl 1.97 vs edited ==--- 1.97/test/udev-test.pl	2005-03-28 04:20:05 +02:00
+++ edited/test/udev-test.pl	2005-03-29 14:51:33 +02:00
@@ -1245,6 +1245,53 @@ KERNEL="sda", SYSFS{vendor}!="", NAME="
 KERNEL="sda", SYSFS{vendor}="", NAME="not-3-ok"
 EOF
 	},
+	{
+		desc		=> "check ACTION value",
+		subsys		=> "block",
+		devpath		=> "/block/sda",
+		exp_name	=> "ok",
+		rules		=> <<EOF
+ACTION="unknown", KERNEL="sda", NAME="unknown-not-ok"
+ACTION="add", KERNEL="sda", NAME="ok"
+EOF
+	},
+	{
+		desc		=> "apply NAME only once",
+		subsys		=> "block",
+		devpath		=> "/block/sda",
+		exp_name	=> "link",
+		exp_target	=> "ok",
+		rules		=> <<EOF
+KERNEL="sda", NAME="ok"
+KERNEL="sda", NAME="not-ok"
+KERNEL="sda", SYMLINK+="link"
+EOF
+	},
+	{
+		desc		=> "test HOTPLUG key",
+		subsys		=> "block",
+		devpath		=> "/block/sda",
+		exp_name	=> "testsymlink",
+		exp_target	=> "ok",
+		exp_rem_error	=> "yes",
+		option		=> "clean",
+		rules		=> <<EOF
+KERNEL="sda", NAME="ok", HOTPLUG+="/bin/ln -s ok %r/testsymlink"
+KERNEL="sda", NAME="not-ok"
+EOF
+	},
+	{
+		desc		=> "test HOTPLUG key remove",
+		subsys		=> "block",
+		devpath		=> "/block/sda",
+		exp_name	=> "testsymlink2",
+		exp_target	=> "ok2",
+		rules		=> <<EOF
+KERNEL="sda", NAME="ok2", HOTPLUG+="/bin/ln -s ok2 %r/testsymlink2"
+KERNEL="sda", ACTION="remove", HOTPLUG+="/bin/rm -f %r/testsymlink2"
+KERNEL="sda", NAME="not-ok2"
+EOF
+	},
 );
 
 # set env



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Kevin P. Fleming]

Kay Sievers wrote:

> On my box I've removed all scripts from /etc/dev.d and replaced it with
> following rules in the main rules file:
> 
>   ACTION="add", SUBSYSTEM="sound", HOTPLUG="/etc/udev/scripts/alsa.dev"
>   ACTION="add", KERNEL="fd*", HOTPLUG="/etc/udev/scripts/MAKEDEV.dev"
>   KERNEL="ttyUSB*", HOTPLUG="/etc/udev/scripts/test-serial-device"
>   SUBSYSTEM="tty", OPTIONS="last_rule"
>   HOTPLUG="/etc/udev/scripts/pam_console.dev"
>   HOTPLUG="/etc/udev/sripts/log.dev"
> 
> This cuts down the execution-time of udevstart from ~10 seconds to ~2
> seconds, cause the hundreds of tty events will not call any script.

Very, very nice! Simple, logical, understandable, usually means it's the 
right solution :-)


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Roman Kagan]

On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> To get this working, I changed the logic to read all rules and not to
> stop at the first match.

That's very nice, indeed.

> If a rule should be the last one to be applied
> to a device it must use OPTION="last_rule".

IMHO this is going to increase admin's chances to shoot himself in the
foot.  Imagine someone having installed a rules file causing the
processing of a particular type to stop early, and then someone (else)
trying to figure out what's wrong with another rules file matching the
same devices but happening to go later in the list.

I beleive all rules must be independently processed; the only reason for
the user to care about the order of the rules should be when a rule
depends on the _results_ of the action of another rule.

As to the notorious "too many tty devices" problem, I guess it can be
worked around with something like

SUBSYSTEM="tty", NAME=""

or

SUBSYSTEM!="tty", HOTPLUG="/some/slow/hotplug/script"

> After the first rule that
> assigns a NAME to a device, all later rules with a NAME key will be
> ignored, so it should not change the current behavior too much.

Same problem here: changing the order of the (seemingly independent!)
rules may cause unexpected change of which rule applies.  What's wrong
with executing all NAME actions?  At worst it'll create multiple device
nodes for the same device - big deal...

Cheers,
  Roman.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Kay Sievers]

On Wed, 2005-03-30 at 11:39 +0400, Roman Kagan wrote:
> On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > To get this working, I changed the logic to read all rules and not to
> > stop at the first match.
> 
> That's very nice, indeed.
> 
> > If a rule should be the last one to be applied
> > to a device it must use OPTION="last_rule".
> 
> IMHO this is going to increase admin's chances to shoot himself in the
> foot.

Thats one of thousand ways to shoot yourself. I don't see any problem
getting bigger here.

>  Imagine someone having installed a rules file causing the
> processing of a particular type to stop early, and then someone (else)
> trying to figure out what's wrong with another rules file matching the
> same devices but happening to go later in the list.

The only thing that can happen is that you get more symlinks or the
systems standard permissions gets applied. And that's not bad, I think.

> I beleive all rules must be independently processed; the only reason for
> the user to care about the order of the rules should be when a rule
> depends on the _results_ of the action of another rule.

Agreed.

> As to the notorious "too many tty devices" problem, I guess it can be
> worked around with something like
> 
> SUBSYSTEM="tty", NAME=""

I thought you were arguing for "independent" rules? This makes the
heaviest dependency on the order of the rules, I can think of. :)

> or
> 
> SUBSYSTEM!="tty", HOTPLUG="/some/slow/hotplug/script"

And if the hotplug-script should be excluded from two subsystems?

> > After the first rule that
> > assigns a NAME to a device, all later rules with a NAME key will be
> > ignored, so it should not change the current behavior too much.
> 
> Same problem here: changing the order of the (seemingly independent!)
> rules may cause unexpected change of which rule applies.  What's wrong
> with executing all NAME actions?  At worst it'll create multiple device
> nodes for the same device - big deal...

No, I don't see any reason to support more than one device node while we
can do an unlimited number of symlinks. It's a security nightmare, to
check the permissions of possibly multiple nodes with the same
major/minor.
And today we have the inconvenience that the kernel logs errors for
devices that don't have the same name as the user visible device node.
We may need something to match these both together in a sane way -
multiple nodes would be a complete mess here.

Thanks,
Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Roman Kagan]

On Wed, Mar 30, 2005 at 02:05:09PM +0200, Kay Sievers wrote:
> On Wed, 2005-03-30 at 11:39 +0400, Roman Kagan wrote:
> > On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > > If a rule should be the last one to be applied
> > > to a device it must use OPTION="last_rule".
> > 
> > IMHO this is going to increase admin's chances to shoot himself in the
> > foot.
> 
> Thats one of thousand ways to shoot yourself. I don't see any problem
> getting bigger here.

Umm, that's not exactly the kind of attitude I'd expect from a developer
of the core subsystem... :)

> >  Imagine someone having installed a rules file causing the
> > processing of a particular type to stop early, and then someone (else)
> > trying to figure out what's wrong with another rules file matching the
> > same devices but happening to go later in the list.
> 
> The only thing that can happen is that you get more symlinks or the
> systems standard permissions gets applied. And that's not bad, I think.

How so?  What if you had a rule to do something to a particular device
type, and then some kind soul included in his package a rules file
telling udev to ignore your devices?

I believe that a rule should be local: the rule writer shold not depend
on other rules to _not_ match the same device.  This appears to be the
only way acceptable to distro maintainers, who would want to add or
remove rules by small independent pieces (files under /etc/udev/rules.d)

> > As to the notorious "too many tty devices" problem, I guess it can be
> > worked around with something like
> > 
> > SUBSYSTEM="tty", NAME=""
> 
> I thought you were arguing for "independent" rules? This makes the
> heaviest dependency on the order of the rules, I can think of. :)

Sorry, this one is busted.  However not in the way you say: rather, it
has no effect at all (as long as the rules are independent).

> > SUBSYSTEM!="tty", HOTPLUG="/some/slow/hotplug/script"
> 
> And if the hotplug-script should be excluded from two subsystems?

Then they all would be listed in the condition clause, e.g.
SUBSYSTEM!="tty"&&SUBSYSTEM!="sound" HOTPLUG="/some/slow/hotplug/script"

Note that it is the creator of the /some/slow/hotplug/script who knows
which devices his rule should, and which houldn't apply to.

BTW isn't the whole "too many tty devices" problem due to the implicitly
appended NAME="%k" catch-all rule?  Maybe it should just be removed, and
instead every device node wanted under /dev be explicitly matched
against a rule in udev config?

> > > After the first rule that
> > > assigns a NAME to a device, all later rules with a NAME key will be
> > > ignored, so it should not change the current behavior too much.
> > 
> > Same problem here: changing the order of the (seemingly independent!)
> > rules may cause unexpected change of which rule applies.  What's wrong
> > with executing all NAME actions?  At worst it'll create multiple device
> > nodes for the same device - big deal...
> 
> No, I don't see any reason to support more than one device node while we
> can do an unlimited number of symlinks.

Again for the locality of the rule definition.

> It's a security nightmare, to
> check the permissions of possibly multiple nodes with the same
> major/minor.

You don't need to.  Each rule's writer is supposed to understand what
he's doing, so you only need to check the permissions of a device node
against the OWNER, GROUP, MODE fields in the NAME action which was
created the node.

In particular, if you happen to have two rules like

KERNEL="kmem", NAME="%k", MODE="0640"

and

KERNEL="kmem", NAME="mykmem", MODE="0666"

your system's security should always be compromised, regardless of the
order in which these rules happen to be evaluated.

> And today we have the inconvenience that the kernel logs errors for
> devices that don't have the same name as the user visible device node.

This must be a recent addition...  My 2.6.11 doesn't care what filenames I
choose for device nodes, nor how many nodes I create for a given device.

Roman.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Kay Sievers]

On Wed, 2005-03-30 at 18:08 +0400, Roman Kagan wrote:
> On Wed, Mar 30, 2005 at 02:05:09PM +0200, Kay Sievers wrote:
> > On Wed, 2005-03-30 at 11:39 +0400, Roman Kagan wrote:
> > > On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > > > If a rule should be the last one to be applied
> > > > to a device it must use OPTION="last_rule".
> > > 
> > > IMHO this is going to increase admin's chances to shoot himself in the
> > > foot.
> > 
> > Thats one of thousand ways to shoot yourself. I don't see any problem
> > getting bigger here.
> 
> Umm, that's not exactly the kind of attitude I'd expect from a developer
> of the core subsystem... :)

Well, sorry if it did sound so. The point is that I don't see any
problem here. We apply all later rules, that don't have a NAME key. That
sounds reasonable to me and is nearly full backwards compatible, which
was the reason to do it that way.

> > >  Imagine someone having installed a rules file causing the
> > > processing of a particular type to stop early, and then someone (else)
> > > trying to figure out what's wrong with another rules file matching the
> > > same devices but happening to go later in the list.
> > 
> > The only thing that can happen is that you get more symlinks or the
> > systems standard permissions gets applied. And that's not bad, I think.
> 
> How so?  What if you had a rule to do something to a particular device
> type, and then some kind soul included in his package a rules file
> telling udev to ignore your devices?

Then this soul instructed udev to ignore the device, exactly the same as
that rule will do it today, and that is expected behavior.

> I believe that a rule should be local: the rule writer shold not depend
> on other rules to _not_ match the same device.  This appears to be the
> only way acceptable to distro maintainers, who would want to add or
> remove rules by small independent pieces (files under /etc/udev/rules.d)

From the first days on, we can set NAME="" to skip any later rule and
ignor the device. Recently "ignore_device" was added for that, which
does the same.
 
> > > As to the notorious "too many tty devices" problem, I guess it can be
> > > worked around with something like
> > > 
> > > SUBSYSTEM="tty", NAME=""
> > 
> > I thought you were arguing for "independent" rules? This makes the
> > heaviest dependency on the order of the rules, I can think of. :)
> 
> Sorry, this one is busted.  However not in the way you say: rather, it
> has no effect at all (as long as the rules are independent).
> 
> > > SUBSYSTEM!="tty", HOTPLUG="/some/slow/hotplug/script"
> > 
> > And if the hotplug-script should be excluded from two subsystems?
> 
> Then they all would be listed in the condition clause, e.g.
> SUBSYSTEM!="tty"&&SUBSYSTEM!="sound" HOTPLUG="/some/slow/hotplug/script"

Oh well, this syntax will not happen with our current udev. If this will
ever be a requirement, we should include a shell instead of our
home-grown parser. :)

> Note that it is the creator of the /some/slow/hotplug/script who knows
> which devices his rule should, and which houldn't apply to.
> 
> BTW isn't the whole "too many tty devices" problem due to the implicitly
> appended NAME="%k" catch-all rule?  Maybe it should just be removed, and
> instead every device node wanted under /dev be explicitly matched
> against a rule in udev config?

The creation of the default kernel name is expected behavior. The
problem this patch addresses is the dev.d/ brute-force logic which is a
completely different problem, right?

> > > > After the first rule that
> > > > assigns a NAME to a device, all later rules with a NAME key will be
> > > > ignored, so it should not change the current behavior too much.
> > > 
> > > Same problem here: changing the order of the (seemingly independent!)
> > > rules may cause unexpected change of which rule applies.  What's wrong
> > > with executing all NAME actions?  At worst it'll create multiple device
> > > nodes for the same device - big deal...
> > 
> > No, I don't see any reason to support more than one device node while we
> > can do an unlimited number of symlinks.
> 
> Again for the locality of the rule definition.
> 
> > It's a security nightmare, to
> > check the permissions of possibly multiple nodes with the same
> > major/minor.
> 
> You don't need to.  Each rule's writer is supposed to understand what
> he's doing, so you only need to check the permissions of a device node
> against the OWNER, GROUP, MODE fields in the NAME action which was
> created the node.

We use SUBSYSTEM-only rules to set permissions. I've also seen some
scripts to return group assignments.
I really don't like the idea, nor see any good reason to support
multiple device nodes with the same numbers.

> In particular, if you happen to have two rules like
> 
> KERNEL="kmem", NAME="%k", MODE="0640"
> 
> and
> 
> KERNEL="kmem", NAME="mykmem", MODE="0666"
> 
> your system's security should always be compromised, regardless of the
> order in which these rules happen to be evaluated.

Why? The first rule gets applied. The second is ignored, cause NAME is
included. It's that easy, isn't it?

> > And today we have the inconvenience that the kernel logs errors for
> > devices that don't have the same name as the user visible device node.
> 
> This must be a recent addition...  My 2.6.11 doesn't care what filenames I
> choose for device nodes, nor how many nodes I create for a given device.

No, it's the same from the beginning on. You get an error in the kernel
log for disk sde, but that disk may be named to "something" by udev and
userspace uses "something" and has no idea what disk it gets the error
for, before it asks the udev-database for the relation. That's not
really nice, if you have hundreds of disks.

Kay



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Greg KH]

On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> This patch on top of the current bk-tree allows to specify programs to
> be executed after the device handling. This is meant as a general
> replacement for the dev.d/ logic which is known to be the main reason for
> the boot-time delay on udev systems.
> 
> This patch introduces the HOTPLUG key. Every matching rule can add one
> HOTPLUG key to a list of program to be executed after the node is
> created/removed.

I like this, except for the name of the key.  The term "hotplug" is just
too overloaded these days.  Can you change it to something else.
Perhaps NOTIFIER?  That more explicitly states what this rule is going
to do.

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/info/Sentarus/hamr30
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Kay Sievers]

On Wed, 2005-03-30 at 11:21 -0800, Greg KH wrote:
> On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > This patch on top of the current bk-tree allows to specify programs to
> > be executed after the device handling. This is meant as a general
> > replacement for the dev.d/ logic which is known to be the main reason for
> > the boot-time delay on udev systems.
> > 
> > This patch introduces the HOTPLUG key. Every matching rule can add one
> > HOTPLUG key to a list of program to be executed after the node is
> > created/removed.
> 
> I like this, except for the name of the key.  The term "hotplug" is just
> too overloaded these days.  Can you change it to something else.

You are right! HOTPLUG says nothing and that key should be renamed.

> Perhaps NOTIFIER?  That more explicitly states what this rule is going
> to do.

How does AGENT sound? It's a bit shorter, but hey, NOTIFIER is still
shorter than SUBSYSTEM. :)

Kay



-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/info/Sentarus/hamr30
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Greg KH]

On Wed, Mar 30, 2005 at 10:20:34PM +0200, Kay Sievers wrote:
> On Wed, 2005-03-30 at 11:21 -0800, Greg KH wrote:
> > On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > > This patch on top of the current bk-tree allows to specify programs to
> > > be executed after the device handling. This is meant as a general
> > > replacement for the dev.d/ logic which is known to be the main reason for
> > > the boot-time delay on udev systems.
> > > 
> > > This patch introduces the HOTPLUG key. Every matching rule can add one
> > > HOTPLUG key to a list of program to be executed after the node is
> > > created/removed.
> > 
> > I like this, except for the name of the key.  The term "hotplug" is just
> > too overloaded these days.  Can you change it to something else.
> 
> You are right! HOTPLUG says nothing and that key should be renamed.
> 
> > Perhaps NOTIFIER?  That more explicitly states what this rule is going
> > to do.
> 
> How does AGENT sound? It's a bit shorter, but hey, NOTIFIER is still
> shorter than SUBSYSTEM. :)

AGENT is again, a term that is very overloaded in the computing field.
I still like NOTIFIER, but if you really don't like that...

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/info/Sentarus/hamr30
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Kay Sievers]

On Thu, 2005-03-31 at 16:18 -0800, Greg KH wrote:
> On Wed, Mar 30, 2005 at 10:20:34PM +0200, Kay Sievers wrote:
> > On Wed, 2005-03-30 at 11:21 -0800, Greg KH wrote:
> > > On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > > > This patch on top of the current bk-tree allows to specify programs to
> > > > be executed after the device handling. This is meant as a general
> > > > replacement for the dev.d/ logic which is known to be the main reason for
> > > > the boot-time delay on udev systems.
> > > > 
> > > > This patch introduces the HOTPLUG key. Every matching rule can add one
> > > > HOTPLUG key to a list of program to be executed after the node is
> > > > created/removed.
> > > 
> > > I like this, except for the name of the key.  The term "hotplug" is just
> > > too overloaded these days.  Can you change it to something else.
> > 
> > You are right! HOTPLUG says nothing and that key should be renamed.
> > 
> > > Perhaps NOTIFIER?  That more explicitly states what this rule is going
> > > to do.
> > 
> > How does AGENT sound? It's a bit shorter, but hey, NOTIFIER is still
> > shorter than SUBSYSTEM. :)
> 
> AGENT is again, a term that is very overloaded in the computing field.
> I still like NOTIFIER, but if you really don't like that...

Heh, seems we don't have bigger problems these days. :)
Last try for a shorter key name. What about RUN?

Kay



-------------------------------------------------------
This SF.net email is sponsored by Demarc:
A global provider of Threat Management Solutions.
Download our HomeAdmin security software for free today!
http://www.demarc.com/info/Sentarus/hamr30
_______________________________________________
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: [PATCH] udev: add rule based program execution

[Greg KH]

On Fri, Apr 01, 2005 at 09:30:35AM +0200, Kay Sievers wrote:
> On Thu, 2005-03-31 at 16:18 -0800, Greg KH wrote:
> > On Wed, Mar 30, 2005 at 10:20:34PM +0200, Kay Sievers wrote:
> > > On Wed, 2005-03-30 at 11:21 -0800, Greg KH wrote:
> > > > On Tue, Mar 29, 2005 at 04:54:03PM +0200, Kay Sievers wrote:
> > > > > This patch on top of the current bk-tree allows to specify programs to
> > > > > be executed after the device handling. This is meant as a general
> > > > > replacement for the dev.d/ logic which is known to be the main reason for
> > > > > the boot-time delay on udev systems.
> > > > > 
> > > > > This patch introduces the HOTPLUG key. Every matching rule can add one
> > > > > HOTPLUG key to a list of program to be executed after the node is
> > > > > created/removed.
> > > > 
> > > > I like this, except for the name of the key.  The term "hotplug" is just
> > > > too overloaded these days.  Can you change it to something else.
> > > 
> > > You are right! HOTPLUG says nothing and that key should be renamed.
> > > 
> > > > Perhaps NOTIFIER?  That more explicitly states what this rule is going
> > > > to do.
> > > 
> > > How does AGENT sound? It's a bit shorter, but hey, NOTIFIER is still
> > > shorter than SUBSYSTEM. :)
> > 
> > AGENT is again, a term that is very overloaded in the computing field.
> > I still like NOTIFIER, but if you really don't like that...
> 
> Heh, seems we don't have bigger problems these days. :)
> Last try for a shorter key name. What about RUN?

Ok, that's fine with me.  Anyone not like this?

thanks,

greg k-h


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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