From cdca8da7ddb915b579214d20395d501da811a4cf Mon Sep 17 00:00:00 2001 From: Tamer Ahmed Date: Wed, 12 May 2021 09:21:22 -0700 Subject: [PATCH] [201811][dhcp-relay]: Launch DHCP Relay On L3 Vlan Only (#7085) Recent changes brought l2 vlan concept which does not have DHCP clients behind them and so DHCP relay is not required. Also, dhcpmon fails to launch on those vlans as their interfaces lack IP addresses. This PR backposts #6527 that limits launch of both DHCP relay and dhcpmon to L3 vlans only. original-pr: #6527 singed-off-by: Tamer Ahmed tamer.ahmed@microsoft.com --- .../docker-dhcp-relay.supervisord.conf.j2 | 29 ++++++++++++------- .../tests/t0-sample-graph.xml | 17 +++++++++++ src/sonic-config-engine/tests/test_cfggen.py | 4 +-- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 index 0f359d38670f..08aad04a8b11 100644 --- a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 +++ b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2 @@ -20,11 +20,18 @@ stdout_logfile=syslog stderr_logfile=syslog {# If our configuration has VLANs... #} -{% if VLAN %} +{% if VLAN_INTERFACE %} +{% set d = namespace(vlan_list=[]) %} +{% for (name, prefix) in VLAN_INTERFACE %} +{% if name not in d.vlan_list %} +{% set d.vlan_list = d.vlan_list + [name] %} +{% endif %} +{% endfor %} +{% set vlan_list = d.vlan_list %} {# Count how many VLANs require a DHCP relay agent... #} {% set num_relays = { 'count': 0 } %} -{% for vlan_name in VLAN %} -{% if VLAN[vlan_name]['dhcp_servers'] %} +{% for vlan_name in vlan_list %} +{% if VLAN and vlan_name in VLAN and VLAN[vlan_name]['dhcp_servers'] %} {% set _dummy = num_relays.update({'count': num_relays.count + 1}) %} {% endif %} {% endfor %} @@ -33,8 +40,8 @@ stderr_logfile=syslog [group:isc-dhcp-relay] programs= {%- set add_preceding_comma = { 'flag': False } %} -{% for vlan_name in VLAN %} -{% if VLAN[vlan_name]['dhcp_servers'] %} +{% for vlan_name in vlan_list %} +{% if VLAN and vlan_name in VLAN and VLAN[vlan_name]['dhcp_servers'] %} {% if add_preceding_comma.flag %},{% endif %} {% set _dummy = add_preceding_comma.update({'flag': True}) %} isc-dhcp-relay-{{ vlan_name }} @@ -43,8 +50,8 @@ isc-dhcp-relay-{{ vlan_name }} {# Create a program entry for each DHCP relay agent instance #} -{% for vlan_name in VLAN %} -{% if VLAN[vlan_name]['dhcp_servers'] %} +{% for vlan_name in vlan_list %} +{% if VLAN and vlan_name in VLAN and VLAN[vlan_name]['dhcp_servers'] %} [program:isc-dhcp-relay-{{ vlan_name }}] {# We treat this VLAN as a downstream interface (-id), as we only want to listen for requests #} command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -id {{ vlan_name }} @@ -72,8 +79,8 @@ stderr_logfile=syslog [group:dhcpmon] programs= {%- set add_preceding_comma = { 'flag': False } %} -{% for vlan_name in VLAN %} -{% if VLAN[vlan_name]['dhcp_servers'] %} +{% for vlan_name in vlan_list %} +{% if VLAN and vlan_name in VLAN and VLAN[vlan_name]['dhcp_servers'] %} {% if add_preceding_comma.flag %},{% endif %} {% set _dummy = add_preceding_comma.update({'flag': True}) %} dhcpmon-{{ vlan_name }} @@ -83,8 +90,8 @@ dhcpmon-{{ vlan_name }} {# Create a program entry for each DHCP MONitor instance #} {% set relay_for_ipv4 = { 'flag': False } %} -{% for vlan_name in VLAN %} -{% if VLAN[vlan_name]['dhcp_servers'] %} +{% for vlan_name in vlan_list %} +{% if VLAN and vlan_name in VLAN and VLAN[vlan_name]['dhcp_servers'] %} {% for dhcp_server in VLAN[vlan_name]['dhcp_servers'] %} {% if dhcp_server | ipv4 %} {% set _dummy = relay_for_ipv4.update({'flag': True}) %} diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index adf3303f6217..35a0a584ad4e 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -237,11 +237,28 @@ fortyGigE0/4;fortyGigE0/8;fortyGigE0/12;fortyGigE0/16;fortyGigE0/20;fortyGigE0/24;fortyGigE0/28;fortyGigE0/32;fortyGigE0/36;fortyGigE0/40;fortyGigE0/44;fortyGigE0/48;fortyGigE0/52;fortyGigE0/56;fortyGigE0/60;fortyGigE0/64;fortyGigE0/68;fortyGigE0/72;fortyGigE0/76;fortyGigE0/80;fortyGigE0/84;fortyGigE0/88;fortyGigE0/92;fortyGigE0/96 False 0.0.0.0/0 + 192.0.0.1;192.0.0.2 1000 1000 192.168.0.0/27 + + + + + Vlan99 + fortyGigE0/100 + False + 0.0.0.0/0 + + UserDefinedL2Vlan + 192.0.0.1;192.0.0.2 + 99 + 99 + + + diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 1e23019f2c3b..e9c9f0f66236 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -181,7 +181,7 @@ def test_minigraph_port_autonegotiation(self): "'Ethernet0': {'alias': 'fortyGigE0/0', 'pfc_asym': 'off', 'lanes': '29,30,31,32', 'description': 'fortyGigE0/0', 'mtu': '9100'}, " "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': '1', 'speed': '100000'}, " "'Ethernet108': {'alias': 'fortyGigE0/108', 'pfc_asym': 'off', 'lanes': '81,82,83,84', 'description': 'fortyGigE0/108', 'mtu': '9100'}, " - "'Ethernet100': {'alias': 'fortyGigE0/100', 'pfc_asym': 'off', 'lanes': '125,126,127,128', 'description': 'fortyGigE0/100', 'mtu': '9100'}, " + "'Ethernet100': {'lanes': '125,126,127,128', 'description': 'fortyGigE0/100', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/100', 'admin_status': 'up'}, " "'Ethernet104': {'alias': 'fortyGigE0/104', 'pfc_asym': 'off', 'lanes': '85,86,87,88', 'description': 'fortyGigE0/104', 'mtu': '9100'}, " "'Ethernet68': {'lanes': '69,70,71,72', 'description': 'fortyGigE0/68', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/68', 'admin_status': 'up'}, " "'Ethernet96': {'lanes': '121,122,123,124', 'description': 'fortyGigE0/96', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/96', 'admin_status': 'up'}, " @@ -218,7 +218,7 @@ def test_minigraph_port_autonegotiation(self): "'Ethernet0': {'lanes': '29,30,31,32', 'description': 'fortyGigE0/0', 'mtu': '9100', 'alias': 'fortyGigE0/0', 'pfc_asym': 'off', 'autoneg': '0'}, " "'Ethernet4': {'lanes': '25,26,27,28', 'description': 'Servers0:eth0', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/4', 'admin_status': 'up', 'autoneg': '1', 'speed': '100000'}, " "'Ethernet108': {'lanes': '81,82,83,84', 'description': 'fortyGigE0/108', 'mtu': '9100', 'alias': 'fortyGigE0/108', 'pfc_asym': 'off', 'autoneg': '0'}, " - "'Ethernet100': {'lanes': '125,126,127,128', 'description': 'fortyGigE0/100', 'mtu': '9100', 'alias': 'fortyGigE0/100', 'pfc_asym': 'off', 'autoneg': '0'}, " + "'Ethernet100': {'lanes': '125,126,127,128', 'description': 'fortyGigE0/100', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/100', 'admin_status': 'up', 'autoneg': '0'}, " "'Ethernet104': {'lanes': '85,86,87,88', 'description': 'fortyGigE0/104', 'mtu': '9100', 'alias': 'fortyGigE0/104', 'pfc_asym': 'off', 'autoneg': '0'}, " "'Ethernet68': {'lanes': '69,70,71,72', 'description': 'fortyGigE0/68', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/68', 'admin_status': 'up', 'autoneg': '0'}, " "'Ethernet96': {'lanes': '121,122,123,124', 'description': 'fortyGigE0/96', 'pfc_asym': 'off', 'mtu': '9100', 'alias': 'fortyGigE0/96', 'admin_status': 'up', 'autoneg': '0'}, "