[201803][dhcp_relay] Add support for DHCP client(s) on one VLAN and D…

…HCP server(s) on another (#2857) * Change URL for isc-dhcp source repository * Port upstream patches to isc-dhcp-relay to support upstream/downstream interfaces * Modify supervisor conf to generate dhcrelay commands with '-id' and '-iu' options * Comments; Also clean up jinja2 syntax * Patch relay to open one socket per interface and send to all servers on all upstream interfaces * Patch relay agent to properly forward BOOTREQUEST only on appropriate interfaace if it is a directed broadcast * Update patch to properly support interfaces with multiple IP addresses assigned * Pass --enable-use-sockets to configure instead of uncommenting USE_SOCKETS directly
sonic-net · May 17, 2019 · bce72c9 · bce72c9
1 parent ca4f587
commit bce72c9
Show file tree

Hide file tree

Showing 9 changed files with 700 additions and 17 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
@@ -32,28 +32,33 @@ stderr_logfile=syslog
 {% if num_relays.count > 0 %}
 [group:isc-dhcp-relay]
 programs=
-{%- set add_preceding_comma = { 'flag': False } -%}
-{%- for vlan_name in VLAN -%}
-{%- if VLAN[vlan_name]['dhcp_servers'] -%}
-{%- if add_preceding_comma.flag %},{% endif -%}
-{%- set _dummy = add_preceding_comma.update({'flag': True}) -%}
+{%- set add_preceding_comma = { 'flag': False } %}
+{% for vlan_name in VLAN %}
+{% if VLAN[vlan_name]['dhcp_servers'] %}
+{% if add_preceding_comma.flag %},{% endif %}
+{% set _dummy = add_preceding_comma.update({'flag': True}) %}
 isc-dhcp-relay-{{ vlan_name }}
 {%- endif %}
 {% endfor %}
 
 
 {# 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 %}
+{% if VLAN[vlan_name]['dhcp_servers'] %}
 [program:isc-dhcp-relay-{{ vlan_name }}]
-command=/usr/sbin/dhcrelay -d -m discard -a %%h:%%p %%P --name-alias-map-file /tmp/port-name-alias-map.txt -i {{ vlan_name }}
-{%- for (name, prefix) in INTERFACE -%}
-{%- if prefix | ipv4 %} -i {{ name }}{% endif -%}
-{%- endfor -%}
-{%- for (name, prefix) in PORTCHANNEL_INTERFACE -%}
-{%- if prefix | ipv4 %} -i {{ name }}{% endif -%}
-{%- endfor -%}
-{%- for dhcp_server in VLAN[vlan_name]['dhcp_servers'] %} {{ dhcp_server }}{% endfor %}
+{# 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 }}
+{#- We treat all other interfaces as upstream interfaces (-iu), as we only want to listen for replies #}
+{% for (name, prefix) in VLAN_INTERFACE %}
+{% if prefix | ipv4 and name != vlan_name %} -iu {{ name }}{% endif -%}
+{% endfor %}
+{% for (name, prefix) in INTERFACE %}
+{% if prefix | ipv4 %} -iu {{ name }}{% endif -%}
+{% endfor %}
+{% for (name, prefix) in PORTCHANNEL_INTERFACE %}
+{% if prefix | ipv4 %} -iu {{ name }}{% endif -%}
+{% endfor %}
+{% for dhcp_server in VLAN[vlan_name]['dhcp_servers'] %} {{ dhcp_server }}{% endfor %}
 
 priority=3
 autostart=false

diff --git a/src/isc-dhcp/Makefile b/src/isc-dhcp/Makefile
@@ -11,7 +11,7 @@ $(addprefix $(DEST)/, $(MAIN_TARGET)): $(DEST)/% :
 	rm -rf ./isc-dhcp
 
 	# Clone isc-dhcp repo
-	git clone https://salsa.debian.org/berni/isc-dhcp.git
+	git clone https://salsa.debian.org/dhcp-team/isc-dhcp.git
 	pushd ./isc-dhcp
 
 	# Reset HEAD to the commit of the proper tag

diff --git a/src/isc-dhcp/patch/0005-Port-upstream-changes-from-commit-f2e70402f0f2955f39.patch b/src/isc-dhcp/patch/0005-Port-upstream-changes-from-commit-f2e70402f0f2955f39.patch
@@ -0,0 +1,102 @@
+From 0092eed7a80f38078a38fedf601952d0d25c6183 Mon Sep 17 00:00:00 2001
+From: Joe LeVeque <jolevequ@microsoft.com>
+Date: Thu, 2 May 2019 19:20:59 +0000
+Subject: [PATCH 1/2] Port upstream changes from commit
+ f2e70402f0f2955f392edc4eb2dd835b820e25bc to add '-iu' option
+
+---
+ common/discover.c |  8 +++++++-
+ relay/dhcrelay.c  | 36 +++++++++++++++++++++++++++++++++++-
+ 2 files changed, 42 insertions(+), 2 deletions(-)
+
+diff --git a/common/discover.c b/common/discover.c
+index 3cd64a7..e20d9d5 100644
+--- a/common/discover.c
++++ b/common/discover.c
+@@ -948,8 +948,14 @@ discover_interfaces(int state) {
+ 		ir = 0;
+ 	else if (state == DISCOVER_UNCONFIGURED)
+ 		ir = INTERFACE_REQUESTED | INTERFACE_AUTOMATIC;
+-	else
++	else {
+ 		ir = INTERFACE_REQUESTED;
++		if (state == DISCOVER_RELAY && local_family == AF_INET) {
++			/* We're a v4 relay without specifically requested
++			 * interfaces, so mark them all as bidirectional. */
++			ir |= INTERFACE_STREAMS;
++		}
++	}
+
+ 	/* Cycle through the list of interfaces looking for IP addresses. */
+ 	while (next_iface(&info, &err, &ifaces)) {
+diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
+index 15f0acf..8051e17 100644
+--- a/relay/dhcrelay.c
++++ b/relay/dhcrelay.c
+@@ -172,6 +172,7 @@ static const char url[] =
+ "                     [-m append|replace|forward|discard]\n" \
+ "                     [--name-alias-map-file <name-alias-map-file>]\n" \
+ "                     [-i interface0 [ ... -i interfaceN]\n" \
++"                     [-iu interface0 [ ... -iu interfaceN]\n" \
+ "                     server0 [ ... serverN]\n\n" \
+ "       dhcrelay -6   [-d] [-q] [-I] [-c <hops>] [-p <port>]\n" \
+ "                     [-pf <pid-file>] [--no-pid]\n" \
+@@ -188,6 +189,7 @@ static const char url[] =
+ "                [-pf <pid-file>] [--no-pid]\n"\
+ "                [-m append|replace|forward|discard]\n" \
+ "                [-i interface0 [ ... -i interfaceN]\n" \
++"                [-iu interface0 [ ... -iu interfaceN]\n" \
+ "                server0 [ ... serverN]\n\n" DHCRELAY_OPTION82_USAGE
+ #endif
+
+@@ -304,7 +306,34 @@ main(int argc, char **argv) {
+ 					  isc_result_totext(status));
+ 			}
+ 			strcpy(tmp->name, argv[i]);
+-			interface_snorf(tmp, INTERFACE_REQUESTED);
++			interface_snorf(tmp, (INTERFACE_REQUESTED |
++					      INTERFACE_STREAMS));
++			interface_dereference(&tmp, MDL);
++		} else if (!strcmp(argv[i], "-iu")) {
++#ifdef DHCPv6
++			if (local_family_set && (local_family == AF_INET6)) {
++				usage();
++			}
++			local_family_set = 1;
++			local_family = AF_INET;
++#endif
++			if (++i == argc) {
++				usage();
++			}
++			if (strlen(argv[i]) >= sizeof(tmp->name)) {
++				log_fatal("%s: interface name too long "
++					  "(is %ld)",
++					  argv[i], (long)strlen(argv[i]));
++			}
++			status = interface_allocate(&tmp, MDL);
++			if (status != ISC_R_SUCCESS) {
++				log_fatal("%s: interface_allocate: %s",
++					  argv[i],
++					  isc_result_totext(status));
++			}
++			strcpy(tmp->name, argv[i]);
++			interface_snorf(tmp, (INTERFACE_REQUESTED |
++					      INTERFACE_UPSTREAM));
+ 			interface_dereference(&tmp, MDL);
+ 		} else if (!strcmp(argv[i], "-a")) {
+ #ifdef DHCPv6
+@@ -691,6 +720,11 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet,
+
+ 	/* If it's a bootreply, forward it to the client. */
+ 	if (packet->op == BOOTREPLY) {
++		if (!(ip->flags & INTERFACE_UPSTREAM)) {
++			log_debug("Dropping reply received on %s", ip->name);
++			return;
++		}
++
+ 		if (!(packet->flags & htons(BOOTP_BROADCAST)) &&
+ 			can_unicast_without_arp(out)) {
+ 			to.sin_addr = packet->yiaddr;
+-- 
+2.17.1
+
diff --git a/src/isc-dhcp/patch/0006-Port-upstream-changes-from-commit-edd6d8881bc4d8ec4b.patch b/src/isc-dhcp/patch/0006-Port-upstream-changes-from-commit-edd6d8881bc4d8ec4b.patch
@@ -0,0 +1,157 @@
+From 1d59a7bd4dc9cb3cd13aedf604a15a8589febe43 Mon Sep 17 00:00:00 2001
+From: Joe LeVeque <jolevequ@microsoft.com>
+Date: Thu, 2 May 2019 19:46:42 +0000
+Subject: [PATCH 2/2] Port upstream changes from commit
+ edd6d8881bc4d8ec4b04173c66c1c840756bbe76 to add '-id' option
+
+---
+ relay/dhcrelay.c | 89 +++++++++++++++++++++++++++++++++---------------
+ 1 file changed, 61 insertions(+), 28 deletions(-)
+
+diff --git a/relay/dhcrelay.c b/relay/dhcrelay.c
+index 8051e17..292ba4f 100644
+--- a/relay/dhcrelay.c
++++ b/relay/dhcrelay.c
+@@ -142,6 +142,8 @@ static int strip_relay_agent_options(struct interface_info *,
+ 				     struct interface_info **,
+ 				     struct dhcp_packet *, unsigned);
+
++static void request_v4_interface(const char* name, int flags);
++
+ static int load_interface_alias_map(const char *port_alias_map_file_path);
+ static int get_interface_alias_by_name(const char *if_name, char *if_alias_out);
+ static void free_interface_alias_map(void);
+@@ -173,6 +175,7 @@ static const char url[] =
+ "                     [--name-alias-map-file <name-alias-map-file>]\n" \
+ "                     [-i interface0 [ ... -i interfaceN]\n" \
+ "                     [-iu interface0 [ ... -iu interfaceN]\n" \
++"                     [-id interface0 [ ... -id interfaceN]\n" \
+ "                     server0 [ ... serverN]\n\n" \
+ "       dhcrelay -6   [-d] [-q] [-I] [-c <hops>] [-p <port>]\n" \
+ "                     [-pf <pid-file>] [--no-pid]\n" \
+@@ -190,6 +193,7 @@ static const char url[] =
+ "                [-m append|replace|forward|discard]\n" \
+ "                [-i interface0 [ ... -i interfaceN]\n" \
+ "                [-iu interface0 [ ... -iu interfaceN]\n" \
++"                [-id interface0 [ ... -id interfaceN]\n" \
+ "                server0 [ ... serverN]\n\n" DHCRELAY_OPTION82_USAGE
+ #endif
+
+@@ -294,21 +298,8 @@ main(int argc, char **argv) {
+ 			if (++i == argc) {
+ 				usage();
+ 			}
+-			if (strlen(argv[i]) >= sizeof(tmp->name)) {
+-				log_fatal("%s: interface name too long "
+-					  "(is %ld)",
+-					  argv[i], (long)strlen(argv[i]));
+-			}
+-			status = interface_allocate(&tmp, MDL);
+-			if (status != ISC_R_SUCCESS) {
+-				log_fatal("%s: interface_allocate: %s",
+-					  argv[i],
+-					  isc_result_totext(status));
+-			}
+-			strcpy(tmp->name, argv[i]);
+-			interface_snorf(tmp, (INTERFACE_REQUESTED |
+-					      INTERFACE_STREAMS));
+-			interface_dereference(&tmp, MDL);
++
++			request_v4_interface(argv[i], INTERFACE_STREAMS);
+ 		} else if (!strcmp(argv[i], "-iu")) {
+ #ifdef DHCPv6
+ 			if (local_family_set && (local_family == AF_INET6)) {
+@@ -320,21 +311,21 @@ main(int argc, char **argv) {
+ 			if (++i == argc) {
+ 				usage();
+ 			}
+-			if (strlen(argv[i]) >= sizeof(tmp->name)) {
+-				log_fatal("%s: interface name too long "
+-					  "(is %ld)",
+-					  argv[i], (long)strlen(argv[i]));
++
++			request_v4_interface(argv[i], INTERFACE_UPSTREAM);
++		} else if (!strcmp(argv[i], "-id")) {
++#ifdef DHCPv6
++			if (local_family_set && (local_family == AF_INET6)) {
++				usage();
+ 			}
+-			status = interface_allocate(&tmp, MDL);
+-			if (status != ISC_R_SUCCESS) {
+-				log_fatal("%s: interface_allocate: %s",
+-					  argv[i],
+-					  isc_result_totext(status));
++			local_family_set = 1;
++			local_family = AF_INET;
++#endif
++			if (++i == argc) {
++				usage();
+ 			}
+-			strcpy(tmp->name, argv[i]);
+-			interface_snorf(tmp, (INTERFACE_REQUESTED |
+-					      INTERFACE_UPSTREAM));
+-			interface_dereference(&tmp, MDL);
++
++			request_v4_interface(argv[i], INTERFACE_DOWNSTREAM);
+ 		} else if (!strcmp(argv[i], "-a")) {
+ #ifdef DHCPv6
+ 			if (local_family_set && (local_family == AF_INET6)) {
+@@ -782,6 +773,11 @@ do_relay4(struct interface_info *ip, struct dhcp_packet *packet,
+ 	if (out)
+ 		return;
+
++	if (!(ip->flags & INTERFACE_DOWNSTREAM)) {
++		log_debug("Dropping request received on %s", ip->name);
++		return;
++	}
++
+ 	/* Add relay agent options if indicated.   If something goes wrong,
+ 	   drop the packet. */
+ 	if (!(length = add_relay_agent_options(ip, packet, length,
+@@ -1991,6 +1987,43 @@ dhcp_set_control_state(control_object_state_t oldstate,
+ 	exit(0);
+ }
+
++/*!
++ *
++ * \brief Allocate an interface as requested with a given set of flags
++ *
++ * The requested interface is allocated, its flags field is set to
++ * INTERFACE_REQUESTED OR'd with the given flags,  and then added to
++ * the list of interfaces.
++ *
++ * \param name - name of the requested interface
++ * \param flags - additional flags for the interface
++ *
++ * \return Nothing
++ */
++void request_v4_interface(const char* name, int flags) {
++	struct interface_info *tmp = NULL;
++	int len = strlen(name);
++	isc_result_t status;
++
++	if (len >= sizeof(tmp->name)) {
++		log_fatal("%s: interface name too long (is %d)", name, len);
++	}
++
++	status = interface_allocate(&tmp, MDL);
++	if (status != ISC_R_SUCCESS) {
++		log_fatal("%s: interface_allocate: %s", name,
++			  isc_result_totext(status));
++	}
++
++       log_debug("Requesting: %s as upstream: %c downstream: %c", name,
++		 (flags & INTERFACE_UPSTREAM ? 'Y' : 'N'),
++		 (flags & INTERFACE_DOWNSTREAM ? 'Y' : 'N'));
++
++	strncpy(tmp->name, name, len);
++	interface_snorf(tmp, (INTERFACE_REQUESTED | flags));
++	interface_dereference(&tmp, MDL);
++}
++
+ #define MAX_PORT_CONFIG_LINE_LEN 1024
+
+ // Allocates and loads global map g_interface_name_alias_map
+-- 
+2.17.1
+
diff --git a/src/isc-dhcp/patch/0007-Add-enable-use-sockets-to-configure-flags-in-debian-.patch b/src/isc-dhcp/patch/0007-Add-enable-use-sockets-to-configure-flags-in-debian-.patch
@@ -0,0 +1,28 @@
+From ab779f1d59f27f66e6e6bea89287c810e0a10a0f Mon Sep 17 00:00:00 2001
+From: Joe LeVeque <jolevequ@microsoft.com>
+Date: Wed, 15 May 2019 23:48:08 +0000
+Subject: [PATCH 1/3] Add --enable-use-sockets to configure flags in
+ debian/rules
+
+This defines USE_SOCKETS at compile time which forces dhcrelay
+to create and bind separate sockets to each interface
+---
+ debian/rules | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/debian/rules b/debian/rules
+index 4184716..46d6527 100755
+--- a/debian/rules
++++ b/debian/rules
+@@ -23,7 +23,7 @@ CFLAGS+=-D_PATH_DHCLIENT_CONF='\"/etc/dhcp/dhclient.conf\"'
+ CFLAGS+=-D_PATH_DHCLIENT_DB='\"$(LEASE_PATH)/dhclient.leases\"'
+ CFLAGS+=-D_PATH_DHCLIENT6_DB='\"$(LEASE_PATH)/dhclient6.leases\"'
+
+-CONFFLAGS=--prefix=/usr --enable-log-pid --enable-paranoia
++CONFFLAGS=--prefix=/usr --enable-log-pid --enable-paranoia --enable-use-sockets
+
+ # cross-architecture building
+ ifneq ($(DEB_HOST_GNU_TYPE),$(DEB_BUILD_GNU_TYPE))
+-- 
+2.17.1
+