From 66727f62bde50620f4a9ac1ba8943df1de775831 Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Tue, 18 Apr 2023 17:38:20 -0700 Subject: [PATCH 1/6] Add a test case to verify that BGP packets use queue7 * All control packets including BGP are expected to use Q7, separate from data packet queues Signed-off-by: Prabhat Aravind --- tests/bgp/test_bgp_queue.py | 73 +++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 tests/bgp/test_bgp_queue.py diff --git a/tests/bgp/test_bgp_queue.py b/tests/bgp/test_bgp_queue.py new file mode 100644 index 00000000000..f3b8a03fe73 --- /dev/null +++ b/tests/bgp/test_bgp_queue.py @@ -0,0 +1,73 @@ +import time +import pytest +import logging + +logger = logging.getLogger(__name__) + +pytestmark = [ + pytest.mark.topology('any'), + pytest.mark.device_type('vs') +] + + +def clear_queue_counters(duthost): + duthost.shell("sonic-clear queuecounters") + + +def get_queue_counters(duthost, port, queue): + """ + Return the counter for a given queue in given port + """ + cmd = "show queue counters {}".format(port) + output = duthost.shell(cmd)['stdout_lines'] + txq = "UC{}".format(queue) + for line in output: + fields = line.split() + if fields[1] == txq: + return int(fields[2]) + return 0 + + +def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinfo): + duthost = duthosts[enum_frontend_dut_hostname] + clear_queue_counters(duthost) + time.sleep(10) + bgp_facts = duthost.bgp_facts(instance_id=enum_asic_index)['ansible_facts'] + mg_facts = duthost.get_extended_minigraph_facts(tbinfo) + + arp_dict = {} + ndp_dict = {} + show_arp = duthost.command('show arp') + show_ndp = duthost.command('show ndp') + for arp_entry in show_arp['stdout_lines']: + items = arp_entry.split() + if (len(items) != 4): + continue + ip = items[0] + iface = items[2] + arp_dict[ip] = iface + for ndp_entry in show_ndp['stdout_lines']: + items = ndp_entry.split() + if (len(items) != 5): + continue + ip = items[0] + iface = items[2] + ndp_dict[ip] = iface + + for k, v in list(bgp_facts['bgp_neighbors'].items()): + # Only consider established bgp sessions + if v['state'] == 'established': + assert (k in arp_dict.keys() or k in ndp_dict.keys()) + if k in arp_dict: + ifname = arp_dict[k] + else: + ifname = ndp_dict[k] + if (ifname.startswith("PortChannel")): + for port in mg_facts['minigraph_portchannels'][ifname]['members']: + logger.info("PortChannel '{}' : port {}".format(ifname, port)) + assert(get_queue_counters(duthost, port, "0") == 0) + assert(get_queue_counters(duthost, port, "7")) + else: + logger.info(ifname) + assert(get_queue_counters(duthost, ifname, "0") == 0) + assert(get_queue_counters(duthost, ifname, "7")) From cf85ef452fc9f7e97a755aad1b922964fff6f296 Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Fri, 5 May 2023 15:30:24 -0700 Subject: [PATCH 2/6] [tests/bgp]: Fix to ensure that script works with vlan interfaces Signed-off-by: Prabhat Aravind --- tests/bgp/test_bgp_queue.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bgp/test_bgp_queue.py b/tests/bgp/test_bgp_queue.py index f3b8a03fe73..3c39d771189 100644 --- a/tests/bgp/test_bgp_queue.py +++ b/tests/bgp/test_bgp_queue.py @@ -59,9 +59,9 @@ def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinf if v['state'] == 'established': assert (k in arp_dict.keys() or k in ndp_dict.keys()) if k in arp_dict: - ifname = arp_dict[k] + ifname = arp_dict[k].split('.',1)[0] else: - ifname = ndp_dict[k] + ifname = ndp_dict[k].split('.',1)[0] if (ifname.startswith("PortChannel")): for port in mg_facts['minigraph_portchannels'][ifname]['members']: logger.info("PortChannel '{}' : port {}".format(ifname, port)) From 90324ff863cb4e91e74fbe060e24d590017f8c25 Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Fri, 5 May 2023 15:35:33 -0700 Subject: [PATCH 3/6] [tests/bgp]: Address pre-commit whitespace issue Signed-off-by: Prabhat Aravind --- tests/bgp/test_bgp_queue.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bgp/test_bgp_queue.py b/tests/bgp/test_bgp_queue.py index 3c39d771189..bf670fde7d7 100644 --- a/tests/bgp/test_bgp_queue.py +++ b/tests/bgp/test_bgp_queue.py @@ -59,9 +59,9 @@ def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinf if v['state'] == 'established': assert (k in arp_dict.keys() or k in ndp_dict.keys()) if k in arp_dict: - ifname = arp_dict[k].split('.',1)[0] + ifname = arp_dict[k].split('.', 1)[0] else: - ifname = ndp_dict[k].split('.',1)[0] + ifname = ndp_dict[k].split('.', 1)[0] if (ifname.startswith("PortChannel")): for port in mg_facts['minigraph_portchannels'][ifname]['members']: logger.info("PortChannel '{}' : port {}".format(ifname, port)) From 717d1e4f663e8bd706fd9bf11737525e6dbf02ff Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Mon, 12 Jun 2023 18:02:22 +0000 Subject: [PATCH 4/6] [docs/testplan]: Add testplan for BGP packet egress queue verification Signed-off-by: Prabhat Aravind --- docs/testplan/BGP-queue-test-plan.md | 31 ++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 docs/testplan/BGP-queue-test-plan.md diff --git a/docs/testplan/BGP-queue-test-plan.md b/docs/testplan/BGP-queue-test-plan.md new file mode 100644 index 00000000000..4895c2dd620 --- /dev/null +++ b/docs/testplan/BGP-queue-test-plan.md @@ -0,0 +1,31 @@ +# BGP-queue test plan + +* [Overview](#Overview) + * [Scope](#Scope) + * [Testbed](#Testbed) +* [Setup configuration](#Setup%20configuration) +* [Test cases](#Test%20cases) + +## Overview +The purpose is to make sure that BGP control packets use unicast queue 7 by default on all SONiC platforms. +The test expects that basic BGP configuration for the test is pre-configured on SONiC device before test run. + +### Scope +The test is targeting a running SONiC system with fully functioning configuration. The purpose of the test is to verify BGP control packets use egress queue 7 on SONiC platforms. + +### Testbed +The test could run on any testbed. + +## Setup configuration +This test requires BGP neighbors to be configured and established before the test run. + +## Test +The test will verify that all BGP packets use unicast queue 7 by default. It is to be noted that if BGP sessions are established over PortChannels, LACP packets will also use the same unicast queue 7, but that does not impact the test functionality. + +## Test cases +### Test case test_bgp_queue +#### Test steps +* Clear all queue counters using "sonic-clear counters" command +* Generate a mapping of neighbors to the corresponding interfaces/ports using ARP/NDP entries +* For all "established" BGP sessions, run "show queue counters" on the corresponding port +* Verify that unicast queue 7 counters are non-zero and that unicast queue 0 counters are zero From f8c59caf27bb477c62d36fbccede7fbf138f0ff9 Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Wed, 2 Aug 2023 18:31:39 +0000 Subject: [PATCH 5/6] [tests/bgp]: Miscellaneous fixes * Check all queues for the test instead of just q0 * Remove check for q7 counters until regression due to https://github.com/sonic-net/sonic-swss/pull/2143 is fixed * Perform interface queue counter checks only once when there are v4 and v6 neighbors over the same interface Signed-off-by: Prabhat Aravind --- tests/bgp/test_bgp_queue.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/bgp/test_bgp_queue.py b/tests/bgp/test_bgp_queue.py index bf670fde7d7..6192cb46076 100644 --- a/tests/bgp/test_bgp_queue.py +++ b/tests/bgp/test_bgp_queue.py @@ -37,6 +37,7 @@ def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinf arp_dict = {} ndp_dict = {} + processed_intfs = set() show_arp = duthost.command('show arp') show_ndp = duthost.command('show ndp') for arp_entry in show_arp['stdout_lines']: @@ -62,12 +63,15 @@ def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinf ifname = arp_dict[k].split('.', 1)[0] else: ifname = ndp_dict[k].split('.', 1)[0] + if ifname in processed_intfs: + continue if (ifname.startswith("PortChannel")): for port in mg_facts['minigraph_portchannels'][ifname]['members']: logger.info("PortChannel '{}' : port {}".format(ifname, port)) - assert(get_queue_counters(duthost, port, "0") == 0) - assert(get_queue_counters(duthost, port, "7")) + for q in range(0, 7): + assert(get_queue_counters(duthost, port, q) == 0) else: logger.info(ifname) - assert(get_queue_counters(duthost, ifname, "0") == 0) - assert(get_queue_counters(duthost, ifname, "7")) + for q in range(0, 7): + assert(get_queue_counters(duthost, ifname, q) == 0) + processed_intfs.add(ifname) From 8fbd2e5c759c7c28c712d09da4f2699a943ff4cd Mon Sep 17 00:00:00 2001 From: Prabhat Aravind Date: Mon, 7 Aug 2023 18:53:41 +0000 Subject: [PATCH 6/6] [tests/bgp]: return -1 in case no queue is found Signed-off-by: Prabhat Aravind --- tests/bgp/test_bgp_queue.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bgp/test_bgp_queue.py b/tests/bgp/test_bgp_queue.py index 6192cb46076..2e83a074c19 100644 --- a/tests/bgp/test_bgp_queue.py +++ b/tests/bgp/test_bgp_queue.py @@ -25,7 +25,7 @@ def get_queue_counters(duthost, port, queue): fields = line.split() if fields[1] == txq: return int(fields[2]) - return 0 + return -1 def test_bgp_queues(duthosts, enum_frontend_dut_hostname, enum_asic_index, tbinfo):