-
Notifications
You must be signed in to change notification settings - Fork 109
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.
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
Fix FdbUpdater crash when SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID attribute missing. #286
Changes from 7 commits
ed53a5c
35b86ed
5d09e93
45aabe0
8fde7b4
0fb0f1d
7934832
7b1aad1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ def __init__(self): | |
self.vlanmac_ifindex_list = [] | ||
self.if_bpid_map = {} | ||
self.bvid_vlan_map = {} | ||
self.broken_fdbs = [] | ||
|
||
def fdb_vlanmac(self, fdb): | ||
if 'vlan' in fdb: | ||
|
@@ -85,8 +86,18 @@ def update_data(self): | |
if not ent: | ||
continue | ||
|
||
bridge_port_id_attr = "" | ||
try: | ||
bridge_port_id_attr = ent["SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"] | ||
except KeyError as e: | ||
# Only write error log once | ||
if fdb_str not in self.broken_fdbs: | ||
mibs.logger.error("SyncD 'ASIC_DB' includes invalid FDB_ENTRY '{}': failed to get bridge_port_id, exception: {}".format(fdb_str, e)) | ||
self.broken_fdbs.append(fdb_str) | ||
continue | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The log will repeat every 5 seconds, seems not necessary to add more delay: in https://github.com/sonic-net/sonic-snmpagent/blob/master/src/sonic_ax_impl/main.py
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed, every FDB only output error log once. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the fix! Further suggestion: flush There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
# Example output: oid:0x3a000000000608 | ||
bridge_port_id = ent["SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID"][6:] | ||
bridge_port_id = bridge_port_id_attr[6:] | ||
if bridge_port_id not in self.if_bpid_map: | ||
continue | ||
port_id = self.if_bpid_map[bridge_port_id] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import os | ||
import sys | ||
from unittest import TestCase | ||
|
||
if sys.version_info.major == 3: | ||
from unittest import mock | ||
else: | ||
import mock | ||
|
||
modules_path = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) | ||
sys.path.insert(0, os.path.join(modules_path, 'src')) | ||
|
||
from sonic_ax_impl.mibs.ietf.rfc4363 import FdbUpdater | ||
|
||
class TestFdbUpdater(TestCase): | ||
|
||
@mock.patch('sonic_ax_impl.mibs.Namespace.dbs_keys', mock.MagicMock(return_value=(['ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{"bvid":"oid:0x26000000000b6c","mac":"60:45:BD:98:6F:48","switch_id":"oid:0x21000000000000"}']))) | ||
@mock.patch('sonic_ax_impl.mibs.Namespace.dbs_get_all', mock.MagicMock(return_value=({"nexthop": "10.0.0.1,10.0.0.3", "ifname": "Ethernet0,Ethernet4"}))) | ||
def test_FdbUpdater_ent_bridge_port_id_attr_missing(self): | ||
updater = FdbUpdater() | ||
|
||
with mock.patch('sonic_ax_impl.mibs.logger.error') as mocked_error: | ||
updater.update_data() | ||
|
||
# check warning | ||
mocked_error.assert_called() | ||
|
||
self.assertTrue(len(updater.vlanmac_ifindex_list) == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log at warning level, because we skip it and continue with others. #Closed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed