Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Test gap] Test dhcp_relay with source port ip enabled #14653

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yejianquan
Copy link
Collaborator

@yejianquan yejianquan commented Sep 19, 2024

Description of PR

Summary:
Fixes #3624
Mitigate the test gap: Test dhcp relay with source port ip in relay enabled.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

Fixes #3624
Mitigate the test gap: Test dhcp relay with source port ip in relay enabled.

How did you do it?

  1. Enhance dhcp_relay ptf test to verify the src_ip in relay packets
  2. Add a fixture which modify deployment_id to 8 and enable source port ip in relay
  3. Add a test case exactly same with test_dhcp_relay_default but include fixture enable_source_port_ip_in_relay.

How did you verify/test it?

Run on local dev vm,
dhcp_relay/test_dhcp_relay.py::test_interface_binding PASSED [ 12%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_default PASSED [ 25%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_with_source_port_ip_in_relay_enabled PASSED [ 37%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_after_link_flap PASSED [ 50%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_start_with_uplinks_down PASSED [ 62%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_unicast_mac PASSED [ 75%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_random_sport PASSED [ 87%]
dhcp_relay/test_dhcp_relay.py::test_dhcp_relay_counter SKIPPED (skip...) [100%]
and PR test will test it again.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

@yejianquan
Copy link
Collaborator Author

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yejianquan
Copy link
Collaborator Author

Hi @Blueve @yaqiangz , could you please help to review this PR?

Copy link
Contributor

@yaqiangz yaqiangz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yaqiangz yaqiangz self-requested a review September 20, 2024 04:24
@yaqiangz
Copy link
Contributor

Hold on..

@yaqiangz
Copy link
Contributor

Hi @lolyu does dualtor need this test? It would affect source IP sent from relay to DHCP server

@lolyu
Copy link
Contributor

lolyu commented Sep 20, 2024

Hi @lolyu does dualtor need this test? It would affect source IP sent from relay to DHCP server

If it is a generic feature on T0, dualtor will need it.
dhcp-relay will enable source port ip on t0, right?

@yejianquan
Copy link
Collaborator Author

yejianquan commented Sep 20, 2024

Hi @lolyu does dualtor need this test? It would affect source IP sent from relay to DHCP server

If it is a generic feature on T0, dualtor will need it. dhcp-relay will enable source port ip on t0, right?

@lolyu What do you mean by 'dhcp-relay will enable source port ip on t0'?

From my understanding and research,
when the -si is used, in the relay packets, the source ip will be client_ip instead of the loopback ip of DUT.

In the buildimage, only when deployment_id==8, we enable source ip in relay.
https://github.com/sonic-net/sonic-buildimage/blob/e0e0c0c1b3c58635bc25fde6a77ca3b0849dfde1/dockers/docker-dhcp-relay/dhcpv4-relay.agents.j2#L16
But not sure when the deployment_id will be 8 in the production ENV.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Need new test: DHCP Relay supports using src intf ip in relay
4 participants