Skip to content

Commit

Permalink
Add wait and check ip address check when dhcp6relay init (#52) (#63)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mssonicbld @yaqiangz
mssonicbld and yaqiangz authored Jan 9, 2025
1 parent 9f8ce5a commit a5f3574
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 55 deletions.
71 changes: 62 additions & 9 deletions src/config_interface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void initialize_swss(std::unordered_map<std::string, relay_config> &vlans)
std::shared_ptr<swss::DBConnector> configDbPtr = std::make_shared<swss::DBConnector> ("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());
Expand Down Expand Up @@ -53,13 +53,15 @@ void deinitialize_swss()

/**
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic)
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
std::shared_ptr<swss::DBConnector> config_db)
*
* @brief initialize and get vlan table information from DHCP_RELAY
*
* @return none
*/
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic) {
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
std::shared_ptr<swss::DBConnector> config_db) {
swss::Selectable *selectable;
int ret = swssSelect.select(&selectable, DEFAULT_TIMEOUT_MSEC);
if (ret == swss::Select::ERROR) {
Expand All @@ -69,7 +71,7 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
}
if (selectable == static_cast<swss::Selectable *> (ipHelpersTable)) {
if (!dynamic) {
handleRelayNotification(*ipHelpersTable, vlans);
handleRelayNotification(*ipHelpersTable, vlans, config_db);
} else {
syslog(LOG_WARNING, "relay config changed, "
"need restart container to take effect");
Expand All @@ -78,7 +80,8 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
}

/**
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
* std::shared_ptr<swss::DBConnector> config_db)
*
* @brief handles DHCPv6 relay configuration change notification
*
Expand All @@ -87,16 +90,18 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
*
* @return none
*/
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
std::shared_ptr<swss::DBConnector> config_db)
{
std::deque<swss::KeyOpFieldsValuesTuple> entries;

ipHelpersTable.pops(entries);
processRelayNotification(entries, vlans);
processRelayNotification(entries, vlans, config_db);
}

/**
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> vlans)
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> vlans,
std::shared_ptr<swss::DBConnector> config_db)
*
* @brief process DHCPv6 relay servers and options configuration change notification
*
Expand All @@ -105,7 +110,8 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un
*
* @return none
*/
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans)
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
std::shared_ptr<swss::DBConnector> config_db)
{
std::vector<std::string> servers;
bool option_79_default = true;
Expand All @@ -119,13 +125,35 @@ void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries,
std::string vlan = kfvKey(entry);
std::string operation = kfvOp(entry);
std::vector<swss::FieldValueTuple> 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;
intf.is_interface_id = interface_id_default;
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);
Expand Down Expand Up @@ -154,3 +182,28 @@ void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &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<char, 256> buffer;
std::string result;
std::unique_ptr<FILE, decltype(&pclose)> 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;
}
29 changes: 23 additions & 6 deletions src/config_interface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ void initialize_swss(std::unordered_map<std::string, relay_config> &vlans);
void deinitialize_swss();

/**
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic)
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
* std::shared_ptr<swss::DBConnector> config_db)
*
* @brief initialize and get vlan information from DHCP_RELAY
*
* @return none
*/
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic);
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
std::shared_ptr<swss::DBConnector> config_db);

/**
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
* std::shared_ptr<swss::DBConnector> config_db)
*
* @brief handles DHCPv6 relay configuration change notification
*
Expand All @@ -51,10 +54,12 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
*
* @return none
*/
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans);
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
std::shared_ptr<swss::DBConnector> config_db);

/**
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans)
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
* std::shared_ptr<swss::DBConnector> config_db)
*
* @brief process DHCPv6 relay servers and options configuration change notification
*
Expand All @@ -63,4 +68,16 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un
*
* @return none
*/
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans);
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
std::shared_ptr<swss::DBConnector> 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);
144 changes: 110 additions & 34 deletions src/relay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1279,40 +1285,29 @@ void loop_relay(std::unordered_map<std::string, relay_config> &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::string, struct relay_config> &,
std::shared_ptr<swss::DBConnector>,
std::shared_ptr<swss::DBConnector>,
std::shared_ptr<swss::Table>,
std::vector<int>,
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();
Expand Down Expand Up @@ -1351,3 +1346,84 @@ void clear_counter(std::shared_ptr<swss::DBConnector> 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::tuple<
std::unordered_map<std::string, struct relay_config> *,
std::shared_ptr<swss::DBConnector>,
std::shared_ptr<swss::DBConnector>,
std::shared_ptr<swss::Table>,
std::vector<int>,
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);
}
}
14 changes: 14 additions & 0 deletions src/relay.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ struct relay_config {
bool is_interface_id;
std::shared_ptr<swss::Table> mux_table;
std::shared_ptr<swss::DBConnector> config_db;
bool is_lla_ready;
};

/* DHCPv6 messages and options */
Expand Down Expand Up @@ -483,3 +484,16 @@ void server_callback(evutil_socket_t fd, short event, void *arg);
*
*/
void clear_counter(std::shared_ptr<swss::DBConnector> 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);
Loading

0 comments on commit a5f3574

Please sign in to comment.