ifplugd: restore auto-ifup unless -a; make iff method less iffy :D

function                                             old     new   delta
up_iface                                               -     112    +112
network_ioctl                                         13      38     +25
detect_link_iff                                       58      71     +13
detect_link                                          143     152      +9
ifplugd_main                                        1107    1109      +2
detect_link_wlan                                     131     125      -6
detect_link_ethtool                                   71      65      -6
detect_link_priv                                      88      80      -8
detect_link_mii                                       88      80      -8
maybe_up_new_iface                                   144      27    -117
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 4/5 up/down: 161/-145)           Total: 16 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

1 files changed, 34 insertions, 38 deletions

diff --git a/networking/ifplugd.c b/networking/ifplugd.c
index 4585530..ac6607c 100644
--- a/networking/ifplugd.c
+++ b/networking/ifplugd.c
@@ -210,9 +210,12 @@ static int run_script(const char *action)
 #endif
 }
 
-static int network_ioctl(int request, void* data)
+static int network_ioctl(int request, void* data, const char *errmsg)
 {
-	return ioctl(ioctl_fd, request, data);
+	int r = ioctl(ioctl_fd, request, data);
+	if (r < 0 && errmsg)
+		bb_perror_msg(errmsg);
+	return r;
 }
 
 static void set_ifreq_to_ifname(struct ifreq *ifreq)
@@ -236,8 +239,7 @@ static void up_iface(void)
 		return;
 
 	set_ifreq_to_ifname(&ifrequest);
-	if (network_ioctl(SIOCGIFFLAGS, &ifrequest) < 0) {
-		bb_perror_msg("can't %cet interface flags", 'g');
+	if (network_ioctl(SIOCGIFFLAGS, &ifrequest, "can't get interface flags") < 0) {
 		G.iface_exists = 0;
 		return;
 	}
@@ -246,24 +248,19 @@ static void up_iface(void)
 		ifrequest.ifr_flags |= IFF_UP;
 		/* Let user know we mess up with interface */
 		bb_error_msg("upping interface");
-		if (network_ioctl(SIOCSIFFLAGS, &ifrequest) < 0)
-			bb_perror_msg_and_die("can't %cet interface flags", 's');
+		if (network_ioctl(SIOCSIFFLAGS, &ifrequest, "can't set interface flags") < 0)
+			xfunc_die();
 	}
 
 #if 0 /* why do we mess with IP addr? It's not our business */
-	if (network_ioctl(SIOCGIFADDR, &ifrequest) < 0) {
-		bb_error_msg("can't get interface address");
+	if (network_ioctl(SIOCGIFADDR, &ifrequest, "can't get interface address") < 0) {
 	} else if (ifrequest.ifr_addr.sa_family != AF_INET) {
 		bb_perror_msg("the interface is not IP-based");
 	} else {
 		((struct sockaddr_in*)(&ifrequest.ifr_addr))->sin_addr.s_addr = INADDR_ANY;
-		if (network_ioctl(SIOCSIFADDR, &ifrequest) < 0)
-			bb_perror_msg("can't set interface address");
-	}
-	if (network_ioctl(SIOCGIFFLAGS, &ifrequest) < 0) {
-		bb_perror_msg("can't get interface flags");
-		return;
+		network_ioctl(SIOCSIFADDR, &ifrequest, "can't set interface address");
 	}
+	network_ioctl(SIOCGIFFLAGS, &ifrequest, "can't get interface flags");
 #endif
 }
 
@@ -279,13 +276,13 @@ static void maybe_up_new_iface(void)
 	set_ifreq_to_ifname(&ifrequest);
 	driver_info.cmd = ETHTOOL_GDRVINFO;
 	ifrequest.ifr_data = &driver_info;
-	if (network_ioctl(SIOCETHTOOL, &ifrequest) == 0) {
+	if (network_ioctl(SIOCETHTOOL, &ifrequest, NULL) == 0) {
 		char buf[sizeof("/xx:xx:xx:xx:xx:xx")];
 
 		/* Get MAC */
 		buf[0] = '\0';
 		set_ifreq_to_ifname(&ifrequest);
-		if (network_ioctl(SIOCGIFHWADDR, &ifrequest) == 0) {
+		if (network_ioctl(SIOCGIFHWADDR, &ifrequest, NULL) == 0) {
 			sprintf(buf, "/%02X:%02X:%02X:%02X:%02X:%02X",
 				(uint8_t)(ifrequest.ifr_hwaddr.sa_data[0]),
 				(uint8_t)(ifrequest.ifr_hwaddr.sa_data[1]),
@@ -310,15 +307,13 @@ static smallint detect_link_mii(void)
 
 	set_ifreq_to_ifname(&ifreq);
 
-	if (network_ioctl(SIOCGMIIPHY, &ifreq) < 0) {
-		bb_perror_msg("SIOCGMIIPHY failed");
+	if (network_ioctl(SIOCGMIIPHY, &ifreq, "SIOCGMIIPHY failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
 	mii->reg_num = 1;
 
-	if (network_ioctl(SIOCGMIIREG, &ifreq) < 0) {
-		bb_perror_msg("SIOCGMIIREG failed");
+	if (network_ioctl(SIOCGMIIREG, &ifreq, "SIOCGMIIREG failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
@@ -332,15 +327,13 @@ static smallint detect_link_priv(void)
 
 	set_ifreq_to_ifname(&ifreq);
 
-	if (network_ioctl(SIOCDEVPRIVATE, &ifreq) < 0) {
-		bb_perror_msg("SIOCDEVPRIVATE failed");
+	if (network_ioctl(SIOCDEVPRIVATE, &ifreq, "SIOCDEVPRIVATE failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
 	mii->reg_num = 1;
 
-	if (network_ioctl(SIOCDEVPRIVATE+1, &ifreq) < 0) {
-		bb_perror_msg("SIOCDEVPRIVATE+1 failed");
+	if (network_ioctl(SIOCDEVPRIVATE+1, &ifreq, "SIOCDEVPRIVATE+1 failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
@@ -357,8 +350,7 @@ static smallint detect_link_ethtool(void)
 	edata.cmd = ETHTOOL_GLINK;
 	ifreq.ifr_data = (void*) &edata;
 
-	if (network_ioctl(SIOCETHTOOL, &ifreq) < 0) {
-		bb_perror_msg("ETHTOOL_GLINK failed");
+	if (network_ioctl(SIOCETHTOOL, &ifreq, "ETHTOOL_GLINK failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
@@ -371,11 +363,19 @@ static smallint detect_link_iff(void)
 
 	set_ifreq_to_ifname(&ifreq);
 
-	if (network_ioctl(SIOCGIFFLAGS, &ifreq) < 0) {
-		bb_perror_msg("SIOCGIFFLAGS failed");
+	if (network_ioctl(SIOCGIFFLAGS, &ifreq, "SIOCGIFFLAGS failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
+	/* If IFF_UP is not set (interface is down), IFF_RUNNING is never set
+	 * regardless of link status. Simply continue to report last status -
+	 * no point in reporting spurious link downs if interface is disabled
+	 * by admin. When/if it will be brought up,
+	 * we'll report real link status.
+	 */
+	if (!(ifreq.ifr_flags & IFF_UP) && G.iface_last_status != IFSTATUS_ERR)
+		return G.iface_last_status;
+
 	return (ifreq.ifr_flags & IFF_RUNNING) ? IFSTATUS_UP : IFSTATUS_DOWN;
 }
 
@@ -387,8 +387,7 @@ static smallint detect_link_wlan(void)
 	memset(&iwrequest, 0, sizeof(struct iwreq));
 	strncpy_IFNAMSIZ(iwrequest.ifr_ifrn.ifrn_name, G.iface);
 
-	if (network_ioctl(SIOCGIWAP, &iwrequest) < 0) {
-		bb_perror_msg("SIOCGIWAP failed");
+	if (network_ioctl(SIOCGIWAP, &iwrequest, "SIOCGIWAP failed") < 0) {
 		return IFSTATUS_ERR;
 	}
 
@@ -469,15 +468,12 @@ static smallint detect_link(void)
 	if (!G.iface_exists)
 		return (option_mask32 & FLAG_MONITOR) ? IFSTATUS_DOWN : IFSTATUS_ERR;
 
-#if 0
-/* Why? This behavior makes it hard to temporary down the iface.
- * It makes a bit more sense to do only in maybe_up_new_iface.
- * OTOH, maybe detect_link_wlan needs this. Then it should be done
- * _only_ there.
- */
+	/* Some drivers can't detect link status when the interface is down.
+	 * I imagine detect_link_iff() is the most vulnerable.
+	 * That's why -a "noauto" in an option, not a hardwired behavior.
+	 */
 	if (!(option_mask32 & FLAG_NO_AUTO))
 		up_iface();
-#endif
 
 	status = G.detect_link_func();
 	if (status == IFSTATUS_ERR) {
@@ -692,7 +688,7 @@ int ifplugd_main(int argc UNUSED_PARAM, char **argv)
 	if (opts & FLAG_MONITOR) {
 		struct ifreq ifrequest;
 		set_ifreq_to_ifname(&ifrequest);
-		G.iface_exists = (network_ioctl(SIOCGIFINDEX, &ifrequest) == 0);
+		G.iface_exists = (network_ioctl(SIOCGIFINDEX, &ifrequest, NULL) == 0);
 	}
 
 	if (G.iface_exists)