Re: dhcpcd kills all connections on Wi-Fi roaming betweenaccesspoints
Roy Marples
Thu Dec 24 06:29:52 2020
On 24/12/2020 03:43, Boris Krasnovskiy wrote:
HI Roy,
sorry, my mistake, I used a config file with disabled DNS options.
The code partially works, the problem is: after roaming DNS interfaces get
reordered, and this is incorrect.
DNS interfaces should be ordered according to their metric always.
I disagree.
The rules are these: Precedence list, fallthrough on equality:
Carrier UP
Roaming
Not IPv4LL
Interface Metric
Oldest
In previous emails you've indicated that in the roaming state it never actually
goes down, just back up again which could take time to find an AP. As such,
you'll have a hard job convincing me that the working ethernet you just plugged
into should not take precedence by default.
Now this could be an issue where the route metric == the interface metric (ie
Linux, not NetBSD where my testing is) so we need to adjust the code to change
the route metric to reflect roaming vs carrier up.
Your quote:
DNS interfaces should be ordered according to their metric always.
becomes:
DNS interfaces should be ordered according to their route metric always.
Then we are both happy if the route metric reflects the above ordering
Does this sound OK?
This means I need to test on my Linux laptop where I broke the OS - it's a
pinebook and the upgrade using KDE neon failed hard. A reflash should fix, but
as always that takes time. I've attached a new patch which should work. Be aware
that existing route offset values have been updated to start from 1000 rather
than 100. All the offsets are now set in route.h rather than magic numbers in
the code to make things clearer. The existing route code *should* already change
the route metric on before running a hook, but that's not been tested in years.
Hopefully you can test while I refresh my laptop.
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/if.c b/src/if.c
index deb5280b..d3852f3d 100644
--- a/src/if.c
+++ b/src/if.c
@@ -697,12 +697,11 @@ if_discover(struct dhcpcd_ctx *ctx, struct ifaddrs **ifaddrs,
ifp->metric = (unsigned int)ifr.ifr_metric;
if_getssid(ifp);
#else
- /* We reserve the 100 range for virtual interfaces, if and when
- * we can work them out. */
- ifp->metric = 200 + ifp->index;
+ /* Leave a low portion for user config */
+ ifp->metric = RTMETRIC_BASE + ifp->index;
if (if_getssid(ifp) != -1) {
ifp->wireless = true;
- ifp->metric += 100;
+ ifp->metric += RTMETRIC_WIRELESS;
}
#endif
diff --git a/src/ipv4ll.c b/src/ipv4ll.c
index 93898109..d5b27c53 100644
--- a/src/ipv4ll.c
+++ b/src/ipv4ll.c
@@ -137,7 +137,7 @@ ipv4ll_defaultroute(rb_tree_t *routes, struct interface *ifp)
sa_in_init(&rt->rt_ifa, &state->addr->addr);
rt->rt_dflags |= RTDF_IPV4LL;
#ifdef HAVE_ROUTE_METRIC
- rt->rt_metric += 10000;
+ rt->rt_metric += RTMETRIC_IPV4LL_OFF;
#endif
return rt_proto_add(routes, rt) ? 1 : 0;
}
diff --git a/src/route.c b/src/route.c
index 47fd6cf2..71523c14 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))
@@ -374,6 +383,8 @@ rt_setif(struct rt *rt, struct interface *ifp)
rt->rt_ifp = ifp;
#ifdef HAVE_ROUTE_METRIC
rt->rt_metric = ifp->metric;
+ if (if_roaming(ifp))
+ rt->rt_metric += RTMETRIC_ROAM;
#endif
}
diff --git a/src/route.h b/src/route.h
index e9ac121a..cb935c83 100644
--- a/src/route.h
+++ b/src/route.h
@@ -93,6 +93,15 @@ struct rt {
#ifdef HAVE_ROUTE_METRIC
unsigned int rt_metric;
#endif
+/* Maximum interface index is generally USHORT_MAX or 65535.
+ * Add some padding for other stuff and we get offsets for the
+ * below that should work automatically.
+ * This is only an issue if the user defines higher metrics in
+ * their configuration, but then they might wish to override also. */
+#define RTMETRIC_BASE 1000U
+#define RTMETRIC_WIRELESS 2000U
+#define RTMETRIC_IPV4LL 1000000U
+#define RTMETRIC_ROAM 2000000U
#ifdef HAVE_ROUTE_PREF
int rt_pref;
#endif
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;
Archive administrator: postmaster@marples.name