From 8d3d39352251af00a4bd16bca72b5fcd0d4672e8 Mon Sep 17 00:00:00 2001 From: kellyyeh <42761586+kellyyeh@users.noreply.github.com> Date: Wed, 6 Mar 2024 22:03:11 -0800 Subject: [PATCH] [202305] Revert DHCPv4 and DHCPv6 counter (#18253) Why I did it Reverting DHCP counter changes due to unexpected packet drops seen in recv buffer, causing counter counts to be inaccurate in dhcpmon and affecting dhcp6relay performance Work item tracking Microsoft ADO (number only): 26918588 How I did it Reset submodule head and revert related dockerfile changes How to verify it Ran mgmt test and stress test --- dockers/docker-dhcp-relay/Dockerfile.j2 | 2 - .../test_show_dhcp6relay_counters.py | 44 +++---- .../cli/show/plugins/show_dhcp_relay.py | 122 +++--------------- sonic-slave-bullseye/Dockerfile.j2 | 1 - src/dhcpmon | 2 +- src/dhcprelay | 2 +- 6 files changed, 40 insertions(+), 133 deletions(-) diff --git a/dockers/docker-dhcp-relay/Dockerfile.j2 b/dockers/docker-dhcp-relay/Dockerfile.j2 index 9d5d658913..7cefc0d985 100644 --- a/dockers/docker-dhcp-relay/Dockerfile.j2 +++ b/dockers/docker-dhcp-relay/Dockerfile.j2 @@ -13,8 +13,6 @@ ENV IMAGE_VERSION=$image_version # Update apt's cache of available packages RUN apt-get update -RUN apt-get install -y libjsoncpp-dev - {% if docker_dhcp_relay_debs.strip() -%} # Copy built Debian packages {{ copy_files("debs/", docker_dhcp_relay_debs.split(' '), "/debs/") }} diff --git a/dockers/docker-dhcp-relay/cli-plugin-tests/test_show_dhcp6relay_counters.py b/dockers/docker-dhcp-relay/cli-plugin-tests/test_show_dhcp6relay_counters.py index 7f7edca64d..3cb2d2bb54 100644 --- a/dockers/docker-dhcp-relay/cli-plugin-tests/test_show_dhcp6relay_counters.py +++ b/dockers/docker-dhcp-relay/cli-plugin-tests/test_show_dhcp6relay_counters.py @@ -17,21 +17,24 @@ try: except KeyError: pass -expected_counts_v6 = """\ - Message Type Vlan1000(RX) --------------- --------------- - - Message Type Vlan1000(TX) --------------- --------------- - -""" - -expected_counts_v4 = """\ - Message Type Vlan1000(RX) --------------- --------------- - - Message Type Vlan1000(TX) --------------- --------------- +expected_counts = """\ + Message Type Vlan1000 +------------------- ----------- + Unknown + Solicit + Advertise + Request + Confirm + Renew + Rebind + Reply + Release + Decline + Reconfigure +Information-Request + Relay-Forward + Relay-Reply + Malformed """ @@ -40,14 +43,5 @@ class TestDhcp6RelayCounters(object): def test_show_counts(self): runner = CliRunner() result = runner.invoke(show.dhcp6relay_counters.commands["counts"], ["-i Vlan1000"]) - print(result.output) - assert result.output == expected_counts_v6 - -class TestDhcpRelayCounters(object): - - def test_show_counts(self): - runner = CliRunner() - result = runner.invoke(show.dhcp4relay_counters.commands["counts"], ["-i Vlan1000"]) - print(result.output) - assert result.output == expected_counts_v4 + assert result.output == expected_counts diff --git a/dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py b/dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py index 0d360aef9b..d76d5f6fa6 100644 --- a/dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py +++ b/dockers/docker-dhcp-relay/cli/show/plugins/show_dhcp_relay.py @@ -1,5 +1,4 @@ import click -import ast from natsort import natsorted from tabulate import tabulate import show.vlan as show_vlan @@ -8,20 +7,13 @@ import utilities_common.cli as clicommon from swsscommon.swsscommon import ConfigDBConnector from swsscommon.swsscommon import SonicV2Connector + # STATE_DB Table -DHCPv4_COUNTER_TABLE = 'DHCP_COUNTER_TABLE' DHCPv6_COUNTER_TABLE = 'DHCPv6_COUNTER_TABLE' -# DHCPv4 Counter Messages -dhcpv4_messages = [ - "Unknown", "Discover", "Offer", "Request", "Decline", "Ack", "Nack", "Release", "Inform" -] - # DHCPv6 Counter Messages -dhcpv6_messages = [ - "Unknown", "Solicit", "Advertise", "Request", "Confirm", "Renew", "Rebind", "Reply", "Release", - "Decline", "Reconfigure", "Information-Request", "Relay-Forward", "Relay-Reply", "Malformed" -] +messages = ["Unknown", "Solicit", "Advertise", "Request", "Confirm", "Renew", "Rebind", "Reply", "Release", "Decline", + "Reconfigure", "Information-Request", "Relay-Forward", "Relay-Reply", "Malformed"] # DHCP_RELAY Config Table DHCP_RELAY = 'DHCP_RELAY' @@ -45,75 +37,6 @@ def get_dhcp_helper_address(ctx, vlan): show_vlan.VlanBrief.register_column('DHCP Helper Address', get_dhcp_helper_address) -class DHCPv4_Counter(object): - def __init__(self): - self.db = SonicV2Connector(use_unix_socket_path=False) - self.db.connect(self.db.STATE_DB) - self.table_name = DHCPv4_COUNTER_TABLE + self.db.get_db_separator(self.db.STATE_DB) - - def get_interface(self): - """ Get all names of all interfaces in DHCPv4_COUNTER_TABLE """ - interfaces = [] - for key in self.db.keys(self.db.STATE_DB): - if DHCPv4_COUNTER_TABLE in key: - interfaces.append(key[21:]) - return interfaces - - def get_dhcp4relay_msg_count(self, interface, dir): - """ Get count of a dhcprelay message """ - value = self.db.get(self.db.STATE_DB, self.table_name + str(interface), str(dir)) - cnts = ast.literal_eval(str(value)) - data = [] - if cnts is not None: - for k, v in cnts.items(): - data.append([k, v]) - return data - - def clear_table(self, interface): - """ Reset all message counts to 0 """ - v4_cnts = {} - for msg in dhcpv4_messages: - v4_cnts[msg] = '0' - self.db.set(self.db.STATE_DB, self.table_name + str(interface), str("RX"), str(v4_cnts)) - self.db.set(self.db.STATE_DB, self.table_name + str(interface), str("TX"), str(v4_cnts)) - -def print_dhcpv4_count(counter, intf): - """Print count of each message""" - rx_data = counter.get_dhcp4relay_msg_count(intf, "RX") - print(tabulate(rx_data, headers=["Message Type", intf+"(RX)"], tablefmt='simple', stralign='right') + "\n") - tx_data = counter.get_dhcp4relay_msg_count(intf, "TX") - print(tabulate(tx_data, headers=["Message Type", intf+"(TX)"], tablefmt='simple', stralign='right') + "\n") - -# -# 'dhcp4relay_counters' group ### -# - - -@click.group(cls=clicommon.AliasedGroup, name="dhcp4relay_counters") -def dhcp4relay_counters(): - """Show DHCPv4 counter""" - pass - - -def ipv4_counters(interface): - counter = DHCPv4_Counter() - counter_intf = counter.get_interface() - - if interface: - print_dhcpv4_count(counter, interface) - else: - for intf in counter_intf: - print_dhcpv4_count(counter, intf) - - -# 'counts' subcommand ("show dhcp4relay_counters counts") -@dhcp4relay_counters.command('counts') -@click.option('-i', '--interface', required=False) -@click.option('--verbose', is_flag=True, help="Enable verbose output") -def counts(interface, verbose): - """Show dhcp4relay message counts""" - ipv4_counters(interface) - class DHCPv6_Counter(object): def __init__(self): @@ -123,37 +46,30 @@ class DHCPv6_Counter(object): def get_interface(self): """ Get all names of all interfaces in DHCPv6_COUNTER_TABLE """ - interfaces = [] + vlans = [] for key in self.db.keys(self.db.STATE_DB): if DHCPv6_COUNTER_TABLE in key: - interfaces.append(key[21:]) - return interfaces + vlans.append(key[21:]) + return vlans - def get_dhcp6relay_msg_count(self, interface, dir): + def get_dhcp6relay_msg_count(self, interface, msg): """ Get count of a dhcp6relay message """ - value = self.db.get(self.db.STATE_DB, self.table_name + str(interface), str(dir)) - cnts = ast.literal_eval(str(value)) - data = [] - if cnts is not None: - for k, v in cnts.items(): - data.append([k, v]) + count = self.db.get(self.db.STATE_DB, self.table_name + str(interface), str(msg)) + data = [str(msg), count] return data def clear_table(self, interface): """ Reset all message counts to 0 """ - v6_cnts = {} - for msg in dhcpv6_messages: - v6_cnts[msg] = '0' - self.db.set(self.db.STATE_DB, self.table_name + str(interface), str("RX"), str(v6_cnts)) - self.db.set(self.db.STATE_DB, self.table_name + str(interface), str("TX"), str(v6_cnts)) + for msg in messages: + self.db.set(self.db.STATE_DB, self.table_name + str(interface), str(msg), '0') -def print_dhcpv6_count(counter, intf): +def print_count(counter, intf): """Print count of each message""" - rx_data = counter.get_dhcp6relay_msg_count(intf, "RX") - print(tabulate(rx_data, headers=["Message Type", intf+"(RX)"], tablefmt='simple', stralign='right') + "\n") - tx_data = counter.get_dhcp6relay_msg_count(intf, "TX") - print(tabulate(tx_data, headers=["Message Type", intf+"(TX)"], tablefmt='simple', stralign='right') + "\n") + data = [] + for i in messages: + data.append(counter.get_dhcp6relay_msg_count(intf, i)) + print(tabulate(data, headers=["Message Type", intf], tablefmt='simple', stralign='right') + "\n") # @@ -172,10 +88,10 @@ def ipv6_counters(interface): counter_intf = counter.get_interface() if interface: - print_dhcpv6_count(counter, interface) + print_count(counter, interface) else: for intf in counter_intf: - print_dhcpv6_count(counter, intf) + print_count(counter, intf) # 'counts' subcommand ("show dhcp6relay_counters counts") @@ -184,6 +100,7 @@ def ipv6_counters(interface): @click.option('--verbose', is_flag=True, help="Enable verbose output") def counts(interface, verbose): """Show dhcp6relay message counts""" + ipv6_counters(interface) @@ -282,7 +199,6 @@ def dhcp_relay_ip6counters(interface): def register(cli): - cli.add_command(dhcp4relay_counters) cli.add_command(dhcp6relay_counters) cli.add_command(dhcp_relay_helper) cli.add_command(dhcp_relay) diff --git a/sonic-slave-bullseye/Dockerfile.j2 b/sonic-slave-bullseye/Dockerfile.j2 index b2d1531b51..777a30d803 100644 --- a/sonic-slave-bullseye/Dockerfile.j2 +++ b/sonic-slave-bullseye/Dockerfile.j2 @@ -359,7 +359,6 @@ RUN apt-get update && apt-get install -y \ # For DHCP Monitor tool libexplain-dev \ libevent-dev \ - libjsoncpp-dev \ # For libyang swig \ # For build dtb diff --git a/src/dhcpmon b/src/dhcpmon index fc20a97ba2..22a7467a8a 160000 --- a/src/dhcpmon +++ b/src/dhcpmon @@ -1 +1 @@ -Subproject commit fc20a97ba2eba753974ea95d504d130093030596 +Subproject commit 22a7467a8ab3e26c3d20e959a0702380f0d4b5ae diff --git a/src/dhcprelay b/src/dhcprelay index 5ae186f4a6..84e4419eef 160000 --- a/src/dhcprelay +++ b/src/dhcprelay @@ -1 +1 @@ -Subproject commit 5ae186f4a6c25647f5ce7b4d2c884b08423455e7 +Subproject commit 84e4419eef4f9b1fafa86ebb7937fe4904aa0795