From 9e465282a109a81c29e8d120a3c282a3a4a868ed Mon Sep 17 00:00:00 2001 From: Mykola Faryma Date: Thu, 9 Jul 2020 14:14:37 +0300 Subject: [PATCH 1/5] [counterpoll] add port buffer drop group Signed-off-by: Mykola Faryma --- counterpoll/main.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/counterpoll/main.py b/counterpoll/main.py index 55f1aefeba..68ee7617f3 100644 --- a/counterpoll/main.py +++ b/counterpoll/main.py @@ -5,7 +5,9 @@ from tabulate import tabulate BUFFER_POOL_WATERMARK = "BUFFER_POOL_WATERMARK" +PORT_BUFFER_DROP = "PORT_BUFFER_DROP" DISABLE = "disable" +DEFLT_60_SEC= "default (60000)" DEFLT_10_SEC= "default (10000)" DEFLT_1_SEC = "default (1000)" @@ -81,6 +83,46 @@ def disable(): port_info['FLEX_COUNTER_STATUS'] = 'disable' configdb.mod_entry("FLEX_COUNTER_TABLE", "PORT", port_info) +# Port buffer drop counter commands +@cli.group() +def port_buffer_drop(): + """ Port buffer drop counter commands """ + +@port_buffer_drop.command() +@click.argument('poll_interval', type=click.IntRange(30000, 300000)) +def interval(poll_interval): + """ + Set port_buffer_drop counter query interval + This counter group causes high CPU usage when polled, + hence the allowed interval is between 30s and 300s. + This is a ahort term solution and + should be changed once the performance is enhanced + """ + configdb = swsssdk.ConfigDBConnector() + configdb.connect() + port_info = {} + if poll_interval is not None: + port_info['POLL_INTERVAL'] = poll_interval + configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) + +@port_buffer_drop.command() +def enable(): + """ Enable port counter query """ + configdb = swsssdk.ConfigDBConnector() + configdb.connect() + port_info = {} + port_info['FLEX_COUNTER_STATUS'] = 'enable' + configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) + +@port_buffer_drop.command() +def disable(): + """ Disable port counter query """ + configdb = swsssdk.ConfigDBConnector() + configdb.connect() + port_info = {} + port_info['FLEX_COUNTER_STATUS'] = 'disable' + configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) + # RIF counter commands @cli.group() def rif(): @@ -166,6 +208,7 @@ def show(): configdb.connect() queue_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'QUEUE') port_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'PORT') + port_drop_info = configdb.get_entry('FLEX_COUNTER_TABLE', PORT_BUFFER_DROP) rif_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'RIF') queue_wm_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'QUEUE_WATERMARK') pg_wm_info = configdb.get_entry('FLEX_COUNTER_TABLE', 'PG_WATERMARK') @@ -177,6 +220,8 @@ def show(): data.append(["QUEUE_STAT", queue_info.get("POLL_INTERVAL", DEFLT_10_SEC), queue_info.get("FLEX_COUNTER_STATUS", DISABLE)]) if port_info: data.append(["PORT_STAT", port_info.get("POLL_INTERVAL", DEFLT_1_SEC), port_info.get("FLEX_COUNTER_STATUS", DISABLE)]) + if port_drop_info: + data.append([PORT_BUFFER_DROP, port_drop_info.get("POLL_INTERVAL", DEFLT_60_SEC), port_drop_info.get("FLEX_COUNTER_STATUS", DISABLE)]) if rif_info: data.append(["RIF_STAT", rif_info.get("POLL_INTERVAL", DEFLT_1_SEC), rif_info.get("FLEX_COUNTER_STATUS", DISABLE)]) if queue_wm_info: From bf3dfffdba83826285273afbaeba9e3d784fac50 Mon Sep 17 00:00:00 2001 From: Mykola F <37578614+mykolaf@users.noreply.github.com> Date: Wed, 12 Aug 2020 19:17:45 +0300 Subject: [PATCH 2/5] Update counterpoll/main.py Co-authored-by: Danny Allen --- counterpoll/main.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/counterpoll/main.py b/counterpoll/main.py index 68ee7617f3..3d4b035e81 100644 --- a/counterpoll/main.py +++ b/counterpoll/main.py @@ -95,7 +95,7 @@ def interval(poll_interval): Set port_buffer_drop counter query interval This counter group causes high CPU usage when polled, hence the allowed interval is between 30s and 300s. - This is a ahort term solution and + This is a short term solution and should be changed once the performance is enhanced """ configdb = swsssdk.ConfigDBConnector() @@ -232,4 +232,3 @@ def show(): data.append(["BUFFER_POOL_WATERMARK_STAT", buffer_pool_wm_info.get("POLL_INTERVAL", DEFLT_10_SEC), buffer_pool_wm_info.get("FLEX_COUNTER_STATUS", DISABLE)]) print tabulate(data, headers=header, tablefmt="simple", missingval="") - From aa725b3e7f21987404f55fbcbe3df4e982dec508 Mon Sep 17 00:00:00 2001 From: Mykola F <37578614+mykolaf@users.noreply.github.com> Date: Wed, 12 Aug 2020 19:17:53 +0300 Subject: [PATCH 3/5] Update counterpoll/main.py Co-authored-by: Danny Allen --- counterpoll/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/counterpoll/main.py b/counterpoll/main.py index 3d4b035e81..c0536d68ac 100644 --- a/counterpoll/main.py +++ b/counterpoll/main.py @@ -101,7 +101,7 @@ def interval(poll_interval): configdb = swsssdk.ConfigDBConnector() configdb.connect() port_info = {} - if poll_interval is not None: + if poll_interval: port_info['POLL_INTERVAL'] = poll_interval configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) From 765eb6a64039dda311dcb39b9fdadc9684350bc0 Mon Sep 17 00:00:00 2001 From: Mykola Faryma Date: Tue, 18 Aug 2020 12:17:49 +0300 Subject: [PATCH 4/5] [UT] add UT for new flex counter group Signed-off-by: Mykola Faryma --- tests/counterpoll_test.py | 58 ++++++++++++++++++++++++++++++++ tests/mock_tables/config_db.json | 20 +++++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/counterpoll_test.py diff --git a/tests/counterpoll_test.py b/tests/counterpoll_test.py new file mode 100644 index 0000000000..cf99bb046d --- /dev/null +++ b/tests/counterpoll_test.py @@ -0,0 +1,58 @@ +import sys +import os +import time +import pytest +import click +import swsssdk +from click.testing import CliRunner + +test_path = os.path.dirname(os.path.abspath(__file__)) +modules_path = os.path.dirname(test_path) +scripts_path = os.path.join(modules_path, "scripts") +sys.path.insert(0, test_path) +sys.path.insert(0, modules_path) + +import mock_tables.dbconnector +import counterpoll.main as counterpoll + +expected_counterpoll_show = """Type Interval (in ms) Status +-------------------- ------------------ -------- +QUEUE_STAT 10000 enable +PORT_STAT 1000 enable +PORT_BUFFER_DROP 60000 enable +QUEUE_WATERMARK_STAT 10000 enable +PG_WATERMARK_STAT 10000 enable +""" + +class TestCounterpoll(object): + @classmethod + def setup_class(cls): + print("SETUP") + os.environ["PATH"] += os.pathsep + scripts_path + os.environ["UTILITIES_UNIT_TESTING"] = "1" + + def test_show(self): + runner = CliRunner() + result = runner.invoke(counterpoll.cli.commands["show"], []) + print(result.output) + assert result.output == expected_counterpoll_show + + def test_port_buffer_drop_interval(self): + runner = CliRunner() + result = runner.invoke(counterpoll.cli.commands["port-buffer-drop"].commands["interval"], ["30000"]) + print(result.output) + assert result.exit_code == 0 + + def test_port_buffer_drop_interval_to_short(self): + runner = CliRunner() + result = runner.invoke(counterpoll.cli.commands["port-buffer-drop"].commands["interval"], ["1000"]) + print(result.output) + expected = "Invalid value for 'POLL_INTERVAL': 1000 is not in the valid range of 30000 to 300000." + assert result.exit_code == 2 + assert expected in result.output + + @classmethod + def teardown_class(cls): + print("TEARDOWN") + os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) + os.environ["UTILITIES_UNIT_TESTING"] = "0" diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 51bd7fb683..9c9e5bd628 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -701,5 +701,25 @@ "lo_addr": "None", "mgmt_addr": "10.250.0.54", "type": "LeafRouter" + }, + "FLEX_COUNTER_TABLE|QUEUE": { + "POLL_INTERVAL": "10000", + "FLEX_COUNTER_STATUS": "enable" + }, + "FLEX_COUNTER_TABLE|PORT": { + "POLL_INTERVAL": "1000", + "FLEX_COUNTER_STATUS": "enable" + }, + "FLEX_COUNTER_TABLE|PORT_BUFFER_DROP": { + "POLL_INTERVAL": "60000", + "FLEX_COUNTER_STATUS": "enable" + }, + "FLEX_COUNTER_TABLE|QUEUE_WATERMARK": { + "POLL_INTERVAL": "10000", + "FLEX_COUNTER_STATUS": "enable" + }, + "FLEX_COUNTER_TABLE|PG_WATERMARK": { + "POLL_INTERVAL": "10000", + "FLEX_COUNTER_STATUS": "enable" } } From baa7bb17d9505ecc4970bb0ee7a3491a65b0a513 Mon Sep 17 00:00:00 2001 From: Volodymyr Samotiy Date: Thu, 27 Aug 2020 14:25:09 +0300 Subject: [PATCH 5/5] [counterpoll] add port buffer drop group * Fix review comments Signed-off-by: Volodymyr Samotiy --- counterpoll/main.py | 5 +++-- tests/counterpoll_test.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/counterpoll/main.py b/counterpoll/main.py index a905406583..8795ee277f 100644 --- a/counterpoll/main.py +++ b/counterpoll/main.py @@ -7,6 +7,7 @@ BUFFER_POOL_WATERMARK = "BUFFER_POOL_WATERMARK" PORT_BUFFER_DROP = "PORT_BUFFER_DROP" DISABLE = "disable" +ENABLE = "enable" DEFLT_60_SEC= "default (60000)" DEFLT_10_SEC= "default (10000)" DEFLT_1_SEC = "default (1000)" @@ -111,7 +112,7 @@ def enable(): configdb = swsssdk.ConfigDBConnector() configdb.connect() port_info = {} - port_info['FLEX_COUNTER_STATUS'] = 'enable' + port_info['FLEX_COUNTER_STATUS'] = ENABLE configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) @port_buffer_drop.command() @@ -120,7 +121,7 @@ def disable(): configdb = swsssdk.ConfigDBConnector() configdb.connect() port_info = {} - port_info['FLEX_COUNTER_STATUS'] = 'disable' + port_info['FLEX_COUNTER_STATUS'] = DISABLE configdb.mod_entry("FLEX_COUNTER_TABLE", PORT_BUFFER_DROP, port_info) # RIF counter commands diff --git a/tests/counterpoll_test.py b/tests/counterpoll_test.py index cf99bb046d..b2c806b95c 100644 --- a/tests/counterpoll_test.py +++ b/tests/counterpoll_test.py @@ -43,7 +43,7 @@ def test_port_buffer_drop_interval(self): print(result.output) assert result.exit_code == 0 - def test_port_buffer_drop_interval_to_short(self): + def test_port_buffer_drop_interval_too_short(self): runner = CliRunner() result = runner.invoke(counterpoll.cli.commands["port-buffer-drop"].commands["interval"], ["1000"]) print(result.output)