Skip to content

Commit 011066a

Browse files
yaqiangzyaqiangz
authored
Add wait and check ip address check when dhcp6relay init (#52)
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
1 parent 2a2fb68 commit 011066a

File tree

5 files changed

+220
-55
lines changed

5 files changed

+220
-55
lines changed

‎src/config_interface.cpp‎

Lines changed: 62 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ void initialize_swss(std::unordered_map<std::string, relay_config> &vlans)
2121
std::shared_ptr<swss::DBConnector> configDbPtr = std::make_shared<swss::DBConnector> ("CONFIG_DB", 0);
2222
swss::SubscriberStateTable ipHelpersTable(configDbPtr.get(), "DHCP_RELAY");
2323
swssSelect.addSelectable(&ipHelpersTable);
24-
get_dhcp(vlans, &ipHelpersTable, false);
24+
get_dhcp(vlans, &ipHelpersTable, false, configDbPtr);
2525
}
2626
catch (const std::bad_alloc &e) {
2727
syslog(LOG_ERR, "Failed allocate memory. Exception details: %s", e.what());
@@ -53,13 +53,15 @@ void deinitialize_swss()
5353

5454
/**
5555
56-
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic)
56+
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
57+
std::shared_ptr<swss::DBConnector> config_db)
5758
*
5859
* @brief initialize and get vlan table information from DHCP_RELAY
5960
*
6061
* @return none
6162
*/
62-
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic) {
63+
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
64+
std::shared_ptr<swss::DBConnector> config_db) {
6365
swss::Selectable *selectable;
6466
int ret = swssSelect.select(&selectable, DEFAULT_TIMEOUT_MSEC);
6567
if (ret == swss::Select::ERROR) {
@@ -69,7 +71,7 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
6971
}
7072
if (selectable == static_cast<swss::Selectable *> (ipHelpersTable)) {
7173
if (!dynamic) {
72-
handleRelayNotification(*ipHelpersTable, vlans);
74+
handleRelayNotification(*ipHelpersTable, vlans, config_db);
7375
} else {
7476
syslog(LOG_WARNING, "relay config changed, "
7577
"need restart container to take effect");
@@ -78,7 +80,8 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
7880
}
7981

8082
/**
81-
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
83+
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
84+
* std::shared_ptr<swss::DBConnector> config_db)
8285
*
8386
* @brief handles DHCPv6 relay configuration change notification
8487
*
@@ -87,16 +90,18 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
8790
*
8891
* @return none
8992
*/
90-
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
93+
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
94+
std::shared_ptr<swss::DBConnector> config_db)
9195
{
9296
std::deque<swss::KeyOpFieldsValuesTuple> entries;
9397

9498
ipHelpersTable.pops(entries);
95-
processRelayNotification(entries, vlans);
99+
processRelayNotification(entries, vlans, config_db);
96100
}
97101

98102
/**
99-
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> vlans)
103+
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> vlans,
104+
std::shared_ptr<swss::DBConnector> config_db)
100105
*
101106
* @brief process DHCPv6 relay servers and options configuration change notification
102107
*
@@ -105,7 +110,8 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un
105110
*
106111
* @return none
107112
*/
108-
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans)
113+
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
114+
std::shared_ptr<swss::DBConnector> config_db)
109115
{
110116
std::vector<std::string> servers;
111117
bool option_79_default = true;
@@ -119,13 +125,35 @@ void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries,
119125
std::string vlan = kfvKey(entry);
120126
std::string operation = kfvOp(entry);
121127
std::vector<swss::FieldValueTuple> fieldValues = kfvFieldsValues(entry);
128+
bool has_ipv6_address = false;
129+
130+
const std::string match_pattern = "VLAN_INTERFACE|" + vlan + "|*";
131+
auto keys = config_db->keys(match_pattern);
132+
for (const auto &itr : keys) {
133+
auto found = itr.find_last_of('|');
134+
if (found == std::string::npos) {
135+
syslog(LOG_WARNING, "%s doesn't exist in VLAN_INTERFACE table, skip it", vlan.c_str());
136+
continue;
137+
}
138+
std::string ip_address = itr.substr(found + 1);
139+
if (ip_address.find(":") != std::string::npos) {
140+
has_ipv6_address = true;
141+
break;
142+
}
143+
}
144+
145+
if (!has_ipv6_address) {
146+
syslog(LOG_WARNING, "%s doesn't have IPv6 address configured, skip it", vlan.c_str());
147+
continue;
148+
}
122149

123150
relay_config intf;
124151
intf.is_option_79 = option_79_default;
125152
intf.is_interface_id = interface_id_default;
126153
intf.interface = vlan;
127154
intf.mux_key = "";
128155
intf.state_db = nullptr;
156+
intf.is_lla_ready = false;
129157
for (auto &fieldValue: fieldValues) {
130158
std::string f = fvField(fieldValue);
131159
std::string v = fvValue(fieldValue);
@@ -154,3 +182,28 @@ void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries,
154182
vlans[vlan] = intf;
155183
}
156184
}
185+
186+
/**
187+
* @code bool check_is_lla_ready(std::string vlan)
188+
*
189+
* @brief Check whether link local address appear in vlan interface
190+
*
191+
* @param vlan string of vlan name
192+
*
193+
* @return bool value indicates whether lla ready
194+
*/
195+
bool check_is_lla_ready(std::string vlan) {
196+
const std::string cmd = "ip -6 addr show " + vlan + " scope link 2> /dev/null";
197+
std::array<char, 256> buffer;
198+
std::string result;
199+
std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
200+
if (pipe) {
201+
while (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
202+
result += buffer.data();
203+
}
204+
if (!result.empty()) {
205+
return true;
206+
}
207+
}
208+
return false;
209+
}

‎src/config_interface.h‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,19 @@ void initialize_swss(std::unordered_map<std::string, relay_config> &vlans);
3333
void deinitialize_swss();
3434

3535
/**
36-
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic)
36+
* @code void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
37+
* std::shared_ptr<swss::DBConnector> config_db)
3738
*
3839
* @brief initialize and get vlan information from DHCP_RELAY
3940
*
4041
* @return none
4142
*/
42-
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic);
43+
void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::SubscriberStateTable *ipHelpersTable, bool dynamic,
44+
std::shared_ptr<swss::DBConnector> config_db);
4345

4446
/**
45-
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans)
47+
* @code void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
48+
* std::shared_ptr<swss::DBConnector> config_db)
4649
*
4750
* @brief handles DHCPv6 relay configuration change notification
4851
*
@@ -51,10 +54,12 @@ void get_dhcp(std::unordered_map<std::string, relay_config> &vlans, swss::Subscr
5154
*
5255
* @return none
5356
*/
54-
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans);
57+
void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::unordered_map<std::string, relay_config> &vlans,
58+
std::shared_ptr<swss::DBConnector> config_db);
5559

5660
/**
57-
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans)
61+
* @code void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
62+
* std::shared_ptr<swss::DBConnector> config_db)
5863
*
5964
* @brief process DHCPv6 relay servers and options configuration change notification
6065
*
@@ -63,4 +68,16 @@ void handleRelayNotification(swss::SubscriberStateTable &ipHelpersTable, std::un
6368
*
6469
* @return none
6570
*/
66-
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans);
71+
void processRelayNotification(std::deque<swss::KeyOpFieldsValuesTuple> &entries, std::unordered_map<std::string, relay_config> &vlans,
72+
std::shared_ptr<swss::DBConnector> config_db);
73+
74+
/**
75+
* @code bool check_is_lla_ready(std::string vlan)
76+
*
77+
* @brief Check whether link local address appear in vlan interface
78+
*
79+
* @param vlan string of vlan name
80+
*
81+
* @return bool value indicates whether lla ready
82+
*/
83+
bool check_is_lla_ready(std::string vlan);

‎src/relay.cpp‎

Lines changed: 110 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,8 @@ void client_callback(evutil_socket_t fd, short event, void *arg) {
895895
}
896896

897897
std::string intf(interfaceName);
898+
// For Vlans that lla is not ready, they wouldn't be added into vlan_map, hence it would be blocked here, no need to
899+
// add is_lla_ready flag check in this callback func
898900
auto vlan = vlan_map.find(intf);
899901
if (vlan == vlan_map.end()) {
900902
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) {
10881090
syslog(LOG_WARNING, "Invalid DHCPv6 header content on loopback socket, packet will be dropped\n");
10891091
continue;
10901092
}
1093+
if (!config->is_lla_ready) {
1094+
syslog(LOG_WARNING, "Link local address for %s is not ready, packet will be dropped\n", config->interface.c_str());
1095+
continue;
1096+
}
10911097
auto loopback_str = std::string(loopback);
10921098
increase_counter(config->state_db, loopback_str, msg_type);
10931099
relay_relay_reply(server_recv_buffer, buffer_sz, config);
@@ -1279,40 +1285,29 @@ void loop_relay(std::unordered_map<std::string, relay_config> &vlans) {
12791285
}
12801286
}
12811287

1282-
for(auto &vlan : vlans) {
1283-
int gua_sock = 0;
1284-
int lla_sock = 0;
1285-
vlan.second.config_db = config_db;
1286-
vlan.second.mux_table = mStateDbMuxTablePtr;
1287-
vlan.second.state_db = state_db;
1288-
vlan.second.mux_key = vlan_member + vlan.second.interface + "|";
1289-
1290-
update_vlan_mapping(vlan.first, config_db);
1291-
1292-
initialize_counter(vlan.second.state_db, vlan.second.interface);
1293-
1294-
if (prepare_vlan_sockets(gua_sock, lla_sock, vlan.second) != -1) {
1295-
vlan.second.gua_sock = gua_sock;
1296-
vlan.second.lla_sock = lla_sock;
1297-
vlan.second.lo_sock = lo_sock;
1298-
1299-
sockets.push_back(gua_sock);
1300-
sockets.push_back(lla_sock);
1301-
prepare_relay_config(vlan.second, gua_sock, filter);
1302-
if (!dual_tor_sock) {
1303-
auto event = event_new(base, gua_sock, EV_READ|EV_PERSIST,
1304-
server_callback, &(vlan.second));
1305-
if (event == NULL) {
1306-
syslog(LOG_ERR, "libevent: Failed to create server listen libevent\n");
1307-
}
1308-
event_add(event, NULL);
1309-
syslog(LOG_INFO, "libevent: add server listen socket for %s\n", vlan.first.c_str());
1310-
}
1311-
} else {
1312-
syslog(LOG_ERR, "Failed to create dualtor loopback listen socket");
1313-
exit(EXIT_FAILURE);
1314-
}
1315-
}
1288+
// Add timer to periodly check lla un-ready vlan
1289+
struct event *timer_event;
1290+
struct timeval tv;
1291+
auto timer_args = new std::tuple<
1292+
std::unordered_map<std::string, struct relay_config> &,
1293+
std::shared_ptr<swss::DBConnector>,
1294+
std::shared_ptr<swss::DBConnector>,
1295+
std::shared_ptr<swss::Table>,
1296+
std::vector<int>,
1297+
int,
1298+
int,
1299+
struct event *
1300+
>(vlans, config_db, state_db, mStateDbMuxTablePtr, sockets, lo_sock, lo_sock, nullptr);
1301+
timer_event = event_new(base, -1, EV_PERSIST, lla_check_callback, timer_args);
1302+
std::get<7>(*timer_args) = timer_event;
1303+
evutil_timerclear(&tv);
1304+
// Check timer is set to 60s
1305+
tv.tv_sec = 60;
1306+
event_add(timer_event, &tv);
1307+
1308+
// We set check timer to be executed every 60s, it would case that its first excution be delayed 60s,
1309+
// hence manually invoke it here to immediate execute it
1310+
lla_check_callback(-1, 0, timer_args);
13161311

13171312
if(signal_init() == 0 && signal_start() == 0) {
13181313
shutdown_relay();
@@ -1351,3 +1346,84 @@ void clear_counter(std::shared_ptr<swss::DBConnector> state_db) {
13511346
state_db->del(itr);
13521347
}
13531348
}
1349+
1350+
/**
1351+
* @code void lla_check_callback(evutil_socket_t fd, short event, void *arg);
1352+
*
1353+
* @brief callback for libevent timer to check whether lla is ready for vlan
1354+
*
1355+
* @param fd libevent socket
1356+
* @param event libevent triggered event
1357+
* @param arg callback argument provided by user
1358+
*
1359+
* @return none
1360+
*/
1361+
void lla_check_callback(evutil_socket_t fd, short event, void *arg) {
1362+
auto args = reinterpret_cast<std::tuple<
1363+
std::unordered_map<std::string, struct relay_config> *,
1364+
std::shared_ptr<swss::DBConnector>,
1365+
std::shared_ptr<swss::DBConnector>,
1366+
std::shared_ptr<swss::Table>,
1367+
std::vector<int>,
1368+
int,
1369+
int,
1370+
struct event *
1371+
> *>(arg);
1372+
auto vlans = std::get<0>(*args);
1373+
auto config_db = std::get<1>(*args);
1374+
auto state_db = std::get<2>(*args);
1375+
auto mStateDbMuxTablePtr = std::get<3>(*args);
1376+
auto sockets = std::get<4>(*args);
1377+
auto lo_sock = std::get<5>(*args);
1378+
auto filter = std::get<6>(*args);
1379+
auto timer_event = std::get<7>(*args);
1380+
1381+
bool all_llas_are_ready = true;
1382+
for(auto &vlan : *vlans) {
1383+
if (vlan.second.is_lla_ready) {
1384+
continue;
1385+
}
1386+
if (!check_is_lla_ready(vlan.first)) {
1387+
syslog(LOG_WARNING, "Link local address for %s is not ready\n", vlan.first.c_str());
1388+
all_llas_are_ready = false;
1389+
continue;
1390+
}
1391+
vlan.second.is_lla_ready = true;
1392+
int gua_sock = 0;
1393+
int lla_sock = 0;
1394+
vlan.second.config_db = config_db;
1395+
vlan.second.mux_table = mStateDbMuxTablePtr;
1396+
vlan.second.state_db = state_db;
1397+
vlan.second.mux_key = vlan_member + vlan.second.interface + "|";
1398+
1399+
update_vlan_mapping(vlan.first, config_db);
1400+
1401+
initialize_counter(vlan.second.state_db, vlan.second.interface);
1402+
1403+
if (prepare_vlan_sockets(gua_sock, lla_sock, vlan.second) != -1) {
1404+
vlan.second.gua_sock = gua_sock;
1405+
vlan.second.lla_sock = lla_sock;
1406+
vlan.second.lo_sock = lo_sock;
1407+
1408+
sockets.push_back(gua_sock);
1409+
sockets.push_back(lla_sock);
1410+
prepare_relay_config(vlan.second, gua_sock, filter);
1411+
if (!dual_tor_sock) {
1412+
auto server_callback_event = event_new(base, gua_sock, EV_READ|EV_PERSIST,
1413+
server_callback, &(vlan.second));
1414+
if (server_callback_event == NULL) {
1415+
syslog(LOG_ERR, "libevent: Failed to create server listen libevent\n");
1416+
}
1417+
event_add(server_callback_event, NULL);
1418+
syslog(LOG_INFO, "libevent: add server listen socket for %s\n", vlan.first.c_str());
1419+
}
1420+
} else {
1421+
syslog(LOG_ERR, "Failed to create dualtor loopback listen socket");
1422+
exit(EXIT_FAILURE);
1423+
}
1424+
}
1425+
if (all_llas_are_ready) {
1426+
syslog(LOG_INFO, "All Vlans' lla are ready, terminate check timer");
1427+
event_del(timer_event);
1428+
}
1429+
}

‎src/relay.h‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ struct relay_config {
7676
bool is_interface_id;
7777
std::shared_ptr<swss::Table> mux_table;
7878
std::shared_ptr<swss::DBConnector> config_db;
79+
bool is_lla_ready;
7980
};
8081

8182
/* DHCPv6 messages and options */
@@ -483,3 +484,16 @@ void server_callback(evutil_socket_t fd, short event, void *arg);
483484
*
484485
*/
485486
void clear_counter(std::shared_ptr<swss::DBConnector> state_db);
487+
488+
/**
489+
* @code void lla_check_callback(evutil_socket_t fd, short event, void *arg);
490+
*
491+
* @brief callback for libevent timer to check whether lla is ready for vlan
492+
*
493+
* @param fd libevent socket
494+
* @param event libevent triggered event
495+
* @param arg callback argument provided by user
496+
*
497+
* @return none
498+
*/
499+
void lla_check_callback(evutil_socket_t fd, short event, void *arg);

0 commit comments

Comments
 (0)