Skip DHCP lease renewal in subnet protection service if nothing has changed (#505)

Author: lmagyar

tailscale/rootfs/etc/NetworkManager/dispatcher.d/protect-subnets

@@ -2,15 +2,30 @@
 # shellcheck shell=bash
 # The shebang 'with-contenv-merge' above is identical with 'with-contenv', but doesn't clear the current environment containing the dispatcher variables
 
+function halt-add-on() {
+  bashio::log.error "Failed to protect subnet routes. Halting add-on to prevent network loss."
+  echo -n 1 > /run/s6-linux-init-container-results/exitcode
+  exec /run/s6/basedir/bin/halt
+}
+
 case "${NM_DISPATCHER_ACTION}" in
-  up|down|dhcp4-change|dhcp6-change)
+  up|down)
     bashio::log.info "Handling Network Manager action ${DEVICE_IP_IFACE-} ${NM_DISPATCHER_ACTION}"
     unprotect-subnet-routes
     if ! protect-subnet-routes; then
       # Better stop add-on than risking losing all network connections
-      bashio::log.error "Failed to protect subnet routes. Halting add-on to prevent network loss."
-      echo -n 1 > /run/s6-linux-init-container-results/exitcode
-      exec /run/s6/basedir/bin/halt
+      halt-add-on
+    fi
+    ;;
+  dhcp4-change|dhcp6-change)
+    # Do anything only when the addresses are really changed
+    if [[ "$(unprotect-subnet-routes test)" != "$(protect-subnet-routes test)" ]]; then
+      bashio::log.info "Handling Network Manager action ${DEVICE_IP_IFACE-} ${NM_DISPATCHER_ACTION}"
+      unprotect-subnet-routes
+      if ! protect-subnet-routes tested; then
+        # Better stop add-on than risking losing all network connections
+        halt-add-on
+      fi
     fi
     ;;
   connectivity-change)

tailscale/rootfs/usr/bin/protect-subnet-routes

@@ -4,33 +4,45 @@
 # In case of non userspace networking,
 # add local subnets to ip rules with higher priority than Tailscale's routing
 # ==============================================================================
+
 readonly PROTECTION_RULE_PRIORITY=5000
+readonly WAIT_DELAY=5   # 5s
+readonly WAIT_COUNT=60  # 60*5s = 300s = 5m
 
 declare -a routes=()
 declare route family
 declare response
 declare wait_counter=0
 
-if bashio::config.false "userspace_networking" && \
-  (! bashio::config.has_value "accept_routes" || bashio::config.true "accept_routes")
-then
+if ! [[ "${1-}" =~ ^(|test|tested)$ ]]; then
+  echo "Usage: $(basename "$0") [test|tested]" 1>&2
+  exit 1
+fi
+
+if [[ "${1-}" != "tested" ]]; then
   # If it is called after network configuration is changed, we need to drop cached network info
   bashio::cache.flush_all
   # It is possible to get "ERROR: Got unexpected response from the API: System is not ready with state: setup"
-  # So we wait a little, 60*5s = 300s = 5m
-  while ! bashio::api.supervisor GET "/addons/self/options/config" false &> /dev/null; do
-    if (( wait_counter++ == 60 )); then
-      bashio::log.error "Supervisor is unreachable"
-      bashio::exit.nok
+  # Test both networking and config Supervisor API availability, these APIs are called in subnet-routes script
+  # And wait a little on inaccessibility
+  while ! bashio::api.supervisor GET "/network/interface/default/info" false &> /dev/null || \
+    ! bashio::api.supervisor GET "/addons/self/options/config" false &> /dev/null
+  do
+    if (( wait_counter++ == $WAIT_COUNT )); then
+      bashio::exit.nok "Supervisor is unreachable"
     fi
     bashio::log.info "Waiting for the supervisor to be ready..."
-    sleep 5
+    sleep $WAIT_DELAY
   done
   if (( wait_counter != 0 )); then
     bashio::log.info "Supervisor is ready"
   fi
+fi
 
-  readarray -t routes < <(subnet-routes local)
+readarray -t routes < <(subnet-routes local)
+if [[ "${1-}" == "test" ]]; then
+  printf "%s" "${routes[@]/%/$'\n'}"
+else
   bashio::log.info \
     "Adding local subnets to ip rules with higher priority than Tailscale's routing," \
     "to prevent routing local subnets if the same subnet is routed within your tailnet."
@@ -53,7 +65,7 @@ then
       if ! response=$(ip "${family}" rule add to "${route}" priority ${PROTECTION_RULE_PRIORITY} table main 2>&1); then
         if [[ "${response}" != "RTNETLINK answers: File exists" ]]; then
           echo "${response}"
-          bashio::exit.nok
+          bashio::exit.nok "  Adding route ${route} to ip rules is unsuccessful"
         else
           bashio::log.notice "  Route ${route} is already added to ip rules"
         fi

tailscale/rootfs/usr/bin/subnet-routes

@@ -16,7 +16,7 @@ function appendarray() {
 }
 
 if ! [[ "${1-}" =~ ^(local|advertised)$ ]]; then
-  echo "Usage: subnet-routes local|advertised" 1>&2
+  echo "Usage: $(basename "$0") local|advertised" 1>&2
   exit 1
 fi
 

tailscale/rootfs/usr/bin/unprotect-subnet-routes

@@ -9,13 +9,19 @@ readonly PROTECTION_RULE_PRIORITY=5000
 declare -a routes=()
 declare route family
 
-if bashio::config.false "userspace_networking" && \
-  (! bashio::config.has_value "accept_routes" || bashio::config.true "accept_routes")
-then
-  readarray -t routes < <( \
-    { ip -4 rule list; ip -6 rule list; } \
-    | { grep -E "^${PROTECTION_RULE_PRIORITY}:" || true ;} \
-    | sed -nr 's/^\d+:\s+from all to ([^\s]+) lookup main$/\1/p')
+if ! [[ "${1-}" =~ ^(|test)$ ]]; then
+  echo "Usage: $(basename "$0") [test]" 1>&2
+  exit 1
+fi
+
+readarray -t routes < <( \
+  { ip -4 rule list; ip -6 rule list; } \
+  | { grep -E "^${PROTECTION_RULE_PRIORITY}:" || true ;} \
+  | sed -nr 's/^\d+:\s+from all to ([^\s]+) lookup main$/\1/p')
+
+if [[ "${1-}" == "test" ]]; then
+  printf "%s" "${routes[@]/%/$'\n'}"
+else
   for route in "${routes[@]}"; do
     bashio::log.info "Removing route ${route} from ip rules"
     if [[ "${route}" =~ .*:.* ]]; then