-
Notifications
You must be signed in to change notification settings - Fork 29
Conversation
I took a quick look at it and it looks good. I like the new additions of DHCP test cases. I will spend some time and look into detail more. |
return ""; | ||
} | ||
|
||
size_t slash_pos = cidr.find("/"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comments for each step will be very helpful (at least for me).
@@ -60,6 +62,31 @@ int Aca_Dhcp_State_Handler::update_dhcp_state_workitem(const DHCPState current_D | |||
stDhcpCfg.ipv6_address = current_DhcpConfiguration.ipv6_address(); | |||
stDhcpCfg.port_host_name = current_DhcpConfiguration.port_host_name(); | |||
|
|||
// handle dhcp dns entries | |||
int cur_dns_index; | |||
for (int i = 0; i < current_DhcpConfiguration.dns_entry_list_size() && i < DHCP_MSG_OPTS_DNS_LENGTH; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking very good. I left some questions and feedback. Should be easy to address them and get this change merged soon. |
@er1cthe0ne there is a problem in DHCP new test case which test DHCP should monitor br-int bridge. But the test program will crash when other test cases delete the br-int bridge. |
@er1cthe0ne I make some changes. Could you take a quick look at it?. And the DHCP code is ready to merge. |
// monitor br-int for dhcp request message | ||
ovs_monitor_thread = | ||
new thread(bind(&ACA_OVS_Control::monitor, &ACA_OVS_Control::get_instance(), "br-int", "resume")); | ||
ovs_monitor_thread->detach(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see if you can try not to detach so that you can join the thread at the end of the test case, and then kill that thread. Let me know if that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thread will block the test case because of the monitor thread does not exit itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change looks great with more DHCP test cases added. Thanks for contributing this important DHCP feature!
* Add a design document for private_ip_manager service. * Add a private_ip_manager service that supports the management of ipv4 and ipv6 address ranges, as well as the allocation of ipv4 and ipv6 addresses from ip address ranges. * Support range ip availability and usage stats. * Add detailed UTs for private_ip_manager service. * Move the db module to the directory lib directory. Co-authored-by: Liguang Xie <[email protected]>
add dhcp gateway and dns support