From 47528c67efac952d08840ed54f0b93682a6008a8 Mon Sep 17 00:00:00 2001 From: Andrew Gallatin Date: Tue, 17 Apr 2018 12:54:58 +0000 Subject: [PATCH] Make lagg creation more fault tolerant - 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. Reviewed by: jtl, glebius Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D15046 --- sbin/ifconfig/ifclone.c | 2 +- sbin/ifconfig/ifconfig.c | 7 ++++--- sbin/ifconfig/ifconfig.h | 1 + sbin/ifconfig/ifgroup.c | 4 ++-- sbin/ifconfig/iflagg.c | 14 +++++++++++--- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/sbin/ifconfig/ifclone.c b/sbin/ifconfig/ifclone.c index 5e23501ae03c..c264a444dcf0 100644 --- a/sbin/ifconfig/ifclone.c +++ b/sbin/ifconfig/ifclone.c @@ -181,7 +181,7 @@ 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]", .cb = clone_Copt_cb }; diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c index b0a2f17586b5..254180eaa7f6 100644 --- a/sbin/ifconfig/ifconfig.c +++ b/sbin/ifconfig/ifconfig.c @@ -99,6 +99,7 @@ int printifname = 0; int supmedia = 0; int printkeys = 0; /* Print keying material for interfaces. */ +int exit_code = 0; /* Formatter Strings */ char *f_inet, *f_inet6, *f_ether, *f_addr; @@ -485,7 +486,7 @@ main(int argc, char *argv[]) errx(1, "%s: cloning name too long", ifname); ifconfig(argc, argv, 1, NULL); - exit(0); + exit(exit_code); } #ifdef JAIL /* @@ -499,7 +500,7 @@ main(int argc, char *argv[]) errx(1, "%s: interface name too long", ifname); ifconfig(argc, argv, 0, NULL); - exit(0); + exit(exit_code); } #endif errx(1, "interface %s does not exist", ifname); @@ -597,7 +598,7 @@ main(int argc, char *argv[]) freeifaddrs(ifap); freeformat(); - exit(0); + exit(exit_code); } static struct afswtch *afs = NULL; diff --git a/sbin/ifconfig/ifconfig.h b/sbin/ifconfig/ifconfig.h index 5a9af3aaa7b8..5b5a839aec0e 100644 --- a/sbin/ifconfig/ifconfig.h +++ b/sbin/ifconfig/ifconfig.h @@ -136,6 +136,7 @@ extern int printkeys; extern int newaddr; extern int verbose; extern int printifname; +extern int exit_code; void setifcap(const char *, int value, int s, const struct afswtch *); diff --git a/sbin/ifconfig/ifgroup.c b/sbin/ifconfig/ifgroup.c index d3792e00a1cb..106d1260ef18 100644 --- a/sbin/ifconfig/ifgroup.c +++ b/sbin/ifconfig/ifgroup.c @@ -140,7 +140,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"); } @@ -159,7 +159,7 @@ printgroup(const char *groupname) } free(ifgr.ifgr_groups); - exit(0); + exit(exit_code); } static struct cmd group_cmds[] = { diff --git a/sbin/ifconfig/iflagg.c b/sbin/ifconfig/iflagg.c index 825acf69d339..89b9ce097f35 100644 --- a/sbin/ifconfig/iflagg.c +++ b/sbin/ifconfig/iflagg.c @@ -41,9 +41,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