DHCPv4 test update for new sonic-dhcpv4-relay design and added functionality by Akshath-17 · Pull Request #19198 · sonic-net/sonic-mgmt

Akshath-17 · 2025-06-25T15:02:12Z

Summary:
This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details:

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

Type of change

Approach

motivation: Run existing sonic-mgmt tests on both ISC and the new sonic-dhcpv4-relay designs and add new tests for functionality introduced in sonic-dhcpv4-relay

mssonicbld · 2025-06-25T15:02:20Z

/azp run

azure-pipelines · 2025-06-25T15:02:35Z

Azure Pipelines successfully started running 1 pipeline(s).

ashutosh-agrawal · 2025-06-25T18:31:01Z

@yaqiangz Please review.

AnantKishorSharma · 2025-06-26T04:11:19Z

+            duthost.shell('sonic-db-cli CONFIG_DB hset "FEATURE|dhcp_relay" "has_sonic_dhcpv4_relay" "True"', module_ignore_errors=True)
+            duthost.shell('sudo config save -y', module_ignore_errors=True)
+            restart_dhcp_service(duthost)
+            # Checking DHCPV4 relay Process on DUT


restart_dhcp_service already has a check for the process.

sonic-mgmt/tests/dhcp_relay/dhcp_relay_utils.py

Line 44 in 935e05a

output = duthost.shell('docker exec dhcp_relay supervisorctl status | grep dhcp | awk \'{print $2}\'',

Please change the pattern from dhcp to dhc to accommodate the isc process name.

AnantKishorSharma · 2025-06-26T06:08:47Z


+def check_interface_status(duthost, relay_agent="isc-relay-agent"):
+    if relay_agent == "sonic-relay-agent":
+        cmd = "docker exec -t dhcp_relay ss -nlp | grep dhcp4relay"


Can we merge this udp port 67 check?

We included 67 port check

AnantKishorSharma · 2025-06-26T07:03:09Z

+        self.server_vrf = self.test_params.get('server_vrf', None)
+        self.vrf_selection = self.test_params.get('vrf_selection', None)
+        self.portchannels_ip_list = self.test_params.get('portchannels_ip_list', None)
+        self.agent_relay_mode = self.test_params.get('agent_relay_mode', None)


Can we just pass the agent relay mode instead of passing each relay mode value separately?

AnantKishorSharma · 2025-06-26T07:03:19Z

+            bootp = scapy.BOOTP(op=1,
+                                htype=1,
+                                hlen=6,
+                                hops=1,


Can we add a test to check the valid/invalid hop count range?

Added new testcase

AnantKishorSharma · 2025-06-26T08:05:34Z


 class DHCPTest(DataplaneBaseTest):

-    BROADCAST_MAC = 'ff:ff:ff:ff:ff:ff'


Please revert change in scope for these.
Also the changes in the rest of the diff caused by this change of scope.

AnantKishorSharma · 2025-06-26T08:05:38Z


 logger = logging.getLogger(__name__)

+#DHCP packet macro's


Please make them DHCPTest class scope

AnantKishorSharma · 2025-06-26T08:10:39Z

+                    pytest_assert(wait_until(40, 5, 0, check_process_and_socket_status, duthost, dut_dhcp_relay_data, "isc_dhcp_process_check"))
+                    pytest_assert(wait_until(40, 5, 0, check_process_and_socket_status, duthost, dut_dhcp_relay_data, "sonic_dhcpv4_socket_check"))
+
+def check_process_and_socket_status(duthost, dut_dhcp_relay_data=None, process_and_socket_check=None):


Please use existing restart_dhcp_service util to check the process status
L52-59 are not needed

AnantKishorSharma · 2025-06-26T08:22:42Z

+                dut_dhcp_relay_data = request.getfixturevalue("dut_dhcp_relay_data")
+                if dut_dhcp_relay_data:
+                    pytest_assert(wait_until(40, 5, 0, check_process_and_socket_status, duthost, dut_dhcp_relay_data, "isc_dhcp_process_check"))
+                    pytest_assert(wait_until(40, 5, 0, check_process_and_socket_status, duthost, dut_dhcp_relay_data, "sonic_dhcpv4_socket_check"))


Why check the status of sonic_dhcpv4_socket_check after cleaning up new design feature flag?
L39-44 are not needed.

AnantKishorSharma · 2025-06-26T08:40:50Z

+
+    """
+
+    CLIENT_VRF_NAME = "Vrf01"


This is also defined at L#1061. Can we make them global to avoid duplicate definitions?

AnantKishorSharma · 2025-06-27T03:38:14Z

+            restart_dhcp_service(duthost)
+            pytest_assert(wait_until(60, 2, 0, dhcp_ready, False), "Source port ip in relay is not disabled!")
+
+@pytest.mark.parametrize("relay_agent", ["isc-relay-agent"])


Please make this global instead of defining for each test and adapt the remaining 2 TC that are not being run for new design to run on both the designs.

AnantKishorSharma · 2025-06-27T03:44:19Z

                )
+
+@pytest.mark.parametrize("relay_agent", ["sonic-relay-agent"])
+@pytest.mark.parametrize("testcase", ["source_intf", "server_id_override"])


Please move these new tests specific to new design to a new script and add a skip for this script based on release 202511 in test_mark_conditions.yaml

AnantKishorSharma · 2025-06-27T04:05:45Z

@@ -0,0 +1,92 @@
+import re


Please add this in common/dhcp_relay_utils.py Instead of creating this new file here.

AnantKishorSharma · 2025-06-27T04:25:42Z

+            delete_checkpoint(duthost, check_point)
+            restart_dhcp_service(duthost)
+            pytest_assert(wait_until(60, 2, 0, dhcp_ready, False), "Source port ip in relay is not disabled!")
+


2 blank line indentation is needed. Please fix all other indentation/pre-commit errors

mssonicbld · 2025-06-27T13:14:00Z

/azp run

azure-pipelines · 2025-06-27T13:14:13Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-29T06:15:03Z

/azp run

azure-pipelines · 2025-07-29T06:15:17Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-29T09:19:03Z

/azp run

azure-pipelines · 2025-07-29T09:19:08Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

mssonicbld · 2025-08-12T10:47:37Z

/azp run

azure-pipelines · 2025-08-12T10:47:44Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

…onality (#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: Ashutosh Agrawal <[email protected]>

…onality (sonic-net#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: Ashutosh Agrawal <[email protected]>

…onality (sonic-net#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]>

…onality (sonic-net#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: selldinesh <[email protected]>

* DHCPv4 test update for new sonic-dhcpv4-relay design and added functionality (#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: Ashutosh Agrawal <[email protected]> * Addressed review comments 3 and 4 from StormLiangMS Signed-off-by: Ashutosh Agrawal <[email protected]> * Remove test_dhcp_relay_on_dualtor_standby - test is fundamentally broken as DHCP relay is dualtor-agnostic (matches PR 22214 and its 202511 backport 23063) Signed-off-by: Ashutosh Agrawal <[email protected]> --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: Ashutosh Agrawal <[email protected]> Co-authored-by: Akshath TR <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]>

…onality (sonic-net#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]>

vmittal-msft · 2026-04-07T03:54:02Z

@Akshath-17 please help raise manual pr for 202511 since there are cherry-pick conflicts

ashutosh-agrawal · 2026-04-07T18:47:32Z

@vmittal-msft I manually backported it to 202511. Here is the merged PR - #23223.

I've also added "Included in 202511" label on this PR.

…onality (sonic-net#19198) This PR is to support the new DHCPv4 Relay design being introduced in SONiC to replace the existing ISC implementation. Please see the following HLD for more details: https://github.com/sonic-net/SONiC/blob/2eb20e2d2bcc181ab8cf1f502173ac0dbd528698/doc/DHCPv4_relay/DHCPv4-relay-agent-High-Level-Design.md --------- Signed-off-by: Venkata Gouri Rajesh Etla <[email protected]> Co-authored-by: vrajeshe <[email protected]> Co-authored-by: Venkata Gouri Rajesh Etla <[email protected]> Signed-off-by: opcoder0 <[email protected]>

PR sonic-net#19198 added relay_agent and downlink_vlan_iface_name as required parameters in DHCPTest.setUp for sonic-relay-agent support, but test_dhcp_relay_stress.py was not updated. This causes KeyError and TypeError failures when the stress test calls DHCPTest to verify relay functionality after stress. Co-authored-by: Copilot <[email protected]> Signed-off-by: Xichen Lin <[email protected]>

…ve excessive buffer PR sonic-net#19198 added relay_agent and downlink_vlan_iface_name as required parameters in DHCPTest.setUp, but test_dhcp_counter_stress.py was not updated. Also removes the excessive BUFFER_SIZE (1GB kernel buffer) that caused memory spikes, using the default instead. Fixes count file extension mismatch (.json removed). Co-authored-by: Copilot <[email protected]> Signed-off-by: Xichen Lin <[email protected]>

Akshath-17 changed the title ~~sonic-mgmt testcase inline with dhcpv4 relay~~ DHCPv4 test update for new sonic-dhcpv4-relay design and added functionality Jun 26, 2025

AnantKishorSharma reviewed Jun 26, 2025

View reviewed changes

ashutosh-agrawal mentioned this pull request Jun 26, 2025

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

Merged

AnantKishorSharma reviewed Jun 27, 2025

View reviewed changes

bpar9 mentioned this pull request Jul 9, 2025

Testplan for new SONiC DHCPv4 Relay #19215

Closed

9 tasks

Akshath-17 force-pushed the dhcpv4_relay branch from 5da0e8c to 5ce3ea6 Compare July 29, 2025 06:14

Akshath-17 requested review from wangxin and yxieca as code owners July 29, 2025 06:14

Copilot AI mentioned this pull request Mar 24, 2026

Review: human-authored PRs opened in past 48 hours (24 PRs) #23231

Draft

zypgithub mentioned this pull request Apr 3, 2026

Regression: test_dhcp_relay failed due to the PR #19198 #23601

Open

rustiqly mentioned this pull request Apr 4, 2026

[dhcp_relay] test_dhcpv4_relay.py flaky due to sniffer/send race condition #23636

Open

ashutosh-agrawal added the Included in 202511 branch label Apr 7, 2026

ZhaohuiS mentioned this pull request Apr 8, 2026

[dhcp_relay] sonic-relay-agent tests fail on 202511 branch after PR #23223 backport — missing image support #23713

Open

Xichen96 mentioned this pull request Apr 14, 2026

[dhcp_relay] Add missing relay_agent params to stress test DHCPTest call #23886

Merged

12 tasks

Xichen96 mentioned this pull request Apr 15, 2026

[dhcp_relay] Fix test_dhcp_counter_stress: add missing relay_agent params and remo… #23939

Merged

12 tasks


		class DHCPTest(DataplaneBaseTest):

		BROADCAST_MAC = 'ff:ff:ff:ff:ff:ff'

Conversation

Akshath-17 commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Approach

Uh oh!

mssonicbld commented Jun 25, 2025

Uh oh!

azure-pipelines bot commented Jun 25, 2025

Uh oh!

ashutosh-agrawal commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnantKishorSharma Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnantKishorSharma Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jun 27, 2025

Uh oh!

azure-pipelines bot commented Jun 27, 2025

Uh oh!

mssonicbld commented Jul 29, 2025

Uh oh!

azure-pipelines bot commented Jul 29, 2025

Uh oh!

mssonicbld commented Jul 29, 2025

Uh oh!

azure-pipelines bot commented Jul 29, 2025

Uh oh!

Akshath-17 commented Jun 25, 2025 •

edited

Loading

AnantKishorSharma Jun 26, 2025 •

edited

Loading

AnantKishorSharma Jun 26, 2025 •

edited

Loading