Skip to content

Commit

Permalink
Don't downgrade connection status on route change
Browse files Browse the repository at this point in the history
The original logic would force the connection status to `:lan` if the IP
address or route changed. The logic was that the Internet might not be
reachible, so let the connectivity checker do it's thing and upgrade the
connection.

This had a couple undesirable side effects:

1. The interface preference would bounce when routes changed even if the route
   change resulted in the status quo.
2. There was a race condition between the connectivity checker. If it
   set "internet" right before the route change, then you'd have to wait
   until the next check before it would be restored to "internet".
3. Not all connectivity checkers were as persistent at setting
   "internet", so it could be degraded for a really long time.

This change keeps the interface connectivity the same or upgrades it to
"lan" on route changes. This basically changes the behavior from
pessimistic (the interface isn't internet-connected, so prove it) to
optimistic (the interface is still connected, and if that's wrong, let
me know).
  • Loading branch information
fhunleth committed Nov 17, 2021
1 parent afdf560 commit f8df93c
Showing 1 changed file with 20 additions and 6 deletions.
26 changes: 20 additions & 6 deletions lib/vintage_net/route_manager.ex
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,27 @@ defmodule VintageNet.RouteManager do
@impl GenServer
def handle_call({:set_route, ifname, ip_subnets, default_gateway}, _from, state) do
if interface_info_changed?(state, ifname, ip_subnets, default_gateway) do
Logger.info("RouteManager: set_route #{ifname} -> :lan")
Logger.info(
"RouteManager: set_route #{ifname}: IP: #{inspect(ip_subnets)}, GW: #{inspect(default_gateway)}"
)

# Mostly keep the status.
#
# All changes to the connectivity status need to come
# from the connectivity checker or reasoning about this gets confusing.
# Note that we know here that the connectivity state may have changed
# since either the IP address or default gateway changed. Nonetheless,
# defer to the connectivity checker to tell us.
#
# If we don't know about the interface yet (no route), then it definitely
# has :lan status so start there.
status =
case Map.fetch(state.interfaces, ifname) do
{:ok, %InterfaceInfo{status: status}} -> status
_ -> :lan
end

# LAN connectivity assumed since if the information changed, then
# internet connectivity needs to be reverified. Plus any existing TCP
# connections going to the internet will be broken given the IP or
# default gateway change.
ifentry = new_interface_info(ifname, ip_subnets, default_gateway, :lan)
ifentry = new_interface_info(ifname, ip_subnets, default_gateway, status)

new_state =
put_in(state.interfaces[ifname], ifentry)
Expand Down

0 comments on commit f8df93c

Please sign in to comment.