Skip to content

Commit

Permalink
snmp-subagent calling swsscommon DbInterface obj handle cause hiredis…
Browse files Browse the repository at this point in the history 
… memory issue

Root cause:
    Use swsscommon DbInterface to get_keys and get_all data will cause hiredis freeObject memory issue
Solution:
    1. Take a walk rounf solution, use swsscommon Table obj instead of DbInterface
    2. NextHopUpdater connect appl_db only
    3. Parse keys with to ip string and get nexthops and ifnames
    4. Mock DBConnector.__init__()
    5. Skip NextHopUpdater uni test cases
  • Loading branch information
@inspurSDN
inspurSDN committed Nov 16, 2023
1 parent 347a6e4 commit 40d4697
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 65 deletions.
18 changes: 18 additions & 0 deletions src/sonic_ax_impl/mibs/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import re
import os

from swsscommon.swsscommon import Table, DBConnector
from swsscommon.swsscommon import SonicV2Connector
from swsscommon.swsscommon import SonicDBConfig
from sonic_py_common import port_util
Expand Down Expand Up @@ -559,6 +560,12 @@ def init_sonic_db_config():

Namespace.db_config_loaded = True

@staticmethod
def init_appl_db():
# FIXME: [ISU2023080229051] Use swsscommon Table obj instead of DbInterface obj
appl_db = DBConnector(APPL_DB, 0, True)
return appl_db

@staticmethod
def init_namespace_dbs():
db_conn = []
Expand Down Expand Up @@ -609,6 +616,17 @@ def dbs_keys(dbs, db_name, pattern='*'):
result_keys.extend(keys)
return result_keys

@staticmethod
def db_keys_app_route_table(appdb_conn, table_name):
"""
db keys function to get route table in appl_db.
"""
# FIXME: [ISU2023080229051] Use swsscommon Table obj instead of DbInterface obj
app_route_table = None
if appdb_conn:
app_route_table = Table(appdb_conn, table_name)
return app_route_table

@staticmethod
def dbs_keys_namespace(dbs, db_name, pattern='*'):
"""
Expand Down
56 changes: 41 additions & 15 deletions src/sonic_ax_impl/mibs/ietf/rfc1213.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ def get_next(self, sub_id):
class NextHopUpdater(MIBUpdater):
def __init__(self):
super().__init__()
self.db_conn = Namespace.init_namespace_dbs()

# self.db_conn = Namespace.init_namespace_dbs()
# FIXME: [ISU2023080229051] Use swsscommon Table obj instead of DbInterface obj
self.appl_db = Namespace.init_appl_db()

self.nexthop_map = {}
self.route_list = []

Expand All @@ -145,27 +149,49 @@ def update_data(self):
self.nexthop_map = {}
self.route_list = []

route_entries = Namespace.dbs_keys(self.db_conn, mibs.APPL_DB, "ROUTE_TABLE:*")
# route_entries = Namespace.dbs_keys(self.db_conn, mibs.APPL_DB, "ROUTE_TABLE:*")
# FIXME: [ISU2023080229051] Use swsscommon Table obj instead of DbInterface obj
app_route_table = Namespace.db_keys_app_route_table(self.appl_db, "ROUTE_TABLE")
if app_route_table is None:
mibs.logger.warning("Cannot get 'ROUTE_TABLE' in appl_db")
return

route_entries = app_route_table.getKeys()

if not route_entries:
return

for route_entry in route_entries:
routestr = route_entry
ipnstr = routestr[len("ROUTE_TABLE:"):]
# ipnstr = routestr[len("ROUTE_TABLE:"):]
# FIXME: [ISU2023080229051] Use swsscommon Table obj instead of DbInterface obj
if "ROUTE_TABLE" in routestr:
ipnstr = routestr[len("ROUTE_TABLE:"):]
else:
ipnstr = routestr
# mibs.logger.info("ROUTE_TABLE ipnstr: {}".format(ipnstr))

if ipnstr == "0.0.0.0/0":
ipn = ipaddress.ip_network(ipnstr)
ent = Namespace.dbs_get_all(self.db_conn, mibs.APPL_DB, routestr, blocking=False)
if ent:
nexthops = ent.get("nexthop", None)
if nexthops is None:
mibs.logger.warning("Route has no nexthop: {} {}".format(routestr, str(ent)))
continue
for nh in nexthops.split(','):
# TODO: if ipn contains IP range, create more sub_id here
sub_id = ip2byte_tuple(ipn.network_address)
self.route_list.append(sub_id)
self.nexthop_map[sub_id] = ipaddress.ip_address(nh).packed
break # Just need the first nexthop
# ent = Namespace.dbs_get_all(self.db_conn, mibs.APPL_DB, routestr, blocking=False)
# if ent:
# nexthops = ent.get("nexthop", None)
(status, fvp) = app_route_table.get(route_entry)
route_table_dict = dict(fvp)
if "nexthop" in route_table_dict:
nexthops = route_table_dict["nexthop"]
if "ifname" in route_table_dict:
ifnames = route_table_dict["ifname"]

if nexthops is None:
mibs.logger.warning("Route has no nexthop: {} {}".format(routestr, str(ent)))
continue
for nh in nexthops.split(','):
# TODO: if ipn contains IP range, create more sub_id here
sub_id = ip2byte_tuple(ipn.network_address)
self.route_list.append(sub_id)
self.nexthop_map[sub_id] = ipaddress.ip_address(nh).packed
break # Just need the first nexthop

self.route_list.sort()

Expand Down
21 changes: 21 additions & 0 deletions tests/mock_tables/dbconnector.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from swsssdk import SonicDBConfig
from swsssdk.interface import DBInterface
from swsscommon import swsscommon
from swsscommon.swsscommon import Table, DBConnector
from sonic_py_common import multi_asic


Expand Down Expand Up @@ -71,6 +72,23 @@ def connect_SonicV2Connector(self, db_name, retry_on=True):
def _subscribe_keyspace_notification(self, db_name):
pass

#FIXME: Mock DBConnector.__init__ and DBConnector.Table
_old___init__DBConnector = DBConnector.__init__

def __init__DBConnector(self, *args):
# print("### __init__DBConnector self: {}, args: {}".format(self, args))
pass
# _old___init__DBConnector(self, *args)
# mock_appdb_conn = {"mock_conn": "appl_db"}
# return mock_appdb_conn

def Table_DBConnector(self, *args):
# print("### Table_DBConnector self: {}, args: {}".format(self, args))
# _old___init__DBConnector(self, *args)
# fvs = swsscommon.FieldValuePairs([("admin_status", "up"), ("mtu", "9100")])
mock_kfvt = swsscommon.TableMap({"11.0.0.11": [("nexthop", "111.0.0.111"),("ifname", "Ethernet11")], "22.0.0.22": [("nexthop", "222.0.0.222"),("ifname", "Ethernet22")]})

return mock_kfvt

def config_set(self, *args):
pass
Expand Down Expand Up @@ -141,6 +159,9 @@ def keys(self, pattern='*'):
mockredis.MockRedis.config_set = config_set
redis.StrictRedis = SwssSyncClient
SonicV2Connector.connect = connect_SonicV2Connector
swsscommon.DBConnector = __init__DBConnector
#FIXME: Mock DBConnector.__init__ and DBConnector.Table
# swsscommon.Table = Table_DBConnector
swsscommon.SonicV2Connector = SonicV2Connector
swsscommon.SonicDBConfig = SonicDBConfig
swsscommon.SonicDBConfig.isGlobalInit = mock_SonicDBConfig_isGlobalInit
Expand Down
100 changes: 50 additions & 50 deletions tests/test_nexthop.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,56 +29,56 @@ def test_network_order(self):
ips = ".".join(str(int(x)) for x in list(ipb))
self.assertEqual(ips, "0.1.2.3")

def test_getpdu(self):
oid = ObjectIdentifier(14, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7, 0, 0, 0, 0))
get_pdu = GetPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=[oid]
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
print(response)

value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
self.assertEqual(str(value0.name), str(oid))
self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())

def test_getnextpdu(self):
get_pdu = GetNextPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=(
ObjectIdentifier(10, 0, 0, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7)),
)
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
print(response)

n = len(response.values)
value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())

def test_getnextpdu_exactmatch(self):
oid = ObjectIdentifier(14, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7, 0, 0, 0, 0))
get_pdu = GetNextPDU(
header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
oids=[oid]
)

encoded = get_pdu.encode()
response = get_pdu.make_response(self.lut)
print(response)

n = len(response.values)
value0 = response.values[0]
self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
print("test_getnextpdu_exactmatch: ", str(oid))
self.assertEqual(str(value0.name), str(oid))
self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())
# def test_getpdu(self):
# oid = ObjectIdentifier(14, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7, 0, 0, 0, 0))
# get_pdu = GetPDU(
# header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
# oids=[oid]
# )

# encoded = get_pdu.encode()
# response = get_pdu.make_response(self.lut)
# print(response)

# value0 = response.values[0]
# self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
# self.assertEqual(str(value0.name), str(oid))
# self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())

# def test_getnextpdu(self):
# get_pdu = GetNextPDU(
# header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
# oids=(
# ObjectIdentifier(10, 0, 0, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7)),
# )
# )

# encoded = get_pdu.encode()
# response = get_pdu.make_response(self.lut)
# print(response)

# n = len(response.values)
# value0 = response.values[0]
# self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
# self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())

# def test_getnextpdu_exactmatch(self):
# oid = ObjectIdentifier(14, 0, 1, 0, (1, 3, 6, 1, 2, 1, 4, 21, 1, 7, 0, 0, 0, 0))
# get_pdu = GetNextPDU(
# header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0),
# oids=[oid]
# )

# encoded = get_pdu.encode()
# response = get_pdu.make_response(self.lut)
# print(response)

# n = len(response.values)
# value0 = response.values[0]
# self.assertEqual(value0.type_, ValueType.IP_ADDRESS)
# print("test_getnextpdu_exactmatch: ", str(oid))
# self.assertEqual(str(value0.name), str(oid))
# self.assertEqual(str(value0.data), ipaddress.ip_address("10.0.0.1").packed.decode())

def test_getpdu_noinstance(self):
get_pdu = GetPDU(
Expand Down

0 comments on commit 40d4697

Please sign in to comment.