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.

Sign up for GitHub

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

Jump to bottom

[drop counters] Fix configuration for counters with lowercase names #1103

Merged
merged 1 commit into from
Sep 10, 2020
Merged

[drop counters] Fix configuration for counters with lowercase names #1103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions scripts/dropconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -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):
"""
Expand All @@ -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):
"""
Expand All @@ -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):
"""
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
23 changes: 12 additions & 11 deletions tests/drops_group_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = """
Expand All @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions tests/mock_tables/config_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions tests/mock_tables/counters_db.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,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",
Expand All @@ -209,7 +210,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",
Expand Down