[dhcp4relay] Smart switch support, defect fixes (#84)

* [sonic-dhcp4relay]: Defect fixes

Below sonic-mgmt issues are fixed:
1. Random source port
2. Padding issues
3. DHCP Server fixes

Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com>

* [sonic-dhcp4relay] Smart switch support

Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com>

* Remove binary zip file and download PcapPlusPlus during build

- Replace 17MB pcappp_v24.09.zip binary with wget download from GitHub
- Download from official release: github.com/seladb/PcapPlusPlus/archive/refs/tags/v24.09.tar.gz
- Add SHA256 checksum verification (b14998ecf5718c2bfb6f709060e53f7ef67639b98bf2222a3cd23d820d70acdc)
- Fix patch file paths to apply cleanly (remove PcapPlusPlus-24.09/ prefix)
- Add .gitignore to exclude downloaded artifacts

Addresses review comment from saiarcot895 on PR #67

---------

Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com>
Co-authored-by: Ashutosh Agrawal <ashu@cisco.com>

Author: cshivashgit

dhcp4relay/.gitignore

@@ -0,0 +1,15 @@
+# Build artifacts
+build/
+build-test/
+*.o
+*.d
+*.gcda
+*.gcno
+*.gcov
+*.html
+*.xml
+
+# PcapPlusPlus downloaded files
+PcapPlusPlus-*/
+PcapPlusPlus-*.tar.gz
+pcappp.stamp

dhcp4relay/Makefile

@@ -15,9 +15,11 @@ GCOVR := gcovr
 
 LD_PCAPPLUSPLUS_LIB := -lPcap++ -lPacket++ -lCommon++
 
-#pcap plus plus zip file pcappp_v24.09.zip
+# PcapPlusPlus library configuration
 PCAPPPVAR := 24.09
-PCAPPPZIP_FILE := pcappp_v${PCAPPPVAR}.zip
+PCAPPP_URL := https://github.com/seladb/PcapPlusPlus/archive/refs/tags/v$(PCAPPPVAR).tar.gz
+PCAPPP_TARBALL := PcapPlusPlus-$(PCAPPPVAR).tar.gz
+PCAPPP_SHA256 := b14998ecf5718c2bfb6f709060e53f7ef67639b98bf2222a3cd23d820d70acdc
 PCAPPLUSPLUS_DIR := $(WORKING_DIR)/PcapPlusPlus-$(PCAPPPVAR)
 PCAPPP_DONE = $(WORKING_DIR)/pcappp.stamp
 INCLUDE_DIR = $(PCAPPLUSPLUS_DIR)/include
@@ -34,10 +36,20 @@ PWD := $(shell pwd)
 .PHONY: $(PCAPPP_DONE)
 $(PCAPPP_DONE):
 
+	# Download PcapPlusPlus from official GitHub release if not already present
+	if [ ! -f $(PCAPPP_TARBALL) ]; then \
+		echo "Downloading PcapPlusPlus v$(PCAPPPVAR)..."; \
+		wget -O $(PCAPPP_TARBALL) $(PCAPPP_URL); \
+	fi
+
+	# Verify checksum
+	echo "$(PCAPPP_SHA256)  $(PCAPPP_TARBALL)" | sha256sum -c -
+
 	# Remove stale files
 	rm -rf $(PCAPPLUSPLUS_DIR)
 
-	unzip ${PCAPPPZIP_FILE}
+	# Extract tarball (GitHub tarball extracts to PcapPlusPlus-<version> without 'v' prefix)
+	tar xzf $(PCAPPP_TARBALL)
 	pushd $(PCAPPLUSPLUS_DIR)
 
 	# Create a git repository here for stg to apply patches
@@ -98,6 +110,7 @@ uninstall:
 
 clean:
 	-$(RM) $(BUILD_DIR) $(BUILD_TEST_DIR) *.html *.xml
+	-$(RM) $(PCAPPLUSPLUS_DIR) $(PCAPPP_TARBALL) $(PCAPPP_DONE)
 	$(FIND) . -name *.gcda -exec rm -f {} \;
 	$(FIND) . -name *.gcno -exec rm -f {} \;
 	$(FIND) . -name *.gcov -exec rm -f {} \;

dhcp4relay/patch/0001-dhcpv4-relay-accept-random-src-port.patch

@@ -1,7 +1,7 @@
-diff --git a/PcapPlusPlus-24.09/Packet++/header/DhcpLayer.h b/PcapPlusPlus-24.09/Packet++/header/DhcpLayer.h
+diff --git a/Packet++/header/DhcpLayer.h b/Packet++/header/DhcpLayer.h
 index 435becb..282d739 100644
---- a/PcapPlusPlus-24.09/Packet++/header/DhcpLayer.h
-+++ b/PcapPlusPlus-24.09/Packet++/header/DhcpLayer.h
+--- a/Packet++/header/DhcpLayer.h
++++ b/Packet++/header/DhcpLayer.h
 @@ -881,8 +881,8 @@ namespace pcpp
 
  	bool DhcpLayer::isDhcpPorts(uint16_t portSrc, uint16_t portDst)

dhcp4relay/pcappp_v24.09.zip

@@ -16,14 +16,15 @@
  * @param len                       length of message
  * @param src_ip                    source IP address as string (optional)
  * @param use_src_ip                if true, use src_ip as source address
+ * @param pad                       if true, do padding
  *
  * @return boolean   True if packet successfully sent
  */
 #ifndef UNIT_TEST
-bool send_udp(int sock, uint8_t *buffer, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip) {
+bool send_udp(int sock, uint8_t *buffer, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip, bool pad) {
    /* Pad additional bytes if length is lesser than 300
     * to make DHCP packet length to minimum of 300 bytes */
-   if (len < BOOTP_MIN_LEN) {
+   if (pad && len < BOOTP_MIN_LEN) {
       auto pad_len = BOOTP_MIN_LEN - len;
       memset(buffer+len, 0, pad_len);
       len = BOOTP_MIN_LEN;

dhcp4relay/src/dhcp4_sender.cpp

@@ -16,7 +16,8 @@
  * @param len                         length of message
  * @param src_ip                    source IP address as string (optional)
  * @param use_src_ip                if true, use src_ip as source address
+ * @param pad                       if true, do padding
  *
  * @return boolean   True if packet successfully sent
  */
-bool send_udp(int sock, uint8_t *buffer, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip);
+bool send_udp(int sock, uint8_t *buffer, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip, bool pad);

dhcp4relay/src/dhcp4_sender.h

@@ -11,6 +11,7 @@
 #include <pcapplusplus/UdpLayer.h>
 #include <signal.h>
 #include <unistd.h>
+#include <fstream>
 
 #include "configdb.h"
 #include "dhcp4_sender.h"
@@ -430,9 +431,22 @@ uint8_t encode_tlv(uint8_t *buf, uint8_t t, uint8_t l, uint8_t *v) {
     return (l + DHCP_SUB_OPT_TLV_HEADER_LEN);
 }
 
+std::string get_mac_address(const std::string &ifname) {
+    std::string path = "/sys/class/net/" + ifname + "/address";
+    std::ifstream file(path);
+    if (!file.is_open()) {
+        syslog(LOG_ERR, "Fetching mac address for interface %s", ifname.c_str());
+        return "";
+    }
+    std::string mac;
+    std::getline(file, mac);
+    return mac;
+}
+
 void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config) {
     uint8_t buf[256] = {0};
     uint8_t buf_offset = 0;
+    std::string bm_mac;
 
     auto vrf = vlan_vrf_map[config->vlan.c_str()];
 
@@ -454,11 +468,22 @@ void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config) {
                              (uint8_t *)circuit_id.c_str());
     buf_offset += offset;
 
+    if (!m_config.midplane_bridge.empty()) {
+        bm_mac = get_mac_address(m_config.midplane_bridge);
+    }
+
     /* Encode remote ID sub-option */
     /* | 2 | 6 | my_mac| */
-    offset = encode_tlv((buf + buf_offset), OPTION82_SUBOPT_REMOTE_ID,
-                        MAC_ADDR_STR_LEN, (uint8_t *)(m_config.host_mac_addr.c_str()));
-    buf_offset += offset;
+    /* if its SmartSwitch we need to fetch mac of bridge-midplane */
+    if ((m_config.is_SmartSwitch) && (!bm_mac.empty())) {
+        offset = encode_tlv((buf + buf_offset), OPTION82_SUBOPT_REMOTE_ID,
+                            MAC_ADDR_STR_LEN, (uint8_t *)(bm_mac.c_str()));
+        buf_offset += offset;
+    } else {
+        offset = encode_tlv((buf + buf_offset), OPTION82_SUBOPT_REMOTE_ID,
+                            MAC_ADDR_STR_LEN, (uint8_t *)(m_config.host_mac_addr.c_str()));
+        buf_offset += offset;
+    }
 
     /* TODO: this sub-option should be set if source interface selection is enabled */
     /* | 5 | 4 | ipv4 | */
@@ -577,7 +602,7 @@ void from_client(pcpp::DhcpLayer *dhcp_pkt, relay_config &config) {
     }
 
     for (auto server : config.servers_sock) {
-        if (send_udp(sock, (uint8_t *)dhcp_pkt->getDhcpHeader(), server, dhcp_pkt->getHeaderLen(), src_ip, use_intf_ip_as_src_ip)) {
+        if (send_udp(sock, (uint8_t *)dhcp_pkt->getDhcpHeader(), server, dhcp_pkt->getHeaderLen(), src_ip, use_intf_ip_as_src_ip, true)) {
             syslog(LOG_INFO, "[DHCPV4_RELAY] DHCP packet is sent to configured server: %s, interface: %s",
                    config.servers[index].c_str(), config.vlan.c_str());
             dhcp_cntr_table.increment_counter(config.vlan, "TX", (int)dhcp_pkt->getMessageType());
@@ -635,6 +660,7 @@ void to_client(pcpp::DhcpLayer *dhcp_pkt, std::unordered_map<std::string, relay_
     struct sockaddr_in target_addr = {0};
     uint32_t giaddr = dhcp_pkt->getDhcpHeader()->gatewayIpAddress;
     uint32_t broadcast_addr = DHCP_BROADCAST_IPADDR;
+    bool pad = false;
     std::unordered_map<std::string, relay_config>::iterator config_itr = vlans->end();
 
     if (getifaddrs(&ifa) == -1) {
@@ -731,8 +757,13 @@ void to_client(pcpp::DhcpLayer *dhcp_pkt, std::unordered_map<std::string, relay_
     in_addr ip_zero = {0};
     /* TODO: Send unicast message to client if BOOTP flag from client is set to unicast */
 
-    dhcp_pkt->removeOption(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS);
-    if (send_udp(config.client_sock, (uint8_t *)dhcp_pkt->getDhcpHeader(), target_addr, dhcp_pkt->getHeaderLen(), ip_zero, false)) {
+    /* Perform padding only when DHCP relay (Option 82) information has been stripped from the packet */
+    if(dhcp_pkt->removeOption(pcpp::DHCPOPT_DHCP_AGENT_OPTIONS)) {
+        syslog(LOG_NOTICE, "Packet is stripped");
+        pad = true;
+    }
+
+    if (send_udp(config.client_sock, (uint8_t *)dhcp_pkt->getDhcpHeader(), target_addr, dhcp_pkt->getHeaderLen(), ip_zero, false, pad)) {
         syslog(LOG_INFO, "[DHCPV4_RELAY] dhcp relay message is broadcast to client %s from server %s",
                config.vlan.c_str(), src_ip.c_str());
         dhcp_cntr_table.increment_counter(config.vlan, "TX", (int)dhcp_pkt->getMessageType());
@@ -896,8 +927,8 @@ void pkt_in_callback(evutil_socket_t fd, short event, void *arg) {
         std::string intf(interface_name);
         auto itr = std::find(interface_list.begin(), interface_list.end(), intf);
         /* To avoid duplicate packets, we are only processing packets from
-           interface in PORT_TABLE and packets from VXLAN interface */
-        if ((itr == interface_list.end()) && (intf.rfind("VXLAN", 0) != 0)) {
+           interface in PORT_TABLE and packets from VXLAN interface and docker0 interfaces */
+        if ((itr == interface_list.end()) && (intf.rfind("VXLAN", 0) != 0) && (intf.rfind("docker0", 0) != 0)) {
             continue;
         }
 
@@ -908,6 +939,9 @@ void pkt_in_callback(evutil_socket_t fd, short event, void *arg) {
             if (vlan == vlan_map.end()) {
                 if (intf.find(CLIENT_IF_PREFIX) != std::string::npos) {
                     syslog(LOG_WARNING, "[DHCPV4_RELAY] Invalid input interface %s\n", interface_name);
+                } else if ((m_config.is_SmartSwitch) && (intf.rfind("dpu", 0) == 0) && !m_config.midplane_bridge.empty()) {
+                    // if its SmartSwitch, we need to check for bridge_midplane interface
+                    vlan_str = m_config.midplane_bridge;
                 }
             } else {
                 vlan_str = vlan->second;

dhcp4relay/src/dhcp4relay.cpp

@@ -162,6 +162,8 @@ struct metadata_config {
     std::string hostname = "sonic";
     uint32_t deployment_id;
     bool is_dualTor;
+    bool is_SmartSwitch;
+    std::string midplane_bridge;
 };
 
 /**

dhcp4relay/src/dhcp4relay.h

@@ -59,12 +59,13 @@ void DHCPMgr::handle_swss_notification() {
     swss::SubscriberStateTable config_db_portchannel_table(config_db_ptr.get(), "PORTCHANNEL_INTERFACE");
     swss::SubscriberStateTable config_db_device_metadata_table(config_db_ptr.get(), "DEVICE_METADATA");
     swss::SubscriberStateTable config_db_vlan_member_table(config_db_ptr.get(), "VLAN_MEMBER");
-    swss::SubscriberStateTable config_db_vlan_interface_table(config_db_ptr.get(), "VLAN_INTERFACE");
     swss::SubscriberStateTable config_db_feature_table(config_db_ptr.get(), "FEATURE");
     swss::SubscriberStateTable config_db_vlan_table(config_db_ptr.get(), "VLAN");
     config_db_dhcp_server_ipv4_ptr = std::make_shared<swss::SubscriberStateTable>(config_db_ptr.get(), "DHCP_SERVER_IPV4");
     state_db_dhcp_server_ipv4_ip_ptr = std::make_shared<swss::SubscriberStateTable>(state_db_ptr.get(), "DHCP_SERVER_IPV4_SERVER_IP");
     swss::SubscriberStateTable config_db_port_table(config_db_ptr.get(), "PORT");
+    swss::SubscriberStateTable config_db_dpu_table(config_db_ptr.get(), "DPUS");
+    swss::SubscriberStateTable state_db_interface_table(state_db_ptr.get(), "INTERFACE_TABLE");
 
     std::deque<swss::KeyOpFieldsValuesTuple> entries;
     swss::Select swss_select;
@@ -74,12 +75,13 @@ void DHCPMgr::handle_swss_notification() {
     swss_select.addSelectable(&config_db_portchannel_table);
     swss_select.addSelectable(&config_db_device_metadata_table);
     swss_select.addSelectable(&config_db_vlan_member_table);
-    swss_select.addSelectable(&config_db_vlan_interface_table);
     swss_select.addSelectable(&config_db_feature_table);
     swss_select.addSelectable(&config_db_vlan_table);
     swss_select.addSelectable(config_db_dhcp_server_ipv4_ptr.get());
     swss_select.addSelectable(state_db_dhcp_server_ipv4_ip_ptr.get());
     swss_select.addSelectable(&config_db_port_table);
+    swss_select.addSelectable(&config_db_dpu_table);
+    swss_select.addSelectable(&state_db_interface_table);
 
     while (!stop_thread) {
         swss::Selectable *selectable;
@@ -125,8 +127,8 @@ void DHCPMgr::handle_swss_notification() {
         } else if (selectable == static_cast<swss::Selectable *>(&config_db_vlan_member_table)) {
             config_db_vlan_member_table.pops(entries);
             process_vlan_member_notification(entries);
-        } else if (selectable == static_cast<swss::Selectable *>(&config_db_vlan_interface_table)) {
-            config_db_vlan_interface_table.pops(entries);
+        } else if (selectable == static_cast<swss::Selectable *>(&state_db_interface_table)) {
+            state_db_interface_table.pops(entries);
             process_vlan_interface_notification(entries);
         } else if (selectable == static_cast<swss::Selectable *>(&config_db_feature_table)) {
             config_db_feature_table.pops(entries);
@@ -137,6 +139,9 @@ void DHCPMgr::handle_swss_notification() {
 	} else if (selectable == static_cast<swss::Selectable *>(&config_db_port_table)) {
             config_db_port_table.pops(entries);
             process_port_notification(entries);
+	} else if (selectable == static_cast<swss::Selectable *>(&config_db_dpu_table)) {
+            config_db_dpu_table.pops(entries);
+            process_port_notification(entries);
 	}
     }
 }
@@ -157,6 +162,8 @@ void DHCPMgr::process_device_metadata_notification(std::deque<swss::KeyOpFieldsV
         std::string key = kfvKey(entry);
         std::vector<swss::FieldValueTuple> field_values = kfvFieldsValues(entry);
         std::string operation = kfvOp(entry);
+        std::shared_ptr<swss::DBConnector> config_db = std::make_shared<swss::DBConnector>("CONFIG_DB", 0);
+        swss::Table midplane_tbl(config_db.get(), "MID_PLANE_BRIDGE");
 
         if (key != "localhost") {
             continue;
@@ -191,6 +198,20 @@ void DHCPMgr::process_device_metadata_notification(std::deque<swss::KeyOpFieldsV
                 send_dualTor_event = true;
             }
 
+            // Handle is_SmartSwitch logic
+            if (subtype_found && subtype_value == "SmartSwitch") {
+                m_config.is_SmartSwitch = true;
+                std::string bridge_name;
+                bool ok = midplane_tbl.hget("GLOBAL", "bridge", bridge_name);
+                if (ok) {
+                    m_config.midplane_bridge = bridge_name;
+                } else {
+                    syslog(LOG_ERR, "Failed to read midplane bridge name\n");
+                }
+            } else if (m_config.is_SmartSwitch) {
+                m_config.is_SmartSwitch = false;
+            }
+
             if (send_dualTor_event) {
                 relay_config *relay_msg = nullptr;
                 try {
@@ -599,7 +620,11 @@ void DHCPMgr::process_vlan_member_notification(std::deque<swss::KeyOpFieldsValue
 
 void DHCPMgr::process_vlan_interface_notification(std::deque<swss::KeyOpFieldsValuesTuple> &entries) {
      for (auto &entry : entries) {
-        std::string key = kfvKey(entry);
+         std::string key = kfvKey(entry);
+         // Only process VLAN interfaces (keys starting with "Vlan" and Vlan with IP suffix)
+         if (key.rfind("Vlan", 0) != 0) {
+             continue;
+         }
 
          std::string vlan;
          std::string vrf;
@@ -608,7 +633,7 @@ void DHCPMgr::process_vlan_interface_notification(std::deque<swss::KeyOpFieldsVa
              vlan = key;
              vrf = "default";
              for (auto &fv : kfvFieldsValues(entry)) {
-                 if (fvField(fv) == "vrf_name") {
+                 if (fvField(fv) == "vrf") {
                      vrf = fvValue(fv);
                      break;
                  }
@@ -720,8 +745,10 @@ void DHCPMgr::process_dhcp_server_ipv4_notification(std::deque<swss::KeyOpFields
 	}
 
 	/*Validation to check vlan is present in VLAN table or not */
+	/*If its a smartswitch, we are checking midplane_bridge details */
 	std::string value;
-        if (!vlan_tbl.hget(vlan, "vlanid", value)) {
+        if ((!vlan_tbl.hget(vlan, "vlanid", value))
+              && (!m_config.is_SmartSwitch || (!m_config.midplane_bridge.empty() && m_config.midplane_bridge != vlan))) {
             delete relay_msg;
             continue;
         }
@@ -808,6 +835,9 @@ void DHCPMgr::process_port_notification(std::deque<swss::KeyOpFieldsValuesTuple>
                  if (fvField(fv) == "alias") {
                      port_msg->alias = fvValue(fv);
                      break;
+                 } else if (fvField(fv) == "midplane_interface") {
+                     port_msg->alias = fvValue(fv);
+                     break;
                  }
             }
         } else {

dhcp4relay/src/dhcp4relay_mgr.cpp

@@ -24,7 +24,7 @@ using namespace swss;
 MOCK_GLOBAL_FUNC1(getifaddrs, int(struct ifaddrs **));
 MOCK_GLOBAL_FUNC1(freeifaddrs, void(struct ifaddrs *));
 MOCK_GLOBAL_FUNC3(write, ssize_t(int, const void*, size_t));
-MOCK_GLOBAL_FUNC6(send_udp, bool(int, uint8_t *, struct sockaddr_in, uint32_t, in_addr, bool));
+MOCK_GLOBAL_FUNC7(send_udp, bool(int, uint8_t *, struct sockaddr_in, uint32_t, in_addr, bool, bool));
 
 void encode_relay_option(pcpp::DhcpLayer *dhcp_pkt, relay_config *config);
 void to_client(pcpp::DhcpLayer* dhcp_pkt, std::unordered_map<std::string, relay_config > *vlans,
@@ -851,8 +851,8 @@ TEST(DHCPRelayTest, to_client) {
     struct ifaddrs *mock_ifaddrs = CreateMockIfaddrs("192.168.1.1", "255.255.255.0", "Vlan100", "192.168.1.2", "Ethernet4");
     EXPECT_GLOBAL_CALL(getifaddrs, getifaddrs(_)).WillOnce(DoAll(testing::SetArgPointee<0>(mock_ifaddrs), Return(0)));
     EXPECT_GLOBAL_CALL(freeifaddrs, freeifaddrs(_)).Times(1);
-    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _)).WillOnce([]
-		(int sock, uint8_t* hdr, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip) {
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([]
+		(int sock, uint8_t* hdr, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip, bool pad) {
         pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr;
         EXPECT_EQ((dhcp_hdr->opCode), 1);
         EXPECT_EQ((dhcp_hdr->hops), 1);
@@ -891,8 +891,8 @@ TEST(DHCPRelayTest, from_client) {
     m_config.host_mac_addr = "12:32:54:24:95:36";
     encode_relay_option(&dhcpLayer, &config);
 
-    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _)).WillOnce([]
-		    (int sock, uint8_t* hdr, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip) {
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([]
+		    (int sock, uint8_t* hdr, struct sockaddr_in target, uint32_t len, in_addr src_ip, bool use_src_ip, bool pad) {
         pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr;
         EXPECT_EQ((dhcp_hdr->opCode), 0);
         EXPECT_EQ((dhcp_hdr->hops), 1);