Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the DHCP reservation manager to track dynamic IP assignments and optimize the dynamic pool allocation. The manager now maintains a cursor pointing to the next available IP in the dynamic pool, improving efficiency when allocating addresses to non-reserved clients.
Key changes:
- Added tracking of dynamic IP assignments with
dynamic_assignmentsHashMap andactive_dynamic_hostsHashSet - Implemented dynamic pool cursor optimization via
recalculate_dynamic_cursor()method to find next available IP - Updated
restore_default()to use the optimized cursor position when restoring the DHCP pool
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.active.is_some() { | ||
| let dynamic_start_host = self | ||
| .next_dynamic_host | ||
| .clamp(DYNAMIC_POOL_START, self.dynamic_end_host); | ||
| let dynamic_start = Ipv4Addr::from( | ||
| self.network_base | ||
| .checked_add(dynamic_start_host) | ||
| .ok_or_else(|| anyhow::anyhow!("Dynamic start exceeds subnet bounds"))?, | ||
| ); | ||
| let dynamic_end = Ipv4Addr::from( | ||
| self.network_base | ||
| .checked_add(self.dynamic_end_host) | ||
| .ok_or_else(|| anyhow::anyhow!("Dynamic end exceeds subnet bounds"))?, | ||
| ); | ||
| self.default = DhcpsLease::from_range(dynamic_start, dynamic_end); |
There was a problem hiding this comment.
The restore_default() method permanently modifies self.default to use next_dynamic_host as the start, but this may not reflect the originally configured default pool. When restore_default() is called again later (e.g., after multiple reservation cycles), self.default will have been mutated to a different range than originally intended. Consider storing the original default separately or recalculating from constants instead of mutating self.default.
| let mut candidate = self | ||
| .next_dynamic_host | ||
| .clamp(DYNAMIC_POOL_START, self.dynamic_end_host); | ||
| for _ in 0..pool_size { |
There was a problem hiding this comment.
The loop iterates pool_size times in the worst case, which could be up to 98 iterations (STATIC_POOL_START - DYNAMIC_POOL_START - 1). For a fully occupied pool, this linear search runs every time recalculate_dynamic_cursor() is called (on every disconnect and IP assignment). Consider using a more efficient data structure or algorithm, such as maintaining a count of occupied hosts and short-circuiting when count equals pool_size.
1 participant
No description provided.