dhcpcd-discuss

Hi Boris

On 12/12/2020 13:20, Roy Marples wrote:
> > 1. It seems 20-resolv.conf hook is trying to remove DNS info on roaming, this 
> > should not happen
>
> Currently it does by design.
> I know it's not desirable for you, but all the hooks need some work as noted in 
> my commit. It's just not as simple as reversing in in dhcpcd.

I found some time to work on this and a patch is attached.
If you use resolvconf, you will need a version with the new -C and -c options.
If you use my openresolv, you can find this in the master branch of the 
openresolv repository.

If you don't use resolvconf or there are no -C or -c options then it will 
function as before, but the DNS information will persist.

Let me know how this works for you.

Roy

diff --git a/BUILDING.md b/BUILDING.md
index f672724a..2856736c 100644
--- a/BUILDING.md
+++ b/BUILDING.md
@@ -148,3 +148,9 @@ copied to `$(libexecdir)/dhcpcd-hooks` for use.
 The configure program attempts to find hooks for systems you have installed.
 To add more simply
 `./configure -with-hook=ntp.conf`
+
+If using resolvconf, the `20-resolv.conf` hook now requires a version with the
+`-C` and `-c` options to deprecate and activate interfaces to support wireless
+roaming (Linux) or carrier just drops (NetBSD).
+If your resolvconf does not support this then you will see a warning
+about an illegal option when the carrier changes, but things should still work.
diff --git a/hooks/20-resolv.conf b/hooks/20-resolv.conf
index 73d33386..86f4873d 100644
--- a/hooks/20-resolv.conf
+++ b/hooks/20-resolv.conf
@@ -10,6 +10,11 @@ resolv_conf_dir="$state_dir/resolv.conf"
 NL="
 "
 : ${resolvconf:=resolvconf}
+if type "$resolvconf" >/dev/null 2>&1; then
+	have_resolvconf=true
+else
+	have_resolvconf=false
+fi
 
 build_resolv_conf()
 {
@@ -164,7 +169,7 @@ add_resolv_conf()
 	for x in ${new_domain_name_servers}; do
 		conf="${conf}nameserver $x$NL"
 	done
-	if type "$resolvconf" >/dev/null 2>&1; then
+	if $have_resolvconf; then
 		[ -n "$ifmetric" ] && export IF_METRIC="$ifmetric"
 		printf %s "$conf" | "$resolvconf" -a "$ifname"
 		return $?
@@ -180,7 +185,7 @@ add_resolv_conf()
 
 remove_resolv_conf()
 {
-	if type "$resolvconf" >/dev/null 2>&1; then
+	if $have_resolvconf; then
 		"$resolvconf" -d "$ifname" -f
 	else
 		if [ -e "$resolv_conf_dir/$ifname" ]; then
@@ -199,7 +204,11 @@ BOUND6|RENEW6|REBIND6|REBOOT6|INFORM6)
 esac
 
 if $if_configured; then
-	if $if_up || [ "$reason" = ROUTERADVERT ]; then
+	if $have_resolvconf && [ "$reason" = NOCARRIER_ROAMING ]; then
+		"$resolvconf" -C "$interface.*"
+	elif $have_resolvconf && [ "$reason" = CARRIER ]; then
+		"$resolvconf" -c "$interface.*"
+	elif $if_up || [ "$reason" = ROUTERADVERT ]; then
 		add_resolv_conf
 	elif $if_down; then
 		remove_resolv_conf
diff --git a/src/dhcpcd.c b/src/dhcpcd.c
index 3acf72c3..7799277a 100644
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -700,26 +700,7 @@ dhcpcd_nocarrier_roaming(struct interface *ifp)
 {
 
 	loginfox("%s: carrier lost - roaming", ifp->name);
-
-	/*
-	 * XXX We should pass something like NOCARRIER_ROAMING
-	 * and set if_up=true; ifdown=false; so that the hook scripts
-	 * can make a decision to keep or discard the interface information.
-	 *
-	 * Currently they discard it (no carrier after all) which is
-	 * generally fine as new connections won't work and current
-	 * connections try to chug along as best as.
-	 * dhcpcd has been doing this since NetBSD-7 at least.
-	 *
-	 * However, for slow roaming this is poor for say web browsing
-	 * as new lookups will fail quickly giving a poor user experience.
-	 * We should improve this, but the hooks will require some work first
-	 * as we need to introduce a mechanism to sort interfaces by
-	 * carrier > roaming > nocarrier. Then the hooks know in which
-	 * order to apply their data, if at all.
-	 * This probably should be a user toggle.
-	 */
-	script_runreason(ifp, "NOCARRIER");
+	script_runreason(ifp, "NOCARRIER_ROAMING");
 
 #ifdef ARP
 	arp_drop(ifp);
diff --git a/src/route.c b/src/route.c
index 47fd6cf2..b0251c2b 100644
--- a/src/route.c
+++ b/src/route.c
@@ -168,6 +168,15 @@ rt_compare_proto(void *context, const void *node1, const void *node2)
 	if (c != 0)
 		return -c;
 
+	/* Prefer roaming over non roaming if both carriers are down. */
+	if (ifp1->carrier == LINK_DOWN && ifp2->carrier == LINK_DOWN) {
+		bool roam1 = if_roaming(ifp1);
+		bool roam2 = if_roaming(ifp2);
+
+		if (roam1 != roam2)
+			return roam1 ? 1 : -1;
+	}
+
 #ifdef INET
 	/* IPv4LL routes always come last */
 	if (rt1->rt_dflags & RTDF_IPV4LL && !(rt2->rt_dflags & RTDF_IPV4LL))
diff --git a/src/script.c b/src/script.c
index 0260845e..bf1e5152 100644
--- a/src/script.c
+++ b/src/script.c
@@ -74,6 +74,9 @@ static const char * const if_params[] = {
 	NULL
 };
 
+static const char * true_str = "true";
+static const char * false_str = "false";
+
 void
 if_printoptions(void)
 {
@@ -228,6 +231,10 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 	const struct if_options *ifo;
 	const struct interface *ifp2;
 	int af;
+	bool is_stdin = ifp->name[0] == '\0';
+	const char *if_up, *if_down;
+	rb_tree_t ifaces;
+	struct rt *rt;
 #ifdef INET
 	const struct dhcp_state *state;
 #ifdef IPV4LL
@@ -237,7 +244,6 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 #ifdef DHCP6
 	const struct dhcp6_state *d6_state;
 #endif
-	bool is_stdin = ifp->name[0] == '\0';
 
 #ifdef HAVE_OPEN_MEMSTREAM
 	if (ctx->script_fp == NULL) {
@@ -276,6 +282,7 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 		if (efprintf(fp, "pid=%d", getpid()) == -1)
 			goto eexit;
 	}
+
 	if (!is_stdin) {
 		if (efprintf(fp, "reason=%s", reason) == -1)
 			goto eexit;
@@ -326,6 +333,7 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 	else if (strcmp(reason, "PREINIT") == 0 ||
 	    strcmp(reason, "CARRIER") == 0 ||
 	    strcmp(reason, "NOCARRIER") == 0 ||
+	    strcmp(reason, "NOCARRIER_ROAMING") == 0 ||
 	    strcmp(reason, "UNKNOWN") == 0 ||
 	    strcmp(reason, "DEPARTED") == 0 ||
 	    strcmp(reason, "STOPPED") == 0)
@@ -382,34 +390,43 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 	if (ifp->ctx->options & DHCPCD_DUMPLEASE)
 		goto dumplease;
 
+	rb_tree_init(&ifaces, &rt_compare_proto_ops);
+	TAILQ_FOREACH(ifp2, ifp->ctx->ifaces, next) {
+		rt = rt_new(UNCONST(ifp2));
+		if (rt == NULL)
+			goto eexit;
+		if (rb_tree_insert_node(&ifaces, rt) != rt)
+			goto eexit;
+	}
 	if (fprintf(fp, "interface_order=") == -1)
 		goto eexit;
-	TAILQ_FOREACH(ifp2, ifp->ctx->ifaces, next) {
-		if (ifp2 != TAILQ_FIRST(ifp->ctx->ifaces)) {
-			if (fputc(' ', fp) == EOF)
-				return -1;
-		}
-		if (fprintf(fp, "%s", ifp2->name) == -1)
-			return -1;
+	RB_TREE_FOREACH(rt, &ifaces) {
+		if (rt != RB_TREE_MIN(&ifaces) &&
+		    fprintf(fp, "%s", " ") == -1)
+			goto eexit;
+		if (fprintf(fp, "%s", rt->rt_ifp->name) == -1)
+			goto eexit;
 	}
+	rt_headclear(&ifaces, AF_UNSPEC);
 	if (fputc('\0', fp) == EOF)
-		return -1;
+		goto eexit;
 
 	if (strcmp(reason, "STOPPED") == 0) {
-		if (efprintf(fp, "if_up=false") == -1)
-			goto eexit;
-		if (efprintf(fp, "if_down=%s",
-		    ifo->options & DHCPCD_RELEASE ? "true" : "false") == -1)
-			goto eexit;
+		if_up = false_str;
+		if_down = ifo->options & DHCPCD_RELEASE ? true_str : false_str;
 	} else if (strcmp(reason, "TEST") == 0 ||
 	    strcmp(reason, "PREINIT") == 0 ||
 	    strcmp(reason, "CARRIER") == 0 ||
 	    strcmp(reason, "UNKNOWN") == 0)
 	{
-		if (efprintf(fp, "if_up=false") == -1)
-			goto eexit;
-		if (efprintf(fp, "if_down=false") == -1)
-			goto eexit;
+		if_up = false_str;
+		if_down = false_str;
+	} else if (strcmp(reason, "NOCARRIER") == 0) {
+		if_up = false_str;
+		if_down = true_str;
+	} else if (strcmp(reason, "NOCARRIER_ROAMING") == 0) {
+		if_up = true_str;
+		if_down = false_str;
 	} else if (1 == 2 /* appease ifdefs */
 #ifdef INET
 	    || (protocol == PROTO_DHCP && state && state->new)
@@ -426,16 +443,17 @@ make_env(struct dhcpcd_ctx *ctx, const struct interface *ifp,
 #endif
 	    )
 	{
-		if (efprintf(fp, "if_up=true") == -1)
-			goto eexit;
-		if (efprintf(fp, "if_down=false") == -1)
-			goto eexit;
+		if_up = true_str;
+		if_down = false_str;
 	} else {
-		if (efprintf(fp, "if_up=false") == -1)
-			goto eexit;
-		if (efprintf(fp, "if_down=true") == -1)
-			goto eexit;
+		if_up = false_str;
+		if_down = true_str;
 	}
+	if (efprintf(fp, "if_up=%s", if_up) == -1)
+		goto eexit;
+	if (efprintf(fp, "if_down=%s", if_down) == -1)
+		goto eexit;
+
 	if ((af = dhcpcd_ifafwaiting(ifp)) != AF_MAX) {
 		if (efprintf(fp, "if_afwaiting=%d", af) == -1)
 			goto eexit;