From a5f3574f616b40036cd169f8a25ea005425604c8 Mon Sep 17 00:00:00 2001 From: mssonicbld <79238446+mssonicbld@users.noreply.github.com> Date: Thu, 9 Jan 2025 11:56:24 +0800 Subject: [PATCH] Add wait and check ip address check when dhcp6relay init (#52) (#63) Why I did it There is wait in wait_for_intf.sh inside dhcp_relay container wait_for_intf.sh.j2 to wait all vlans' ipv6 link local address to be ready. Only after that, related dhcp relay processes could be started (i.e. dhcrelay, dhcp6relay, dhcpmon) If ipv6 link local address for one vlan is not ready, it would block all vlans' dhcpv4 and dhcpv6 relay, which is unexpected. Work item tracking Microsoft ADO (number only): 30491632 How I did it Add wait LLA in dhcp6relay process with crrent PR Add libevent timer to periodicly check LLA and initialization for no-ready Vlan, after they are all ready, delete the timer. With this change, dhcp6relay would only wait vlans whose LLA are not ready, and vlans whose LLA are ready would not be blocked. Remove wait logic in wait script by this PR: [dhcp_relay] Remove wait LLA in wati_for_intf.sh in dhcp_relay container sonic-buildimage#21182 How to verify it Install new dhcp6relay, and run below test: In 2 Vlans scenraio (Vlan1000 and Vlan2000), remove lla for Vlan2000, then restart dhcp6relay. Then dhcp6relay successfully bind 547 for Vlan1000. After adding lla back for Vlan2000, we can see dhcp6relay successfully bind 547 for Vlan2000. With new dhcp6relay, run dhcpv6_relay tests and all passed https://github.com/sonic-net/sonic-mgmt/blob/master/tests/dhcp_relay/test_dhcpv6_relay.py Co-authored-by: Yaqiang Zhu --- src/config_interface.cpp | 71 +++++++++++++--- src/config_interface.h | 29 +++++-- src/relay.cpp | 144 +++++++++++++++++++++++++-------- src/relay.h | 14 ++++ test/mock_config_interface.cpp | 17 ++-- 5 files changed, 220 insertions(+), 55 deletions(-) diff --git a/src/config_interface.cpp b/src/config_interface.cpp index e6b07fa..2834eae 100644 --- a/src/config_interface.cpp +++ b/src/config_interface.cpp @@ -21,7 +21,7 @@ void initialize_swss(std::unordered_map &vlans) std::shared_ptr configDbPtr = std::make_shared ("CONFIG_DB", 0); swss::SubscriberStateTable ipHelpersTable(configDbPtr.get(), "DHCP_RELAY"); swssSelect.addSelectable(&ipHelpersTable); - get_dhcp(vlans, &ipHelpersTable, false); + get_dhcp(vlans, &ipHelpersTable, false, configDbPtr); } catch (const std::bad_alloc &e) { syslog(LOG_ERR, "Failed allocate memory. Exception details: %s", e.what()); @@ -53,13 +53,15 @@ void deinitialize_swss() /** - * @code void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic) + * @code void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic, + std::shared_ptr config_db) * * @brief initialize and get vlan table information from DHCP_RELAY * * @return none */ -void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic) { +void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic, + std::shared_ptr config_db) { swss::Selectable *selectable; int ret = swssSelect.select(&selectable, DEFAULT_TIMEOUT_MSEC); if (ret == swss::Select::ERROR) { @@ -69,7 +71,7 @@ void get_dhcp(std::unordered_map &vlans, swss::Subscr } if (selectable == static_cast (ipHelpersTable)) { if (!dynamic) { - handleRelayNotification(*ipHelpersTable, vlans); + handleRelayNotification(*ipHelpersTable, vlans, config_db); } else { syslog(LOG_WARNING, "relay config changed, " "need restart container to take effect"); @@ -78,7 +80,8 @@ void get_dhcp(std::unordered_map &vlans, swss::Subscr } /** - * @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans) + * @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans, + * std::shared_ptr config_db) * * @brief handles DHCPv6 relay configuration change notification * @@ -87,16 +90,18 @@ void get_dhcp(std::unordered_map &vlans, swss::Subscr * * @return none */ -void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans) +void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans, + std::shared_ptr config_db) { std::deque entries; ipHelpersTable.pops(entries); - processRelayNotification(entries, vlans); + processRelayNotification(entries, vlans, config_db); } /** - * @code void processRelayNotification(std::deque &entries, std::unordered_map vlans) + * @code void processRelayNotification(std::deque &entries, std::unordered_map vlans, + std::shared_ptr config_db) * * @brief process DHCPv6 relay servers and options configuration change notification * @@ -105,7 +110,8 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un * * @return none */ -void processRelayNotification(std::deque &entries, std::unordered_map &vlans) +void processRelayNotification(std::deque &entries, std::unordered_map &vlans, + std::shared_ptr config_db) { std::vector servers; bool option_79_default = true; @@ -119,6 +125,27 @@ void processRelayNotification(std::deque &entries, std::string vlan = kfvKey(entry); std::string operation = kfvOp(entry); std::vector fieldValues = kfvFieldsValues(entry); + bool has_ipv6_address = false; + + const std::string match_pattern = "VLAN_INTERFACE|" + vlan + "|*"; + auto keys = config_db->keys(match_pattern); + for (const auto &itr : keys) { + auto found = itr.find_last_of('|'); + if (found == std::string::npos) { + syslog(LOG_WARNING, "%s doesn't exist in VLAN_INTERFACE table, skip it", vlan.c_str()); + continue; + } + std::string ip_address = itr.substr(found + 1); + if (ip_address.find(":") != std::string::npos) { + has_ipv6_address = true; + break; + } + } + + if (!has_ipv6_address) { + syslog(LOG_WARNING, "%s doesn't have IPv6 address configured, skip it", vlan.c_str()); + continue; + } relay_config intf; intf.is_option_79 = option_79_default; @@ -126,6 +153,7 @@ void processRelayNotification(std::deque &entries, intf.interface = vlan; intf.mux_key = ""; intf.state_db = nullptr; + intf.is_lla_ready = false; for (auto &fieldValue: fieldValues) { std::string f = fvField(fieldValue); std::string v = fvValue(fieldValue); @@ -154,3 +182,28 @@ void processRelayNotification(std::deque &entries, vlans[vlan] = intf; } } + +/** + * @code bool check_is_lla_ready(std::string vlan) + * + * @brief Check whether link local address appear in vlan interface + * + * @param vlan string of vlan name + * + * @return bool value indicates whether lla ready + */ +bool check_is_lla_ready(std::string vlan) { + const std::string cmd = "ip -6 addr show " + vlan + " scope link 2> /dev/null"; + std::array buffer; + std::string result; + std::unique_ptr pipe(popen(cmd.c_str(), "r"), pclose); + if (pipe) { + while (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) { + result += buffer.data(); + } + if (!result.empty()) { + return true; + } + } + return false; +} diff --git a/src/config_interface.h b/src/config_interface.h index 019838a..31d25e6 100644 --- a/src/config_interface.h +++ b/src/config_interface.h @@ -33,16 +33,19 @@ void initialize_swss(std::unordered_map &vlans); void deinitialize_swss(); /** - * @code void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic) + * @code void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic, + * std::shared_ptr config_db) * * @brief initialize and get vlan information from DHCP_RELAY * * @return none */ -void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic); +void get_dhcp(std::unordered_map &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic, + std::shared_ptr config_db); /** - * @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans) + * @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans, + * std::shared_ptr config_db) * * @brief handles DHCPv6 relay configuration change notification * @@ -51,10 +54,12 @@ void get_dhcp(std::unordered_map &vlans, swss::Subscr * * @return none */ -void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans); +void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map &vlans, + std::shared_ptr config_db); /** - * @code void processRelayNotification(std::deque &entries, std::unordered_map &vlans) + * @code void processRelayNotification(std::deque &entries, std::unordered_map &vlans, + * std::shared_ptr config_db) * * @brief process DHCPv6 relay servers and options configuration change notification * @@ -63,4 +68,16 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un * * @return none */ -void processRelayNotification(std::deque &entries, std::unordered_map &vlans); +void processRelayNotification(std::deque &entries, std::unordered_map &vlans, + std::shared_ptr config_db); + +/** + * @code bool check_is_lla_ready(std::string vlan) + * + * @brief Check whether link local address appear in vlan interface + * + * @param vlan string of vlan name + * + * @return bool value indicates whether lla ready + */ +bool check_is_lla_ready(std::string vlan); diff --git a/src/relay.cpp b/src/relay.cpp index 10fe635..d2b29b7 100644 --- a/src/relay.cpp +++ b/src/relay.cpp @@ -895,6 +895,8 @@ void client_callback(evutil_socket_t fd, short event, void *arg) { } std::string intf(interfaceName); + // For Vlans that lla is not ready, they wouldn't be added into vlan_map, hence it would be blocked here, no need to + // add is_lla_ready flag check in this callback func auto vlan = vlan_map.find(intf); if (vlan == vlan_map.end()) { if (intf.find(CLIENT_IF_PREFIX) != std::string::npos) { @@ -1088,6 +1090,10 @@ void server_callback_dualtor(evutil_socket_t fd, short event, void *arg) { syslog(LOG_WARNING, "Invalid DHCPv6 header content on loopback socket, packet will be dropped\n"); continue; } + if (!config->is_lla_ready) { + syslog(LOG_WARNING, "Link local address for %s is not ready, packet will be dropped\n", config->interface.c_str()); + continue; + } auto loopback_str = std::string(loopback); increase_counter(config->state_db, loopback_str, msg_type); relay_relay_reply(server_recv_buffer, buffer_sz, config); @@ -1279,40 +1285,29 @@ void loop_relay(std::unordered_map &vlans) { } } - for(auto &vlan : vlans) { - int gua_sock = 0; - int lla_sock = 0; - vlan.second.config_db = config_db; - vlan.second.mux_table = mStateDbMuxTablePtr; - vlan.second.state_db = state_db; - vlan.second.mux_key = vlan_member + vlan.second.interface + "|"; - - update_vlan_mapping(vlan.first, config_db); - - initialize_counter(vlan.second.state_db, vlan.second.interface); - - if (prepare_vlan_sockets(gua_sock, lla_sock, vlan.second) != -1) { - vlan.second.gua_sock = gua_sock; - vlan.second.lla_sock = lla_sock; - vlan.second.lo_sock = lo_sock; - - sockets.push_back(gua_sock); - sockets.push_back(lla_sock); - prepare_relay_config(vlan.second, gua_sock, filter); - if (!dual_tor_sock) { - auto event = event_new(base, gua_sock, EV_READ|EV_PERSIST, - server_callback, &(vlan.second)); - if (event == NULL) { - syslog(LOG_ERR, "libevent: Failed to create server listen libevent\n"); - } - event_add(event, NULL); - syslog(LOG_INFO, "libevent: add server listen socket for %s\n", vlan.first.c_str()); - } - } else { - syslog(LOG_ERR, "Failed to create dualtor loopback listen socket"); - exit(EXIT_FAILURE); - } - } + // Add timer to periodly check lla un-ready vlan + struct event *timer_event; + struct timeval tv; + auto timer_args = new std::tuple< + std::unordered_map &, + std::shared_ptr, + std::shared_ptr, + std::shared_ptr, + std::vector, + int, + int, + struct event * + >(vlans, config_db, state_db, mStateDbMuxTablePtr, sockets, lo_sock, lo_sock, nullptr); + timer_event = event_new(base, -1, EV_PERSIST, lla_check_callback, timer_args); + std::get<7>(*timer_args) = timer_event; + evutil_timerclear(&tv); + // Check timer is set to 60s + tv.tv_sec = 60; + event_add(timer_event, &tv); + + // We set check timer to be executed every 60s, it would case that its first excution be delayed 60s, + // hence manually invoke it here to immediate execute it + lla_check_callback(-1, 0, timer_args); if(signal_init() == 0 && signal_start() == 0) { shutdown_relay(); @@ -1351,3 +1346,84 @@ void clear_counter(std::shared_ptr state_db) { state_db->del(itr); } } + +/** + * @code void lla_check_callback(evutil_socket_t fd, short event, void *arg); + * + * @brief callback for libevent timer to check whether lla is ready for vlan + * + * @param fd libevent socket + * @param event libevent triggered event + * @param arg callback argument provided by user + * + * @return none + */ +void lla_check_callback(evutil_socket_t fd, short event, void *arg) { + auto args = reinterpret_cast *, + std::shared_ptr, + std::shared_ptr, + std::shared_ptr, + std::vector, + int, + int, + struct event * + > *>(arg); + auto vlans = std::get<0>(*args); + auto config_db = std::get<1>(*args); + auto state_db = std::get<2>(*args); + auto mStateDbMuxTablePtr = std::get<3>(*args); + auto sockets = std::get<4>(*args); + auto lo_sock = std::get<5>(*args); + auto filter = std::get<6>(*args); + auto timer_event = std::get<7>(*args); + + bool all_llas_are_ready = true; + for(auto &vlan : *vlans) { + if (vlan.second.is_lla_ready) { + continue; + } + if (!check_is_lla_ready(vlan.first)) { + syslog(LOG_WARNING, "Link local address for %s is not ready\n", vlan.first.c_str()); + all_llas_are_ready = false; + continue; + } + vlan.second.is_lla_ready = true; + int gua_sock = 0; + int lla_sock = 0; + vlan.second.config_db = config_db; + vlan.second.mux_table = mStateDbMuxTablePtr; + vlan.second.state_db = state_db; + vlan.second.mux_key = vlan_member + vlan.second.interface + "|"; + + update_vlan_mapping(vlan.first, config_db); + + initialize_counter(vlan.second.state_db, vlan.second.interface); + + if (prepare_vlan_sockets(gua_sock, lla_sock, vlan.second) != -1) { + vlan.second.gua_sock = gua_sock; + vlan.second.lla_sock = lla_sock; + vlan.second.lo_sock = lo_sock; + + sockets.push_back(gua_sock); + sockets.push_back(lla_sock); + prepare_relay_config(vlan.second, gua_sock, filter); + if (!dual_tor_sock) { + auto server_callback_event = event_new(base, gua_sock, EV_READ|EV_PERSIST, + server_callback, &(vlan.second)); + if (server_callback_event == NULL) { + syslog(LOG_ERR, "libevent: Failed to create server listen libevent\n"); + } + event_add(server_callback_event, NULL); + syslog(LOG_INFO, "libevent: add server listen socket for %s\n", vlan.first.c_str()); + } + } else { + syslog(LOG_ERR, "Failed to create dualtor loopback listen socket"); + exit(EXIT_FAILURE); + } + } + if (all_llas_are_ready) { + syslog(LOG_INFO, "All Vlans' lla are ready, terminate check timer"); + event_del(timer_event); + } +} diff --git a/src/relay.h b/src/relay.h index fa10cf7..2b129fc 100644 --- a/src/relay.h +++ b/src/relay.h @@ -76,6 +76,7 @@ struct relay_config { bool is_interface_id; std::shared_ptr mux_table; std::shared_ptr config_db; + bool is_lla_ready; }; /* DHCPv6 messages and options */ @@ -483,3 +484,16 @@ void server_callback(evutil_socket_t fd, short event, void *arg); * */ void clear_counter(std::shared_ptr state_db); + +/** + * @code void lla_check_callback(evutil_socket_t fd, short event, void *arg); + * + * @brief callback for libevent timer to check whether lla is ready for vlan + * + * @param fd libevent socket + * @param event libevent triggered event + * @param arg callback argument provided by user + * + * @return none + */ +void lla_check_callback(evutil_socket_t fd, short event, void *arg); diff --git a/test/mock_config_interface.cpp b/test/mock_config_interface.cpp index 9298f3c..104ef96 100644 --- a/test/mock_config_interface.cpp +++ b/test/mock_config_interface.cpp @@ -7,6 +7,7 @@ TEST(configInterface, initialize_swss) { config_db->hset("DHCP_RELAY|Vlan1000", "dhcpv6_servers@", "fc02:2000::1,fc02:2000::2,fc02:2000::3,fc02:2000::4"); config_db->hset("DHCP_RELAY|Vlan1000", "dhcpv6_option|rfc6939_support", "false"); config_db->hset("DHCP_RELAY|Vlan1000", "dhcpv6_option|interface_id", "true"); + config_db->hset("VLAN_INTERFACE|Vlan1000|fc02:1000::1", "", ""); std::unordered_map vlans; ASSERT_NO_THROW(initialize_swss(vlans)); EXPECT_EQ(vlans.size(), 1); @@ -23,13 +24,13 @@ TEST(configInterface, get_dhcp) { config_db->hset("DHCP_RELAY|Vlan1000", "dhcpv6_option|interface_id", "true"); swss::SubscriberStateTable ipHelpersTable(config_db.get(), "DHCP_RELAY"); std::unordered_map vlans; - - ASSERT_NO_THROW(get_dhcp(vlans, &ipHelpersTable, false)); + + ASSERT_NO_THROW(get_dhcp(vlans, &ipHelpersTable, false, config_db)); EXPECT_EQ(vlans.size(), 0); swssSelect.addSelectable(&ipHelpersTable); - ASSERT_NO_THROW(get_dhcp(vlans, &ipHelpersTable, false)); + ASSERT_NO_THROW(get_dhcp(vlans, &ipHelpersTable, false, config_db)); EXPECT_EQ(vlans.size(), 1); } @@ -37,7 +38,7 @@ TEST(configInterface, handleRelayNotification) { std::shared_ptr cfg_db = std::make_shared ("CONFIG_DB", 0); swss::SubscriberStateTable ipHelpersTable(cfg_db.get(), "DHCP_RELAY"); std::unordered_map vlans; - handleRelayNotification(ipHelpersTable, vlans); + handleRelayNotification(ipHelpersTable, vlans, cfg_db); } TEST(configInterface, processRelayNotification) { @@ -51,7 +52,7 @@ TEST(configInterface, processRelayNotification) { ipHelpersTable.pops(entries); std::unordered_map vlans; - processRelayNotification(entries, vlans); + processRelayNotification(entries, vlans, config_db); EXPECT_EQ(vlans.size(), 1); EXPECT_FALSE(vlans["Vlan1000"].is_option_79); @@ -64,4 +65,8 @@ MOCK_GLOBAL_FUNC0(stopSwssNotificationPoll, void(void)); TEST(configInterface, stopSwssNotificationPoll) { EXPECT_GLOBAL_CALL(stopSwssNotificationPoll, stopSwssNotificationPoll()).Times(1); ASSERT_NO_THROW(stopSwssNotificationPoll()); -} \ No newline at end of file +} + +TEST(configInterface, check_is_lla_ready) { + EXPECT_FALSE(check_is_lla_ready("Vlan1000")); +}