ifconfig(8): Make lagg creation more fault-tolerant
authorAaron LI <aly@aaronly.me>
Sun, 12 Apr 2020 13:29:32 +0000 (21:29 +0800)
committerAaron LI <aly@aaronly.me>
Fri, 26 Jun 2020 14:52:59 +0000 (22:52 +0800)
* Warn, don't exit, when SIOCSLAGGPORT returns an error.

  When we exit with an error during lagg creation, a single failed NIC
  (which no longer attaches) can prevent lagg creation and other
  configuration, such as adding an IPv4 address, and thus leave a
  machine unreachable.

* Preserve non-EEXISTS errors for exit status from SIOCSLAGGPORT, in
  case scripts are looking for it. Hopefully this can be extended if
  other parts of ifconfig can allow a "soft" failure.

* Improve the warning message to mention what lagg and what member are
  problematic.

Obtained from FreeBSD:
https://github.com/freebsd/freebsd/commit/dde41c97866152e8c7eb1f52f07e3e5ac43b2652
https://reviews.freebsd.org/D15046

sbin/ifconfig/ifclone.c
sbin/ifconfig/ifconfig.c
sbin/ifconfig/ifconfig.h
sbin/ifconfig/ifgroup.c
sbin/ifconfig/iflagg.c

index b98b643..a5e16d4 100644 (file)
@@ -179,8 +179,9 @@ static void
 clone_Copt_cb(const char *optarg __unused)
 {
        list_cloners();
-       exit(0);
+       exit(exit_code);
 }
+
 static struct option clone_Copt = {
        .opt = "C",
        .opt_usage = "[-C]",
@@ -192,7 +193,7 @@ clone_ctor(void)
 {
        size_t i;
 
-       for (i = 0; i < nitems(clone_cmds);  i++)
+       for (i = 0; i < nitems(clone_cmds); i++)
                cmd_register(&clone_cmds[i]);
        opt_register(&clone_Copt);
 }
index 97bcd2b..353e622 100644 (file)
@@ -82,6 +82,7 @@ int   noload;
 int    supmedia = 0;
 int    printkeys = 0;          /* Print keying material for interfaces. */
 int    printifname = 0;        /* Print the name of the created interface. */
+int    exit_code = 0;
 
 static int ifconfig(int argc, char *const *argv, int iscreate,
                     const struct afswtch *afp);
@@ -258,7 +259,7 @@ main(int argc, char *argv[])
                                        errx(1, "%s: cloning name too long",
                                            ifname);
                                ifconfig(argc, argv, 1, NULL);
-                               return (0);
+                               exit(exit_code);
                        }
                        errx(1, "interface %s does not exist", ifname);
                } else {
@@ -389,7 +390,7 @@ retry:
        if (namesonly && need_nl > 0)
                putchar('\n');
 
-       return (0);
+       return (exit_code);
 }
 
 static struct afswtch *afs = NULL;
index af301bc..002d516 100644 (file)
@@ -142,6 +142,7 @@ extern      int printifname;
 extern int flags;
 extern int newaddr;
 extern int verbose;
+extern int exit_code;
 
 void   setifcap(const char *, int value, int s, const struct afswtch *);
 
index 0d611f8..fc3242c 100644 (file)
@@ -137,7 +137,7 @@ printgroup(const char *groupname)
        if (ioctl(s, SIOCGIFGMEMB, (caddr_t)&ifgr) == -1) {
                if (errno == EINVAL || errno == ENOTTY ||
                    errno == ENOENT)
-                       exit(0);
+                       exit(exit_code);
                else
                        err(1, "SIOCGIFGMEMB");
        }
@@ -157,7 +157,7 @@ printgroup(const char *groupname)
        }
        free(ifgr.ifgr_groups);
 
-       exit(0);
+       exit(exit_code);
 }
 
 static struct cmd group_cmds[] = {
index 9c3273c..5460047 100644 (file)
@@ -34,9 +34,17 @@ setlaggport(const char *val, int d, int s, const struct afswtch *afp)
        strlcpy(rp.rp_ifname, name, sizeof(rp.rp_ifname));
        strlcpy(rp.rp_portname, val, sizeof(rp.rp_portname));
 
-       /* Don't choke if the port is already in this lagg. */
-       if (ioctl(s, SIOCSLAGGPORT, &rp) && errno != EEXIST)
-               err(1, "SIOCSLAGGPORT");
+       /*
+        * Do not exit with an error here.  Doing so permits a failed NIC
+        * to take down an entire lagg.
+        *
+        * Don't error at all if the port is already in the lagg.
+        */
+       if (ioctl(s, SIOCSLAGGPORT, &rp) && errno != EEXIST) {
+               warnx("%s %s: SIOCSLAGGPORT: %s",
+                     name, val, strerror(errno));
+               exit_code = 1;
+       }
 }
 
 static void