Merge branch 'master' into pr-template-standardization

oleksandrivantsiv · web-flow · commit ab65ded4a389 · 2026-05-21T15:17:15.000-07:00
diff --git a/dhcp4relay/src/dhcp4_sender.cpp b/dhcp4relay/src/dhcp4_sender.cpp
@@ -2,7 +2,7 @@
 
 #include <arpa/inet.h>
 #include <errno.h>
-#include <syslog.h>
+#include "logger.h"
 
 #include <cstring>
 
@@ -63,14 +63,14 @@ bool send_udp(int sock, uint8_t *buffer, struct sockaddr_in target, uint32_t len
         if (sendmsg(sock, &msg, 0) == -1) {
             char server_addr[INET_ADDRSTRLEN];
             inet_ntop(AF_INET, &(target.sin_addr), server_addr, INET_ADDRSTRLEN);
-            syslog(LOG_ERR, "sendmsg: Failed to send to target address: %s, error: %s\n", server_addr, strerror(errno));
+            SWSS_LOG_ERROR("sendmsg: Failed to send to target address: %s, error: %s", server_addr, strerror(errno));
             return false;
         }
     } else {
         if (sendto(sock, buffer, len, 0, (const struct sockaddr *)&target, sizeof(target)) == -1) {
             char server_addr[INET_ADDRSTRLEN];
             inet_ntop(AF_INET, &(target.sin_addr), server_addr, INET_ADDRSTRLEN);
-            syslog(LOG_ERR, "sendto: Failed to send to target address: %s, error: %s\n", server_addr, strerror(errno));
+            SWSS_LOG_ERROR("sendto: Failed to send to target address: %s, error: %s", server_addr, strerror(errno));
             return false;
         }
     }
diff --git a/dhcp4relay/src/dhcp4relay.cpp b/dhcp4relay/src/dhcp4relay.cpp
diff --git a/dhcp4relay/src/dhcp4relay.h b/dhcp4relay/src/dhcp4relay.h
@@ -9,7 +9,7 @@
 #include <netinet/if_ether.h>
 #include <netinet/ip.h>
 #include <netinet/udp.h>
-#include <syslog.h>
+#include "logger.h"
 
 #include <map>
 #include <string>
@@ -27,6 +27,11 @@
 #define DHCPv4_OPTION_LIMIT 255
 #define RAWSOCKET_RECV_SIZE 1048576
 #define CLIENT_IF_PREFIX "Ethernet"
+#define VRF_NAME_FIELD "vrf_name"  // typo "vrf" caused VRF-update miss; field is "vrf_name"
+#define SERVER_VRF_FIELD "server_vrf"
+#define SOURCE_INTERFACE_FIELD "source_interface"
+#define LINK_SELECTION_FIELD "link_selection"
+#define STATE_FIELD "state"
 #define BUFFER_SIZE 9200        // TODO: change to dynamic size based on MTU
 #define MAX_DHCP_PKT_SIZE 1472  // 1500 - (IP + UDP)headers
 #define MAC_ADDR_STR_LEN 17
@@ -104,7 +109,6 @@ struct relay_config {
     sockaddr_in link_address_netmask;
     sockaddr_in src_intf_sel_addr;
     uint32_t link_ifindex;
-    std::shared_ptr<swss::DBConnector> state_db;
     std::string vlan;
     std::string phy_interface;
     std::string vrf;  // This is server VRF.
@@ -118,7 +122,6 @@ struct relay_config {
     std::vector<sockaddr_in> servers_sock;
     bool is_interface_id;
     bool is_add;
-    std::shared_ptr<swss::DBConnector> config_db;
 };
 
 typedef enum {
@@ -132,7 +135,16 @@ typedef enum {
     DHCPv4_SERVER_IP_UPDATE,
     DHCPv4_SERVER_IP_DELETE,
     DHCPv4_RELAY_DUAL_TOR_UPDATE,
-    DHCPv4_RELAY_PORT_UPDATE
+    DHCPv4_RELAY_PORT_UPDATE,
+    /*
+     * General-purpose main<->mgr synchronisation barrier on
+     * config_pipe. The mgr thread emits one to mark a coherent
+     * point in the event stream; the main thread blocks until it
+     * consumes one. Currently used to gate pkt_in_callback on the
+     * initial DHCPV4_RELAY snapshot. Reusable for Redis-reconnect
+     * resync, atomic config replace, etc.
+     */
+    DHCPv4_RELAY_SYNC_BARRIER
 } event_type;
 
 struct event_config {
diff --git a/dhcp4relay/src/dhcp4relay_mgr.cpp b/dhcp4relay/src/dhcp4relay_mgr.cpp
diff --git a/dhcp4relay/src/dhcp4relay_stats.cpp b/dhcp4relay/src/dhcp4relay_stats.cpp
@@ -73,7 +73,12 @@ void update_interface_counters_in_db(
 
     // Populate existing counters
     for (const auto& field : existing_fields) {
-        counter_map[fvField(field)] = std::stoi(fvValue(field));
+        try {
+            counter_map[fvField(field)] = std::stoi(fvValue(field));
+        } catch (const std::exception &e) {
+            SWSS_LOG_WARN("[DHCPV4_RELAY] update_interface_counters_in_db: invalid counter value '%s' for field '%s': %s",
+                   fvValue(field).c_str(), fvField(field).c_str(), e.what());
+        }
     }
 
     // Update with new values
@@ -135,7 +140,7 @@ void DHCPCounter_table::db_update_loop() {
                 }
             }
         }
-        syslog(LOG_INFO, "DHCPV4_RELAY: DHCPCounter_table::db_update_loop() : Data Updated to DB \n");
+        SWSS_LOG_INFO("DHCPV4_RELAY: DHCPCounter_table::db_update_loop() : Data Updated to DB ");
     }
 }
 
@@ -198,13 +203,27 @@ void DHCPCounter_table::initialize_interface(const std::string& interface) {
 void DHCPCounter_table::increment_counter(const std::string& interface,
                                         const std::string& direction,
                                         int msg_type) {
-    std::string type = counter_map.find(msg_type)->second;
-    // Initialize counters if not present
-    if (interfaces_cntr_table.find(interface) == interfaces_cntr_table.end())
-        DHCPCounter_table::initialize_interface(interface);
+    auto type_itr = counter_map.find(msg_type);
+    if (type_itr == counter_map.end()) {
+        SWSS_LOG_WARN("[DHCPV4_RELAY] increment_counter: unknown msg_type %d, counting as Unknown", msg_type);
+        type_itr = counter_map.find(DHCPv4_MESSAGE_TYPE_UNKNOWN);
+        if (type_itr == counter_map.end()) {
+            return;
+        }
+    }
+    const std::string &type = type_itr->second;
 
     std::lock_guard<std::mutex> lock(interfaces_mutex);
 
+    if (interfaces_cntr_table.find(interface) == interfaces_cntr_table.end()) {
+        DHCPCounters counter;
+        for (const auto& t : counter_map) {
+            counter.RX[t.second] = 0;
+            counter.TX[t.second] = 0;
+        }
+        interfaces_cntr_table[interface] = counter;
+    }
+
     if (direction == "RX") {
         interfaces_cntr_table[interface].RX[type]++;
     } else if (direction == "TX") {
diff --git a/dhcp4relay/src/main.cpp b/dhcp4relay/src/main.cpp
@@ -1,5 +1,5 @@
 #include <stdlib.h>
-#include <syslog.h>
+#include "logger.h"
 
 #include <unordered_map>
 
@@ -9,11 +9,12 @@ bool dual_tor_sock = false;
 char loopback[IF_NAMESIZE] = "Loopback0";
 
 int main(int argc, char *argv[]) {
+    swss::Logger::linkToDbNative("dhcp4relay");
     try {
         std::unordered_map<std::string, relay_config> vlans;
         loop_relay(vlans);
     } catch (std::exception &e) {
-        syslog(LOG_ERR, "An exception occurred.\n");
+        SWSS_LOG_ERROR("An exception occurred.");
         return 1;
     }
     return 0;
diff --git a/dhcp4relay/test/mock_relay.cpp b/dhcp4relay/test/mock_relay.cpp
@@ -333,8 +333,11 @@ TEST(relayConfig, handle_interface_events) {
                      .Times(AtLeast(1))
                      .WillRepeatedly(Invoke(RealWrite));
     ASSERT_NE(pipe(pipe_fds), -1);
-    relay_config *config = new relay_config();
+
     std::unordered_map<std::string, relay_config> vlans;
+    vlans["Vlan100"].vlan = "Vlan100";
+
+    relay_config *config = new relay_config();
     config->vlan = "Vlan100";
     config->is_add = true;
     config->src_intf_sel_addr.sin_family = AF_INET;
@@ -370,8 +373,41 @@ TEST(relayConfig, handle_interface_events) {
     close(pipe_fds[1]);
 }
 
+TEST(relayConfig, handle_interface_events_unknown_vlan) {
+    int pipe_fds[2];
+    EXPECT_GLOBAL_CALL(write, write(_, _, _))
+                     .Times(AtLeast(1))
+                     .WillRepeatedly(Invoke(RealWrite));
+    ASSERT_NE(pipe(pipe_fds), -1);
+
+    std::unordered_map<std::string, relay_config> vlans;
+
+    relay_config *config = new relay_config();
+    config->vlan = "Vlan999";
+    config->is_add = true;
+    config->src_intf_sel_addr.sin_family = AF_INET;
+    config->src_intf_sel_addr.sin_addr.s_addr = inet_addr("10.0.0.1");
+
+    event_config event;
+    event.type = DHCPv4_RELAY_INTERFACE_UPDATE;
+    event.msg = static_cast<void *>(config);
+
+    ASSERT_NE(write(pipe_fds[1], &event, sizeof(event)), -1);
+
+    config_event_callback(pipe_fds[0], 0, &vlans);
+
+    EXPECT_TRUE(vlans.find("Vlan999") == vlans.end());
+
+    close(pipe_fds[0]);
+    close(pipe_fds[1]);
+}
+
 TEST(relayConfig, handle_vlan_member_events) {
+    struct ifaddrs *mock_ifaddrs = CreateMockIfaddrs("192.168.1.1", "255.255.255.0", "Vlan100", "192.168.1.2", "Ethernet4");
     int pipe_fds[2];
+    EXPECT_GLOBAL_CALL(getifaddrs, getifaddrs(_))
+        .WillRepeatedly(DoAll(testing::SetArgPointee<0>(mock_ifaddrs), Return(0)));
+    EXPECT_GLOBAL_CALL(freeifaddrs, freeifaddrs(_)).Times(AtLeast(2));
     EXPECT_GLOBAL_CALL(write, write(_, _, _))
                      .Times(AtLeast(1))
                      .WillRepeatedly(Invoke(RealWrite));
@@ -417,6 +453,7 @@ TEST(relayConfig, handle_vlan_member_events) {
 
     close(pipe_fds[0]);
     close(pipe_fds[1]);
+    FreeMockIfaddrs(mock_ifaddrs);
 }
 
 TEST(relayConfig, handle_vlan_interface_events) {
@@ -937,3 +974,100 @@ TEST(DHCPRelayTest, from_client) {
     });
     from_client(&dhcpLayer, config);
 }
+
+/* Helper: build a relay-of-relay packet (giaddr already set) with a pre-existing Option 82. */
+static relay_config make_relay_of_relay_config(const std::string &agent_relay_mode) {
+    interface_list.push_back("Ethernet12");
+    phy_interface_alias_map["Ethernet12"] = "eth12";
+
+    relay_config config = {};
+    config.phy_interface = "Ethernet12";
+    config.vlan = "Vlan10";
+    config.agent_relay_mode = agent_relay_mode;
+    config.max_hop_count = 16;
+    struct sockaddr_in addr = {0};
+    addr.sin_family = AF_INET;
+    addr.sin_addr.s_addr = inet_addr("192.168.20.100");
+    config.servers_sock = {addr};
+    config.servers = {"192.168.20.100"};
+    config.link_address.sin_addr.s_addr = inet_addr("192.168.10.10");
+    config.link_address_netmask.sin_addr.s_addr = inet_addr("255.255.255.0");
+    vlan_vrf_map["Vlan10"] = "Vrf01";
+    m_config.host_mac_addr = "12:32:54:24:95:36";
+    return config;
+}
+
+/* agent_relay_mode=append: packet already has Option 82; we should append ours and forward. */
+TEST(DHCPRelayTest, from_client_relay_of_relay_append) {
+    relay_config config = make_relay_of_relay_config("append");
+
+    pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75"));
+    pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac);
+    dhcpLayer.getDhcpHeader()->hops = 0;
+    /* Non-zero giaddr signals relay-of-relay path */
+    dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1");
+    encode_relay_option(&dhcpLayer, &config);
+
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([]
+            (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) {
+        pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr;
+        EXPECT_EQ(dhcp_hdr->hops, 1);
+        EXPECT_EQ(dhcp_hdr->gatewayIpAddress, inet_addr("192.168.1.1"));
+        return true;
+    });
+    from_client(&dhcpLayer, config);
+}
+
+/* agent_relay_mode=replace: existing Option 82 stripped, ours added, packet forwarded. */
+TEST(DHCPRelayTest, from_client_relay_of_relay_replace) {
+    relay_config config = make_relay_of_relay_config("replace");
+
+    pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75"));
+    pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac);
+    dhcpLayer.getDhcpHeader()->hops = 0;
+    dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1");
+    encode_relay_option(&dhcpLayer, &config);
+
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([]
+            (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) {
+        pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr;
+        EXPECT_EQ(dhcp_hdr->hops, 1);
+        EXPECT_EQ(dhcp_hdr->gatewayIpAddress, inet_addr("192.168.1.1"));
+        return true;
+    });
+    from_client(&dhcpLayer, config);
+}
+
+/* agent_relay_mode=forward: packet forwarded unchanged, Option 82 not modified. */
+TEST(DHCPRelayTest, from_client_relay_of_relay_forward) {
+    relay_config config = make_relay_of_relay_config("forward");
+
+    pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75"));
+    pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac);
+    dhcpLayer.getDhcpHeader()->hops = 0;
+    dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1");
+    encode_relay_option(&dhcpLayer, &config);
+
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).WillOnce([]
+            (int, uint8_t* hdr, struct sockaddr_in, uint32_t, in_addr, bool, bool) {
+        pcpp::dhcp_header* dhcp_hdr = (pcpp::dhcp_header*)hdr;
+        EXPECT_EQ(dhcp_hdr->hops, 1);
+        EXPECT_EQ(dhcp_hdr->gatewayIpAddress, inet_addr("192.168.1.1"));
+        return true;
+    });
+    from_client(&dhcpLayer, config);
+}
+
+/* agent_relay_mode=discard: packet must be dropped, send_udp must NOT be called. */
+TEST(DHCPRelayTest, from_client_relay_of_relay_discard) {
+    relay_config config = make_relay_of_relay_config("discard");
+
+    pcpp::MacAddress clientMac(std::string("00:0e:86:11:c0:75"));
+    pcpp::DhcpLayer dhcpLayer(pcpp::DHCP_DISCOVER, clientMac);
+    dhcpLayer.getDhcpHeader()->hops = 0;
+    dhcpLayer.getDhcpHeader()->gatewayIpAddress = inet_addr("192.168.1.1");
+    encode_relay_option(&dhcpLayer, &config);
+
+    EXPECT_GLOBAL_CALL(send_udp, send_udp(_, _, _, _, _, _, _)).Times(0);
+    from_client(&dhcpLayer, config);
+}
diff --git a/dhcp6relay/src/relay.cpp b/dhcp6relay/src/relay.cpp
@@ -947,7 +947,7 @@ void client_packet_handler(uint8_t *buffer, ssize_t length, struct relay_config
         const struct ip6_ext *ext_header;
         do {
             ext_header = (const struct ip6_ext *)current_position;
-            current_position += ext_header->ip6e_len;
+            current_position += (ext_header->ip6e_len + 1) * 8;
             if((current_position == prev) ||
                (current_position + sizeof(*ext_header) >= buffer_end)) {
                 return;
diff --git a/dhcp6relay/test/mock_relay.cpp b/dhcp6relay/test/mock_relay.cpp
@@ -721,6 +721,18 @@ TEST(relay, client_packet_handler) {
     0x00, 0x00, 0x00, 0x19, 0x00, 0x0c, 0x27, 0xfe, 0x8f, 0x95, 0x00, 0x00, 0x0e, 0x10, 0x00, 0x00,
     0x15, 0x18
   };
+  // Packet with Hop-by-Hop extension header (ip6e_len=1 => 16 bytes per RFC 2460: (1+1)*8=16).
+  // Tests that the extension header pointer arithmetic is correct for nonzero ip6e_len.
+  // IPv6 next_header=0x00 (HbH), ext: nxt=0x11(UDP), len=0x01 -> advance 16 bytes -> UDP dst=547
+  uint8_t client_raw_solicit_with_hbh_extension[] = {
+    0x33, 0x33, 0x00, 0x01, 0x00, 0x02, 0x08, 0x00, 0x27, 0xfe, 0x8f, 0x95, 0x86, 0xdd, 0x60, 0x00,
+    0x00, 0x00, 0x00, 0x1c, 0x00, 0x01, 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00,
+    0x27, 0xff, 0xfe, 0xfe, 0x8f, 0x95, 0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x01, 0x00, 0x02, 0x11, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x22, 0x02, 0x23, 0x00, 0x0c, 0x00, 0x00, 0x01, 0x10,
+    0x08, 0x74
+  };
+
   uint8_t non_udp_with_externsion[] = {
     0x33, 0x33, 0x00, 0x01, 0x00, 0x02, 0x08, 0x00, 0x27, 0xfe, 0x8f, 0x95, 0x86, 0xdd, 0x60, 0x00,
     0x00, 0x00, 0x00, 0x44, 0x2c, 0x01, 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0a, 0x00,
@@ -745,6 +757,9 @@ TEST(relay, client_packet_handler) {
   ASSERT_NO_THROW(client_packet_handler(client_raw_solicit_with_externsion, sizeof(client_raw_solicit_with_externsion), &config, ifname));
 
   ASSERT_NO_THROW(client_packet_handler(non_udp_with_externsion, sizeof(non_udp_with_externsion), &config, ifname));
+
+  // nonzero ip6e_len=1: must advance (1+1)*8=16 bytes per RFC 2460, not 1 byte
+  ASSERT_NO_THROW(client_packet_handler(client_raw_solicit_with_hbh_extension, sizeof(client_raw_solicit_with_hbh_extension), &config, ifname));
 }
 
 MOCK_GLOBAL_FUNC6(recvfrom, ssize_t(int, void *, size_t, int, struct sockaddr *, socklen_t *));
