Bug 166656 - [patch] dhclient(8) doesn't exit os link down
Summary: [patch] dhclient(8) doesn't exit os link down
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 8.2-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: John Baldwin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-05 02:50 UTC by Peter Jeremy
Modified: 2013-02-26 19:40 UTC (History)
0 users

See Also:


Attachments
file.diff (413 bytes, patch)
2012-04-05 02:50 UTC, Peter Jeremy
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Jeremy 2012-04-05 02:50:00 UTC
	/etc/devd.conf includes a rule to start dhclient when an Ethernet or
	802.11 interface reports "link up", with a comment: "No link down rule
	exists because dhclient automatically exits when the link goes down."
	IMHO, this is the desired behaviour, unfortunately it's not the way
	dhclient actually behaves.  In my experience, dhclient will exit when
	the interface goes down but ignores link status changes.
	 
	Looking at the source, it seems to exit only when there's no usable
	route (by monitoring RTF_UP).
	 
	dhclient does monitor the link status using SIOCGIFMEDIA but only at
	startup (it will exit if the link doesn't come up within 10s of
	dhclient starting) and during DHCP exchanges (if the link goes down
	when it's expecting a DHCP response then it exits).

Fix: dhclient receives RTM_IFINFO messages on link change.  Currently
	the code just monitors RTF_UP on link change.  The attached patch
	adds a SIOCGIFMEDIA check when a RTM_IFINFO messages is received
	and exits if the link is now down.
How-To-Repeat: 
	Start dhclient and verify a valid lease is obtained.
	Unplug network cable and verify that dhclient did not exit.
Comment 1 Torfinn Ingolfsen 2012-07-12 10:57:19 UTC
Hello,
FWIW, I tested this patch on FreeBSD 8.1-stable:
root@kg-omni1# uname -a
FreeBSD kg-omni1.kg4.no 8.1-STABLE FreeBSD 8.1-STABLE #0: Sun Oct 17 12:35:38 CEST 2010     root@kg-i82.kg4.no:/usr/obj/usr/src/sys/GENERIC  i386

It works flawlessly.
Without the patch, dhclient would not exit, resulting in no new lease being aquired when the link goes down and up again. 
With the patch, dhclient exits and gets started when the link comes up again. From /var/log/messages:
Jul 12 11:37:07 kg-omni1 dhclient[30335]: Link xl0 is down, dhclient exiting
Jul 12 11:37:07 kg-omni1 kernel: xl0: link state changed to DOWN
Jul 12 11:37:07 kg-omni1 dhclient[30322]: connection closed
Jul 12 11:37:07 kg-omni1 dhclient[30322]: exiting.
Jul 12 11:37:20 kg-omni1 kernel: xl0: link state changed to UP
Jul 12 11:37:20 kg-omni1 dhclient: New IP Address (xl0): 84.215.134.159
Jul 12 11:37:20 kg-omni1 dhclient: New Subnet Mask (xl0): 255.255.192.0
Jul 12 11:37:20 kg-omni1 dhclient: New Broadcast Address (xl0): 255.255.255.255
Jul 12 11:37:20 kg-omni1 dhclient: New Routers (xl0): 84.215.128.1
HTH
-- 
Torfinn Ingolfsen <torfinn.ingolfsen@getmail.no>
Comment 2 Yuri Victorovich freebsd_committer freebsd_triage 2012-07-13 19:16:30 UTC
dhclient on exit should also remove the IP address it has set.
Today the problem is not only that dhclient doesn't exit, but also that 
it doesn't remove the IP address and it is left intil and after the next 
DHCP setup.

Yuri
Comment 3 Peter Jeremy 2012-08-15 01:19:13 UTC
Sorry for the delay in responding, I got side-tracked.

On 2012-Jul-13 11:16:30 -0700, Yuri <yuri@rawbw.com> wrote:
>dhclient on exit should also remove the IP address it has set.


Yes, and the patch in the existing PR _should_ do that - it invokes
the failure handler in the same way as the other dhclient failure
modes.  Unfortunately, there seems to be a separate issue in dhclient
so that it is not passing the IP address to the failure handler.

-- 
Peter Jeremy
Comment 4 dfilter service freebsd_committer freebsd_triage 2012-08-17 16:53:54 UTC
Author: jhb
Date: Fri Aug 17 15:53:43 2012
New Revision: 239356
URL: http://svn.freebsd.org/changeset/base/239356

Log:
  Fix dhclient to properly exit and teardown the configured lease when
  link is lost.  devd will start a new dhclient instance when link is
  restored.
  
  PR:		bin/166656
  Submitted by:	Peter Jeremy (mostly)
  Reviewed by:	brooks (earlier version from Peter)
  MFC after:	1 month

Modified:
  head/sbin/dhclient/dhclient.c

Modified: head/sbin/dhclient/dhclient.c
==============================================================================
--- head/sbin/dhclient/dhclient.c	Fri Aug 17 14:22:56 2012	(r239355)
+++ head/sbin/dhclient/dhclient.c	Fri Aug 17 15:53:43 2012	(r239356)
@@ -278,6 +278,11 @@ routehandler(struct protocol *p)
 			    ifi->name);
 			goto die;
 		}
+		if (!interface_link_status(ifi->name)) {
+			warning("Interface %s is down, dhclient exiting",
+			    ifi->name);
+			goto die;
+		}
 		break;
 	case RTM_IFANNOUNCE:
 		ifan = (struct if_announcemsghdr *)rtm;
@@ -316,6 +321,8 @@ routehandler(struct protocol *p)
 
 die:
 	script_init("FAIL", NULL);
+	if (ifi->client->active)
+		script_write_params("old_", ifi->client->active);
 	if (ifi->client->alias)
 		script_write_params("alias_", ifi->client->alias);
 	script_go();
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 dfilter service freebsd_committer freebsd_triage 2012-08-22 14:53:49 UTC
Author: jhb
Date: Wed Aug 22 13:53:37 2012
New Revision: 239564
URL: http://svn.freebsd.org/changeset/base/239564

Log:
  Revert r239356 and use an alternate algorithm.
  
  First, don't exit when the link goes down on an interface.  Instead,
  teach dhclient to track changes in link state and to enter the reboot
  state when the link on an interface goes up causing dhclient to attempt
  to renew its existing lease.
  
  Second, remove the change I added to clear the old lease when dhclient
  exits due to an error (such as ifconfig down).  If an interface is
  using autoconfiguration it should keep its autoconfiguration as much as
  possible.  If the next time it needs a configuration it is able to reuse
  the previous autoconfiguration, then leaving the settings intact allows
  existing connections to survive temporary outages, etc.
  
  PR:		bin/166656
  MFC after:	1 month

Modified:
  head/sbin/dhclient/dhclient.c
  head/sbin/dhclient/dhcpd.h

Modified: head/sbin/dhclient/dhclient.c
==============================================================================
--- head/sbin/dhclient/dhclient.c	Wed Aug 22 08:27:37 2012	(r239563)
+++ head/sbin/dhclient/dhclient.c	Wed Aug 22 13:53:37 2012	(r239564)
@@ -218,6 +218,7 @@ routehandler(struct protocol *p)
 	struct sockaddr *sa;
 	struct iaddr a;
 	ssize_t n;
+	int linkstat;
 
 	n = read(routefd, &msg, sizeof(msg));
 	rtm = (struct rt_msghdr *)msg;
@@ -278,10 +279,14 @@ routehandler(struct protocol *p)
 			    ifi->name);
 			goto die;
 		}
-		if (!interface_link_status(ifi->name)) {
-			warning("Interface %s is down, dhclient exiting",
-			    ifi->name);
-			goto die;
+		linkstat = interface_link_status(ifi->name);
+		if (linkstat != ifi->linkstat) {
+			debug("%s link state %s -> %s", ifi->name,
+			    ifi->linkstat ? "up" : "down",
+			    linkstat ? "up" : "down");
+			ifi->linkstat = linkstat;
+			if (linkstat)
+				state_reboot(ifi);
 		}
 		break;
 	case RTM_IFANNOUNCE:
@@ -321,8 +326,6 @@ routehandler(struct protocol *p)
 
 die:
 	script_init("FAIL", NULL);
-	if (ifi->client->active)
-		script_write_params("old_", ifi->client->active);
 	if (ifi->client->alias)
 		script_write_params("alias_", ifi->client->alias);
 	script_go();
@@ -437,6 +440,7 @@ main(int argc, char *argv[])
 		}
 		fprintf(stderr, " got link\n");
 	}
+	ifi->linkstat = 1;
 
 	if ((nullfd = open(_PATH_DEVNULL, O_RDWR, 0)) == -1)
 		error("cannot open %s: %m", _PATH_DEVNULL);

Modified: head/sbin/dhclient/dhcpd.h
==============================================================================
--- head/sbin/dhclient/dhcpd.h	Wed Aug 22 08:27:37 2012	(r239563)
+++ head/sbin/dhclient/dhcpd.h	Wed Aug 22 13:53:37 2012	(r239564)
@@ -208,6 +208,7 @@ struct interface_info {
 	int			 errors;
 	int			 dead;
 	u_int16_t		 index;
+	int			 linkstat;
 };
 
 struct timeout {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 John Baldwin freebsd_committer freebsd_triage 2012-08-29 13:45:32 UTC
State Changed
From-To: open->patched

I think the current patches to dhclient in HEAD satisfy this now. 


Comment 7 John Baldwin freebsd_committer freebsd_triage 2012-08-29 13:45:32 UTC
Responsible Changed
From-To: freebsd-bugs->jhb

I think the current patches to dhclient in HEAD satisfy this now.
Comment 8 dfilter service freebsd_committer freebsd_triage 2013-02-26 19:14:18 UTC
Author: jhb
Date: Tue Feb 26 19:14:05 2013
New Revision: 247335
URL: http://svnweb.freebsd.org/changeset/base/247335

Log:
  MFC 239356,239564:
  Teach dhclient to track changes in link state and to enter the reboot
  state when the link on an interface goes up causing dhclient to attempt
  to renew its existing lease.
  
  PR:		bin/166656

Modified:
  stable/9/sbin/dhclient/dhclient.c
  stable/9/sbin/dhclient/dhcpd.h
Directory Properties:
  stable/9/sbin/dhclient/   (props changed)

Modified: stable/9/sbin/dhclient/dhclient.c
==============================================================================
--- stable/9/sbin/dhclient/dhclient.c	Tue Feb 26 18:33:23 2013	(r247334)
+++ stable/9/sbin/dhclient/dhclient.c	Tue Feb 26 19:14:05 2013	(r247335)
@@ -218,6 +218,7 @@ routehandler(struct protocol *p)
 	struct sockaddr *sa;
 	struct iaddr a;
 	ssize_t n;
+	int linkstat;
 
 	n = read(routefd, &msg, sizeof(msg));
 	rtm = (struct rt_msghdr *)msg;
@@ -278,6 +279,15 @@ routehandler(struct protocol *p)
 			    ifi->name);
 			goto die;
 		}
+		linkstat = interface_link_status(ifi->name);
+		if (linkstat != ifi->linkstat) {
+			debug("%s link state %s -> %s", ifi->name,
+			    ifi->linkstat ? "up" : "down",
+			    linkstat ? "up" : "down");
+			ifi->linkstat = linkstat;
+			if (linkstat)
+				state_reboot(ifi);
+		}
 		break;
 	case RTM_IFANNOUNCE:
 		ifan = (struct if_announcemsghdr *)rtm;
@@ -430,6 +440,7 @@ main(int argc, char *argv[])
 		}
 		fprintf(stderr, " got link\n");
 	}
+	ifi->linkstat = 1;
 
 	if ((nullfd = open(_PATH_DEVNULL, O_RDWR, 0)) == -1)
 		error("cannot open %s: %m", _PATH_DEVNULL);

Modified: stable/9/sbin/dhclient/dhcpd.h
==============================================================================
--- stable/9/sbin/dhclient/dhcpd.h	Tue Feb 26 18:33:23 2013	(r247334)
+++ stable/9/sbin/dhclient/dhcpd.h	Tue Feb 26 19:14:05 2013	(r247335)
@@ -208,6 +208,7 @@ struct interface_info {
 	int			 errors;
 	int			 dead;
 	u_int16_t		 index;
+	int			 linkstat;
 };
 
 struct timeout {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 John Baldwin freebsd_committer freebsd_triage 2013-02-26 19:14:37 UTC
State Changed
From-To: patched->closed

Fix merged to 8 and 9.
Comment 10 dfilter service freebsd_committer freebsd_triage 2013-02-26 19:14:38 UTC
Author: jhb
Date: Tue Feb 26 19:14:29 2013
New Revision: 247336
URL: http://svnweb.freebsd.org/changeset/base/247336

Log:
  MFC 239356,239564:
  Teach dhclient to track changes in link state and to enter the reboot
  state when the link on an interface goes up causing dhclient to attempt
  to renew its existing lease.
  
  PR:		bin/166656

Modified:
  stable/8/sbin/dhclient/dhclient.c
  stable/8/sbin/dhclient/dhcpd.h
Directory Properties:
  stable/8/sbin/dhclient/   (props changed)

Modified: stable/8/sbin/dhclient/dhclient.c
==============================================================================
--- stable/8/sbin/dhclient/dhclient.c	Tue Feb 26 19:14:05 2013	(r247335)
+++ stable/8/sbin/dhclient/dhclient.c	Tue Feb 26 19:14:29 2013	(r247336)
@@ -218,6 +218,7 @@ routehandler(struct protocol *p)
 	struct sockaddr *sa;
 	struct iaddr a;
 	ssize_t n;
+	int linkstat;
 
 	n = read(routefd, &msg, sizeof(msg));
 	rtm = (struct rt_msghdr *)msg;
@@ -278,6 +279,15 @@ routehandler(struct protocol *p)
 			    ifi->name);
 			goto die;
 		}
+		linkstat = interface_link_status(ifi->name);
+		if (linkstat != ifi->linkstat) {
+			debug("%s link state %s -> %s", ifi->name,
+			    ifi->linkstat ? "up" : "down",
+			    linkstat ? "up" : "down");
+			ifi->linkstat = linkstat;
+			if (linkstat)
+				state_reboot(ifi);
+		}
 		break;
 	case RTM_IFANNOUNCE:
 		ifan = (struct if_announcemsghdr *)rtm;
@@ -430,6 +440,7 @@ main(int argc, char *argv[])
 		}
 		fprintf(stderr, " got link\n");
 	}
+	ifi->linkstat = 1;
 
 	if ((nullfd = open(_PATH_DEVNULL, O_RDWR, 0)) == -1)
 		error("cannot open %s: %m", _PATH_DEVNULL);

Modified: stable/8/sbin/dhclient/dhcpd.h
==============================================================================
--- stable/8/sbin/dhclient/dhcpd.h	Tue Feb 26 19:14:05 2013	(r247335)
+++ stable/8/sbin/dhclient/dhcpd.h	Tue Feb 26 19:14:29 2013	(r247336)
@@ -208,6 +208,7 @@ struct interface_info {
 	int			 errors;
 	int			 dead;
 	u_int16_t		 index;
+	int			 linkstat;
 };
 
 struct timeout {
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"