[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#615600: BOOTIF= kernel commandline option does not work



tags 615600 pending
thanks

On Wed, Mar 23, 2011 at 08:31:56PM -0500, Dustin Kirkland wrote:
> On Sun, 27 Feb 2011 16:22:28 -0400, Joey Hess wrote:
> > BOOTIF is a pxelinux boot parameter. It is supported by the Debian
> > initramfs when pxe booting, but it is not supported by the Debian
> > installer.
> >
> > Perhaps it should be. In the meantime, you can use the documented
> > preseeding interface of booting with interface=eth1. I don't think
> > netcfg allows specifying a interface by MAC though.
> 
> Floris Bos mentioned that this is similar to an oldish bug (at least
> by Ubuntu standards) in Ubuntu's bug tracker against the netcfg
> package:
>  provide a method to use a specified MAC-address as the installation device
>  * https://bugs.launchpad.net/ubuntu/+source/netcfg/+bug/56679
> 
> We have a proposed solution, in the attached script, S31pxedust.

Thanks for sending this along.

I think honouring BOOTIF is the right thing to do.  You can always
remove 'ipappend 2' if you don't like it, but I think the overwhelming
probability is that you want to use the same interface for installation
as you did for PXE-booting.

However, I think this implementation has two problems:

 * It's at too high a priority: it overrides any other setting of
   netcfg/choose_interface.  If you have 'ipappend 2' but also for some
   reason have (say) interface=eth1 in your boot parameters, then the
   latter is a more explicit instruction to d-i and therefore should
   win.  (Andrew noted this in
   https://bugs.launchpad.net/ubuntu/+source/netcfg/+bug/56679/comments/13.)

 * It runs too early.  Network interfaces may not necessarily be
   available when debian-installer-startup runs; this isn't too likely
   in a netboot image, granted, but it's possible if you're adding
   drivers later by way of a USB stick or something.

I think Andrew's script was expedient for him - it's a lot simpler to
drop a shell script into d-i's initramfs than it is to modify C programs
there - but a better long-term solution for us would be to do this in
netcfg itself.  I have put my money where my mouth is and implemented
this.  My patch overrides the results of link detection with the
interface that matches BOOTIF, but preseeding an interface name
overrides both of those.  This feels right to me.  I've done some
testing in a qemu instance with multiple network interfaces and it's
behaving exactly as I'd expect.

I've committed the following patch, and I plan to release netcfg 1.63
with it shortly.

commit 33aeb7186837a6d7fa4762d72455e70672501cbe
Author: Colin Watson <cjwatson@debian.org>
Date:   Thu Jun 16 18:00:44 2011 +0100

    If BOOTIF= is set on the Linux command line, look for an interface with that address and use it by default (closes: #615600, LP: #56679).

diff --git a/debian/changelog b/debian/changelog
index 61df533..1bcd302 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+netcfg (1.63) UNRELEASED; urgency=low
+
+  * If BOOTIF= is set on the Linux command line, look for an interface with
+    that address and use it by default (closes: #615600, LP: #56679).
+
+ -- Colin Watson <cjwatson@debian.org>  Thu, 16 Jun 2011 14:20:04 +0100
+
 netcfg (1.62) unstable; urgency=low
 
   [ Updated translations ]
diff --git a/netcfg-common.c b/netcfg-common.c
index 972ce86..7e5a3fa 100644
--- a/netcfg-common.c
+++ b/netcfg-common.c
@@ -46,6 +46,10 @@
 
 #include <ifaddrs.h>
 
+#ifdef __linux__
+#define SYSCLASSNET "/sys/class/net/"
+#endif /* __linux__ */
+
 #ifdef __FreeBSD_kernel__
 #define LO_IF	"lo0"
 #else
@@ -128,8 +132,6 @@ void open_sockets (void)
 
 #ifdef __linux__
 
-#define SYSCLASSNET "/sys/class/net/"
-
 /* Returns non-zero if this interface has an enabled kill switch, otherwise
  * zero.
  */
@@ -187,8 +189,6 @@ int check_kill_switch(const char *iface)
     return ret;
 }
 
-#undef SYSCLASSNET
-
 #else /* !__linux__ */
 int check_kill_switch(const char *iface)
 {
@@ -471,6 +471,102 @@ FILE *file_open(char *path, const char *opentype)
     }
 }
 
+const char *find_bootif_iface(int num_interfaces, char **ifs)
+{
+#ifdef __linux__
+#define PROC_CMDLINE "/proc/cmdline"
+#define ADDRESS_LEN 17 /* aa:bb:cc:dd:ee:ff */
+
+    int i;
+    FILE *cmdline_file;
+    char *cmdline = NULL;
+    size_t dummy;
+    const char *s;
+    char *bootif = NULL;
+
+    /* Look for BOOTIF= entry in kernel command line. */
+
+    cmdline_file = file_open(PROC_CMDLINE, "r");
+    if (!cmdline_file) {
+        di_error("Failed to open " PROC_CMDLINE ": %s", strerror(errno));
+        return NULL;
+    }
+    if (getline(&cmdline, &dummy, cmdline_file) < 0) {
+        di_error("Failed to read line from " PROC_CMDLINE ": %s",
+                 strerror(errno));
+        fclose(cmdline_file);
+        return NULL;
+    }
+
+    s = cmdline;
+    while ((s = strstr(s, "BOOTIF=")) != NULL) {
+        if (s == cmdline || s[-1] == ' ') {
+            size_t bootif_len;
+            char *subst;
+
+            s += sizeof("BOOTIF=") - 1;
+            bootif_len = strcspn(s, " ");
+            if (bootif_len != ADDRESS_LEN + 3)
+                continue;
+            bootif = strndup(s + 3, bootif_len - 3); /* skip hardware type */
+            for (subst = bootif; *subst; subst++)
+                if (*subst == '-')
+                    *subst = ':';
+            break;
+        }
+        s++;
+    }
+
+    if (!bootif) {
+        di_info("Could not find valid BOOTIF= entry in " PROC_CMDLINE);
+        free(cmdline);
+        fclose(cmdline_file);
+        return NULL;
+    }
+
+    free(cmdline);
+    fclose(cmdline_file);
+
+    /* Look for an interface matching BOOTIF. */
+
+    for (i = 0; i < num_interfaces; i++) {
+        size_t len;
+        char *kobj;
+        FILE *kobj_file;
+        char address[ADDRESS_LEN + 1];
+
+        len = strlen(SYSCLASSNET) + strlen(ifs[i]) + strlen("/address") + 1;
+        kobj = malloc(len);
+        snprintf(kobj, len, SYSCLASSNET "%s/address", ifs[i]);
+        kobj_file = file_open(kobj, "r");
+        if (!kobj_file)
+            goto next;
+        if (!fgets(address, ADDRESS_LEN + 1, kobj_file))
+            goto next;
+        if (strncmp(address, bootif, ADDRESS_LEN) == 0) {
+            di_info("Found interface %s with BOOTIF address %s",
+                    ifs[i], bootif);
+            fclose(kobj_file);
+            free(kobj);
+            free(bootif);
+            return ifs[i];
+        }
+
+next:
+        fclose(kobj_file);
+        free(kobj);
+    }
+
+    di_error("Could not find any interface with address %s", bootif);
+    free(bootif);
+
+#undef ADDRESS_LEN
+#undef PROC_CMDLINE
+#endif /* __linux__ */
+
+    return NULL;
+}
+
 void netcfg_die(struct debconfclient *client)
 {
     if (netcfg_progress_displayed)
@@ -486,15 +582,17 @@ void netcfg_die(struct debconfclient *client)
  * @param client - client
  * @param interface      - set to the answer
  * @param numif - number of interfaces found.
+ * @param defif - default interface from link detection.
  */
 
 int netcfg_get_interface(struct debconfclient *client, char **interface,
-                         int *numif, char* defif)
+                         int *numif, const char *defif)
 {
     char *inter = NULL, **ifs;
     size_t len;
     int ret, i, asked;
     int num_interfaces = 0;
+    const char *bootif;
     char *ptr = NULL;
     char *ifdsc = NULL;
     char *old_selection = NULL;
@@ -512,6 +610,12 @@ int netcfg_get_interface(struct debconfclient *client, char **interface,
 
     num_interfaces = get_all_ifs(1, &ifs);
 
+    /* If BOOTIF is set and matches an interface, override any provided
+     * default from link detection. */
+    bootif = find_bootif_iface(num_interfaces, ifs);
+    if (bootif)
+        defif = bootif;
+
     /* If no default was provided, use the first in the list of interfaces. */
     if (! defif && num_interfaces > 0) {
         defif=ifs[0];
diff --git a/netcfg.h b/netcfg.h
index e79f445..9304bb3 100644
--- a/netcfg.h
+++ b/netcfg.h
@@ -97,7 +97,7 @@ extern FILE *file_open (char *path, const char *opentype);
 
 extern void netcfg_die (struct debconfclient *client);
 
-extern int netcfg_get_interface(struct debconfclient *client, char **interface, int *num_interfaces, char* defif);
+extern int netcfg_get_interface(struct debconfclient *client, char **interface, int *num_interfaces, const char *defif);
 
 extern short valid_hostname (const char *hname);
 extern short valid_domain (const char *dname);

Regards,

-- 
Colin Watson                                       [cjwatson@debian.org]



Reply to: