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

SONiC Yang model for DHCP-Relay parameters #8946

Merged
merged 9 commits into from
Nov 4, 2021

Conversation

AkhileshSamineni
Copy link
Contributor

@AkhileshSamineni AkhileshSamineni commented Oct 11, 2021

SONiC Yang model for DHCP-Relay parameters

Signed-off-by: Akhilesh Samineni [email protected]

Added DHCPv4 Source interface, Link select, Max hop count, Server VRF, Policy action and VRF Select options.
Added DHCPv6 Servers, Source interface, Max hop count, Server VRF and VRF Select options.
Tables: VLAN.
Copy link
Collaborator

@dgsudharsan dgsudharsan left a comment

Choose a reason for hiding this comment

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

Please remove the fields for which we don't have backend support

@dgsudharsan
Copy link
Collaborator

@shlomibitton Please review

@AkhileshSamineni
Copy link
Contributor Author

Please remove the fields for which we don't have backend support

@dgsudharsan Removed those fields.

@AkhileshSamineni
Copy link
Contributor Author

@shlomibitton I still see the "dhcpv6_servers" added to VLAN table only from below configuration.

root@sonic:/home/admin# config vlan add 100
root@sonic:/home/admin# stty cols 200
root@sonic:/home/admin# config vlan member add 100 Ethernet48
root@sonic:/home/admin# config interface ip add Vlan100 10.0.0.1/24
root@sonic:/home/admin# 
root@sonic:/home/admin# config vlan dhcp_relay add 100 20.20.0.1
root@sonic:/home/admin# config vlan dhcp_relay add 100 20.20.20.20
root@sonic:/home/admin#
root@sonic:/home/admin# config interface ip add Vlan100 1001::1/24 
root@sonic:/home/admin# config vlan  dhcp_relay add 100 2001::2

root@sonic:/home/admin# show vlan brief
+-----------+--------------+------------+----------------+-------------+-----------------------+
|   VLAN ID | IP Address   | Ports      | Port Tagging   | Proxy ARP   | DHCP Helper Address   |
+===========+==============+============+================+=============+=======================+
|       100 | 10.0.0.1/24  | Ethernet48 | tagged         | disabled    | 20.20.0.1             |
|           | 1001::1/24   |            |                |             | 20.20.20.20           |
|           |              |            |                |             | 2001::2               |
+-----------+--------------+------------+----------------+-------------+-----------------------+
root@sonic:/home/admin# 
root@sonic:/home/admin# sonic-db-cli CONFIG_DB hgetall "VLAN|Vlan100"
{'dhcp_servers@': '20.20.0.1,20.20.20.20', 'vlanid': '100', 'dhcpv6_servers@': '2001::2'}
root@sonic:/home/admin#

@shlomibitton
Copy link
Contributor

@AkhileshSamineni by design the DHCPv6 helpers should be configured from the configuration you mentioned.
Is there any problem with this?
https://github.com/Azure/SONiC/blob/master/doc/DHCPv6_Relay/DHCPv6_Relay_HLD.md#3-cli

@AkhileshSamineni
Copy link
Contributor Author

@AkhileshSamineni by design the DHCPv6 helpers should be configured from the configuration you mentioned. Is there any problem with this? https://github.com/Azure/SONiC/blob/master/doc/DHCPv6_Relay/DHCPv6_Relay_HLD.md#3-cli

@shlomibitton So "dhcpv6_servers" field is getting added to "VLAN" table only right. Please review the yang model changes.

shlomibitton
shlomibitton previously approved these changes Oct 25, 2021
Copy link
Contributor

@shlomibitton shlomibitton left a comment

Choose a reason for hiding this comment

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

LGTM

@lguohan lguohan requested a review from kellyyeh October 28, 2021 17:26
@lguohan
Copy link
Collaborator

lguohan commented Oct 28, 2021

@kellyyeh, can you review?

@@ -229,6 +232,9 @@
"dhcp_servers": [
"10.222.72.116"
],
"dhcpv6_servers": [
Copy link
Contributor

Choose a reason for hiding this comment

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

We currently store dhcpv6_servers in a separate table called DHCP_RELAY, and SONiC dhcp6relay fetches dhcpv6 server addresses from this table instead of VLAN. This is the draft sonic-dhcpv6-relay.yang

module DHCP_RELAY  
    container DHCP_RELAY {  	
        list VLAN_LIST {
    		key name;
   		    leaf name {
    			type string;
  		    }
   		    leaf dhcpv6_servers {
     		    	type inet6:ip-address;
  		    }
		    leaf dhcpv6_option|rfc6939_support {
			    type bool;
		    }
        }
    }
}

And config_db schema looks like this:

DHCP_RELAY|intf-i|dhcpv6_servers: ["dhcp-server-0", "dhcp-server-1", ...., "dhcp-server-n-1"]
DHCP_RELAY|intf-i|dhcpv6_option|rfc6939_support: "true"

202106 version currently still uses ISC-DHCP which reads from VLAN table, so we need to have dhcpv6_servers in both VLAN and DHCP_RELAY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new 'dhcpv6-relay' yang file changes for DHCP_RELAY table.

@kellyyeh Seeing parsing issues with leaf "dhcpv6_option|rfc6939_support", as it has '|' character.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add it as just "rfc6939_support" please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, added leaf as 'rfc6939_support'.

@AkhileshSamineni
Copy link
Contributor Author

@lguohan Please approve and merge it.

@zhangyanzhao zhangyanzhao added the YANG YANG model related changes label Nov 4, 2021
@lguohan lguohan linked an issue Nov 4, 2021 that may be closed by this pull request
@lguohan lguohan merged commit 2c801ef into sonic-net:master Nov 4, 2021
praveen-li pushed a commit to praveen-li/sonic-buildimage that referenced this pull request Feb 15, 2022
This commit brings part of
sonic-net#8946.

Signed-off-by: Praveen Chaudhary <[email protected]>
RB=2977661
G=lnos-reviewers
R=pchaudhary,pmao,samaity,zxu
A=
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[yang-models] VLAN yang model is not up-to-date
6 participants