From 8d857fab16526136a907014131fafbe0db783ede Mon Sep 17 00:00:00 2001
From: Tamer Ahmed <tamer.ahmed@microsoft.com>
Date: Mon, 25 Jan 2021 10:48:48 -0800
Subject: [PATCH] [dhcp-relay]: Launch DHCP Relay On L3 Vlan (#6527)

Recent changes brought l2 vlan concept which do 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 limit launch of both DHCP relay
and dhcpmon to L3 vlans only.

singed-off-by: Tamer Ahmed <tamer.ahmed@microsoft.com>
---
 .../docker-dhcp-relay.supervisord.conf.j2     | 22 +++++++++----------
 .../tests/t0-sample-graph.xml                 | 17 ++++++++++++++
 2 files changed, 28 insertions(+), 11 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 1ec3ddb63d..19a6cc294f 100644
--- a/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2
+++ b/dockers/docker-dhcp-relay/docker-dhcp-relay.supervisord.conf.j2
@@ -39,11 +39,11 @@ dependent_startup=true
 dependent_startup_wait_for=rsyslogd:running
 
 {# If our configuration has VLANs... #}
-{% if VLAN %}
+{% if VLAN_INTERFACE %}
 {# 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_INTERFACE %}
+{% 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 %}
@@ -52,8 +52,8 @@ dependent_startup_wait_for=rsyslogd:running
 [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_INTERFACE %}
+{% 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 }}
@@ -63,8 +63,8 @@ isc-dhcp-relay-{{ vlan_name }}
 
 {# Create a program entry for each DHCP relay agent instance #}
 {% set relay_for_ipv4 = { 'flag': False } %}
-{% for vlan_name in VLAN %}
-{% if VLAN[vlan_name]['dhcp_servers'] %}
+{% for vlan_name in VLAN_INTERFACE %}
+{% 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}) %}
@@ -106,8 +106,8 @@ dependent_startup_wait_for=start:exited
 [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_INTERFACE %}
+{% 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 }}
@@ -117,8 +117,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_INTERFACE %}
+{% 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 237a2ff0f5..63f892fe6d 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 @@
           <AttachTo>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</AttachTo>
           <NoDhcpRelay>False</NoDhcpRelay>
           <StaticDHCPRelay>0.0.0.0/0</StaticDHCPRelay>
+          <FhrpProtoType i:nil="true"/>
           <Type i:nil="true"/>
           <DhcpRelays>192.0.0.1;192.0.0.2</DhcpRelays>
           <VlanID>1000</VlanID>
           <Tag>1000</Tag>
           <Subnets>192.168.0.0/27</Subnets>
+          <MacAddress i:nil="true"/>
+          <SecondarySubnets/>
+        </VlanInterface>
+        <VlanInterface>
+          <Name>Vlan99</Name>
+          <AttachTo>fortyGigE0/100</AttachTo>
+          <NoDhcpRelay>False</NoDhcpRelay>
+          <StaticDHCPRelay>0.0.0.0/0</StaticDHCPRelay>
+          <FhrpProtoType i:nil="true"/>
+          <Type>UserDefinedL2Vlan</Type>
+          <DhcpRelays>192.0.0.1;192.0.0.2</DhcpRelays>
+          <VlanID>99</VlanID>
+          <Tag>99</Tag>
+          <Subnets/>
+          <MacAddress i:nil="true"/>
+          <SecondarySubnets/>
         </VlanInterface>
       </VlanInterfaces>
       <IPInterfaces>