Skip to content

Commit

Permalink
[drop counters] Fix configuration for counters with lowercase names (s…
Browse files Browse the repository at this point in the history 
…onic-net#1103)

Signed-off-by: Danny Allen <[email protected]>
  • Loading branch information
@daall
daall authored Sep 10, 2020
1 parent 8d802d4 commit 144bccb
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 29 deletions.
32 changes: 16 additions & 16 deletions scripts/dropconfig
View file
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down

0 comments on commit 144bccb

Please sign in to comment.