Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add route flow counter related test cases #5134

Closed
28 changes: 15 additions & 13 deletions tests/bgp/test_bgp_speaker.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from tests.common.utilities import wait_tcp_connection
from tests.common.helpers.assertions import pytest_require
from tests.common.utilities import wait_until
from tests.flow_counter.flow_counter_utils import RouteFlowCounterTestContext


pytestmark = [
Expand Down Expand Up @@ -308,19 +309,20 @@ def bgp_speaker_announce_routes_common(common_setup_teardown,

logger.info("run ptf test")

ptf_runner(ptfhost,
"ptftests",
"fib_test.FibTest",
platform_dir="ptftests",
params={"router_macs": [duthost.facts['router_mac']],
"ptf_test_port_map": PTF_TEST_PORT_MAP,
"fib_info_files": ["/root/bgp_speaker_route_%s.txt" % family],
"ipv4": ipv4,
"ipv6": ipv6,
"testbed_mtu": mtu,
"test_balancing": False},
log_file="/tmp/bgp_speaker_test.FibTest.log",
socket_recv_size=16384)
with RouteFlowCounterTestContext(duthost, [prefix], {prefix : {'packets': 3, 'bytes': 4554}}):
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
ptf_runner(ptfhost,
"ptftests",
"fib_test.FibTest",
platform_dir="ptftests",
params={"router_macs": [duthost.facts['router_mac']],
"ptf_test_port_map": PTF_TEST_PORT_MAP,
"fib_info_files": ["/root/bgp_speaker_route_%s.txt" % family],
"ipv4": ipv4,
"ipv6": ipv6,
"testbed_mtu": mtu,
"test_balancing": False},
log_file="/tmp/bgp_speaker_test.FibTest.log",
socket_recv_size=16384)

logger.info("Withdraw routes")
withdraw_route(ptfip, lo_addr, prefix, nexthop_ips[1].ip, port_num[0])
Expand Down
Empty file added tests/flow_counter/__init__.py
Empty file.
278 changes: 278 additions & 0 deletions tests/flow_counter/flow_counter_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,278 @@
import logging
import random
from tests.common.helpers.assertions import pytest_assert
from tests.common.utilities import wait_until, check_skip_release

logger = logging.getLogger(__name__)

support_route_flow_counter = None
skip_versions = ['201811', '201911', '202012', '202106', '202111']
CAPABILITY_WAIT_TIME_IN_SEC = 180
CAPABILITY_CHECK_INTERVAL_IN_SEC = 5


class RouteFlowCounterTestContext:
"""Allow caller to use "with" key words to run router flow counter test.
"""
def __init__(self, dut, route_pattern_list, expected_stats, interval=1000):
"""Init RouteFlowCounterTestContext

Args:
dut (object): DUT object
route_pattern_list (list): a list of route pattern, e.g. ['1.1.1.0/24', 'Vrf1|1.1.1.0/24', 'Vnet1|2.2.2.0/24']
expected_stats (dict): Expected result value. e.g. {'1.1.1.0/24': {'packets': '5', 'bytes': '4500'}}
interval (int, optional): Route flow counter query interval. Defaults to 1000.
"""
self.dut = dut
self.route_pattern_list = route_pattern_list
self.expected_stats = expected_stats
self.interval = interval

def __enter__(self):
"""Enable route flow counter and configure route pattern
"""
if not is_route_flow_counter_supported(self.dut):
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
return
set_route_flow_counter_interval(self.dut, self.interval)
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
set_route_flow_counter_status(self.dut, True)
for route_pattern in self.route_pattern_list:
set_route_flow_counter_pattern(self.dut, route_pattern)

def __exit__(self, exc_type, exc_val, exc_tb):
"""Do following tasks:
1. Verify route flow counter stats agaist expected value
2. Disable route flow coutern and remove route pattern

Args:
exc_type (object): not used
exc_val (object): not used
exc_tb (object): not used
"""
if not is_route_flow_counter_supported(self.dut):
return

try:
result, message = self.check_stats()
pytest_assert(result, message)
finally:
set_route_flow_counter_status(self.dut, False)
for route_pattern in self.route_pattern_list:
remove_route_flow_counter_pattern(self.dut, route_pattern)


def check_stats(self):
"""Verify route flow counter statistic

Returns:
tuple: (status, error message)
"""
logger.info('Checking route flow counter stats')
actual_stats = parse_route_flow_counter_stats(self.dut)
result, message = verify_route_flow_counter_stats(self.expected_stats, actual_stats)
if not result:
return result, message

if len(self.expected_stats) > 0:
logger.info('Checking route flow counter stats after clearing by route')
to_clear = random.sample(list(self.expected_stats.keys()), 1)[0]
clear_route_flow_counter_by_route(self.dut, to_clear)
for key in self.expected_stats[to_clear]:
self.expected_stats[to_clear][key] = '0'
actual_stats = parse_route_flow_counter_stats(self.dut)
result, message = verify_route_flow_counter_stats(self.expected_stats, actual_stats)
if not result:
return result, message

if len(self.expected_stats) == 1 and len(self.route_pattern_list) == 1:
logger.info('Checking route flow counter stats after clearing by pattern')
clear_route_flow_counter_by_pattern(self.dut, self.route_pattern_list[0])
else:
logger.info('Checking route flow counter stats after clearing all routes')
clear_route_flow_counter(self.dut)
for prefix, value in self.expected_stats.items():
for key in value:
self.expected_stats[prefix][key] = '0'

actual_stats = parse_route_flow_counter_stats(self.dut)
return verify_route_flow_counter_stats(self.expected_stats, actual_stats)


def is_route_flow_counter_supported(dut):
"""Check if route flow counter is supported on this platform

Args:
dut (object): DUT object

Returns:
bool: True if supported
"""
skip, _ = check_skip_release(dut, skip_versions)
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
if skip:
return False

global support_route_flow_counter
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
if support_route_flow_counter is None:
if not wait_until(CAPABILITY_WAIT_TIME_IN_SEC, CAPABILITY_CHECK_INTERVAL_IN_SEC, 0, get_route_flow_counter_capability, dut):
support_route_flow_counter = False
pytest_assert(False, 'Failed to get route flow counter capability')
if not support_route_flow_counter:
logger.info('Route flow counter is not supported on this platform')
return support_route_flow_counter


def get_route_flow_counter_capability(dut):
"""Get route flow counter capability from STATE DB

Args:
dut (object): DUT object

Returns:
bool: True if capability is successfully retrieved from STATE DB
"""
global support_route_flow_counter
support = dut.shell('sudo sonic-db-cli STATE_DB HGET "FLOW_COUNTER_CAPABILITY_TABLE|route" support')['stdout'].strip()
if support == 'true':
Junchao-Mellanox marked this conversation as resolved.
Show resolved Hide resolved
support_route_flow_counter = True
elif support == 'false':
support_route_flow_counter = False
return support_route_flow_counter is not None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we simply return the variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. get_route_flow_counter_capability is a callback of wait_until, the return value indicates that whether wait_until need continue running. If we return the support_route_flow_counter, there could be case that: support_route_flow_counter is False and wait_until will continue running until timeout.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about raising an exception if the value is None? wait_until should return False in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantic here is to "wait support_route_flow_counter until there is a value", so I support "return support_route_flow_counter is not None" just matches the semantic.



def set_route_flow_counter_status(dut, status):
"""Set route flow counter status

Args:
dut (object): DUT object
status (bool): Enable if True else disable
"""
dut.command('counterpoll flowcnt-route {}'.format('enable' if status else 'disable'))


def set_route_flow_counter_interval(dut, interval):
"""Set route flow counter interval

Args:
dut (object): DUT object
interval (int): Query interval value in ms
"""
dut.command('counterpoll flowcnt-route interval {}'.format(interval))


def set_route_flow_counter_pattern(dut, route_pattern, max_match_count=30):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me it seems more readable to get another parameter- VRF, rather than encapsulating it in the route pattern string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, using VRF and prefix here makes this function more readable. However, I would like to keep this based on two reasons:

  1. Most of case VRF is none(default VRF)
  2. If we use VRF and prefix, the caller has to use a tuple (, ) to represent the route pattern, it increases unneeded complexity of caller side. And a tuple is not so readable.

"""Set route pattern for route flow counter

Args:
dut (object): DUT object
route_pattern (str): Route pattern. e.g. "1.1.1.0/24", "2000::/64", "Vrf1|2.2.2.0/24"
max_match_count (int, optional): Max allowed match count. Defaults to 30.
"""
items = route_pattern.split('|')
if len(items) == 2:
dut.command('sudo config flowcnt-route pattern add {} --vrf {} --max {} -y'.format(items[1], items[0], max_match_count))
elif len(items) == 1:
dut.command('sudo config flowcnt-route pattern add {} --max {} -y'.format(items[0], max_match_count))
else:
logger.error('Invalid route pattern {}'.format(route_pattern))


def remove_route_flow_counter_pattern(dut, route_pattern):
"""Remove route pattern for route flow counter

Args:
dut (object): DUT object
route_pattern (str): Route pattern. e.g. "1.1.1.0/24", "2000::/64", "Vrf1|2.2.2.0/24"
"""
items = route_pattern.split('|')
if len(items) == 2:
dut.command('sudo config flowcnt-route pattern remove {} --vrf {}'.format(items[1], items[0]))
elif len(items) == 1:
dut.command('sudo config flowcnt-route pattern remove {}'.format(items[0]))
else:
logger.error('Invalid route pattern {}'.format(route_pattern))


def clear_route_flow_counter(dut):
"""Clear all route flow counter statistics

Args:
dut (object): DUT object
"""
dut.command('sonic-clear flowcnt-route')


def clear_route_flow_counter_by_pattern(dut, route_pattern):
"""Clear route flow counter statistics by pattern

Args:
dut (object): DUT object
route_pattern (str): Route pattern. e.g. "1.1.1.0/24", "2000::/64", "Vrf1|2.2.2.0/24"
"""
items = route_pattern.split('|')
if len(items) == 2:
dut.command('sonic-clear flowcnt-route pattern {} --vrf {}'.format(items[1], items[0]))
elif len(items) == 1:
dut.command('sonic-clear flowcnt-route pattern {}'.format(items[0]))
else:
logger.error('Invalid route pattern {}'.format(route_pattern))


def clear_route_flow_counter_by_route(dut, prefix):
"""Clear route flow counter statistics by route

Args:
dut (object): DUT object
prefix (str): Prefix pattern. e.g. "1.1.1.0/24", "2000::/64", "Vrf1|2.2.2.0/24"
"""
items = prefix.split('|')
if len(items) == 2:
dut.command('sonic-clear flowcnt-route route {} --vrf {}'.format(items[1], items[0]))
elif len(items) == 1:
dut.command('sonic-clear flowcnt-route route {}'.format(items[0]))
else:
logger.error('Invalid prefix pattern {}'.format(prefix))


def parse_route_flow_counter_stats(dut):
"""Parse command output of "show flowcnt-route stats"

Args:
dut (object): DUT object

Returns:
dict: Parsed result. e.g. {'1.1.1.0/24': {'packets': '5', 'bytes': '4500'}}
"""
stats_list = dut.show_and_parse('show flowcnt-route stats')
parse_result = {}
for stats in stats_list:
if stats['vrf'] == 'default':
key = stats['matched routes']
else:
key = '|'.join([stats['vrf'], stats['matched routes']])
parse_result[key] = {
'packets': stats['packets'],
'bytes': stats['bytes']
}
return parse_result


def verify_route_flow_counter_stats(expect_stats, actual_stats):
"""Verify actual statistic with expected statistic

Args:
expect_stats (dict): Expected stats. e.g. {'1.1.1.0/24': {'packets': '5', 'bytes': '4500'}}
actual_stats (dict): Actual stats. e.g. {'1.1.1.0/24': {'packets': '5', 'bytes': '4500'}}

Returns:
bool: Match if True.
"""
logger.info('Expected stats: {}'.format(expect_stats))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here I suggest to exit the function only once(at the end), so we can set the rc and message when needed and return it at the end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer to keep this. There is a "for loop in for loop" in this function, it is hard to use rc in the inner loop.

    for key, value in expect_stats.items():
        if key not in actual_stats:
            return False, 'Failed to find {} in result'.format(key)

        for stats_type, expect_value in value.items():
            if int(expect_value) != int(actual_stats[key][stats_type].replace(',', '')):
                return False, 'Expected {} value of {} is {}, but got {}'.format(stats_type, key, expect_value, actual_stats[key][stats_type])

logger.info('Actual stats: {}'.format(actual_stats))
for key, value in expect_stats.items():
if key not in actual_stats:
return False, 'Failed to find {} in result'.format(key)

for stats_type, expect_value in value.items():
if int(expect_value) != int(actual_stats[key][stats_type].replace(',', '')):
return False, 'Expected {} value of {} is {}, but got {}'.format(stats_type, key, expect_value, actual_stats[key][stats_type])

return True, None
Empty file added tests/route/__init__.py
Empty file.
Loading