Skip to content

Commit 144bccb

Browse files
daalldaall
authored
[drop counters] Fix configuration for counters with lowercase names (sonic-net#1103)
Signed-off-by: Danny Allen <[email protected]>
1 parent 8d802d4 commit 144bccb

File tree

4 files changed

+36
-29
lines changed

4 files changed

+36
-29
lines changed

‎scripts/dropconfig

+16-16
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ except KeyError:
3030

3131
# CONFIG_DB Tables
3232
DEBUG_COUNTER_CONFIG_TABLE = 'DEBUG_COUNTER'
33-
DROP_REASON_TABLE_PREFIX = 'DEBUG_COUNTER_DROP_REASON'
33+
DROP_REASON_CONFIG_TABLE = 'DEBUG_COUNTER_DROP_REASON'
3434

3535
# STATE_DB Tables
3636
DEBUG_COUNTER_CAPABILITY_TABLE = 'DEBUG_COUNTER_CAPABILITIES'
@@ -136,10 +136,7 @@ class DropConfig(object):
136136
raise InvalidArgumentError('One or more provided drop reason not supported on this device')
137137

138138
for reason in reasons:
139-
self.config_db.set_entry(self.config_db.serialize_key(
140-
(DROP_REASON_TABLE_PREFIX, counter_name)),
141-
reason,
142-
{})
139+
self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), {})
143140

144141
drop_counter_entry = {'type': counter_type}
145142

@@ -168,8 +165,12 @@ class DropConfig(object):
168165
self.config_db.set_entry(DEBUG_COUNTER_CONFIG_TABLE,
169166
counter_name,
170167
None)
171-
self.config_db.delete_table(self.config_db.serialize_key(
172-
(DROP_REASON_TABLE_PREFIX, counter_name)))
168+
169+
# We can't use `delete_table` here because table names are normalized to uppercase.
170+
# Counter names can be lowercase (e.g. "test_counter|ACL_ANY"), so we have to go
171+
# through and treat each drop reason as an entry to get the correct behavior.
172+
for reason in self.get_counter_drop_reasons(counter_name):
173+
self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, reason, None)
173174

174175
def add_reasons(self, counter_name, reasons):
175176
"""
@@ -192,10 +193,7 @@ class DropConfig(object):
192193
raise InvalidArgumentError('One or more provided drop reason not supported on this device')
193194

194195
for reason in reasons:
195-
self.config_db.set_entry(self.config_db.serialize_key(
196-
(DROP_REASON_TABLE_PREFIX, counter_name)),
197-
reason,
198-
{})
196+
self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), {})
199197

200198
def remove_reasons(self, counter_name, reasons):
201199
"""
@@ -212,10 +210,7 @@ class DropConfig(object):
212210
raise InvalidArgumentError('Counter \'{}\' not found'.format(counter_name))
213211

214212
for reason in reasons:
215-
self.config_db.set_entry(self.config_db.serialize_key(
216-
(DROP_REASON_TABLE_PREFIX, counter_name)),
217-
reason,
218-
None)
213+
self.config_db.set_entry(DROP_REASON_CONFIG_TABLE, (counter_name, reason), None)
219214

220215
def get_config(self, group):
221216
"""
@@ -236,7 +231,7 @@ class DropConfig(object):
236231
}
237232

238233
# Get the drop reasons for this counter
239-
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])
234+
drop_reason_keys = sorted(self.get_counter_drop_reasons(counter_name), key=lambda x: x[1])
240235

241236
# Fill in the first drop reason
242237
num_reasons = len(drop_reason_keys)
@@ -308,6 +303,11 @@ class DropConfig(object):
308303

309304
return deserialize_reason_list(cap_query.get('reasons', ''))
310305

306+
def get_counter_drop_reasons(self, counter_name):
307+
# get_keys won't filter on counter_name for us because the counter name is case sensitive and
308+
# get_keys will normalize the table name to uppercase.
309+
return [key for key in self.config_db.get_keys(DROP_REASON_CONFIG_TABLE) if key[0] == counter_name]
310+
311311

312312
def deserialize_reason_list(list_str):
313313
if list_str is None:

‎tests/drops_group_test.py

+12-11
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,12 @@
2828
L3_ANY
2929
"""
3030

31-
expected_counter_configuration = """Counter Alias Group Type Reasons Description
32-
--------- ------------ ------------ ------------------- --------- --------------------------------------------------
33-
DEBUG_0 DEBUG_0 N/A PORT_INGRESS_DROPS None N/A
34-
DEBUG_1 SWITCH_DROPS PACKET_DROPS SWITCH_EGRESS_DROPS None Outgoing packet drops, tracked at the switch level
35-
DEBUG_2 DEBUG_2 N/A PORT_INGRESS_DROPS None
31+
expected_counter_configuration = """Counter Alias Group Type Reasons Description
32+
----------------- ----------------- ------------ ------------------- --------- --------------------------------------------------
33+
DEBUG_0 DEBUG_0 N/A PORT_INGRESS_DROPS None N/A
34+
DEBUG_1 SWITCH_DROPS PACKET_DROPS SWITCH_EGRESS_DROPS None Outgoing packet drops, tracked at the switch level
35+
DEBUG_2 DEBUG_2 N/A PORT_INGRESS_DROPS None
36+
lowercase_counter lowercase_counter N/A SWITCH_EGRESS_DROPS None N/A
3637
"""
3738

3839
expected_counter_configuration_with_group = """Counter Alias Group Type Reasons Description
@@ -46,9 +47,9 @@
4647
Ethernet4 N/A 0 1000 0 0 100 800
4748
Ethernet8 N/A 100 10 0 0 0 10
4849
49-
DEVICE SWITCH_DROPS
50-
---------------- --------------
51-
sonic_drops_test 1000
50+
DEVICE SWITCH_DROPS lowercase_counter
51+
---------------- -------------- -------------------
52+
sonic_drops_test 1000 0
5253
"""
5354

5455
expected_counts_with_group = """
@@ -71,9 +72,9 @@
7172
Ethernet4 N/A 0 0 0 0 0 0
7273
Ethernet8 N/A 0 0 0 0 0 0
7374
74-
DEVICE SWITCH_DROPS
75-
---------------- --------------
76-
sonic_drops_test 0
75+
DEVICE SWITCH_DROPS lowercase_counter
76+
---------------- -------------- -------------------
77+
sonic_drops_test 0 0
7778
"""
7879

7980
dropstat_path = "/tmp/dropstat"

‎tests/mock_tables/config_db.json

+4
Original file line numberDiff line numberDiff line change
@@ -558,10 +558,14 @@
558558
"type": "PORT_INGRESS_DROPS",
559559
"desc": ""
560560
},
561+
"DEBUG_COUNTER|lowercase_counter": {
562+
"type": "SWITCH_EGRESS_DROPS"
563+
},
561564
"DEBUG_COUNTER_DROP_REASON|DEBUG_0|IP_HEADER_ERROR": {},
562565
"DEBUG_COUNTER_DROP_REASON|DEBUG_1|ACL_ANY": {},
563566
"DEBUG_COUNTER_DROP_REASON|DEBUG_2|IP_HEADER_ERROR": {},
564567
"DEBUG_COUNTER_DROP_REASON|DEBUG_2|NO_L3_HEADER": {},
568+
"DEBUG_COUNTER_DROP_REASON|lowercase_counter|L2_ANY": {},
565569
"FEATURE|bgp": {
566570
"state": "enabled",
567571
"auto_restart": "enabled",

‎tests/mock_tables/counters_db.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,8 @@
204204
"SAI_PORT_STAT_PFC_7_TX_PKTS": "807"
205205
},
206206
"COUNTERS:oid:0x21000000000000": {
207-
"SAI_SWITCH_STAT_IN_DROP_REASON_RANGE_BASE": "1000"
207+
"SAI_SWITCH_STAT_OUT_DROP_REASON_RANGE_BASE": "1000",
208+
"SAI_SWITCH_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS": "0"
208209
},
209210
"COUNTERS_PORT_NAME_MAP": {
210211
"Ethernet0": "oid:0x1000000000002",
@@ -227,7 +228,8 @@
227228
"DEBUG_2": "SAI_PORT_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS"
228229
},
229230
"COUNTERS_DEBUG_NAME_SWITCH_STAT_MAP": {
230-
"DEBUG_1": "SAI_SWITCH_STAT_IN_DROP_REASON_RANGE_BASE"
231+
"DEBUG_1": "SAI_SWITCH_STAT_OUT_DROP_REASON_RANGE_BASE",
232+
"lowercase_counter": "SAI_SWITCH_STAT_OUT_CONFIGURED_DROP_REASONS_1_DROPPED_PKTS"
231233
},
232234
"COUNTERS:oid:0x1500000000035b": {
233235
"PFC_WD_ACTION": "drop",

0 commit comments

Comments
 (0)