Add routed port support to dhcp_relay#2276
Conversation
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
dce99b6 to
0131de2
Compare
|
/azp run |
|
No pipelines are associated with this pull request. |
Signed-off-by: Devesh Pathak <250206677+devesh-nexthop@users.noreply.github.com>
0131de2 to
f0e7e4e
Compare
|
/azp run |
|
No pipelines are associated with this pull request. |
|
@devesh-nexthop can we get some reviewers tagged here who have been involved in the discussion |
Updated scalability requirements and enhanced routed port support in the DHCPv4 relay agent documentation. Adjusted circuit ID and chassis ID handling, and refined CLI output examples.
|
/azp run |
|
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. |
There was a problem hiding this comment.
My Understanding is
- PORT | EthernetX contains dhcp_servers/dhcpv6_servers .
- INTERFACE|EthernetX| makes EthernetX a routed L3 Interface
- A Plain L2 PORT Without IP should not be treated as valid DHCP relay Interface.
Can we document whether this invalid case is rejected by config/YANG validation, ignored by dhcp_relay, or logged as an error?
| } | ||
|
|
||
| leaf circuit_id { | ||
| description "Custom Circuit ID value (used when circuit_id_format is 'custom')"; |
There was a problem hiding this comment.
Please add the YANG validation to use this field only when circuit_id_format is 'custom'
| description "VLAN ID"; | ||
| description "Interface name - VLAN or routed port"; | ||
| type union { | ||
| // Comment VLAN leaf reference here until libyang back-links issue is resolved and use VLAN string pattern |
There was a problem hiding this comment.
If libyang issue is no longer present, use the VLAN table reference. If this issue still present, don't remove the comment.
| // VLAN interface | ||
| type string { | ||
| pattern 'Vlan([0-9]{1,3}|[1-3][0-9]{3}|[4][0][0-8][0-9]|[4][0][9][0-4])'; | ||
| } |
There was a problem hiding this comment.
Either add other L3 interfaces (e.g portchannel, sub-intf) or change the scope to support for physical port.
| list INTERFACE_LIST { | ||
| key name; | ||
| leaf name { | ||
| type string; |
There was a problem hiding this comment.
Make sure to align this model based on https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-dhcpv6-relay.yang
| - [5.1.3 Interop with Port-Based DHCP Server](#513-interop-with-port-based-dhcp-server) | ||
| - [5.1.4 DHCP Monitor](#514-dhcp-monitor) | ||
| - [5.1.5 Dual-Tor Support](#515-dual-tor-support) | ||
| - [5.1.6 Routed Port Support](#516-physical-port-support) |
There was a problem hiding this comment.
Like we discussed in the meeting, please clarify whether the same support is available for port-channel interfaces and routed VLAN subinterfaces as well. If it is not supported, it would be better to explicitly document this limitation and explain why the same support cannot be provided.
Code changes PR:
[sonic-dhcp-relay] sonic-net/sonic-dhcp-relay#102
[sonic-buildimage] sonic-net/sonic-buildimage#26897
[sonic-mgmt] sonic-net/sonic-mgmt#24117