[dhcp4relay] Fix bug: DHCP relay broken after "config vrf bind <Vlan> <Vrf>" (socket not rebound to new VRF)#104
Open
Xichen96 wants to merge 1 commit intosonic-net:masterfrom
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Xichen96
force-pushed
the
dev/xichenlin/fix-dhcp4relay-vlan-interface-vrf-update
branch
from
bac7808 to
88f23cc
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Xichen96
force-pushed
the
dev/xichenlin/fix-dhcp4relay-vlan-interface-vrf-update
branch
from
88f23cc to
d11235d
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Xichen96
force-pushed
the
dev/xichenlin/fix-dhcp4relay-vlan-interface-vrf-update
branch
from
d11235d to
fab233f
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Xichen96
changed the title
Xichen96
force-pushed
the
dev/xichenlin/fix-dhcp4relay-vlan-interface-vrf-update
branch
from
fab233f to
7c9a551
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
… <Vrf>" (socket not rebound to new VRF) process_vlan_interface_notification() reads field "vrf" instead of the schema's "vrf_name", so the runtime VRF-update message arrives with an empty vrf, the consumer short-circuits, and the upstream socket stays bound to the original VRF. Fix the typo and centralize reused CONFIG_DB field names in dhcp4relay.h. Co-authored-by: Copilot <[email protected]> Signed-off-by: Xichen96 <[email protected]>
Xichen96
force-pushed
the
dev/xichenlin/fix-dhcp4relay-vlan-interface-vrf-update
branch
from
7c9a551 to
20f28b1
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why I did it
On a SONiC device with dhcp_relay running on a VLAN, moving that VLAN to a non-default VRF at runtime via
config vrf bind <Vlan> <Vrf>silently breaks DHCP: the relay's upstream socket stays bound to the original VRF, so OFFER/ACK never reach clients. The bug is silent — no log, no counter, no restart. Restarting dhcp_relay (orconfig reload) papersover it because the startup path uses the correct CONFIG_DB lookup. Bug has been latent since PR #67 / #84.
How I did it
process_vlan_interface_notification()indhcp4relay_mgr.cppreads field"vrf"from theVLAN_INTERFACEupdate, but the schema field is"vrf_name"(the same module's startup path uses the correct name). The mismatch leavesmsg->vrfempty, the consumer indhcp4relay.cppshort-circuits atif (msg->vrf.empty()) return;, andhandle_server_sock()— which wouldsetsockopt(SO_BINDTODEVICE, vrf)— never runs.Fix: read
VRF_NAME_FIELDinstead of the typo, and pull the five reused CONFIG_DB field-name literals (vrf_name,server_vrf,source_interface,link_selection,state) into named macros indhcp4relay.hso this typo class cannot recur silently. The IP-suffix branch is left untouched — bare-key event drives the rebind.How to verify it
sonic-mgmt
tests/dhcp_relay/test_dhcpv4_relay.py::test_dhcp_relay_with_non_default_vrf(4 cases). Without the fix all 4 fail with PTFexpected 48, got 0. With the fix applied to the on-DUT dhcp_relay deb, all 4 pass; the test-siderestart_dhcp_serviceworkaround can be removed.