Skip to content

Commit

Permalink
[debug dump] Missing Dict Key handled in the MatchOptimizer (sonic-ne…
Browse files Browse the repository at this point in the history
…t#2014)

- Why I did
The hardcoded delimiter used in the match_optimizer was updated to be assigned dynamically based on the db name provided.
Sometimes, the kv-pairs received from match engine may not contain the necessary fields requested and in those cases MatchEngine Optimizer is throwing a KeyError Exception on the missing field. Added Unit tests to cover the scenario

Traceback (most recent call last):
  File "/usr/local/bin/dump", line 8, in <module>
    sys.exit(dump())
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/dump/main.py", line 85, in state
    collected_info[arg] = obj.execute(params)
  File "/usr/local/lib/python3.9/dist-packages/dump/plugins/route.py", line 82, in execute
    self.init_asic_nh()
  File "/usr/local/lib/python3.9/dist-packages/dump/plugins/route.py", line 142, in init_asic_nh
    nh_ex.collect()
  File "/usr/local/lib/python3.9/dist-packages/dump/plugins/route.py", line 211, in collect
    rif_oid = self.init_asic_next_hop_info(self.rt.nh_id)
  File "/usr/local/lib/python3.9/dist-packages/dump/plugins/route.py", line 191, in init_asic_next_hop_info
    ret = self.rt.nhgrp_match_engine.fetch(req)
  File "/usr/local/lib/python3.9/dist-packages/dump/match_infra.py", line 430, in fetch
    return self.__mutate_response(ret, fv_requested, ret_just_keys)
  File "/usr/local/lib/python3.9/dist-packages/dump/match_infra.py", line 389, in __mutate_response
    new_ret["return_values"][key][field] = key_fv[key][field]
KeyError: 'SAI_NEXT_HOP_ATTR_ROUTER_INTERFACE_ID'

- How I did it
Handled the no-field present case gracefully

Signed-off-by: vkarri <vkarri@contoso.com>
  • Loading branch information
vivekrnv authored Jan 20, 2022
1 parent f614803 commit 48b5e73
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
7 changes: 5 additions & 2 deletions dump/match_infra.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ def __mutate_response(self, ret, fv_requested, ret_just_keys):
for key in keys:
new_ret["return_values"][key] = {}
for field in fv_requested:
new_ret["return_values"][key][field] = key_fv[key][field]
new_ret["return_values"][key][field] = key_fv[key].get(field, "")
return new_ret

def __fill_cache(self, ret):
Expand Down Expand Up @@ -415,7 +415,10 @@ def __fetch_from_cache(self, key, req):

def fetch(self, req_orig):
req = copy.deepcopy(req_orig)
key = req.table + ":" + req.key_pattern
sep = "|"
if req.db:
sep = SonicDBConfig.getSeparator(req.db)
key = req.table + sep + req.key_pattern
if key in self.__key_cache:
verbose_print("Cache Hit for Key: {}".format(key))
return self.__fetch_from_cache(key, req)
Expand Down
40 changes: 39 additions & 1 deletion tests/dump_tests/match_engine_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
import sys
import unittest
import pytest
from dump.match_infra import MatchEngine, EXCEP_DICT, MatchRequest
from dump.match_infra import MatchEngine, EXCEP_DICT, MatchRequest, MatchRequestOptimizer
from unittest.mock import MagicMock
from deepdiff import DeepDiff
from importlib import reload

Expand Down Expand Up @@ -248,3 +249,40 @@ def test_namespace_asic1(self):
assert ret["error"] == ""
assert len(ret["keys"]) == 1
assert "PORT|Ethernet-BP256" in ret["keys"]

class TestMatchEngineOptimizer(unittest.TestCase):

def test_caching(self):
rv = {"COPP_GROUP|queue4_group2": {"trap_action": "copy", "trap_priority": "4", "queue": "4", "meter_type": "packets", "mode": "sr_tcm", "cir": "600", "cbs": "600", "red_action": "drop"}}
template = {"error": "", "keys": [rv], "return_values": {}}
m_engine = MatchEngine()
m_engine.fetch = MagicMock(return_value=template)
m_engine_optim = MatchRequestOptimizer(m_engine)
req = MatchRequest(db="CONFIG_DB", table="COPP_GROUP", key_pattern="queue4*", field="red_action", value="drop")
ret = m_engine_optim.fetch(req)
assert ret["error"] == ""
assert len(ret["keys"]) == 1
assert "COPP_GROUP|queue4_group2" in ret["keys"]

req = MatchRequest(db="CONFIG_DB", table="COPP_GROUP", key_pattern="queue4_group2")
ret = m_engine_optim.fetch(req)
assert ret["error"] == ""
assert len(ret["keys"]) == 1
assert "COPP_GROUP|queue4_group2" in ret["keys"]

assert m_engine.fetch.call_count == 1

def test_missing_field(self):
rv = {"COPP_GROUP|queue4_group2": {"trap_action": "copy", "trap_priority": "4", "queue": "4", "meter_type": "packets", "mode": "sr_tcm", "cir": "600", "cbs": "600", "red_action": "drop"}}
template = {"error": "", "keys": [rv], "return_values": {}}
m_engine = MatchEngine()
m_engine.fetch = MagicMock(return_value=template)
m_engine_optim = MatchRequestOptimizer(m_engine)
req = MatchRequest(db="CONFIG_DB", table="COPP_GROUP", key_pattern="queue4*", field="red_action", value="drop", return_fields=["whatever"])
ret = m_engine_optim.fetch(req)
assert ret["error"] == ""
assert len(ret["keys"]) == 1
assert "COPP_GROUP|queue4_group2" in ret["keys"]
# missing filed should not cause an excpetion in the optimizer
assert "whatever" in ret["return_values"]["COPP_GROUP|queue4_group2"]
assert not ret["return_values"]["COPP_GROUP|queue4_group2"]["whatever"]

0 comments on commit 48b5e73

Please sign in to comment.