-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
ZTP infrastructure changes to support DHCP discovery provisioning data #3298
Conversation
- Dynamically generate DHCP client configuration based on current ZTP state - Added support to request and process hostname when using DHCPv6 - Do not process graphservice url dhcp option if ZTP is enabled, ZTP service will process it - Generate /e/n/i file with all active interfaces seeking address assignment via DHCP. Only interfaces that are created in Linux will be added to /e/n/i. Also DHCP is started only on linked up in-band interfaces. Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>
discovery is in progress.
retest broadcom please |
…ned in MGMT_INTERFACE table.
Build finished. No test results found. |
retest vsimage please |
src/sonic-config-engine/sonic-cfggen
Outdated
@@ -281,6 +289,7 @@ def main(): | |||
redis_bcc = RedisBytecodeCache(SonicV2Connector(host='127.0.0.1')) | |||
env = jinja2.Environment(loader=loader, trim_blocks=True, bytecode_cache=redis_bcc) | |||
env.filters['sort_by_port_index'] = sort_by_port_index | |||
env.filters['is_intf_exists'] = is_intf_exists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config engine should not take current system state, it only parses the config db or minigraph. otherwise, it is making sonic-cfggen complicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and I will remove the newly added filter. It is not required as well.
@@ -387,9 +387,6 @@ set /files/etc/sysctl.conf/net.ipv6.conf.default.keep_addr_on_down 1 | |||
set /files/etc/sysctl.conf/net.ipv6.conf.all.keep_addr_on_down 1 | |||
set /files/etc/sysctl.conf/net.ipv6.conf.eth0.keep_addr_on_down 1 | |||
|
|||
set /files/etc/sysctl.conf/net.ipv6.conf.eth0.accept_ra_defrtr 0 | |||
set /files/etc/sysctl.conf/net.ipv6.conf.eth0.accept_ra 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i prefer to keep it as it is. if you need dhcp, then you can enable it. I saw you have this in you interfaces.j2 file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have it in /etc/sysctl.conf, there can be a case where the setting done by interface.j2 is overwritten when sysctl files are re-read by a different service. I moved setting of accept_ra to a new file "files/dhcp/90-dhcp6-systcl.conf.j2". This will decide whether accept_ra is 1/0. For DHCPv6 on eth0 to work, accept_ra must be 1.
The changes done as part of this PR enable DHCPv6 on eth0 by default just like how DHCP is enabled on eth0. This allows access to the switch using an IPv6 address. It has dual purpose to be used for ZTP over IPv6 as well as using it for management of SONiC switch using DHCPv6 assigned IPv6 address.
If we set accept_ra to 0 by default, the access to the switch using DHCPv6 is not possible.
Following are some options for us to consider:
- DHCPv6 is enabled when MGMT_INTERFACE table is not defined
This is part of the proposed change.
- DHCPv6 is enabled only when ZTP discovery is being performed.
This might momentarily work but user may lose connectivity to management interface if MGMT_INTRFACE configuration is not done or if accept_ra setting is not changed as part of ZTP configuration steps.
- DHCPv6 is enabled only when ZTP is enabled in the build
This can be confusing as certain build combinations allows IPv6 management access out of the box and not in others.
|
||
{% if ZTP['mode']['inband'] == 'true' %} | ||
{% for port in PORT %} | ||
{% set var = {'port_exists': True if port | is_intf_exists else False } %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you are doing inband, when do you care whether the interface exists or not. by default all interface should be created by swss. in line 58, you have hotplug meaning you can wait for the interface to appear and then start doing dhcp.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and I will remove the interface exists check.
Also, the hotplug even though mentioned in the configuration, the feature is not fully functional in ifupdown2. I checked with the ifupdown2 maintainer and it is on the ToDo list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sonic-cfggen should not take system state
#3298) * ZTP infrastructure changes to support DHCP discovery provisioning data - Dynamically generate DHCP client configuration based on current ZTP state - Added support to request and process hostname when using DHCPv6 - Do not process graphservice url dhcp option if ZTP is enabled, ZTP service will process it - Generate /e/n/i file with all active interfaces seeking address assignment via DHCP. Only interfaces that are created in Linux will be added to /e/n/i. Also DHCP is started only on linked up in-band interfaces. Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>
sonic-net#3298) * ZTP infrastructure changes to support DHCP discovery provisioning data - Dynamically generate DHCP client configuration based on current ZTP state - Added support to request and process hostname when using DHCPv6 - Do not process graphservice url dhcp option if ZTP is enabled, ZTP service will process it - Generate /e/n/i file with all active interfaces seeking address assignment via DHCP. Only interfaces that are created in Linux will be added to /e/n/i. Also DHCP is started only on linked up in-band interfaces. Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>
sonic-net#3298) * ZTP infrastructure changes to support DHCP discovery provisioning data - Dynamically generate DHCP client configuration based on current ZTP state - Added support to request and process hostname when using DHCPv6 - Do not process graphservice url dhcp option if ZTP is enabled, ZTP service will process it - Generate /e/n/i file with all active interfaces seeking address assignment via DHCP. Only interfaces that are created in Linux will be added to /e/n/i. Also DHCP is started only on linked up in-band interfaces. Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>
will process it
via DHCP. Only interfaces that are created in Linux will be added to /e/n/i.
Also DHCP is started only on linked up in-band interfaces.
Signed-off-by: Rajendra Dendukuri rajendra.dendukuri@broadcom.com
- What I did
Dynamically generate contents of /e/n/i based on ZTP configuration loaded in Config DB. This allows
DHCP discovery on in-band and oob interfaces
- How I did it
Config DB is loaded with ZTP specific configuration by the config-setup service. The ZTP configuration in config DB used as input to sonic-cfggen to decide which interfaces need to running
DHCP
Also modified dhclient.conf file also modified to dynamically generate dhcp configuration when ZTP
is in progress.
- How to verify it
- Description for the changelog
Generate interfaces and dhcp client configuration for interfaces participating in Zero Touch Provisioning
- A picture of a cute animal (not mandatory but encouraged)