[dhcp_relay] sonic dhcp relay agent for IPv4 by cshivashgit · Pull Request #67 · sonic-net/sonic-dhcp-relay

cshivashgit · 2025-04-30T13:26:10Z

Why I did it
DHCP Relay agent for IPv4

How I did it
Merging dhcp4 changes into this repo, created dhcp6[existing code] and dhcp4[new code] folders

HLD link:
https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md

mssonicbld · 2025-04-30T13:26:19Z

/azp run

azure-pipelines · 2025-04-30T13:26:28Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-04-30T13:53:05Z

/azp run

azure-pipelines · 2025-04-30T13:53:15Z

Azure Pipelines successfully started running 1 pipeline(s).

ashutosh-agrawal · 2025-04-30T16:40:00Z

@yaqiangz Appreciate if you can provide feedback on the directory/file structure.

mssonicbld · 2025-05-02T10:39:18Z

/azp run

azure-pipelines · 2025-05-02T10:39:27Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-05-02T13:57:34Z

/azp run

azure-pipelines · 2025-05-02T13:57:43Z

Azure Pipelines successfully started running 1 pipeline(s).

…and dhcp4relay[new code] folders Co-authored-by: Akshath TR <akshath.t.r@gmail.com> Co-authored-by: Ashutosh Agrawal <ashu@cisco.com>

mssonicbld · 2025-05-07T03:55:19Z

/azp run

azure-pipelines · 2025-05-07T03:55:28Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-05-22T04:28:15Z

/azp run

azure-pipelines · 2025-05-22T04:28:24Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-03T14:13:08Z

/azp run

azure-pipelines · 2025-07-03T14:13:17Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-10-30T06:45:28Z

/azp run

azure-pipelines · 2025-10-30T06:45:37Z

Azure Pipelines successfully started running 1 pipeline(s).

Move pcappp_v24.09.zip and patch folder inside dhcp4relay directory

mssonicbld · 2025-10-30T10:15:35Z

/azp run

azure-pipelines · 2025-10-30T10:15:45Z

Azure Pipelines successfully started running 1 pipeline(s).

Blueve · 2025-11-03T04:04:44Z

Close and re-open to rerun PR test

mssonicbld · 2025-11-03T04:04:56Z

/azp run

azure-pipelines · 2025-11-03T04:05:05Z

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Ashutosh Agrawal <ashu@cisco.com>

Fixed a typo to retrigger PR checks

mssonicbld · 2025-11-03T21:15:12Z

/azp run

azure-pipelines · 2025-11-03T21:15:20Z

Azure Pipelines successfully started running 1 pipeline(s).

saiarcot895 · 2025-12-07T23:38:03Z

Why is there a random zip file added here? If this is actually needed as a build dependency, compile it as a separate application or add it into the slave container.

@saiarcot895 Since, we are applying a minor patch on top of the downloaded PCAP++ source code, we didn't include it in the slave container. But I've modified the build process to now download the file instead of including it in the code. Please review the changes in this PR

- Replace 17MB pcappp_v24.09.zip binary with wget download from GitHub - Download from official release: github.com/seladb/PcapPlusPlus/archive/refs/tags/v24.09.tar.gz - Add SHA256 checksum verification (b14998ecf5718c2bfb6f709060e53f7ef67639b98bf2222a3cd23d820d70acdc) - Fix patch file paths to apply cleanly (remove PcapPlusPlus-24.09/ prefix) - Add .gitignore to exclude downloaded artifacts Addresses review comment from saiarcot895 on PR sonic-net#67

* [sonic-dhcp4relay]: Defect fixes Below sonic-mgmt issues are fixed: 1. Random source port 2. Padding issues 3. DHCP Server fixes Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com> * [sonic-dhcp4relay] Smart switch support Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com> * Remove binary zip file and download PcapPlusPlus during build - Replace 17MB pcappp_v24.09.zip binary with wget download from GitHub - Download from official release: github.com/seladb/PcapPlusPlus/archive/refs/tags/v24.09.tar.gz - Add SHA256 checksum verification (b14998ecf5718c2bfb6f709060e53f7ef67639b98bf2222a3cd23d820d70acdc) - Fix patch file paths to apply cleanly (remove PcapPlusPlus-24.09/ prefix) - Add .gitignore to exclude downloaded artifacts Addresses review comment from saiarcot895 on PR #67 --------- Signed-off-by: Shivashankar CR <shivashankar.c.r@gmail.com> Co-authored-by: Ashutosh Agrawal <ashu@cisco.com>

… <Vrf>" (socket not rebound to new VRF) Bug --- On a SONiC device with dhcp_relay configured on a VLAN, moving that VLAN to a non-default VRF at runtime via config vrf bind <Vlan> <Vrf> leaves the dhcp_relay container running but functionally broken: client DISCOVER/REQUEST still reach the relay (they arrive on the client-facing VLAN, which is a kernel-level event), but every relayed unicast to the upstream DHCP server is sent out the wrong interface (typically eth0 in the management VRF, since that is what the upstream socket is still bound to). The server's OFFER/ACK either never reach the device or arrive on an interface the relay is not listening on, so DHCP discovery for any client behind that VLAN times out indefinitely. The bug is silent: no error log, no counter increment, no relay restart. The only externally visible symptom is "clients in the new VRF cannot get a lease". Restarting the dhcp_relay container (or `config reload`) papers over the issue, because the daemon's startup path opens its sockets via a separate, correct CONFIG_DB lookup. Impact ------ * Any production workflow that relies on rebinding a VLAN's VRF without restarting dhcp_relay is broken. This includes day-2 multi-tenant onboarding, customer VRF migrations, and the sonic-mgmt test `tests/dhcp_relay/test_dhcpv4_relay.py:: test_dhcp_relay_with_non_default_vrf`, which currently has to work around the bug by explicitly restarting dhcp_relay after VRF binding. * The bug has been latent since the field was first introduced (PRs sonic-net#67 and sonic-net#84) — every release that ships sonic-dhcp-relay >= those PRs is affected. Root cause ---------- process_vlan_interface_notification() in dhcp4relay_mgr.cpp is the runtime handler for VLAN_INTERFACE updates from CONFIG_DB. For the bare-key form ("VLAN_INTERFACE|Vlan1000") it iterates the entry's fields and copies a vrf name into the outbound relay-message: if (fvField(fv) == "vrf") { // <-- wrong field name vrf = fvValue(fv); break; } The schema field name is "vrf_name", not "vrf" — the same module's own startup-time lookup at dhcp4relay.cpp:864 and dhcp4relay_mgr.cpp:382 correctly uses "vrf_name". The comparison therefore never matches, the outbound message's vrf stays empty, and the consumer in dhcp4relay.cpp at the equivalent of: if ((msg->vrf.empty()) || ...) { return; } short-circuits, so handle_server_sock() — which is what would close the upstream socket and re-open it with `setsockopt(SO_BINDTODEVICE, vrf)` — is never called. The socket remains bound to whichever VRF was current at startup. Fix --- * Read field VRF_NAME_FIELD ("vrf_name") instead of the typo'd "vrf" in the bare-key branch of process_vlan_interface_notification(). This makes the runtime rebind path consistent with the startup path and with the schema. * Pull the five reused CONFIG_DB field-name string literals ("vrf_name", "server_vrf", "source_interface", "link_selection", "state") into named macros in dhcp4relay.h. These literals were previously duplicated across the two .cpp files, which is exactly how the typo above slipped through review: the broken site was the only place writing the field name, so a reader had nothing to compare against. Centralizing them lets the compiler and a single grep enforce consistency. The IP-suffix branch ("VLAN_INTERFACE|Vlan1000|<ip>/<mask>") is left exactly as upstream — those events do not carry vrf_name and the bare-key event is what drives the rebind. Limiting the change to the buggy branch avoids any spurious extra socket rebinds on plain `config interface ip add/remove`. Validation ---------- Reproduced in KVM testbed with sonic-mgmt `test_dhcp_relay_with_non_default_vrf` (4 cases). Without the fix, all 4 fail at the PTF check `expected 48, got 0` (no DHCP packet makes it back). With the fix applied to the dhcp_relay deb on the DUT, all 4 pass, and the test-side `restart_dhcp_service` workaround can be removed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Xichen96 <lukelin0907@gmail.com>

… <Vrf>" (socket not rebound to new VRF) (#107) ### 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 (or `config reload`) papers over 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()` in `dhcp4relay_mgr.cpp` reads field `"vrf"` from the `VLAN_INTERFACE` update, but the schema field is `"vrf_name"` (the same module's startup path uses the correct name). The mismatch leaves `msg->vrf` empty, the consumer in `dhcp4relay.cpp` short-circuits at `if (msg->vrf.empty()) return;`, and `handle_server_sock()` — which would `setsockopt(SO_BINDTODEVICE, vrf)` — never runs. Fix: read `VRF_NAME_FIELD` instead 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 in `dhcp4relay.h` so 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 PTF `expected 48, got 0`. With the fix applied to the on-DUT dhcp_relay deb, all 4 pass; the test-side `restart_dhcp_service` workaround can be removed. Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>

sumati3031-arista · 2026-07-02T05:04:44Z

@cshivashgit after the introduction of sonic relay agent, we are seeing a lot of failures in dhcp_relay testcases
We want to understand if you are aware and if any work is in progress to fix those failures.
I have proposed fix for a few in the following PRs.
sonic-net/sonic-dhcpmon#81
sonic-net/sonic-mgmt#25394

ashutosh-agrawal · 2026-07-02T20:36:43Z

@sumati3031-arista We are constantly working on improving quality and stability of dhcp_relay. We have multiple merged PRs since the original design and have two more open PRs. Please help with the review:

#112
#109

For any other issues, please feel free to tag @cshivashgit or myself and we will look at it promptly.

cshivashgit force-pushed the sonic-dhcp4relay branch from f4847c1 to cfd6970 Compare April 30, 2025 13:52

ashutosh-agrawal mentioned this pull request Apr 30, 2025

DHCPv4 Relay Agent High Level Design sonic-net/SONiC#1938

Merged

cshivashgit force-pushed the sonic-dhcp4relay branch from cfd6970 to 4ab5782 Compare May 2, 2025 10:39

cshivashgit force-pushed the sonic-dhcp4relay branch from 4ab5782 to cb1b92f Compare May 2, 2025 13:57

Introducing dhcp v4 relay changes, created dhcp6relay[existing code] …

37d3981

…and dhcp4relay[new code] folders Co-authored-by: Akshath TR <akshath.t.r@gmail.com> Co-authored-by: Ashutosh Agrawal <ashu@cisco.com>

cshivashgit force-pushed the sonic-dhcp4relay branch from cb1b92f to 37d3981 Compare May 7, 2025 03:55

Addressing MSFT comments on HLD

dffb7e7

yaqiangz reviewed Jun 24, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay.cpp Outdated

yaqiangz reviewed Jun 24, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay_mgr.cpp Outdated

yaqiangz reviewed Jul 1, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay_stats.cpp Outdated

yaqiangz reviewed Jul 1, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay_stats.cpp Outdated

yaqiangz reviewed Jul 1, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay.cpp Outdated

yaqiangz reviewed Jul 1, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay.cpp

yaqiangz reviewed Jul 1, 2025

View reviewed changes

Comment thread dhcp4relay/src/dhcp4relay.cpp

Added code changes to support DualTor and dhcp_server

a4f3fe1

cshivashgit force-pushed the sonic-dhcp4relay branch from 8248735 to 92e6506 Compare October 30, 2025 06:45

Additional CFG definitions update

c2ce158

Move pcappp_v24.09.zip and patch folder inside dhcp4relay directory

Blueve previously approved these changes Nov 3, 2025

View reviewed changes

Blueve closed this Nov 3, 2025

Blueve reopened this Nov 3, 2025

ashutosh-agrawal added 2 commits November 3, 2025 13:13

fixed a typo to retrigger PR checks

9faa343

Signed-off-by: Ashutosh Agrawal <ashu@cisco.com>

Merge pull request #2 from cshivashgit/ashu/typo-fix

b565380

Fixed a typo to retrigger PR checks

ashutosh-agrawal dismissed Blueve’s stale review via b565380 November 3, 2025 21:15

Blueve approved these changes Nov 5, 2025

View reviewed changes

Blueve merged commit 8772dd5 into sonic-net:master Nov 5, 2025
8 checks passed

saiarcot895 reviewed Dec 7, 2025

View reviewed changes

ashutosh-agrawal mentioned this pull request Dec 8, 2025

Remove binary zip file and download PcapPlusPlus during build cshivashgit/sonic-dhcp-relay#4

Merged

Xichen96 mentioned this pull request Apr 25, 2026

[dhcp4relay] Fix bug: DHCP relay broken after "config vrf bind <Vlan> <Vrf>" (socket not rebound to new VRF) #104

Merged

mssonicbld mentioned this pull request Apr 28, 2026

[action] [PR:104] [dhcp4relay] Fix bug: DHCP relay broken after "config vrf bind <Vlan> <Vrf>" (socket not rebound to new VRF) #107

Merged

Uh oh!

Conversation

cshivashgit commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Apr 30, 2025

Uh oh!

azure-pipelines Bot commented Apr 30, 2025

Uh oh!

mssonicbld commented Apr 30, 2025

Uh oh!

azure-pipelines Bot commented Apr 30, 2025

Uh oh!

ashutosh-agrawal commented Apr 30, 2025

Uh oh!

mssonicbld commented May 2, 2025

Uh oh!

azure-pipelines Bot commented May 2, 2025

Uh oh!

mssonicbld commented May 2, 2025

Uh oh!

azure-pipelines Bot commented May 2, 2025

Uh oh!

mssonicbld commented May 7, 2025

Uh oh!

azure-pipelines Bot commented May 7, 2025

Uh oh!

mssonicbld commented May 22, 2025

Uh oh!

azure-pipelines Bot commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 3, 2025

Uh oh!

azure-pipelines Bot commented Jul 3, 2025

Uh oh!

mssonicbld commented Oct 30, 2025

Uh oh!

azure-pipelines Bot commented Oct 30, 2025

Uh oh!

mssonicbld commented Oct 30, 2025

Uh oh!

azure-pipelines Bot commented Oct 30, 2025

Uh oh!

Blueve commented Nov 3, 2025

Uh oh!

mssonicbld commented Nov 3, 2025

Uh oh!

azure-pipelines Bot commented Nov 3, 2025

Uh oh!

mssonicbld commented Nov 3, 2025

Uh oh!

azure-pipelines Bot commented Nov 3, 2025

Uh oh!

Uh oh!

saiarcot895 Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

ashutosh-agrawal Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sumati3031-arista commented Jul 2, 2026

Uh oh!

ashutosh-agrawal commented Jul 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

cshivashgit commented Apr 30, 2025 •

edited

Loading