dhcp4relay drops packets on first VLANs in large batch - #112
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@Xichen96 please review |
|
@StormLiangMS Please help with review. |
When a large batch of DHCPV4_RELAY VLANs is configured alongside their
underlying VLAN_INTERFACEs and a Loopback used as source_interface, the
first ~10 VLANs silently drop relayed DHCP packets with
"No IPv4 address configured".
Root cause: prepare_relay_interface_config() resolves the source
interface IP via getifaddrs(). At scale, intfmgrd has not yet programmed
the Loopback IP into the kernel by the time the DHCPV4_RELAY config
arrives, so getifaddrs() misses it, src_intf_sel_addr is left at
0.0.0.0, and from_client() drops every packet for those VLANs. The
authoritative source IP is already known to the dhcp4relay process at
that point (as a *INTERFACE pub/sub event has either landed in the
SubscriberStateTable buffer or already passed through
process_interface_notification with no matching vlan in vlans_copy);
only the kernel state is behind.
Fix: maintain an in-process intf_to_addr_cache map populated as a
side-effect of process_interface_notification, and dispatch
DHCPv4_RELAY_INTERFACE_UPDATE from that cache when a DHCPV4_RELAY batch
registers a vlan whose source_interface IP was cached but never
replayed. No Redis access on the hot path.
Wired in two places:
- In the startup predrain block of handle_swss_notification(), after
process_relay_notification(initial_entries) and before the
DHCPv4_RELAY_SYNC_BARRIER write, drain the *INTERFACE
SubscriberStateTables and feed them through
process_interface_notification. The buffers were seeded by their
construction-time SCAN, so this both populates the cache and
dispatches INTERFACE_UPDATE for vlans registered by the relay
drain immediately above.
- In the runtime select loop, immediately after the DHCPV4_RELAY
branch's process_relay_notification(entries) call, replay matching
cache entries via dispatch_source_intf_from_cache() so a vlan whose
source_interface IP arrived earlier (and was discarded by
process_interface_notification because vlans_copy didn't yet
contain the vlan) still receives a correct src_intf_sel_addr
before any packet reaches prepare_relay_interface_config().
INTERFACE_UPDATE is the same event type produced by
process_interface_notification, so the main thread handler in
dhcp4relay.cpp needs no change. Idempotent: the existing kernel-driven
INTERFACE flow simply rewrites the same src_intf_sel_addr if it later
delivers another event for the same interface.
Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com>
3c88092 to
c2b6e29
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR addresses a startup/config-ordering race in dhcp4relay where VLANs can drop relayed DHCP packets (“No IPv4 address configured”) when a large DHCPV4_RELAY batch arrives before the kernel reflects the source_interface (e.g., Loopback) IPv4 address. The fix avoids depending solely on getifaddrs() timing by caching interface IPv4 addresses in-process and replaying INTERFACE updates when VLAN relay config is applied.
Changes:
- Add an in-process
intf_to_addr_cachepopulated from*INTERFACEtable notifications (IPv4 only). - During startup, drain
*INTERFACESubscriberStateTable buffers before the initial sync barrier to ensure initial VLANs getsrc_intf_sel_addr. - During runtime, after processing DHCPV4_RELAY updates, replay cached interface updates to cover the case where the interface IP event arrived before the VLAN relay config existed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dhcp4relay/src/dhcp4relay_mgr.h | Declares dispatch_source_intf_from_cache() for replaying cached interface updates. |
| dhcp4relay/src/dhcp4relay_mgr.cpp | Implements interface IPv4 caching, startup interface-buffer draining, and runtime replay of cached interface updates after relay config batches. |
|
@cshivashgit Thanks for identifying and fixing this subtle ordering issue—the cache and replay approach looks good. I rebased the branch onto the latest master for you. I prefer keeping comments focused on non-obvious behavior, as this volume of comments can drown out the code and disrupt its flow. The detailed rationale would fit better in the commit message and PR description. |
|
@cshivashgit Could you also fix the copilot comments? Thx. |
Limit cached source-interface replay to VLANs touched by the current DHCPV4_RELAY batch, avoiding a full vlans_copy scan on each update. Trim the replay/cache source comments to only the non-obvious ordering behavior and add a regression test for interface-before-relay config ordering. Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
@Xichen96 comments are addressed, please review |
When a large batch of DHCPV4_RELAY VLANs is configured alongside their underlying VLAN_INTERFACEs and a Loopback used as source_interface, the first ~10 VLANs silently drop relayed DHCP packets with "No IPv4 address configured".
Root cause: prepare_relay_interface_config() resolves the source interface IP via getifaddrs(). At scale, intfmgrd has not yet programmed the Loopback IP into the kernel by the time the DHCPV4_RELAY config arrives, so getifaddrs() misses it, src_intf_sel_addr is left at 0.0.0.0, and from_client() drops every packet for those VLANs. The authoritative source IP is already known to the dhcp4relay process at that point (as a *INTERFACE pub/sub event has either landed in the SubscriberStateTable buffer or already passed through process_interface_notification with no matching vlan in vlans_copy); only the kernel state is behind.
Fix: maintain an in-process intf_to_addr_cache map populated as a side-effect of process_interface_notification, and dispatch DHCPv4_RELAY_INTERFACE_UPDATE from that cache when a DHCPV4_RELAY batch registers a vlan whose source_interface IP was cached but never replayed. No Redis access on the hot path.
Wired in two places:
In the startup predrain block of handle_swss_notification(), after process_relay_notification(initial_entries) and before the DHCPv4_RELAY_SYNC_BARRIER write, drain the *INTERFACE SubscriberStateTables and feed them through process_interface_notification. The buffers were seeded by their construction-time SCAN, so this both populates the cache and dispatches INTERFACE_UPDATE for vlans registered by the relay drain immediately above.
In the runtime select loop, immediately after the DHCPV4_RELAY branch's process_relay_notification(entries) call, replay matching cache entries via dispatch_source_intf_from_cache() so a vlan whose source_interface IP arrived earlier (and was discarded by process_interface_notification because vlans_copy didn't yet contain the vlan) still receives a correct src_intf_sel_addr before any packet reaches prepare_relay_interface_config().
INTERFACE_UPDATE is the same event type produced by process_interface_notification, so the main thread handler in dhcp4relay.cpp needs no change. Idempotent: the existing kernel-driven INTERFACE flow simply rewrites the same src_intf_sel_addr if it later delivers another event for the same interface.