[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 <[email protected]>

Author: cshivashgit

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/src/dhcp4_sender.cpp

@@ -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.h

@@ -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/dhcp4relay.cpp

@@ -577,7 +577,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 +635,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 +732,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 +902,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;
         }
 

dhcp4relay/src/dhcp4relay_mgr.cpp

@@ -59,12 +59,12 @@ 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 state_db_interface_table(state_db_ptr.get(), "INTERFACE_TABLE");
 
     std::deque<swss::KeyOpFieldsValuesTuple> entries;
     swss::Select swss_select;
@@ -74,12 +74,12 @@ 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(&state_db_interface_table);
 
     while (!stop_thread) {
         swss::Selectable *selectable;
@@ -125,8 +125,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);
@@ -599,7 +599,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 +612,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;
                  }

dhcp4relay/test/mock_relay.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);