interfaces: stop last internal use of /var/run/booting #5637

At last, we seem to be free...

To be precise here move staticarp configure before reload
block in interface_configure() to avoid passing a stale
ifconfig cache as that would trigger a transition twice.
Pass ifconfig cache from where it is available or read it
on the fly (e.g. rc.linkup).

With that cache we can figure out if a transition is required
and so can avoid most of the boot stalling except when staticarp
is enabled on a lot of interfaces, but that was always slow(er)
later on.  It should even be faster now avoiding the ifconfig
in the common case.

There is a side effect that dhcp wants to populate the ARP
table and that is still unconditional because we do not know
whether we have new entries added or others removed.  Having
them removed might leave them in the ARP table for longer
than necessary, however.

It's not that the current implementation is particularly bad,
but it relies heavily on implied regular flushing of ARP entries
just to keep a consistent functionality which is a big design
flaw.  As a stopgap measure remove an ARP entry when we delete
the static mapping for it to keep the entries in sync.

/var/run/booting remains in backend scripts that should not
interfere with boot but we will clean these up later as they
do not need removal but rather a transition to a safer way
than checking for a file (that might not get deleted for
one reason or another.. it has been known to happen).

src/etc/inc/interfaces.inc

@@ -2363,6 +2363,9 @@ function interface_configure($verbose = false, $interface = 'wan', $reload = fal
         /* XXX bridges can only load if all interfaces are there */
     }
 
+    /* XXX may be called twice since also tied to dhcp configure */
+    interfaces_staticarp_configure($interface, $ifconfig_details);
+
     if ($verbose) {
         echo "done.\n";
     }
@@ -2377,8 +2380,6 @@ function interface_configure($verbose = false, $interface = 'wan', $reload = fal
         configdp_run('rfc2136 reload', array($interface));
     }
 
-    interfaces_staticarp_configure($interface);
-
     return $loaded;
 }
 
@@ -3807,7 +3808,7 @@ function get_vip_descr($ipaddress)
     return '';
 }
 
-function interfaces_staticarp_configure($if)
+function interfaces_staticarp_configure($if, $ifconfig_details = null)
 {
     global $config;
 
@@ -3816,22 +3817,31 @@ function interfaces_staticarp_configure($if)
         return;
     }
 
-    mwexecf('/sbin/ifconfig %s %sstaticarp', [$ifcfg['if'], isset($config['dhcpd'][$if]['staticarp']) ? '' : '-']);
+    if (empty($ifconfig_details)) {
+        $ifconfig_details = legacy_interfaces_details($ifcfg['if']);
+    }
 
-    if (!file_exists('/var/run/booting')) {
-        // XXX: arp is really slow when there are many interfaces, while booting -d -a shouldn't be needed anyway
+    if (empty($ifconfig_details[$ifcfg['if']])) {
+        return;
+    }
+
+    $have = in_array('staticarp', $ifconfig_details[$ifcfg['if']]['flags']);
+    $want = isset($config['dhcpd'][$if]['staticarp']);
+
+    if ($have !== $want) {
+        mwexecf('/sbin/ifconfig %s %sstaticarp', [$ifcfg['if'], $want ? '' : '-']);
         mwexecf('/usr/sbin/arp -d -i %s -a', [$ifcfg['if']]);
     }
 
     if (isset($config['dhcpd'][$if]['staticmap'])) {
         foreach ($config['dhcpd'][$if]['staticmap'] as $arpent) {
-            if (!isset($config['dhcpd'][$if]['staticarp']) && !isset($arpent['arp_table_static_entry'])) {
+            if (!$want && !isset($arpent['arp_table_static_entry'])) {
                 continue;
             }
             if (!isset($arpent['ipaddr'])) {
                 continue;
             }
-            mwexecf( '/usr/sbin/arp -s %s %s', [$arpent['ipaddr'], $arpent['mac']]);
+            mwexecf('/usr/sbin/arp -s %s %s', [$arpent['ipaddr'], $arpent['mac']]);
         }
     }
 }

src/etc/inc/plugins.inc.d/dhcpd.inc

@@ -633,7 +633,8 @@ EOD;
      * to setup failover peer "bleh" entries
      */
     foreach ($config['dhcpd'] as $dhcpif => $dhcpifconf) {
-        interfaces_staticarp_configure($dhcpif);
+        /* certainly odd but documented and side effect populates ARP table */
+        interfaces_staticarp_configure($dhcpif, $ifconfig_details);
 
         if (!isset($dhcpifconf['enable'])) {
             continue;

src/opnsense/service/conf/actions.d/actions_interface.conf

@@ -47,6 +47,12 @@ parameters: %s
 type:script_output
 message:request arp table
 
+[remove.arp]
+command:/usr/sbin/arp -d
+parameters:%s 2> /dev/null; exit 0
+type:script
+message:remove arp entry for %s
+
 [flush.arp]
 command:arp -da
 parameters:

src/www/services_dhcp.php

@@ -435,6 +435,11 @@ if ($_SERVER['REQUEST_METHOD'] === 'GET') {
         exit;
     } elseif ($act ==  "del") {
         if (!empty($config['dhcpd'][$if]['staticmap'][$_POST['id']])) {
+            if (isset($config['dhcpd'][$if]['staticmap'][$_POST['id']]['ipaddr'])) {
+                configdp_run('interface remove arp', [
+                    $config['dhcpd'][$if]['staticmap'][$_POST['id']]['ipaddr']
+                ]);
+            }
             unset($config['dhcpd'][$if]['staticmap'][$_POST['id']]);
             write_config();
             if (isset($config['dhcpd'][$if]['enable'])) {