diff --git a/scripts/dropconfig b/scripts/dropconfig index 292214a31f..950706df44 100755 --- a/scripts/dropconfig +++ b/scripts/dropconfig @@ -30,7 +30,7 @@ except KeyError: # CONFIG_DB Tables DEBUG_COUNTER_CONFIG_TABLE = 'DEBUG_COUNTER' -DROP_REASON_TABLE_PREFIX = 'DEBUG_COUNTER_DROP_REASON' +DROP_REASON_CONFIG_TABLE = 'DEBUG_COUNTER_DROP_REASON' # STATE_DB Tables DEBUG_COUNTER_CAPABILITY_TABLE = 'DEBUG_COUNTER_CAPABILITIES' @@ -136,10 +136,7 @@ class DropConfig(object): raise InvalidArgumentError('One or more provided drop reason not supported on this device') for reason in reasons: - self.config_db.set_entry(self.config_db.serialize_key( - (DROP_REASON_TABLE_PREFIX, counter_name)), - reason, - {}) + self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), {}) drop_counter_entry = {'type': counter_type} @@ -168,8 +165,12 @@ class DropConfig(object): self.config_db.set_entry(DEBUG_COUNTER_CONFIG_TABLE, counter_name, None) - self.config_db.delete_table(self.config_db.serialize_key( - (DROP_REASON_TABLE_PREFIX, counter_name))) + + # We can't use `delete_table` here because table names are normalized to uppercase. + # Counter names can be lowercase (e.g. "test_counter|ACL_ANY"), so we have to go + # through and treat each drop reason as an entry to get the correct behavior. + for reason in self.get_counter_drop_reasons(counter_name): + self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, reason, None) def add_reasons(self, counter_name, reasons): """ @@ -192,10 +193,7 @@ class DropConfig(object): raise InvalidArgumentError('One or more provided drop reason not supported on this device') for reason in reasons: - self.config_db.set_entry(self.config_db.serialize_key( - (DROP_REASON_TABLE_PREFIX, counter_name)), - reason, - {}) + self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), {}) def remove_reasons(self, counter_name, reasons): """ @@ -212,10 +210,7 @@ class DropConfig(object): raise InvalidArgumentError('Counter \'{}\' not found'.format(counter_name)) for reason in reasons: - self.config_db.set_entry(self.config_db.serialize_key( - (DROP_REASON_TABLE_PREFIX, counter_name)), - reason, - None) + self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), None) def get_config(self, group): """ @@ -236,7 +231,7 @@ class DropConfig(object): } # Get the drop reasons for this counter - drop_reason_keys = sorted(self.config_db.get_keys(self.config_db.serialize_key((DROP_REASON_TABLE_PREFIX, counter_name))), key=lambda x: x[1]) + drop_reason_keys = sorted(self.get_counter_drop_reasons(counter_name), key=lambda x: x[1]) # Fill in the first drop reason num_reasons = len(drop_reason_keys) @@ -308,6 +303,11 @@ class DropConfig(object): return deserialize_reason_list(cap_query.get('reasons', '')) + def get_counter_drop_reasons(self, counter_name): + # get_keys won't filter on counter_name for us because the counter name is case sensitive and + # get_keys will normalize the table name to uppercase. + return [key for key in self.config_db.get_keys(DROP_REASON_CONFIG_TABLE) if key[0] == counter_name] + def deserialize_reason_list(list_str): if list_str is None: diff --git a/tests/drops_group_test.py b/tests/drops_group_test.py index 0076e7b477..06bbfdcc3b 100644 --- a/tests/drops_group_test.py +++ b/tests/drops_group_test.py @@ -28,11 +28,12 @@ L3_ANY """ -expected_counter_configuration = """Counter Alias Group Type Reasons Description ---------- ------------ ------------ ------------------- --------- -------------------------------------------------- -DEBUG_0 DEBUG_0 N/A PORT_INGRESS_DROPS None N/A -DEBUG_1 SWITCH_DROPS PACKET_DROPS SWITCH_EGRESS_DROPS None Outgoing packet drops, tracked at the switch level -DEBUG_2 DEBUG_2 N/A PORT_INGRESS_DROPS None +expected_counter_configuration = """Counter Alias Group Type Reasons Description +----------------- ----------------- ------------ ------------------- --------- -------------------------------------------------- +DEBUG_0 DEBUG_0 N/A PORT_INGRESS_DROPS None N/A +DEBUG_1 SWITCH_DROPS PACKET_DROPS SWITCH_EGRESS_DROPS None Outgoing packet drops, tracked at the switch level +DEBUG_2 DEBUG_2 N/A PORT_INGRESS_DROPS None +lowercase_counter lowercase_counter N/A SWITCH_EGRESS_DROPS None N/A """ expected_counter_configuration_with_group = """Counter Alias Group Type Reasons Description @@ -46,9 +47,9 @@ Ethernet4 N/A 0 1000 0 0 100 800 Ethernet8 N/A 100 10 0 0 0 10 - DEVICE SWITCH_DROPS ----------------- -------------- -sonic_drops_test 1000 + DEVICE SWITCH_DROPS lowercase_counter +---------------- -------------- ------------------- +sonic_drops_test 1000 0 """ expected_counts_with_group = """ @@ -71,9 +72,9 @@ Ethernet4 N/A 0 0 0 0 0 0 Ethernet8 N/A 0 0 0 0 0 0 - DEVICE SWITCH_DROPS ----------------- -------------- -sonic_drops_test 0 + DEVICE SWITCH_DROPS lowercase_counter +---------------- -------------- ------------------- +sonic_drops_test 0 0 """ dropstat_path = "/tmp/dropstat" diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index de781b1384..2ed0106020 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -558,10 +558,14 @@ "type": "PORT_INGRESS_DROPS", "desc": "" }, + "DEBUG_COUNTER|lowercase_counter": { + "type": "SWITCH_EGRESS_DROPS" + }, "DEBUG_COUNTER_DROP_REASON|DEBUG_0|IP_HEADER_ERROR": {}, "DEBUG_COUNTER_DROP_REASON|DEBUG_1|ACL_ANY": {}, "DEBUG_COUNTER_DROP_REASON|DEBUG_2|IP_HEADER_ERROR": {}, "DEBUG_COUNTER_DROP_REASON|DEBUG_2|NO_L3_HEADER": {}, + "DEBUG_COUNTER_DROP_REASON|lowercase_counter|L2_ANY": {}, "FEATURE|bgp": { "state": "enabled", "auto_restart": "enabled", diff --git a/tests/mock_tables/counters_db.json b/tests/mock_tables/counters_db.json index 8ba3d2e3a5..7c616ca500 100644 --- a/tests/mock_tables/counters_db.json +++ b/tests/mock_tables/counters_db.json @@ -204,7 +204,8 @@ "SAI_PORT_STAT_PFC_7_TX_PKTS": "807" }, "COUNTERS:oid:0x21000000000000": { - "SAI_SWITCH_STAT_IN_DROP_REASON_RANGE_BASE": "1000" + "SAI_SWITCH_STAT_OUT_DROP_REASON_RANGE_BASE": "1000", + "SAI_SWITCH_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS": "0" }, "COUNTERS_PORT_NAME_MAP": { "Ethernet0": "oid:0x1000000000002", @@ -227,7 +228,8 @@ "DEBUG_2": "SAI_PORT_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS" }, "COUNTERS_DEBUG_NAME_SWITCH_STAT_MAP": { - "DEBUG_1": "SAI_SWITCH_STAT_IN_DROP_REASON_RANGE_BASE" + "DEBUG_1": "SAI_SWITCH_STAT_OUT_DROP_REASON_RANGE_BASE", + "lowercase_counter": "SAI_SWITCH_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS" }, "COUNTERS:oid:0x1500000000035b": { "PFC_WD_ACTION": "drop",