feat: custom DHCP relay source interface by edipascale · Pull Request #1348 · githedgehog/fabric

edipascale · 2026-03-13T15:50:48Z

No description provided.

github-actions · 2026-03-13T15:56:32Z

🚀 Temp artifacts published: v0-006df36f1 🚀

github-actions · 2026-03-16T10:51:49Z

🚀 Temp artifacts published: v0-06251f167 🚀

github-actions · 2026-03-24T13:10:32Z

🚀 Temp artifacts published: v0-6a101c614 🚀

github-actions · 2026-03-24T15:15:09Z

🚀 Temp artifacts published: v0-4e15df5d3 🚀

github-actions · 2026-03-24T15:48:34Z

🚀 Temp artifacts published: v0-60d874cc1 🚀

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>

github-actions · 2026-03-24T17:28:38Z

🚀 Temp artifacts published: v0-6a98cd9e2 🚀

edipascale · 2026-03-24T17:30:50Z

@Frostman this appears to work fine now, in that I see the DHCP packets sent to a server in another VPC. However we should probably decide what exactly the user should be able to configure when using DHCP relay, e.g.:

source interface: @mrbojangles3 was of the opinion that it's best not to expose this, if I understood him right
link-select: does not work if source interface is not specified (BCM SONiC will not accept that config), so it's linked to the previous item
vrf-select: right now I am always keeping it on but maybe that is not what customers want

Copilot

Pull request overview

Adds support for selecting a DHCP relay “source VRF” (via a referenced VPC) when configuring DHCP relay behavior, plumbing this through the VPC API → agent planning → BCM OpenConfig rendering, and documenting/exposing the new field in CRDs.

Changes:

Add relayVPC to VPCDHCP (API + CRDs + docs) and validate its usage/existence.
Extend dozer DHCP relay spec with an optional vrf and render/unmarshal it in BCM OpenConfig DHCP relay config.
Update BCM planning to omit the relay source interface for non-control-plane relay, and optionally set relay VRF derived from relayVPC.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/agent/dozer/dozer.go	Extends internal dozer DHCP relay spec with optional VRF.
pkg/agent/dozer/bcm/spec_dhcp.go	Plumbs new VRF field into OpenConfig marshal/unmarshal for DHCP relay.
pkg/agent/dozer/bcm/plan.go	Sets relay source interface conditionally and derives relay VRF from `relayVPC`.
api/vpc/v1beta1/vpc_types.go	Adds `relayVPC` to the API and validates correct usage + referenced VPC existence.
config/crd/bases/vpc.githedgehog.com_vpcs.yaml	Exposes `relayVPC` in the VPC CRD schema.
config/crd/bases/agent.githedgehog.com_agents.yaml	Exposes `relayVPC` in the Agent CRD schema (embedded VPC DHCP config).
docs/api.md	Documents the new `relayVPC` field in `VPCDHCP`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T17:38:43Z

api/vpc/v1beta1/vpc_types.go

+			if subnetCfg.DHCP.RelayVPC != "" {
+				relayVPC := &VPC{}
+				err := kube.Get(ctx, ktypes.NamespacedName{Name: subnetCfg.DHCP.RelayVPC, Namespace: vpc.Namespace}, relayVPC)
+				if err != nil {
+					if kapierrors.IsNotFound(err) {
+						return nil, errors.Errorf("subnet %s: dhcp relay VPC %s not found", subnetName, subnetCfg.DHCP.RelayVPC)
+					}
+
+					return nil, errors.Wrapf(err, "subnet %s: failed to get dhcp relay VPC %s", subnetName, subnetCfg.DHCP.RelayVPC) // TODO replace with some internal error to not expose to the user
+				}


New RelayVPC validation branches are added (rejecting relayVPC without relay, and verifying the referenced VPC exists via kube.Get), but there are currently no unit tests covering VPC.Validate. Since this package already uses fake-client validation tests for other resources, consider adding tests to cover the new RelayVPC cases (missing relay, not-found VPC, success path).

Copilot · 2026-03-24T17:38:44Z

pkg/agent/dozer/bcm/plan.go

 				return errors.Wrapf(err, "failed to parse DHCP relay %s for vpc %s", subnet.DHCP.Relay, vpcName)
 			}
+			if subnet.DHCP.RelayVPC != "" {
+				relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))


When RelayVPC is set, the plan derives a relay VRF name but doesn't verify that VRF is actually present in spec.VRFs for this agent. If the relay VPC isn't configured on this switch, this can generate a DHCP relay config that references a non-existent VRF and fail during apply. Consider validating spec.VRFs[vpcVrfName(subnet.DHCP.RelayVPC)] exists (similar to the static external WithinVPC check) or ensuring the relay VPC VRF is planned on-demand.

Suggested change

relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))

relayVRFName := vpcVrfName(subnet.DHCP.RelayVPC)

if _, ok := spec.VRFs[relayVRFName]; !ok {

return errors.Errorf("DHCP relay VPC %s is not configured on this switch (missing VRF %s)", subnet.DHCP.RelayVPC, relayVRFName)

}

relayVRF = pointer.To(relayVRFName)

Copilot · 2026-03-24T17:38:44Z

pkg/agent/dozer/bcm/plan.go

 				return errors.Wrapf(err, "failed to parse DHCP relay %s for vpc %s", subnet.DHCP.Relay, vpcName)
 			}
+			if subnet.DHCP.RelayVPC != "" {
+				relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))


Same issue as above in the L3Flat planning path: RelayVPC-derived VRF is used without checking that the VRF exists in spec.VRFs for this agent, which can yield invalid relay-agent config if the relay VPC isn't otherwise planned on this switch. Add a presence check (or plan the VRF) before setting relayVRF.

Suggested change

relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))

vrfName := vpcVrfName(subnet.DHCP.RelayVPC)

if _, ok := spec.VRFs[vrfName]; ok {

relayVRF = pointer.To(vrfName)

} else {

slog.Warn("DHCP relay VRF not found in spec.VRFs; skipping relayVRF assignment",

"vpc", vpcName,

"subnet", subnetName,

"relayVPC", subnet.DHCP.RelayVPC,

"vrf", vrfName,

)

}

Frostman · 2026-03-24T17:43:44Z

@Frostman this appears to work fine now, in that I see the DHCP packets sent to a server in another VPC. However we should probably decide what exactly the user should be able to configure when using DHCP relay, e.g.:

source interface: @mrbojangles3 was of the opinion that it's best not to expose this, if I understood him right

link-select: does not work if source interface is not specified (BCM SONiC will not accept that config), so it's linked to the previous item

vrf-select: right now I am always keeping it on but maybe that is not what customers want

link/vrf-select is just extra metadata on the request, so it should be fine
as for the source interface it's a bit more interesting - the main question would be if server running in other vpc would be able to reply to the request and it'll be relayed back to the client correctly

edipascale · 2026-03-24T17:49:53Z

@Frostman this appears to work fine now, in that I see the DHCP packets sent to a server in another VPC. However we should probably decide what exactly the user should be able to configure when using DHCP relay, e.g.:

source interface: @mrbojangles3 was of the opinion that it's best not to expose this, if I understood him right

link-select: does not work if source interface is not specified (BCM SONiC will not accept that config), so it's linked to the previous item

vrf-select: right now I am always keeping it on but maybe that is not what customers want

link/vrf-select is just extra metadata on the request, so it should be fine as for the source interface it's a bit more interesting - the main question would be if server running in other vpc would be able to reply to the request and it'll be relayed back to the client correctly

from the broadcom user guide, that shouldn't matter:

If you do not specify the source interface, the source IP address in the relayed packet is automatically determined based on the outgoing interface. The system chooses the first address (IPv4 or IPv6) configured on the interface which falls in the same network as the destination address or next hop router.

So the request will have the SAG address of the VPC, as confirmed by my testing:

core@server-02 ~ $ sudo tcpdump -ni enp2s1.1002 udp port 67 or port 68
dropped privs to pcap
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on enp2s1.1002, link-type EN10MB (Ethernet), snapshot length 262144 bytes
17:24:09.971594 IP 10.0.2.1.67 > 10.0.2.2.67: BOOTP/DHCP, Request from 0c:20:12:fe:03:01, length 348

edipascale force-pushed the ema/custom-dhcp-source-iface branch from 006df36 to 06251f1 Compare March 16, 2026 10:47

edipascale force-pushed the ema/custom-dhcp-source-iface branch from 06251f1 to 6a101c6 Compare March 24, 2026 13:06

edipascale force-pushed the ema/custom-dhcp-source-iface branch from 6a101c6 to 4e15df5 Compare March 24, 2026 15:10

edipascale force-pushed the ema/custom-dhcp-source-iface branch from 4e15df5 to 60d874c Compare March 24, 2026 15:41

feat: enable DHCP relay to other VPC

6a98cd9

Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>

edipascale force-pushed the ema/custom-dhcp-source-iface branch from 60d874c to 6a98cd9 Compare March 24, 2026 17:24

edipascale marked this pull request as ready for review March 24, 2026 17:27

edipascale requested review from a team as code owners March 24, 2026 17:27

edipascale requested a review from Copilot March 24, 2026 17:34

Copilot started reviewing on behalf of edipascale March 24, 2026 17:34 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Frostman approved these changes Mar 24, 2026

View reviewed changes

Frostman merged commit 4bedfc5 into master Mar 24, 2026
29 of 31 checks passed

Frostman deleted the ema/custom-dhcp-source-iface branch March 24, 2026 19:24

-				relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))
+				relayVRFName := vpcVrfName(subnet.DHCP.RelayVPC)
+				if _, ok := spec.VRFs[relayVRFName]; !ok {
+					return errors.Errorf("DHCP relay VPC %s is not configured on this switch (missing VRF %s)", subnet.DHCP.RelayVPC, relayVRFName)
+				}
+				relayVRF = pointer.To(relayVRFName)

-				relayVRF = pointer.To(vpcVrfName(subnet.DHCP.RelayVPC))
+				vrfName := vpcVrfName(subnet.DHCP.RelayVPC)
+				if _, ok := spec.VRFs[vrfName]; ok {
+					relayVRF = pointer.To(vrfName)
+				} else {
+					slog.Warn("DHCP relay VRF not found in spec.VRFs; skipping relayVRF assignment",
+						"vpc", vpcName,
+						"subnet", subnetName,
+						"relayVPC", subnet.DHCP.RelayVPC,
+						"vrf", vrfName,
+					)
+				}

Conversation

edipascale commented Mar 13, 2026

Uh oh!

github-actions bot commented Mar 13, 2026

Uh oh!

github-actions bot commented Mar 16, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

edipascale commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Frostman commented Mar 24, 2026

Uh oh!

edipascale commented Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants