Skip to content

Commit

Permalink
[dhcp_server] Fix the issue with "kea-dhcp4.conf" file generation for…
Browse files Browse the repository at this point in the history
… Smart Switch. (sonic-net#19200)

The configuration generated from the template for the Smart Switch contained incorrect data in the "subnet4:id" field. For regular cases, the subnet ID is deduced from the VLAN name. For the Smart Switch, there is always one subnet, and the ID is set to 0.
  • Loading branch information
oleksandrivantsiv authored and arun1355492 committed Jul 26, 2024
1 parent c64e9ed commit bd5ce30
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
VLAN_MEMBER = "VLAN_MEMBER"
DPUS = "DPUS"
MID_PLANE_BRIDGE = "MID_PLANE_BRIDGE"
MID_PLANE_BRIDGE_SUBNET_ID = 10000
PORT_MODE_CHECKER = ["DhcpServerTableCfgChangeEventChecker", "DhcpPortTableEventChecker", "DhcpRangeTableEventChecker",
"DhcpOptionTableEventChecker", "VlanTableEventChecker", "VlanIntfTableEventChecker",
"VlanMemberTableEventChecker"]
Expand Down Expand Up @@ -90,7 +91,7 @@ def generate(self):
port_ips, used_ranges = self._parse_port(port_ipv4, dhcp_interfaces, dhcp_members, ranges)
customized_options = self._parse_customized_options(customized_options_ipv4)
render_obj, enabled_dhcp_interfaces, used_options, subscribe_table = \
self._construct_obj_for_template(dhcp_server_ipv4, port_ips, hostname, customized_options)
self._construct_obj_for_template(dhcp_server_ipv4, port_ips, hostname, customized_options, smart_switch)

if smart_switch:
subscribe_table |= set(SMART_SWITCH_CHECKER)
Expand Down Expand Up @@ -175,7 +176,7 @@ def _parse_port_map_alias(self):
for pc_name in pc_table.keys():
self.port_alias_map[pc_name] = pc_name

def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, customized_options):
def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, customized_options, smart_switch=False):
subnets = []
client_classes = []
enabled_dhcp_interfaces = set()
Expand Down Expand Up @@ -223,8 +224,9 @@ def _construct_obj_for_template(self, dhcp_server_ipv4, port_ips, hostname, cust
"condition": "substring(relay4[1].hex, -{}, {}) == '{}'".format(class_len, class_len,
client_class)
})

subnet_obj = {
"id": dhcp_interface_name.replace("Vlan", ""),
"id": MID_PLANE_BRIDGE_SUBNET_ID if smart_switch else dhcp_interface_name.replace("Vlan", ""),
"subnet": str(ipaddress.ip_network(dhcp_interface_ip, strict=False)),
"pools": pools,
"gateway": dhcp_config["gateway"],
Expand Down
16 changes: 15 additions & 1 deletion src/sonic-dhcp-utilities/dhcp_utilities/dhcpservd/dhcp_lease.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from abc import abstractmethod
from collections import deque
from datetime import datetime
from dhcp_utilities.common.utils import is_smart_switch

DHCP_SERVER_IPV4_LEASE = "DHCP_SERVER_IPV4_LEASE"
KEA_LEASE_FILE_PATH = "/tmp/kea-lease.csv"
Expand All @@ -29,6 +30,13 @@ def __init__(self, db_connector, lease_update_interval=DEFAULE_LEASE_UPDATE_INTE
self.lease_update_interval = lease_update_interval
self.last_update_time = None
self.lock = threading.Lock()
device_metadata = self.db_connector.get_config_db_table("DEVICE_METADATA")
self.is_smart_switch = is_smart_switch(device_metadata)

if self.is_smart_switch:
mid_plane_table = self.db_connector.get_config_db_table("MID_PLANE_BRIDGE")
if "GLOBAL" in mid_plane_table and mid_plane_table["GLOBAL"]:
self.midplane_bridge_name = mid_plane_table["GLOBAL"]["bridge"]

@abstractmethod
def _read(self):
Expand Down Expand Up @@ -97,6 +105,12 @@ def register(self):
"""
signal.signal(signal.SIGUSR1, self._update_lease)

def _lease_key(self, subnet_id, mac_address):
if self.is_smart_switch:
return f"{self.midplane_bridge_name}|{mac_address}"
else:
return f"Vlan{subnet_id}|{mac_address}"

def _read(self):
# Read lease file generated by kea-dhcp4
try:
Expand All @@ -120,7 +134,7 @@ def _read(self):
lease_end = splits[4]
subnet_id = splits[5]

new_key = "{}|{}".format("Vlan" + subnet_id, mac_address)
new_key = self._lease_key(subnet_id, mac_address)
if new_key in new_lease:
continue
new_lease[new_key] = {
Expand Down
1 change: 1 addition & 0 deletions src/sonic-dhcp-utilities/tests/test_data/kea-dhcp4.conf.j2
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
{%- if add_subnet_preceding_comma.flag -%},{%- endif -%}
{%- set _dummy = add_subnet_preceding_comma.update({'flag': True}) %}
{
"id": {{ subnet_info["id"] }},
"subnet": "{{ subnet_info["subnet"] }}",
"pools": [
{%- set add_pool_preceding_comma = { 'flag': False } %}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
address,hwaddr,client_id,valid_lifetime,expire,subnet_id,fqdn_fwd,fqdn_rev,hostname,state,user_context
169.254.200.4,aa:bb:cc:dd:ff:04,,900,1718053209,10000,0,0,sonic,0,
169.254.200.1,aa:bb:cc:dd:ff:01,,900,1718053209,10000,0,0,sonic,0,
169.254.200.3,aa:bb:cc:dd:ff:03,,900,1718053210,10000,0,0,sonic,0,
169.254.200.2,aa:bb:cc:dd:ff:02,,900,1718053210,10000,0,0,sonic,0,
1 change: 1 addition & 0 deletions src/sonic-dhcp-utilities/tests/test_dhcp_cfggen.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
},
"subnet4": [
{
"id": 1000,
"subnet": "192.168.0.0/21",
"pools": [
{
Expand Down
46 changes: 25 additions & 21 deletions src/sonic-dhcp-utilities/tests/test_dhcp_lease.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from freezegun import freeze_time
from swsscommon import swsscommon
from unittest.mock import patch, call, MagicMock
from common_utils import mock_get_config_db_table

expected_lease = {
"Vlan1000|10:70:fd:b6:13:00": {
Expand Down Expand Up @@ -34,20 +35,22 @@


def test_read_kea_lease_with_file_not_found(mock_swsscommon_dbconnector_init):
db_connector = DhcpDbConnector()
kea_lease_handler = KeaDhcp4LeaseHandler(db_connector)
try:
kea_lease_handler._read()
except FileNotFoundError:
pass
with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table):
db_connector = DhcpDbConnector()
kea_lease_handler = KeaDhcp4LeaseHandler(db_connector)
try:
kea_lease_handler._read()
except FileNotFoundError:
pass


def test_read_kea_lease(mock_swsscommon_dbconnector_init):
db_connector = DhcpDbConnector()
kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease.csv")
# Verify whether lease information read is as expected
lease = kea_lease_handler._read()
assert lease == expected_lease
with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table):
db_connector = DhcpDbConnector()
kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease.csv")
# Verify whether lease information read is as expected
lease = kea_lease_handler._read()
assert lease == expected_lease


# Cannot mock built-in/extension type function(datetime.datetime.timestamp), need to free time
Expand Down Expand Up @@ -88,13 +91,14 @@ def test_update_kea_lease(mock_swsscommon_dbconnector_init, mock_swsscommon_tabl


def test_no_implement(mock_swsscommon_dbconnector_init):
db_connector = DhcpDbConnector()
lease_handler = LeaseHanlder(db_connector)
try:
lease_handler._read()
except NotImplementedError:
pass
try:
lease_handler.register()
except NotImplementedError:
pass
with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table):
db_connector = DhcpDbConnector()
lease_handler = LeaseHanlder(db_connector)
try:
lease_handler._read()
except NotImplementedError:
pass
try:
lease_handler.register()
except NotImplementedError:
pass
5 changes: 3 additions & 2 deletions src/sonic-dhcp-utilities/tests/test_dhcpservd.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import signal
import sys
import time
from common_utils import MockProc
from common_utils import MockProc, mock_get_config_db_table
from dhcp_utilities.common.utils import DhcpDbConnector
from dhcp_utilities.common.dhcp_db_monitor import DhcpServdDbMonitor
from dhcp_utilities.dhcpservd.dhcp_cfggen import DhcpServCfgGenerator
Expand Down Expand Up @@ -110,7 +110,8 @@ def test_update_dhcp_server_ip(mock_swsscommon_dbconnector_init, mock_parse_port
def test_start(mock_swsscommon_dbconnector_init, mock_parse_port_map_alias, mock_get_render_template):
with patch.object(DhcpServd, "dump_dhcp4_config") as mock_dump, \
patch.object(DhcpServd, "_update_dhcp_server_ip") as mock_update_dhcp_server_ip, \
patch.object(DhcpServdDbMonitor, "enable_checkers"):
patch.object(DhcpServdDbMonitor, "enable_checkers"), \
patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table):
dhcp_db_connector = DhcpDbConnector()
dhcp_cfg_generator = DhcpServCfgGenerator(dhcp_db_connector, "/usr/local/lib/kea/hooks/libdhcp_run_script.so")
dhcpservd = DhcpServd(dhcp_cfg_generator, dhcp_db_connector, MagicMock())
Expand Down
35 changes: 35 additions & 0 deletions src/sonic-dhcp-utilities/tests/test_smart_switch.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import pytest
from common_utils import MockConfigDb, dhcprelayd_refresh_dhcrelay_test, dhcprelayd_proceed_with_check_res_test
from dhcp_utilities.dhcpservd.dhcp_lease import KeaDhcp4LeaseHandler
from dhcp_utilities.dhcprelayd.dhcprelayd import DHCP_SERVER_CHECKER, MID_PLANE_CHECKER
from dhcp_utilities.dhcpservd.dhcp_cfggen import DhcpServCfgGenerator
from dhcp_utilities.common.utils import DhcpDbConnector
Expand Down Expand Up @@ -35,6 +36,7 @@
},
"subnet4": [
{
"id": 10000,
"subnet": "169.254.200.0/24",
"pools": [
{
Expand Down Expand Up @@ -103,6 +105,30 @@
}


expected_lease = {
'bridge_midplane|aa:bb:cc:dd:ff:01': {
'ip': '169.254.200.1',
'lease_end': '1718053209',
'lease_start': '1718052309'
},
'bridge_midplane|aa:bb:cc:dd:ff:02': {
'ip': '169.254.200.2',
'lease_end': '1718053210',
'lease_start': '1718052310'
},
'bridge_midplane|aa:bb:cc:dd:ff:03': {
'ip': '169.254.200.3',
'lease_end': '1718053210',
'lease_start': '1718052310'
},
'bridge_midplane|aa:bb:cc:dd:ff:04': {
'ip': '169.254.200.4',
'lease_end': '1718053209',
'lease_start': '1718052309'
}
}


def test_dhcprelayd_refresh_dhcrelay(mock_swsscommon_dbconnector_init):
expected_checkers = set(["MidPlaneTableEventChecker"])
dhcprelayd_refresh_dhcrelay_test(expected_checkers, True, mock_get_config_db_table)
Expand Down Expand Up @@ -140,3 +166,12 @@ def test_dhcp_dhcp_cfggen_generate(mock_swsscommon_dbconnector_init, mock_parse_
def mock_get_config_db_table(table_name):
mock_config_db = MockConfigDb(MOCK_CONFIG_DB_PATH_SMART_SWITCH)
return mock_config_db.get_config_db_table(table_name)


def test_read_kea_lease(mock_swsscommon_dbconnector_init):
with patch.object(DhcpDbConnector, "get_config_db_table", side_effect=mock_get_config_db_table):
db_connector = DhcpDbConnector()
kea_lease_handler = KeaDhcp4LeaseHandler(db_connector, lease_file="tests/test_data/kea-lease_smart_switch.csv")
# Verify whether lease information read is as expected
lease = kea_lease_handler._read()
assert lease == expected_lease

0 comments on commit bd5ce30

Please sign in to comment.