From d9488d78ebc2dde8ac4065381780e4d974056e92 Mon Sep 17 00:00:00 2001 From: Ian Stakenvicius Date: Thu, 11 Jul 2013 14:04:02 -0400 Subject: rule-generator: proactively rename ifaces to avoid conflicts When a new network iface device is added, scan through the list of rules to see if its kernel-assigned name is used as a target for another device. If so, and said target device is not this device, rename it to a temporary interface name. Then rename the device in accordance with any rename rules that may apply to this device, if applicable. The temporary name assigned is the basename of the interface with a numeric compoment which is close to the inverse of the numeric id (127 - id#). This should provide a more user-friendly output than the old 'rename#' behaviour, when there is no final target name for the iface. This proactive temporary rename will prevent cases where old-style rule-generator rules are used and a target NAME= is set for one iface, assigning it to the iface name used by a second iface, and that second iface has no rename rule to apply. The original rename code would be blocked due to the conflict and time out when attempting to rename, leaving the interface assigned to the temporary 'rename[id#]' name and/or failing to rename other ifaces in accordance with the existing rules. This is a corner case that only occurrs when 75-persistent-net-generator.rules or the write_net_rules script it 'IMPORTS' fails to generate a new rule and rename the interface and there is no other interface-renaming rules that apply. There may also be performance benefits to renaming ifaces early, but no benchmarks have been run to confirm this. Signed-off-by: Ian Stakenvicius --- src/udev/udev-event.c | 129 ++++++++++++++++++++++++++++++++++---------------- src/udev/udev-rules.c | 65 +++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 42 deletions(-) (limited to 'src') diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 7cf82fd8a6..0779ebc30b 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -752,36 +752,18 @@ out: } #ifdef ENABLE_RULE_GENERATOR -static void rename_netif_kernel_log(struct ifreq ifr) -{ - int klog; - FILE *f; - - klog = open("/dev/kmsg", O_WRONLY|O_CLOEXEC); - if (klog < 0) - return; - - f = fdopen(klog, "w"); - if (f == NULL) { - close(klog); - return; - } - - fprintf(f, "<30>udevd[%u]: renamed network interface %s to %s\n", - getpid(), ifr.ifr_name, ifr.ifr_newname); - fclose(f); -} +/* function to return the count of rules that assign NAME= to a value matching arg#2 , defined in udev-rules.c */ +int udev_rules_assigning_name_to(struct udev_rules *rules,const char *match_name); #endif -static int rename_netif(struct udev_event *event) +static int rename_netif_dev_fromname_toname(struct udev_device *dev,const char *oldname,const char *newname) { - struct udev_device *dev = event->dev; int sk; struct ifreq ifr; int err; log_debug("changing net interface name from '%s' to '%s'\n", - udev_device_get_sysname(dev), event->name); + oldname, newname); sk = socket(PF_INET, SOCK_DGRAM, 0); if (sk < 0) { @@ -791,37 +773,26 @@ static int rename_netif(struct udev_event *event) } memset(&ifr, 0x00, sizeof(struct ifreq)); - strscpy(ifr.ifr_name, IFNAMSIZ, udev_device_get_sysname(dev)); - strscpy(ifr.ifr_newname, IFNAMSIZ, event->name); + strscpy(ifr.ifr_name, IFNAMSIZ, oldname); + strscpy(ifr.ifr_newname, IFNAMSIZ, newname); err = ioctl(sk, SIOCSIFNAME, &ifr); #ifdef ENABLE_RULE_GENERATOR int loop; if (err == 0) { - rename_netif_kernel_log(ifr); + print_kmsg("renamed network interface %s to %s\n", ifr.ifr_name, ifr.ifr_newname); goto out; } - /* keep trying if the destination interface name already exists */ + log_debug("collision on rename of network interface %s to %s , retrying until timeout\n", + ifr.ifr_name, ifr.ifr_newname); + err = -errno; if (err != -EEXIST) goto out; - /* free our own name, another process may wait for us */ - snprintf(ifr.ifr_newname, IFNAMSIZ, "rename%u", udev_device_get_ifindex(dev)); - err = ioctl(sk, SIOCSIFNAME, &ifr); - if (err < 0) { - err = -errno; - goto out; - } - - /* log temporary name */ - rename_netif_kernel_log(ifr); - /* wait a maximum of 90 seconds for our target to become available */ - strscpy(ifr.ifr_name, IFNAMSIZ, ifr.ifr_newname); - strscpy(ifr.ifr_newname, IFNAMSIZ, event->name); loop = 90 * 20; while (loop--) { const struct timespec duration = { 0, 1000 * 1000 * 1000 / 20 }; @@ -830,7 +801,7 @@ static int rename_netif(struct udev_event *event) err = ioctl(sk, SIOCSIFNAME, &ifr); if (err == 0) { - rename_netif_kernel_log(ifr); + print_kmsg("renamed network interface %s to %s\n", ifr.ifr_name, ifr.ifr_newname); break; } err = -errno; @@ -854,6 +825,11 @@ out: return err; } +static int rename_netif(struct udev_event *event) +{ + return rename_netif_dev_fromname_toname(event->dev,udev_device_get_sysname(event->dev),event->name); +} + int udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules, const sigset_t *sigmask) { struct udev_device *dev = event->dev; @@ -890,14 +866,83 @@ int udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules, udev_rules_apply_to_event(rules, event, sigmask); /* rename a new network interface, if needed */ + + /* ENABLE_RULE_GENERATOR conditional: + * if this is a net iface, and it is an add event, + * and as long as all of the following are FALSE: + * - no NAME target and the current name is not being used + * - there is a NAME target and it is the same as the current name + * - the rules can successfully be searched for the current name (not really part of the conditional) + * the run the rename. + * + * note - udev_rules_assigning_name_to is run when event->name is NULL to ensure renames happen, + * but also on its own to check if a temp-rename is necessary when event->name exists. + * + * A temp-rename is necessary when: + * - there is no rule renaming the current iface but the current name IS used in some other rule + * - there is a rule renaming the current iface, + * the current name IS used AND the target name != the current name + */ + if (udev_device_get_ifindex(dev) > 0 && streq(udev_device_get_action(dev), "add") && +#ifdef ENABLE_RULE_GENERATOR + (event->name == NULL && (err=udev_rules_assigning_name_to(rules,udev_device_get_sysname(dev))) > 0 || + event->name != NULL && !streq(event->name, udev_device_get_sysname(dev)))) { +#else event->name != NULL && !streq(event->name, udev_device_get_sysname(dev))) { +#endif char syspath[UTIL_PATH_SIZE]; char *pos; + char *finalifname = event->name; +#ifdef ENABLE_RULE_GENERATOR + char newifname[IFNAMSIZ]; + + /* err is the number of rules that assign a device with NAME= this sysname */ + if (err > 0 || (err=udev_rules_assigning_name_to(rules,udev_device_get_sysname(dev))) > 0) { + /* have a conflict, rename to a temp name */ + char *newpos; + int ifidnum; + + /* build the temporary iface name */ + strscpy(newifname, IFNAMSIZ, udev_device_get_sysname(dev)); + newpos=pos=&newifname[strcspn(newifname,"0123456789")]; + ifidnum=(int)strtol(pos,&newpos,10); + *pos='\0'; + if (newpos > pos && *newpos == '\0') /* append new iface num to name */ + /* use udev_device_get_ifindex(dev) as it is unique to every iface */ + snprintf(pos,IFNAMSIZ+(newifname-pos), "%d", 128 - udev_device_get_ifindex(dev)); + + /* note, err > 0, which will skip the post-rename stuff if no rename occurs */ + + /* if sysname isn't already the tmpname (ie there is no numeric component), do the rename */ + if (!streq(newifname,udev_device_get_sysname(dev))) { + err = rename_netif_dev_fromname_toname(dev,udev_device_get_sysname(dev),newifname); + if (err == 0) { + finalifname = newifname; + log_debug("renamed netif to '%s' for collision avoidance\n", newifname); + } else { + log_error("could not rename netif to '%s' for collision avoidance\n",newifname); + } + } + /* rename it now to its final target if its not already there */ + if (event->name != NULL && !streq(event->name, newifname)) { + err = rename_netif_dev_fromname_toname(dev,newifname,event->name); + if (err == 0) + finalifname = event->name; + } + + } else { /* no need to rename to a tempname first, do a regular direct rename to event->name */ + + err = 1; /* skip the post-rename stuff if no rename occurs */ + if (!streq(event->name, udev_device_get_sysname(dev))) + err = rename_netif(event); + } +#else err = rename_netif(event); +#endif if (err == 0) { - log_debug("renamed netif to '%s'\n", event->name); + log_debug("renamed netif to '%s'\n", finalifname); /* remember old name */ udev_device_add_property(dev, "INTERFACE_OLD", udev_device_get_sysname(dev)); @@ -907,7 +952,7 @@ int udev_event_execute_rules(struct udev_event *event, struct udev_rules *rules, pos = strrchr(syspath, '/'); if (pos != NULL) { pos++; - strscpy(pos, sizeof(syspath) - (pos - syspath), event->name); + strscpy(pos, sizeof(syspath) - (pos - syspath), finalifname); udev_device_set_syspath(event->dev, syspath); udev_device_add_property(dev, "INTERFACE", udev_device_get_sysname(dev)); log_debug("changed devpath to '%s'\n", udev_device_get_devpath(dev)); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index f6c2b7943c..2c28ef7e20 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1849,6 +1849,71 @@ enum escape_type { ESCAPE_REPLACE, }; +#ifdef ENABLE_RULE_GENERATOR +/* function to return the count of rules that assign NAME= to a value matching arg#2 - returns 0,1 */ +int udev_rules_assigning_name_to(struct udev_rules *rules, const char *match_name) +{ + struct token *cur; + struct token *rule; + enum escape_type esc = ESCAPE_UNSET; + bool name_final = false; + + if (rules->tokens == NULL) + return 0; + + /* loop through token list, match, run actions or forward to next rule */ + cur = &rules->tokens[0]; + rule = cur; + for (;;) { + dump_token(rules, cur); + switch (cur->type) { + case TK_RULE: + /* current rule */ + rule = cur; + if (!rule->rule.can_set_name) + goto nomatch; + break; + case TK_M_SUBSYSTEM: + if (match_key(rules, cur, "net") != 0) + goto nomatch; + break; + case TK_M_ACTION: + if (match_key(rules, cur, "add") != 0) + goto nomatch; + break; + case TK_A_NAME: { + const char *name = rules_str(rules, cur->key.value_off); + char name_str[UTIL_PATH_SIZE]; + int count; + + strscpy(name_str,UTIL_PATH_SIZE,name); + count = util_replace_chars(name_str, "/"); + if (count > 0) + log_debug("%i character(s) replaced\n", count); + if (streq(name_str,match_name)) { + log_debug("found a match! NAME assigns %s in: %s:%u\n", + name, + rules_str(rules, rule->rule.filename_off), + rule->rule.filename_line); + return 1; /* no need to find more than one */ + } + + /* skip to next rule */ + goto nomatch; + } + case TK_END: + return 0; + } + + cur++; + continue; + nomatch: + /* fast-forward to next rule */ + cur = rule + rule->rule.token_count; + } +} +#endif + int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event, const sigset_t *sigmask) { struct token *cur; -- cgit v1.2.3-54-g00ecf