Add a reason for connection status changes

This hopefully will make connectivity change issues easier to debug in
the future. The idea is to give a reason why the status changes so that
any unexpected transistions can more easily be debugged.

lib/vintage_net/connectivity/internet_checker.ex

@@ -53,17 +53,17 @@ defmodule VintageNet.Connectivity.InternetChecker do
     # get our connectivity status back in sync.
     new_state =
       if VintageNet.get(lower_up_property(ifname)) do
-        state |> ifup() |> report_connectivity()
+        state |> ifup() |> report_connectivity("ifup")
       else
-        state |> ifdown() |> report_connectivity()
+        state |> ifdown() |> report_connectivity("ifdown")
       end
 
     {:noreply, new_state, new_state.status.interval}
   end
 
   @impl GenServer
   def handle_info(:timeout, state) do
-    new_state = state |> check_connectivity() |> report_connectivity()
+    new_state = state |> check_connectivity() |> report_connectivity("timeout")
 
     {:noreply, new_state, new_state.status.interval}
   end
@@ -72,7 +72,7 @@ defmodule VintageNet.Connectivity.InternetChecker do
         {VintageNet, ["interface", ifname, "lower_up"], _old_value, false, _meta},
         %{ifname: ifname} = state
       ) do
-    new_state = state |> ifdown() |> report_connectivity()
+    new_state = state |> ifdown() |> report_connectivity("ifdown")
 
     {:noreply, new_state, new_state.status.interval}
   end
@@ -81,7 +81,7 @@ defmodule VintageNet.Connectivity.InternetChecker do
         {VintageNet, ["interface", ifname, "lower_up"], _old_value, true, _meta},
         %{ifname: ifname} = state
       ) do
-    new_state = state |> ifup() |> report_connectivity()
+    new_state = state |> ifup() |> report_connectivity("ifup")
 
     {:noreply, new_state, new_state.status.interval}
   end
@@ -91,7 +91,7 @@ defmodule VintageNet.Connectivity.InternetChecker do
         %{ifname: ifname} = state
       ) do
     # The interface was completely removed!
-    new_state = state |> ifdown() |> report_connectivity()
+    new_state = state |> ifdown() |> report_connectivity("removed!")
     {:noreply, new_state, new_state.status.interval}
   end
 
@@ -118,13 +118,12 @@ defmodule VintageNet.Connectivity.InternetChecker do
     end
   end
 
-  defp report_connectivity(state) do
+  defp report_connectivity(state, why) do
     # It's desirable to set these even if redundant since the checks in this
     # modules are authoritative. I.e., the internet isn't connected unless we
-    # declare it detected. Other modules can reset the connection to :lan
-    # if, for example, a new IP address gets set by DHCP. The following call
+    # declare it detected.The following call
     # will optimize out redundant updates if they really are redundant.
-    RouteManager.set_connection_status(state.ifname, state.status.connectivity)
+    RouteManager.set_connection_status(state.ifname, state.status.connectivity, why)
     state
   end
 

lib/vintage_net/connectivity/lan_checker.ex

@@ -35,12 +35,12 @@ defmodule VintageNet.Connectivity.LANChecker do
 
     case VintageNet.get(lower_up_property(ifname)) do
       true ->
-        RouteManager.set_connection_status(ifname, :lan)
+        RouteManager.set_connection_status(ifname, :lan, "ifup")
 
       _not_true ->
         # If the physical layer isn't up, don't start polling until
         # we're notified that it is available.
-        RouteManager.set_connection_status(ifname, :disconnected)
+        RouteManager.set_connection_status(ifname, :disconnected, "ifdown")
     end
 
     {:noreply, state}
@@ -52,7 +52,7 @@ defmodule VintageNet.Connectivity.LANChecker do
         %{ifname: ifname} = state
       ) do
     # Physical layer is down. We're definitely disconnected.
-    RouteManager.set_connection_status(ifname, :disconnected)
+    RouteManager.set_connection_status(ifname, :disconnected, "ifdown")
     {:noreply, state}
   end
 
@@ -64,7 +64,7 @@ defmodule VintageNet.Connectivity.LANChecker do
     # Physical layer is up. Optimistically assume that the LAN is accessible.
 
     # NOTE: Consider triggering based on whether the interface has an IP address or not.
-    RouteManager.set_connection_status(ifname, :lan)
+    RouteManager.set_connection_status(ifname, :lan, "ifup")
 
     {:noreply, state}
   end
@@ -75,7 +75,7 @@ defmodule VintageNet.Connectivity.LANChecker do
         %{ifname: ifname} = state
       ) do
     # The interface was completely removed!
-    if old_value, do: RouteManager.set_connection_status(ifname, :disconnected)
+    if old_value, do: RouteManager.set_connection_status(ifname, :disconnected, "removed!")
     {:noreply, state}
   end
 

lib/vintage_net/route_manager.ex

@@ -99,9 +99,10 @@ defmodule VintageNet.RouteManager do
   Changing the connection status can re-prioritize routing. The
   specified interface doesn't need to have a default route.
   """
-  @spec set_connection_status(VintageNet.ifname(), VintageNet.connection_status()) :: :ok
-  def set_connection_status(ifname, status) do
-    GenServer.call(__MODULE__, {:set_connection_status, ifname, status})
+  @spec set_connection_status(VintageNet.ifname(), VintageNet.connection_status(), String.t()) ::
+          :ok
+  def set_connection_status(ifname, status, why \\ "unknown") do
+    GenServer.call(__MODULE__, {:set_connection_status, ifname, status, why})
   end
 
   @doc """
@@ -172,10 +173,10 @@ defmodule VintageNet.RouteManager do
   end
 
   @impl GenServer
-  def handle_call({:set_connection_status, ifname, status}, _from, state) do
+  def handle_call({:set_connection_status, ifname, status, why}, _from, state) do
     new_state =
       state
-      |> update_connection_status(ifname, status)
+      |> update_connection_status(ifname, status, why)
 
     {:reply, :ok, new_state}
   end
@@ -240,11 +241,11 @@ defmodule VintageNet.RouteManager do
   end
 
   # Only process routes if the status changes
-  defp update_connection_status(state, ifname, new_status) do
+  defp update_connection_status(state, ifname, new_status, why) do
     case state.interfaces[ifname] do
       nil ->
         Logger.warn(
-          "RouteManager: set_connection_status to #{inspect(new_status)} on unknown ifname: #{ifname}"
+          "RouteManager: new set_connection_status #{ifname}} -> #{inspect(new_status)} (#{why})"
         )
 
         ifentry = new_interface_info(ifname, [], nil, new_status)
@@ -254,7 +255,9 @@ defmodule VintageNet.RouteManager do
 
       ifentry ->
         if ifentry.status != new_status do
-          Logger.info("RouteManager: set_connection_status #{ifname} -> #{inspect(new_status)}")
+          Logger.info(
+            "RouteManager: set_connection_status #{ifname} -> #{inspect(new_status)} (#{why})"
+          )
 
           put_in(state.interfaces[ifname].status, new_status)
           |> update_route_tables()