Skip to content

[dhcp4relay] Forward chained requests without relay information - #125

Draft
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/item10-chained-no-option82
Draft

[dhcp4relay] Forward chained requests without relay information#125
Xichen96 wants to merge 2 commits into
sonic-net:masterfrom
Xichen96:dev/xichenlin/item10-chained-no-option82

Conversation

@Xichen96

@Xichen96 Xichen96 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:

Forward nonzero-giaddr chained requests that do not contain Option 82 without
applying agent relay mode, adding relay information, or changing giaddr.

This is a low-priority placeholder for a non-current deployment scenario. This
draft must not merge yet.

Type of change

  • Bug fix

Approach

What is the motivation for this PR?

The configured agent relay mode describes handling of packets that already
contain relay information. Native relay currently applies it to every nonzero-
giaddr request, so default discard incorrectly drops a valid chained request
with no Option 82.

How did you do it?

  • Apply agent relay mode only when a nonzero-giaddr request actually contains
    Option 82.
  • Otherwise preserve giaddr, leave Option 82 absent, and continue normal hop
    limit checking, increment, and forwarding.
  • Add focused native unit coverage using discard mode.

How did you verify/test it?

Focused native unit code is included but was NOT RUN, per explicit test
deferral. Test execution is deferred until the higher-priority DHCP relay PRs
listed below merge.

Any platform specific information?

None. This covers a lower-priority, non-current deployment scenario.

Dependencies and future validation TODOs

Validation sequencing depends on these higher-priority relay PRs merging first
(no code dependency; this branch is independently based on fresh origin/master):

Before marking ready or merging:

  • Review this policy against ISC relay behavior before merge.
  • Execute the focused native unit coverage included in this PR.
  • Open the dependent sonic-mgmt PR using discard mode and prove a nonzero-
    giaddr request without Option 82 forwards, increments hops, preserves
    giaddr, and still contains no Option 82:
    [dhcp_relay] Cover chained requests without Option 82 sonic-mgmt#26324
  • Run applicable PR CI/hardware validation after the dependency PRs merge.
  • Keep this PR in draft; do not merge before these TODOs are complete.

Documentation

The policy must be reviewed against ISC behavior before this draft can merge.

Apply agent relay mode only when a chained request actually contains Option 82, preserving giaddr and normal hop processing otherwise.

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

Copilot-Session: 2b881aa9-3a3a-4aaf-b2ca-b941705b2438
Signed-off-by: Xichen96 <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).

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

Walk DHCP options with cookie, length, PAD, and END validation so bytes after END cannot make a no-option chained request subject to relay-agent mode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2b881aa9-3a3a-4aaf-b2ca-b941705b2438
Signed-off-by: Xichen96 <lukelin0907@gmail.com>
@Xichen96
Xichen96 force-pushed the dev/xichenlin/item10-chained-no-option82 branch from 31dd0bf to 0c4588b Compare July 19, 2026 17:52
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

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

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.

2 participants