Merge pull request #2391 from pqarmitage/updates

Add options for setting interface group of VMACs and ipvlans

INSTALL

@@ -97,7 +97,7 @@ To build using clang/llvm
 Alpine Linux
 ------------
 The following libraries need to be installed:
-	iptables-dev ipset-dev libnl3-dev musl-dev libnftnl-dev and openssl-dev or libressl-dev
+	linux-headers iptables-dev ipset-dev libnl3-dev musl-dev libnftnl-dev and openssl-dev or libressl-dev
 For magic file identification support:
 	file-dev
 For SNMP support:

doc/man/man5/keepalived.conf.5.in

@@ -1625,7 +1625,11 @@ The syntax for vrrp_instance is :
     # interface it will force a netlink message to be sent for the base interface
     # since the kernel does not send one, even if the promiscuity of the base
     # interface has been updated.
-    \fBuse_vmac \fR[<VMAC_INTERFACE_NAME>] [MAC_ADDRESS] [netlink_notify_msg]
+    # By default the VMAC is created in the same link group as the parent interface.
+    # Specifying group GROUP_ID (where GROUP_ID is either a valid group number, or a
+    # name in /etc/iproute2/group) will create the interface in the specied group.
+    # The name option can be specified if you want to use an interface name "group".
+    \fBuse_vmac \fR[[name] <VMAC_INTERFACE_NAME>] [MAC_ADDRESS] [netlink_notify_msg] [group GROUP_ID]
 
     # use_vmac_addr is used to create VMAC (macvlan) interfaces for
     # each interface that is used by a VIP or eVIP where the interface
@@ -1649,12 +1653,15 @@ The syntax for vrrp_instance is :
     # For IPv4 instances, an IP address is required, for IPv6
     # the address is optional, in which case the link local
     # address will be used.
-    # The mode flags default to bridge. NOTE: the mode flags must be the
+    # The mode flags defaults to bridge. NOTE: the mode flags must be the
     # same for all ipvlans on the same underlying interface.
     # It is safer to configure an interface name, in case keepalived crashes
     # and restarts, in which case it can more reliably find a previously
     # created interface.
-    \fBuse_ipvlan \fR[<INTERFACE_NAME>] [IP_ADDRESS] [bridge|private|vepa]
+    # The name option can be specified if you want to use a name that would cause
+    # a parsing error (e.g. "bridge").
+    # For a description of the group option, see use_vmac.
+    \fBuse_ipvlan \fR[[name] <INTERFACE_NAME>] [IP_ADDRESS] [bridge|private|vepa] [group GROUP_ID]
 
     # force instance to use IPv6 (this option is deprecated since
     # the virtual ip addresses determine whether IPv4 or IPv6 is used).

keepalived/include/vrrp.h

@@ -73,6 +73,7 @@ enum vrrp_flags_bits {
 	VRRP_VMAC_XMITBASE_BIT,
 	VRRP_VMAC_ADDR_BIT,
 	VRRP_VMAC_NETLINK_NOTIFY,
+	VRRP_VMAC_GROUP,
 #ifdef _HAVE_VRRP_IPVLAN_
 	VRRP_IPVLAN_BIT,
 #endif
@@ -260,6 +261,7 @@ typedef struct _vrrp_t {
 #ifdef _HAVE_VRRP_VMAC_
 	char			vmac_ifname[IFNAMSIZ];	/* Name of VRRP VMAC interface */
 	u_char			ll_addr[ETH_ALEN];	/* Override MAC address */
+	uint32_t		vmac_group;		/* interface group for VMAC/ipvlan */
 #ifdef _HAVE_VRRP_IPVLAN_
 	struct _ip_address	*ipvlan_addr;		/* Address to configure on an ipvlan interface */
 	int			ipvlan_type;		/* Bridge, private or VEPA mode */

keepalived/vrrp/vrrp_data.c

@@ -648,10 +648,6 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
 					vrrp->ll_addr[0], vrrp->ll_addr[1], vrrp->ll_addr[2], vrrp->ll_addr[3], vrrp->ll_addr[4], vrrp->ll_addr[5],
 					__test_bit(VRRP_VMAC_MAC_USE_VRID, &vrrp->flags) ? " (using VRID)" : "");
 	}
-	if (__test_bit(VRRP_VMAC_NETLINK_NOTIFY, &vrrp->flags))
-		conf_write(fp, "   Force netlink update for base interface");
-	if (__test_bit(VRRP_VMAC_ADDR_BIT, &vrrp->flags))
-		conf_write(fp, "   Use VMAC for VIPs on other interfaces");
 #ifdef _HAVE_VRRP_IPVLAN_
 	else if (__test_bit(VRRP_IPVLAN_BIT, &vrrp->flags))
 		conf_write(fp, "   Use IPVLAN, i/f %s, is_up = %s%s%s, type %s",
@@ -666,6 +662,20 @@ dump_vrrp(FILE *fp, const vrrp_t *vrrp)
 #endif
 					);
 #endif
+
+	/* The following two flags should only be set on VMACs, but
+	 * we check them for any interface type, just incase ... */
+	if (__test_bit(VRRP_VMAC_NETLINK_NOTIFY, &vrrp->flags))
+		conf_write(fp, "     Force netlink update for base interface");
+	if (__test_bit(VRRP_VMAC_ADDR_BIT, &vrrp->flags))
+		conf_write(fp, "     Use VMAC for VIPs on other interfaces");
+
+	/* The following should only be specified for VMACs and ipvlans */
+	if (__test_bit(VRRP_VMAC_GROUP, &vrrp->flags))
+		conf_write(fp, "     Interface group %u", vrrp->vmac_group);
+	else if (vrrp->ifp->base_ifp->group)
+		conf_write(fp, "     Interface group %u (copied from parent)", vrrp->ifp->base_ifp->group);
+
 	if (vrrp->ifp && vrrp->ifp->is_ours) {
 		conf_write(fp, "   Interface = %s, %s on %s%s", IF_NAME(vrrp->ifp),
 				__test_bit(VRRP_VMAC_BIT, &vrrp->flags) ? "vmac" : "ipvlan",

keepalived/vrrp/vrrp_parser.c

@@ -44,6 +44,8 @@
 #include "global_data.h"
 #include "global_parser.h"
 
+#include "rttables.h"
+
 #include "vrrp_data.h"
 #include "vrrp_ipaddress.h"
 #include "vrrp_sync.h"
@@ -514,7 +516,7 @@ vrrp_vmac_handler(const vector_t *strvec)
 	__set_bit(VRRP_VMAC_BIT, &current_vrrp->flags);
 
 	/* Ifname and MAC address can be specified */
-	for (i = 1; i < vector_size(strvec) && i <= 2; i++) {
+	for (i = 1; i < vector_size(strvec); i++) {
 		if (strchr(strvec_slot(strvec, i), ':')) {
 			/* It's a MAC address - interface names cannot include a ':' */
 			if (__test_bit(VRRP_VMAC_MAC_SPECIFIED, &current_vrrp->flags)) {
@@ -560,49 +562,65 @@ vrrp_vmac_handler(const vector_t *strvec)
 				report_config_error(CONFIG_GENERAL_ERROR, "VMAC MAC address not allowed to be RFC5798 address (%s) - ignoring", strvec_slot(strvec, i));
 			else
 				__set_bit(VRRP_VMAC_MAC_SPECIFIED, &current_vrrp->flags);
-		} else {
-			name = strvec_slot(strvec, i);
 
-			if (current_vrrp->vmac_ifname[0]) {
-				report_config_error(CONFIG_GENERAL_ERROR, "VMAC interface name already specified");
-				continue;
-			}
+			continue;
+		}
 
-			/* The string "netlink_notify_msg" needs to be longer than IFNAMSIZ
-			 * so that it cannot be a valid interface name. */
-			if (!strcmp(name, "netlink_notify_msg")) {
-				__set_bit(VRRP_VMAC_NETLINK_NOTIFY, &current_vrrp->flags);
-				continue;
-			}
+		if (!strcmp(strvec_slot(strvec, i), "netlink_notify_msg")) {
+			__set_bit(VRRP_VMAC_NETLINK_NOTIFY, &current_vrrp->flags);
+			continue;
+		}
 
-			if (!dev_name_valid(name)) {
-				report_config_error(CONFIG_GENERAL_ERROR, "VMAC interface name '%s' too long or invalid characters - ignoring", name);
+		if (!strcmp(strvec_slot(strvec, i), "group")) {
+			uint32_t group;
+			if (!find_rttables_group(strvec_slot(strvec, ++i), &group)) {
+				report_config_error(CONFIG_GENERAL_ERROR, "VMAC group %s not found", strvec_slot(strvec, i));
 				continue;
 			}
+			__set_bit(VRRP_VMAC_GROUP, &current_vrrp->flags);
+			current_vrrp->vmac_group = group;
+			continue;
+		}
 
-			/* Check another vrrp instance isn't using this name */
-			list_for_each_entry(ovrrp, &vrrp_data->vrrp, e_list) {
-				if (!strcmp(name, ovrrp->vmac_ifname)) {
-					report_config_error(CONFIG_GENERAL_ERROR, "(%s) VRRP instance %s is already using %s - ignoring name", current_vrrp->iname, ovrrp->iname, name);
-					name = NULL;
-					break;
-				}
-			}
+		if (!strcmp(strvec_slot(strvec, i), "name")) {
+			/* Skip over "name" */
+			i++;
+		}
 
-			if (!name)
-				continue;
+		if (current_vrrp->vmac_ifname[0]) {
+			report_config_error(CONFIG_GENERAL_ERROR, "VMAC interface name already specified");
+			continue;
+		}
 
-			strcpy(current_vrrp->vmac_ifname, name);
+		name = strvec_slot(strvec, i);
 
-			/* Check if the interface exists and is a macvlan we can use */
-			if ((ifp = if_get_by_ifname(current_vrrp->vmac_ifname, IF_NO_CREATE)) &&
-			    (ifp->if_type != IF_TYPE_MACVLAN ||
-			     ifp->vmac_type != MACVLAN_MODE_PRIVATE)) {
-				/* ??? also check ADDR_GEN_MODE and VRF enslavement matches parent */
-				report_config_error(CONFIG_GENERAL_ERROR, "(%s) interface %s already exists and is not a private macvlan; ignoring vmac if_name", current_vrrp->iname, current_vrrp->vmac_ifname);
-				current_vrrp->vmac_ifname[0] = '\0';
+		if (!dev_name_valid(name)) {
+			report_config_error(CONFIG_GENERAL_ERROR, "VMAC interface name '%s' too long or invalid characters - ignoring", name);
+			continue;
+		}
+
+		/* Check another vrrp instance isn't using this name */
+		list_for_each_entry(ovrrp, &vrrp_data->vrrp, e_list) {
+			if (!strcmp(name, ovrrp->vmac_ifname)) {
+				report_config_error(CONFIG_GENERAL_ERROR, "(%s) VRRP instance %s is already using %s - ignoring name", current_vrrp->iname, ovrrp->iname, name);
+				name = NULL;
+				break;
 			}
 		}
+
+		if (!name)
+			continue;
+
+		strcpy(current_vrrp->vmac_ifname, name);
+
+		/* Check if the interface exists and is a macvlan we can use */
+		if ((ifp = if_get_by_ifname(current_vrrp->vmac_ifname, IF_NO_CREATE)) &&
+		    (ifp->if_type != IF_TYPE_MACVLAN ||
+		     ifp->vmac_type != MACVLAN_MODE_PRIVATE)) {
+			/* ??? also check ADDR_GEN_MODE and VRF enslavement matches parent */
+			report_config_error(CONFIG_GENERAL_ERROR, "(%s) interface %s already exists and is not a private macvlan; ignoring vmac if_name", current_vrrp->iname, current_vrrp->vmac_ifname);
+			current_vrrp->vmac_ifname[0] = '\0';
+		}
 	}
 }
 
@@ -677,7 +695,20 @@ vrrp_ipvlan_handler(const vector_t *strvec)
 			continue;
 		}
 
-		if (check_valid_ipaddress(strvec_slot(strvec, i), true)) {
+		if (!strcmp(strvec_slot(strvec, i), "group")) {
+			uint32_t group;
+			if (!find_rttables_group(strvec_slot(strvec, ++i), &group)) {
+				report_config_error(CONFIG_GENERAL_ERROR, "ipvlan group %s not found", strvec_slot(strvec, i));
+				continue;
+			}
+			__set_bit(VRRP_VMAC_GROUP, &current_vrrp->flags);
+			current_vrrp->vmac_group = group;
+			continue;
+		}
+
+		if (!strcmp(strvec_slot(strvec, i), "name")) {
+			i++;
+		} else if (check_valid_ipaddress(strvec_slot(strvec, i), true)) {
 			parse_ipaddress(&addr, strvec_slot(strvec, i), true);
 			if (current_vrrp->ipvlan_addr) {
 				report_config_error(CONFIG_GENERAL_ERROR, "(%s) ipvlan address already specified - ignoring '%s'", current_vrrp->iname, strvec_slot(strvec, i));

keepalived/vrrp/vrrp_vmac.c

@@ -233,7 +233,6 @@ netlink_link_up(vrrp_t *vrrp)
 static void
 netlink_link_group(interface_t *base_ifp)
 {
-	uint32_t group = base_ifp->group;
 	struct {
 		struct nlmsghdr n;
 		struct ifinfomsg ifi;
@@ -246,7 +245,7 @@ netlink_link_group(interface_t *base_ifp)
 	req.ifi.ifi_family = AF_UNSPEC;
 	req.ifi.ifi_index = (int)IF_INDEX(base_ifp);
 
-	addattr_l(&req.n, sizeof(req), IFLA_GROUP, &group, sizeof(group));
+	addattr32(&req.n, sizeof(req), IFLA_GROUP, base_ifp->group);
 	netlink_talk(&nl_cmd, &req.n);
 }
 
@@ -277,7 +276,6 @@ netlink_link_add_vmac(vrrp_t *vrrp, const interface_t *old_interface)
 	struct rtattr *linkinfo;
 	struct rtattr *data;
 	interface_t *ifp;
-	uint32_t group;
 	bool create_interface = true;
 	struct {
 		struct nlmsghdr n;
@@ -393,8 +391,9 @@ netlink_link_add_vmac(vrrp_t *vrrp, const interface_t *old_interface)
 		 * (iptables devgroup or nftables iifgroup, oifgroup) to continue
 		 * working regardless of the use_vmac setting.
 		 */
-		group = vrrp->configured_ifp->base_ifp->group;
-		addattr_l(&req.n, sizeof(req), IFLA_GROUP, &group, sizeof(group));
+		addattr32(&req.n, sizeof(req), IFLA_GROUP,
+			__test_bit(VRRP_VMAC_GROUP, &vrrp->flags) ? vrrp->vmac_group
+								  : vrrp->configured_ifp->base_ifp->group);
 		addattr_l(&req.n, sizeof(req), IFLA_ADDRESS, if_ll_addr, ETH_ALEN);
 
 #ifdef _HAVE_VRF_
@@ -619,9 +618,15 @@ netlink_link_add_ipvlan(vrrp_t *vrrp)
 		/* ipvlan settings */
 
 		/* Note: if the underlying interface is a ipvlan, then the kernel will configure the
-		 * interface only the underlying interface of the ipvlan */
+		 * interface only the underlying interface of the ipvlan.
+		 * We copy the group from the base interface to allow firewall rules
+		 * (iptables devgroup or nftables iifgroup, oifgroup) to continue
+		 * working regardless of the use_vmac setting. */
 		addattr32(&req.n, sizeof(req), IFLA_LINK, vrrp->configured_ifp->ifindex);
 		addattr_l(&req.n, sizeof(req), IFLA_IFNAME, vrrp->vmac_ifname, strlen(vrrp->vmac_ifname));
+		addattr32(&req.n, sizeof(req), IFLA_GROUP,
+			__test_bit(VRRP_VMAC_GROUP, &vrrp->flags) ? vrrp->vmac_group
+								  : vrrp->configured_ifp->base_ifp->group);
 		linkinfo = PTR_CAST(struct rtattr, NLMSG_TAIL(&req.n));
 		addattr_l(&req.n, sizeof(req), IFLA_LINKINFO, NULL, 0);
 		addattr_l(&req.n, sizeof(req), IFLA_INFO_KIND, (const void *)ipvlan_ll_kind, strlen(ipvlan_ll_kind));

lib/rttables.c

@@ -322,13 +322,11 @@ find_rttables_dsfield(const char *name, uint8_t *id)
 	return ret;
 }
 
-#if HAVE_DECL_FRA_SUPPRESS_IFGROUP
 bool
 find_rttables_group(const char *name, uint32_t *id)
 {
 	return find_entry(name, id, &rt_groups, RT_GROUPS_FILE, NULL, INT32_MAX);
 }
-#endif
 
 bool
 find_rttables_realms(const char *name, uint32_t *id)
@@ -386,7 +384,7 @@ get_entry(unsigned int id, list_head_t *l, const char* file_name, const rt_entry
 	return ret_buf;
 }
 
-#if HAVE_DECL_FRA_SUPPRESS_IFGROUP
+#if HAVE_DECL_FRA_SUPPRESS_IFGROUP && defined _WITH_SNMP_VRRP_
 const char *
 get_rttables_group(uint32_t id)
 {

lib/rttables.h

@@ -28,24 +28,18 @@
 #include <stdint.h>
 
 extern void clear_rt_names(void);
-#ifdef _WITH_VRRP_
 extern bool find_rttables_table(const char *, uint32_t *);
 extern bool find_rttables_dsfield(const char *, uint8_t *);
 extern bool find_rttables_realms(const char *, uint32_t *);
-#if HAVE_DECL_FRA_SUPPRESS_IFGROUP
 extern bool find_rttables_group(const char *, uint32_t *);
-#endif
 extern bool find_rttables_proto(const char *, uint8_t *);
 extern bool find_rttables_rtntype(const char *, uint8_t *);
-#endif
 extern bool find_rttables_scope(const char *, uint8_t *);
 
 extern const char *get_rttables_scope(uint32_t);
-#ifdef _WITH_VRRP_
-#if HAVE_DECL_FRA_SUPPRESS_IFGROUP
+#if HAVE_DECL_FRA_SUPPRESS_IFGROUP && defined _WITH_SNMP_VRRP_
 extern const char *get_rttables_group(uint32_t);
 #endif
 extern const char *get_rttables_rtntype(uint8_t);
-#endif
 
 #endif