From 97b08eec0c594a2ff953c32fd5cac55f93fd2be7 Mon Sep 17 00:00:00 2001
From: Franco Fichtner <franco@opnsense.org>
Date: Thu, 7 Sep 2023 09:08:23 +0200
Subject: [PATCH] system: kill the unused "alert" "interval"

The terminology is confusing -- it's just a poll interval in
dpinger's alert thread like we have in gateway watcher (also
defaulting to 1 second).  Since we do not want to expose
such values to the GUI without a pressing reason we better
do it this way than add more knobs that make support harder
and expectations less clear (reaction time suffers quickly when
increasing this value).

While here disable the alert thread completely in dpinger which
seems to be steered by passing the loss and delay.  Also adjust
the default for the loss interval which is calculated by dpinger
as 4 times the probe interval, so the default was wrong also.
---
 src/etc/inc/plugins.inc.d/dpinger.inc |  6 +-----
 src/www/system_gateways_edit.php      | 22 +++-------------------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/src/etc/inc/plugins.inc.d/dpinger.inc b/src/etc/inc/plugins.inc.d/dpinger.inc
index a8f90a017..1800b2cf3 100644
--- a/src/etc/inc/plugins.inc.d/dpinger.inc
+++ b/src/etc/inc/plugins.inc.d/dpinger.inc
@@ -69,12 +69,11 @@ function dpinger_services()
 function dpinger_defaults()
 {
     return [
-        'alert_interval' => '1',
         'data_length' => '0',
         'interval' => '1',
         'latencyhigh' => '500',
         'latencylow' => '200',
-        'loss_interval' => '2',
+        'loss_interval' => '4',
         'losshigh' => '20',
         'losslow' => '10',
         'time_period' => '60',
@@ -286,9 +285,6 @@ function dpinger_configure_do($verbose = false, $gwname = null, $bootup = false)
             'interval' => '-s %ss ',
             'loss_interval' => '-l %ss ',
             'time_period' => '-t %ss ',
-            'alert_interval' => '-A %ss ',
-            'latencyhigh' => '-D %s ',
-            'losshigh' => '-L %s ',
             'data_length' => '-d %s '
             ] as $pname => $ppattern
         ) {
diff --git a/src/www/system_gateways_edit.php b/src/www/system_gateways_edit.php
index 05fe10685..daadd3ae3 100644
--- a/src/www/system_gateways_edit.php
+++ b/src/www/system_gateways_edit.php
@@ -249,12 +249,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
         $input_errors[] = gettext("The probe interval needs to be positive.");
     }
 
-    if (!is_numeric($dpinger_config['alert_interval'])) {
-        $input_errors[] = gettext("The alert interval needs to be a numeric value.");
-    } elseif ($dpinger_config['alert_interval'] < 1) {
-        $input_errors[] = gettext("The alert interval needs to be positive.");
-    }
-
     if (!is_numeric($dpinger_config['data_length'])) {
         $input_errors[] = gettext("The data length needs to be a numeric value.");
     } elseif ($dpinger_config['data_length'] < 0) {
@@ -324,7 +318,7 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
 
         $gateway['defaultgw'] = ($pconfig['defaultgw'] == "yes" || $pconfig['defaultgw'] == "on");
 
-        foreach (array('alert_interval', 'latencylow', 'latencyhigh', 'loss_interval', 'losslow', 'losshigh', 'time_period', 'data_length') as $fieldname) {
+        foreach (['latencylow', 'latencyhigh', 'loss_interval', 'losslow', 'losshigh', 'time_period', 'data_length'] as $fieldname) {
             if (!empty($pconfig[$fieldname])) {
                 $gateway[$fieldname] = $pconfig[$fieldname];
             }
@@ -404,7 +398,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST') {
         'monitor_noroute',
         'name',
         'weight',
-        'alert_interval',
         'data_length',
         'time_period',
         'loss_interval',
@@ -451,7 +444,7 @@ $( document ).ready(function() {
 
     // (un)hide advanced on form load when any advanced setting is provided
 <?php
-  if ((!empty($pconfig['latencylow']) || !empty($pconfig['latencyhigh']) || !empty($pconfig['data_length']) || !empty($pconfig['losslow']) || !empty($pconfig['losshigh']) || (isset($pconfig['weight']) && $pconfig['weight'] > 1) || (!empty($pconfig['interval']) && ($pconfig['interval'] > $dpinger_default['interval'])) || (!empty($pconfig['alert_interval']) && ($pconfig['alert_interval'] > $dpinger_default['alert_interval'])) || (!empty($pconfig['time_period']) && ($pconfig['time_period'] > $dpinger_default['time_period'])) || (!empty($pconfig['loss_interval']) && ($pconfig['loss_interval'] > $dpinger_default['loss_interval'])))): ?>
+  if ((!empty($pconfig['latencylow']) || !empty($pconfig['latencyhigh']) || !empty($pconfig['data_length']) || !empty($pconfig['losslow']) || !empty($pconfig['losshigh']) || (isset($pconfig['weight']) && $pconfig['weight'] > 1) || (!empty($pconfig['interval']) && ($pconfig['interval'] > $dpinger_default['interval'])) || (!empty($pconfig['time_period']) && ($pconfig['time_period'] > $dpinger_default['time_period'])) || (!empty($pconfig['loss_interval']) && ($pconfig['loss_interval'] > $dpinger_default['loss_interval'])))): ?>
     $("#btn_advanced").click();
 <?php
   endif;?>
@@ -701,15 +694,6 @@ $( document ).ready(function() {
                       <?= sprintf(gettext('How often that an ICMP probe will be sent in seconds. Default is %d.'), $dpinger_default['interval']) ?>
                     </div>
                   </td>
-                </tr>
-                 <tr class="advanced hidden">
-                  <td><a id="help_for_alert_interval" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Alert Interval");?></td>
-                  <td>
-                    <input name="alert_interval" id="alert_interval" type="text" value="<?=$pconfig['alert_interval'];?>" />
-                    <div class="hidden" data-for="help_for_alert_interval">
-                      <?= sprintf(gettext('Time interval between alerts. Default is %d.'), $dpinger_default['alert_interval']) ?>
-                    </div>
-                  </td>
                 </tr>
                 <tr class="advanced hidden">
                   <td><a id="help_for_time_period" href="#" class="showhelp"><i class="fa fa-info-circle"></i></a> <?=gettext("Time Period");?></td>
@@ -725,7 +709,7 @@ $( document ).ready(function() {
                   <td>
                     <input name="loss_interval" id="loss_interval" type="text" value="<?=$pconfig['loss_interval'];?>" />
                     <div class="hidden" data-for="help_for_loss_interval">
-                      <?= sprintf(gettext('Time interval before packets are treated as lost. Default is %d.'), $dpinger_default['loss_interval']) ?>
+                      <?= sprintf(gettext('Time interval before packets are treated as lost. Default is %d, calculated as four times the probe interval).'), $dpinger_default['loss_interval']) ?>
                     </div>
                   </td>
                 </tr>