Skip to content

[dhcp_relay] Validate RFC 6607 VSS-Control#26315

Draft
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/fix-vss-control
Draft

[dhcp_relay] Validate RFC 6607 VSS-Control#26315
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/fix-vss-control

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:

Model RFC 6607 VSS-Control request and reply behavior in the existing
different-non-default-VRF DHCPv4 relay test.

Depends on sonic-net/sonic-dhcp-relay#122.

Integration TODO before final validation

  • After sonic-mgmt PR [dhcp_relay] Cover Option 82 reply lifecycle for both agents #26296 reaches its final stack, rebase this PR onto
    it and make the shared server-reply helper use server_response_option82
    without VSS-Control Suboption 152. Keep request option82 unchanged with
    Suboptions 151 and 152.
  • Isolate or flush the retained-152 negative OFFER before sending the
    aware-server OFFER so a delayed negative packet cannot satisfy the following
    positive verification.

Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

  • master
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • N/A

Test result

PR CI is pending. Hardware validation requires a SONiC image containing
sonic-dhcp-relay PR #122.

Approach

What is the motivation for this PR?

The PTF model currently uses one Option 82 byte string for both relay requests
and simulated server replies. RFC 6607 requires relay requests containing VSS
Suboption 151 to also contain VSS-Control Suboption 152. A VSS-aware server
removes 152 from its reply, while an unaware server returns both.

Without direction-specific Option 82 data, the test cannot model either
behavior correctly after the relay implements VSS-Control validation.

How did you do it?

  • Expect relay requests in the different-VRF case to contain
    1 -> 2 -> 5 -> 11 -> 151 -> 152.
  • Correct the Suboption 151 description: its leading zero byte is the Type-0
    ASCII VPN identifier field, not padding.
  • Use reply Option 82 data containing matching Suboption 151 without
    Suboption 152 for the successful server path.
  • First send an OFFER retaining 152 and verify that no matching client-facing
    OFFER is forwarded, then continue the existing DHCP exchange with a
    VSS-aware reply.

How did you verify/test it?

The changed Python files pass syntax compilation and the repository's commit
hooks, including AST and flake8 checks. Full packet validation is delegated to
PR CI and hardware testing with the dependent relay image.

Any platform specific information?

None.

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

This updates the existing SONiC relay different-non-default-VRF coverage; it
does not add a new topology or standalone test case.

Documentation

The test comments and expected-results documentation now describe RFC 6607
Suboptions 151 and 152 explicitly.

Model relay requests with VSS and VSS-Control, VSS-aware replies without VSS-Control, and verify that replies retaining VSS-Control are not forwarded.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2b881aa9-3a3a-4aaf-b2ca-b941705b2438
Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enhances the existing DHCPv4 relay different-non-default-VRF test/ptf model to validate RFC 6607 VSS behavior by distinguishing Option 82 contents between relay requests and simulated server replies, including the expected handling of VSS-Control (Suboption 152).

Changes:

  • Update test case expected-results documentation to explicitly cover VSS Suboption 151 and VSS-Control Suboption 152 behavior.
  • Extend the PTF DHCP relay test to model VSS-aware vs VSS-unaware server replies by using direction-specific Option 82 payloads.
  • Add a negative check ensuring offers that retain VSS-Control are not forwarded to the client in the different-VRF scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/dhcp_relay/test_dhcpv4_relay.py Documents new expected behavior for RFC 6607 VSS-Control validation in the different-VRF DHCP relay test.
ansible/roles/test/files/ptftests/py3/dhcp_relay_test.py Implements request vs reply Option 82 separation and adds a negative test for dropping unprocessed VSS-Control offers.

Comment thread ansible/roles/test/files/ptftests/py3/dhcp_relay_test.py Outdated
Build the RFC 6607 Type-0 VPN identifier as ASCII bytes before calculating the Suboption 151 length, keeping the length and payload representation consistent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 2b881aa9-3a3a-4aaf-b2ca-b941705b2438
Signed-off-by: Xichen Lin <lukelin0907@gmail.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +1296 to 1300
if self.relay_agent == "sonic-relay-agent" and self.server_vrf:
self.dataplane.flush()
self.server_send_unprocessed_vss_offer()
self.verify_unprocessed_vss_offer_dropped()
self.server_send_offer()
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.

3 participants