CLI for Configuring PFC Historical Statistics (sonic-net#3779)

* Add pfc stat history options to PFCWD CLI
* Unit tests for PFCWD pfc history cli
Includes modifications made to pass pre-commit (fixing import *).
* Add pfc-stat-history command and flag for PFCWD to docs

Author: peterbailey-arista

config/main.py

@@ -3125,10 +3125,11 @@ def pfcwd():
 @pfcwd.command()
 @click.option('--action', '-a', type=click.Choice(['drop', 'forward', 'alert']))
 @click.option('--restoration-time', '-r', type=click.IntRange(100, 60000))
+@click.option('--pfc-stat-history', is_flag=True, help="Enable historical statistics tracking")
 @click.option('--verbose', is_flag=True, help="Enable verbose output")
 @click.argument('ports', nargs=-1)
 @click.argument('detection-time', type=click.IntRange(100, 5000))
-def start(action, restoration_time, ports, detection_time, verbose):
+def start(action, restoration_time, pfc_stat_history, ports, detection_time, verbose):
     """
     Start PFC watchdog on port(s). To config all ports, use all as input.
 
@@ -3150,6 +3151,9 @@ def start(action, restoration_time, ports, detection_time, verbose):
     if restoration_time:
         cmd += ['--restoration-time', str(restoration_time)]
 
+    if pfc_stat_history:
+        cmd += ['--pfc-stat-history']
+
     clicommon.run_command(cmd, display_cmd=verbose)
 
 @pfcwd.command()
@@ -3191,6 +3195,21 @@ def big_red_switch(big_red_switch, verbose):
 
     clicommon.run_command(cmd, display_cmd=verbose)
 
+
+@pfcwd.command('pfc_stat_history')
+@click.option('--verbose', is_flag=True, help="Enable verbose output")
+@click.argument('pfc_stat_history', type=click.Choice(['enable', 'disable']))
+@click.argument('ports', nargs=-1)
+def pfc_stat_history(ports, pfc_stat_history, verbose):
+    """ Enable/disable PFC Historical Statistics mode on ports"""
+
+    cmd = ['pfcwd', 'pfc_stat_history', pfc_stat_history]
+    ports = set(ports) - set(['ports'])
+    cmd += list(ports)
+
+    clicommon.run_command(cmd, display_cmd=verbose)
+
+
 @pfcwd.command('start_default')
 @click.option('--verbose', is_flag=True, help="Enable verbose output")
 def start_default(verbose):

doc/Command-Reference.md

@@ -8663,6 +8663,7 @@ This command starts PFC Watchdog
   ```
   config pfcwd start --action drop all 400 --restoration-time 400
   config pfcwd start --action forward Ethernet0 Ethernet8 400
+  config pfcwd start --action drop all 400 --restoration-time 400 --pfc-stat-history
   ```
 
 **config pfcwd stop**
@@ -8701,6 +8702,18 @@ This command enables or disables PFCWD's "BIG RED SWITCH"(BRS). After enabling B
   config pfcwd big_red_switch enable
   ```
 
+**config pfcwd pfc_stat_history \<enable/disable\> \<ports>**
+
+This command enables or disables PFCWD's PFC Historical Statistics estimation. After enabling, PFC Watchdog will be configured to estimate pause transitions, total pause time, and the pause time and timstamp of the most recent pause activity on those ports.
+
+NOTE: The estimation will only be performed on ports the PFCWD has been started on, alternatively use the --pfc-stat-history flag with the `start` command to simultaneously enable history on those ports.
+
+- Usage:
+  ```
+  config pfcwd pfc_stat_history enable all
+  config pfcwd pfc_stat_history disable Ethernet0 Ethernet8
+  ```
+
 **config pfcwd start_default**
 
 This command starts PFC Watchdog with the default settings.
@@ -8716,6 +8729,7 @@ Default values are the following:
    - restoration time - 200ms
    - polling interval - 200ms
    - action - 'drop'
+   - pfc stat history - disable
 
 Additionally if number of ports in the system exceeds 32, all times will be multiplied by roughly <num_ports\>/32.
 

pfcwd/main.py

@@ -38,6 +38,7 @@
 DEFAULT_POLL_INTERVAL = 200
 DEFAULT_PORT_NUM = 32
 DEFAULT_ACTION = 'drop'
+DEFAULT_PFC_HISTORY_STATUS = "disable"
 
 STATS_DESCRIPTION = [
     ('STORM DETECTED/RESTORED', 'PFC_WD_QUEUE_STATS_DEADLOCK_DETECTED', 'PFC_WD_QUEUE_STATS_DEADLOCK_RESTORED'),
@@ -50,7 +51,8 @@
 CONFIG_DESCRIPTION = [
     ('ACTION',           'action',           'drop'),
     ('DETECTION TIME',   'detection_time',   'N/A'),
-    ('RESTORATION TIME', 'restoration_time', 'infinite')
+    ('RESTORATION TIME', 'restoration_time', 'infinite'),
+    ('HISTORY',          'pfc_stat_history', 'disable')
 ]
 
 STATS_HEADER = ('QUEUE', 'STATUS',) + list(zip(*STATS_DESCRIPTION))[0]
@@ -248,34 +250,47 @@ def config(self, ports):
             tablefmt='simple'
         ))
 
-    def start(self, action, restoration_time, ports, detection_time):
+    def start(self, action, restoration_time, ports, detection_time, pfc_stat_history):
         invalid_ports = self.get_invalid_ports(ports)
         if len(invalid_ports):
             click.echo("Failed to run command, invalid options:")
             for opt in invalid_ports:
                 click.echo(opt)
             sys.exit(1)
-        self.start_cmd(action, restoration_time, ports, detection_time)
+        self.start_cmd(action, restoration_time, ports, detection_time, pfc_stat_history)
 
+    def pfc_stat_history(self, pfc_stat_history, ports):
+        invalid_ports = self.get_invalid_ports(ports)
+        if len(invalid_ports):
+            click.echo("Failed to run command, invalid options:")
+            for opt in invalid_ports:
+                click.echo(opt)
+            sys.exit(1)
+        self.pfc_stat_history_cmd(pfc_stat_history, ports)
 
-    def verify_pfc_enable_status_per_port(self, port, pfcwd_info):
+    def verify_pfc_enable_status_per_port(self, port, pfcwd_info, overwrite=True):
         pfc_status = self.config_db.get_entry(PORT_QOS_MAP, port).get('pfc_enable')
         if pfc_status is None:
             log.log_warning("SKIPPED: PFC is not enabled on port: {}".format(port), also_print_to_console=True)
             return
 
-        self.config_db.mod_entry(
-            CONFIG_DB_PFC_WD_TABLE_NAME, port, None
-        )
-        self.config_db.mod_entry(
-            CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
-        )
+        if overwrite:
+            # don't clear existing pfc history setting unless set explicitely
+            cur_pfc_history = self.config_db.get_entry(
+                CONFIG_DB_PFC_WD_TABLE_NAME, port
+            ).get("pfc_stat_history", DEFAULT_PFC_HISTORY_STATUS)
 
-    @multi_asic_util.run_on_multi_asic
-    def start_cmd(self, action, restoration_time, ports, detection_time):
-        if os.geteuid() != 0:
-            sys.exit("Root privileges are required for this operation")
+            pfcwd_info.setdefault("pfc_stat_history", cur_pfc_history)
 
+            self.config_db.set_entry(
+                CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
+            )
+        else:
+            self.config_db.mod_entry(
+                CONFIG_DB_PFC_WD_TABLE_NAME, port, pfcwd_info
+            )
+
+    def configure_ports(self, ports, pfcwd_info, overwrite=True):
         all_ports = get_all_ports(
             self.db, self.multi_asic.current_namespace,
             self.multi_asic.display_option
@@ -284,6 +299,20 @@ def start_cmd(self, action, restoration_time, ports, detection_time):
         if len(ports) == 0:
             ports = all_ports
 
+        for port in ports:
+            if port == "all":
+                for p in all_ports:
+                    self.verify_pfc_enable_status_per_port(p, pfcwd_info, overwrite)
+            else:
+                if port not in all_ports:
+                    continue
+                self.verify_pfc_enable_status_per_port(port, pfcwd_info, overwrite)
+
+    @multi_asic_util.run_on_multi_asic
+    def start_cmd(self, action, restoration_time, ports, detection_time, pfc_stat_history):
+        if os.geteuid() != 0:
+            sys.exit("Root privileges are required for this operation")
+
         pfcwd_info = {
             'detection_time': detection_time,
         }
@@ -297,15 +326,10 @@ def start_cmd(self, action, restoration_time, ports, detection_time):
                 "restoration time not defined; default to 2 times "
                 "detection time: {} ms".format(2 * detection_time)
             )
+        if pfc_stat_history:
+            pfcwd_info["pfc_stat_history"] = "enable"
 
-        for port in ports:
-            if port == "all":
-                for p in all_ports:
-                    self.verify_pfc_enable_status_per_port(p, pfcwd_info)
-            else:
-                if port not in all_ports:
-                    continue
-                self.verify_pfc_enable_status_per_port(port, pfcwd_info)
+        self.configure_ports(ports, pfcwd_info, overwrite=True)
 
     @multi_asic_util.run_on_multi_asic
     def interval(self, poll_interval):
@@ -391,11 +415,12 @@ def start_default(self):
         pfcwd_info = {
             'detection_time': DEFAULT_DETECTION_TIME * multiply,
             'restoration_time': DEFAULT_RESTORATION_TIME * multiply,
-            'action': DEFAULT_ACTION
+            'action': DEFAULT_ACTION,
+            'pfc_stat_history': DEFAULT_PFC_HISTORY_STATUS
         }
 
         for port in active_ports:
-            self.verify_pfc_enable_status_per_port(port, pfcwd_info)
+            self.verify_pfc_enable_status_per_port(port, pfcwd_info, overwrite=True)
 
         pfcwd_info = {}
         pfcwd_info['POLL_INTERVAL'] = DEFAULT_POLL_INTERVAL * multiply
@@ -423,6 +448,15 @@ def big_red_switch(self, big_red_switch):
             pfcwd_info
         )
 
+    @multi_asic_util.run_on_multi_asic
+    def pfc_stat_history_cmd(self, pfc_stat_history, ports):
+        if os.geteuid() != 0:
+            sys.exit("Root privileges are required for this operation")
+
+        pfcwd_info = {
+            'pfc_stat_history': pfc_stat_history
+        }
+        self.configure_ports(ports, pfcwd_info, overwrite=False)
 
 # Show stats
 class Show(object):
@@ -459,20 +493,21 @@ class Start(object):
         '--action', '-a', type=click.Choice(['drop', 'forward', 'alert'])
     )
     @click.option('--restoration-time', '-r', type=click.IntRange(100, 60000))
+    @click.option('--pfc-stat-history', is_flag=True)
     @click.argument('ports', nargs=-1)
     @click.argument('detection-time', type=click.IntRange(100, 5000))
     @clicommon.pass_db
-    def start(db, action, restoration_time, ports, detection_time):
+    def start(db, action, restoration_time, ports, detection_time, pfc_stat_history):
         """
         Start PFC watchdog on port(s). To config all ports, use all as input.
 
         Example:
 
-        sudo pfcwd start --action drop all 400 --restoration-time 400
+        sudo pfcwd start --action drop all 400 --restoration-time 400 --pfc-stat-history enable
 
         """
         PfcwdCli(db).start(
-            action, restoration_time, ports, detection_time
+            action, restoration_time, ports, detection_time, pfc_stat_history
         )
 
 
@@ -525,7 +560,19 @@ def big_red_switch(db, big_red_switch):
         PfcwdCli(db).big_red_switch(big_red_switch)
 
 
+# Enable/disable PFC WD PFC_STAT_HISTORY mode
+class PfcStatHistory(object):
+    @cli.command('pfc_stat_history')
+    @click.argument('pfc_stat_history', type=click.Choice(['enable', 'disable']))
+    @click.argument('ports', nargs=-1)
+    @clicommon.pass_db
+    def pfc_stat_history(db, pfc_stat_history, ports):
+        """ Enable/disable PFC Historical Statistics mode on ports"""
+        PfcwdCli(db).pfc_stat_history(pfc_stat_history, ports)
+
+
 def get_pfcwd_clis():
+    cli.add_command(PfcStatHistory().pfc_stat_history)
     cli.add_command(BigRedSwitch().big_red_switch)
     cli.add_command(CounterPoll().counter_poll)
     cli.add_command(StartDefault().start_default)

tests/mock_tables/asic0/config_db.json

@@ -99,22 +99,26 @@
     "PFC_WD|Ethernet0": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet4": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet-BP0": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet-BP4": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|GLOBAL": {
         "BIG_RED_SWITCH": "enable",

tests/mock_tables/asic1/config_db.json

@@ -66,12 +66,14 @@
     "PFC_WD|Ethernet-BP256": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet-BP260": {
         "action": "drop",
         "detection_time": "200",
-        "restoration_time": "200"
+        "restoration_time": "200",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|GLOBAL": {
         "BIG_RED_SWITCH": "enable",

tests/mock_tables/config_db.json

@@ -1835,17 +1835,20 @@
     "PFC_WD|Ethernet0": {
         "action": "drop",
         "detection_time": "600",
-        "restoration_time": "600"
+        "restoration_time": "600",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet4": {
         "action": "drop",
         "detection_time": "600",
-        "restoration_time": "600"
+        "restoration_time": "600",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|Ethernet8": {
         "action": "drop",
         "detection_time": "600",
-        "restoration_time": "600"
+        "restoration_time": "600",
+        "pfc_stat_history": "disable"
     },
     "PFC_WD|GLOBAL": {
         "POLL_INTERVAL": "600"