[PATCH net-next v2 0/6] vxlan: cleanup and IPv6 link-local support

[PATCH net-next v2 0/6] vxlan: cleanup and IPv6 link-local support

[Matthias Schiffer]

Running VXLANs over IPv6 link-local addresses allows to use them as a
drop-in replacement for VLANs, avoiding to allocate additional outer IP
addresses to run the VXLAN over.

Since v1, I have added a lot more consistency checks to the address
configuration, making sure address families and scopes match. To simplify
the implementation, I also did some general refactoring of the
configuration handling in the new first patch of the series.

The second patch is more cleanup; is slightly touches OVS code, so that
list is in CC this time, too.

As in v1, the last two patches actually make VXLAN over IPv6 link-local
work, and allow multiple VXLANs wit the same VNI and port, as long as
link-local addresses on different interfaces are used. As suggested, I now
store in the flags field if the VXLAN uses link-local addresses or not.


Matthias Schiffer (6):
  vxlan: refactor verification and application of configuration
  vxlan: get rid of redundant vxlan_dev.flags
  vxlan: improve validation of address family configuration
  vxlan: check valid combinations of address scopes
  vxlan: fix snooping for link-local IPv6 addresses
  vxlan: allow multiple VXLANs with same VNI for IPv6 link-local
    addresses

 drivers/net/vxlan.c           | 411 ++++++++++++++++++++++++++----------------
 include/net/vxlan.h           |   3 +-
 net/openvswitch/vport-vxlan.c |   4 +-
 3 files changed, 263 insertions(+), 155 deletions(-)

-- 
2.12.2

[PATCH net-next v2 1/6] vxlan: refactor verification and application of configuration

[Matthias Schiffer]

The vxlan_dev_configure function was mixing validation and application of
the vxlan configuration; this could easily lead to bugs with the changelink
operation, as it was hard to see if the function wcould return an error
after parts of the configuration had already been applied.

This commit splits validation and application out of vxlan_dev_configure as
separate functions to make it clearer where error returns are allowed and
where the vxlan_dev or net_device may be configured.

In addition, some validation is moved to vxlan_validate and some
initialization to vxlan_setup where this improves grouping of similar
settings.

Finally, this also fixes two actual bugs:

* if set, conf->mtu would overwrite dev->mtu in each changelink operation,
  reverting other changes of dev->mtu
* the "if (!conf->dst_port)" branch would never be run, as conf->dst_port
  was set in vxlan_setup before. This caused VXLAN-GPE to use the same
  default port as other VXLAN sockets instead of the intended IANA-assigned
  4790.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: new patch

 drivers/net/vxlan.c | 202 +++++++++++++++++++++++++++++-----------------------
 1 file changed, 114 insertions(+), 88 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index ebc98bb17a51..86471e961708 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2598,6 +2598,10 @@ static void vxlan_setup(struct net_device *dev)
 	netif_keep_dst(dev);
 	dev->priv_flags |= IFF_NO_QUEUE;
 
+	/* MTU range: 68 - 65535 */
+	dev->min_mtu = ETH_MIN_MTU;
+	dev->max_mtu = ETH_MAX_MTU;
+
 	INIT_LIST_HEAD(&vxlan->next);
 	spin_lock_init(&vxlan->hash_lock);
 
@@ -2605,9 +2609,8 @@ static void vxlan_setup(struct net_device *dev)
 	vxlan->age_timer.function = vxlan_cleanup;
 	vxlan->age_timer.data = (unsigned long) vxlan;
 
-	vxlan->cfg.dst_port = htons(vxlan_port);
-
 	vxlan->dev = dev;
+	vxlan->net = dev_net(dev);
 
 	gro_cells_init(&vxlan->gro_cells, dev);
 
@@ -2676,11 +2679,19 @@ static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[])
 		}
 	}
 
+	if (tb[IFLA_MTU]) {
+		u32 mtu = nla_get_u32(data[IFLA_MTU]);
+
+		if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU)
+			return -EINVAL;
+	}
+
 	if (!data)
 		return -EINVAL;
 
 	if (data[IFLA_VXLAN_ID]) {
-		__u32 id = nla_get_u32(data[IFLA_VXLAN_ID]);
+		u32 id = nla_get_u32(data[IFLA_VXLAN_ID]);
+
 		if (id >= VXLAN_N_VID)
 			return -ERANGE;
 	}
@@ -2839,55 +2850,36 @@ static int vxlan_sock_add(struct vxlan_dev *vxlan)
 	return ret;
 }
 
-static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
-			       struct vxlan_config *conf,
-			       bool changelink)
+static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
+				 struct net_device **lower,
+				 struct vxlan_dev *old)
 {
 	struct vxlan_net *vn = net_generic(src_net, vxlan_net_id);
-	struct vxlan_dev *vxlan = netdev_priv(dev), *tmp;
-	struct vxlan_rdst *dst = &vxlan->default_dst;
-	unsigned short needed_headroom = ETH_HLEN;
+	struct vxlan_dev *tmp;
 	bool use_ipv6 = false;
-	__be16 default_port = vxlan->cfg.dst_port;
-	struct net_device *lowerdev = NULL;
 
-	if (!changelink) {
-		if (conf->flags & VXLAN_F_GPE) {
-			/* For now, allow GPE only together with
-			 * COLLECT_METADATA. This can be relaxed later; in such
-			 * case, the other side of the PtP link will have to be
-			 * provided.
-			 */
-			if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) ||
-			    !(conf->flags & VXLAN_F_COLLECT_METADATA)) {
-				pr_info("unsupported combination of extensions\n");
-				return -EINVAL;
-			}
-			vxlan_raw_setup(dev);
-		} else {
-			vxlan_ether_setup(dev);
+	if (conf->flags & VXLAN_F_GPE) {
+		/* For now, allow GPE only together with
+		 * COLLECT_METADATA. This can be relaxed later; in such
+		 * case, the other side of the PtP link will have to be
+		 * provided.
+		 */
+		if ((conf->flags & ~VXLAN_F_ALLOWED_GPE) ||
+		    !(conf->flags & VXLAN_F_COLLECT_METADATA)) {
+			pr_info("unsupported combination of extensions\n");
+			return -EINVAL;
 		}
-
-		/* MTU range: 68 - 65535 */
-		dev->min_mtu = ETH_MIN_MTU;
-		dev->max_mtu = ETH_MAX_MTU;
-		vxlan->net = src_net;
 	}
 
-	dst->remote_vni = conf->vni;
-
-	memcpy(&dst->remote_ip, &conf->remote_ip, sizeof(conf->remote_ip));
-
-	/* Unless IPv6 is explicitly requested, assume IPv4 */
-	if (!dst->remote_ip.sa.sa_family)
-		dst->remote_ip.sa.sa_family = AF_INET;
+	if (!conf->remote_ip.sa.sa_family)
+		conf->remote_ip.sa.sa_family = AF_INET;
 
-	if (dst->remote_ip.sa.sa_family == AF_INET6 ||
-	    vxlan->cfg.saddr.sa.sa_family == AF_INET6) {
+	if (conf->remote_ip.sa.sa_family == AF_INET6 ||
+	    conf->saddr.sa.sa_family == AF_INET6) {
 		if (!IS_ENABLED(CONFIG_IPV6))
 			return -EPFNOSUPPORT;
 		use_ipv6 = true;
-		vxlan->flags |= VXLAN_F_IPV6;
+		conf->flags |= VXLAN_F_IPV6;
 	}
 
 	if (conf->label && !use_ipv6) {
@@ -2895,14 +2887,13 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 		return -EINVAL;
 	}
 
-	if (conf->remote_ifindex &&
-	    conf->remote_ifindex != vxlan->cfg.remote_ifindex) {
-		lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
-		dst->remote_ifindex = conf->remote_ifindex;
+	if (conf->remote_ifindex) {
+		struct net_device *lowerdev;
 
+		lowerdev = __dev_get_by_index(src_net, conf->remote_ifindex);
 		if (!lowerdev) {
 			pr_info("ifindex %d does not exist\n",
-				dst->remote_ifindex);
+				conf->remote_ifindex);
 			return -ENODEV;
 		}
 
@@ -2916,39 +2907,83 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 		}
 #endif
 
-		if (!conf->mtu)
-			dev->mtu = lowerdev->mtu -
-				   (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM);
+		*lower = lowerdev;
+	} else {
+		if (vxlan_addr_multicast(&conf->remote_ip)) {
+			pr_info("multicast destination requires interface to be specified\n");
+			return -EINVAL;
+		}
 
-		needed_headroom = lowerdev->hard_header_len;
-	} else if (!conf->remote_ifindex &&
-		   vxlan_addr_multicast(&dst->remote_ip)) {
-		pr_info("multicast destination requires interface to be specified\n");
-		return -EINVAL;
+		*lower = NULL;
 	}
 
-	if (lowerdev) {
-		dev->gso_max_size = lowerdev->gso_max_size;
-		dev->gso_max_segs = lowerdev->gso_max_segs;
+	if (!conf->dst_port) {
+		if (conf->flags & VXLAN_F_GPE)
+			conf->dst_port = htons(4790); /* IANA VXLAN-GPE port */
+		else
+			conf->dst_port = htons(vxlan_port);
 	}
 
-	if (conf->mtu) {
-		int max_mtu = ETH_MAX_MTU;
+	if (!conf->age_interval)
+		conf->age_interval = FDB_AGE_DEFAULT;
 
-		if (lowerdev)
-			max_mtu = lowerdev->mtu;
+	list_for_each_entry(tmp, &vn->vxlan_list, next) {
+		if (tmp == old)
+			continue;
 
-		max_mtu -= (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM);
+		if (tmp->cfg.vni == conf->vni &&
+		    (tmp->default_dst.remote_ip.sa.sa_family == AF_INET6 ||
+		     tmp->cfg.saddr.sa.sa_family == AF_INET6) == use_ipv6 &&
+		    tmp->cfg.dst_port == conf->dst_port &&
+		    (tmp->flags & VXLAN_F_RCV_FLAGS) ==
+		    (conf->flags & VXLAN_F_RCV_FLAGS)) {
+			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
+			return -EEXIST;
+		}
+	}
 
-		if (conf->mtu < dev->min_mtu || conf->mtu > dev->max_mtu)
-			return -EINVAL;
+	return 0;
+}
 
-		dev->mtu = conf->mtu;
+static void vxlan_config_apply(struct net_device *dev,
+			       struct vxlan_config *conf,
+			       struct net_device *lowerdev, bool changelink)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct vxlan_rdst *dst = &vxlan->default_dst;
+	unsigned short needed_headroom = ETH_HLEN;
+	bool use_ipv6 = !!(conf->flags & VXLAN_F_IPV6);
+	int max_mtu = ETH_MAX_MTU;
 
-		if (conf->mtu > max_mtu)
-			dev->mtu = max_mtu;
+	if (!changelink) {
+		if (conf->flags & VXLAN_F_GPE)
+			vxlan_raw_setup(dev);
+		else
+			vxlan_ether_setup(dev);
+
+		if (conf->mtu)
+			dev->mtu = conf->mtu;
 	}
 
+	dst->remote_vni = conf->vni;
+
+	memcpy(&dst->remote_ip, &conf->remote_ip, sizeof(conf->remote_ip));
+
+	if (lowerdev) {
+		dst->remote_ifindex = conf->remote_ifindex;
+
+		dev->gso_max_size = lowerdev->gso_max_size;
+		dev->gso_max_segs = lowerdev->gso_max_segs;
+
+		needed_headroom = lowerdev->hard_header_len;
+
+		max_mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM :
+					   VXLAN_HEADROOM);
+	}
+
+	if (dev->mtu > max_mtu)
+		dev->mtu = max_mtu;
+
 	if (use_ipv6 || conf->flags & VXLAN_F_COLLECT_METADATA)
 		needed_headroom += VXLAN6_HEADROOM;
 	else
@@ -2956,31 +2991,22 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	dev->needed_headroom = needed_headroom;
 
 	memcpy(&vxlan->cfg, conf, sizeof(*conf));
-	if (!vxlan->cfg.dst_port) {
-		if (conf->flags & VXLAN_F_GPE)
-			vxlan->cfg.dst_port = htons(4790); /* IANA VXLAN-GPE port */
-		else
-			vxlan->cfg.dst_port = default_port;
-	}
 	vxlan->flags |= conf->flags;
+}
 
-	if (!vxlan->cfg.age_interval)
-		vxlan->cfg.age_interval = FDB_AGE_DEFAULT;
+static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
+			       struct vxlan_config *conf,
+			       bool changelink)
+{
+	struct vxlan_dev *vxlan = netdev_priv(dev);
+	struct net_device *lowerdev;
+	int ret;
 
-	if (changelink)
-		return 0;
+	ret = vxlan_config_validate(src_net, conf, &lowerdev, vxlan);
+	if (ret)
+		return ret;
 
-	list_for_each_entry(tmp, &vn->vxlan_list, next) {
-		if (tmp->cfg.vni == conf->vni &&
-		    (tmp->default_dst.remote_ip.sa.sa_family == AF_INET6 ||
-		     tmp->cfg.saddr.sa.sa_family == AF_INET6) == use_ipv6 &&
-		    tmp->cfg.dst_port == vxlan->cfg.dst_port &&
-		    (tmp->flags & VXLAN_F_RCV_FLAGS) ==
-		    (vxlan->flags & VXLAN_F_RCV_FLAGS)) {
-			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
-			return -EEXIST;
-		}
-	}
+	vxlan_config_apply(dev, conf, lowerdev, changelink);
 
 	return 0;
 }
-- 
2.12.2

[PATCH net-next v2 2/6] vxlan: get rid of redundant vxlan_dev.flags

[Matthias Schiffer]

There is no good reason to keep the flags twice in vxlan_dev and
vxlan_config.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: new patch

 drivers/net/vxlan.c           | 76 +++++++++++++++++++++----------------------
 include/net/vxlan.h           |  1 -
 net/openvswitch/vport-vxlan.c |  4 +--
 3 files changed, 39 insertions(+), 42 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 86471e961708..bdd19e5037b0 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -303,7 +303,7 @@ static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
 	if (rdst->remote_vni != vxlan->default_dst.remote_vni &&
 	    nla_put_u32(skb, NDA_VNI, be32_to_cpu(rdst->remote_vni)))
 		goto nla_put_failure;
-	if ((vxlan->flags & VXLAN_F_COLLECT_METADATA) && fdb->vni &&
+	if ((vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA) && fdb->vni &&
 	    nla_put_u32(skb, NDA_SRC_VNI,
 			be32_to_cpu(fdb->vni)))
 		goto nla_put_failure;
@@ -417,7 +417,7 @@ static u32 eth_vni_hash(const unsigned char *addr, __be32 vni)
 static inline struct hlist_head *vxlan_fdb_head(struct vxlan_dev *vxlan,
 						const u8 *mac, __be32 vni)
 {
-	if (vxlan->flags & VXLAN_F_COLLECT_METADATA)
+	if (vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA)
 		return &vxlan->fdb_head[eth_vni_hash(mac, vni)];
 	else
 		return &vxlan->fdb_head[eth_hash(mac)];
@@ -432,7 +432,7 @@ static struct vxlan_fdb *__vxlan_find_mac(struct vxlan_dev *vxlan,
 
 	hlist_for_each_entry_rcu(f, head, hlist) {
 		if (ether_addr_equal(mac, f->eth_addr)) {
-			if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
+			if (vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA) {
 				if (vni == f->vni)
 					return f;
 			} else {
@@ -1266,7 +1266,7 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
 #endif
 	}
 
-	if ((vxlan->flags & VXLAN_F_LEARN) &&
+	if ((vxlan->cfg.flags & VXLAN_F_LEARN) &&
 	    vxlan_snoop(skb->dev, &saddr, eth_hdr(skb)->h_source, vni))
 		return false;
 
@@ -1489,7 +1489,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 
 		if (netif_rx_ni(reply) == NET_RX_DROP)
 			dev->stats.rx_dropped++;
-	} else if (vxlan->flags & VXLAN_F_L3MISS) {
+	} else if (vxlan->cfg.flags & VXLAN_F_L3MISS) {
 		union vxlan_addr ipa = {
 			.sin.sin_addr.s_addr = tip,
 			.sin.sin_family = AF_INET,
@@ -1649,7 +1649,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
 		if (netif_rx_ni(reply) == NET_RX_DROP)
 			dev->stats.rx_dropped++;
 
-	} else if (vxlan->flags & VXLAN_F_L3MISS) {
+	} else if (vxlan->cfg.flags & VXLAN_F_L3MISS) {
 		union vxlan_addr ipa = {
 			.sin6.sin6_addr = msg->target,
 			.sin6.sin6_family = AF_INET6,
@@ -1682,7 +1682,7 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
 			return false;
 		pip = ip_hdr(skb);
 		n = neigh_lookup(&arp_tbl, &pip->daddr, dev);
-		if (!n && (vxlan->flags & VXLAN_F_L3MISS)) {
+		if (!n && (vxlan->cfg.flags & VXLAN_F_L3MISS)) {
 			union vxlan_addr ipa = {
 				.sin.sin_addr.s_addr = pip->daddr,
 				.sin.sin_family = AF_INET,
@@ -1703,7 +1703,7 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
 			return false;
 		pip6 = ipv6_hdr(skb);
 		n = neigh_lookup(ipv6_stub->nd_tbl, &pip6->daddr, dev);
-		if (!n && (vxlan->flags & VXLAN_F_L3MISS)) {
+		if (!n && (vxlan->cfg.flags & VXLAN_F_L3MISS)) {
 			union vxlan_addr ipa = {
 				.sin6.sin6_addr = pip6->daddr,
 				.sin6.sin6_family = AF_INET6,
@@ -1977,7 +1977,7 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 #endif
 	}
 
-	if (dst_vxlan->flags & VXLAN_F_LEARN)
+	if (dst_vxlan->cfg.flags & VXLAN_F_LEARN)
 		vxlan_snoop(skb->dev, &loopback, eth_hdr(skb)->h_source, vni);
 
 	u64_stats_update_begin(&tx_stats->syncp);
@@ -2015,7 +2015,7 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
 		dst_release(dst);
 		dst_vxlan = vxlan_find_vni(vxlan->net, vni,
 					   daddr->sa.sa_family, dst_port,
-					   vxlan->flags);
+					   vxlan->cfg.flags);
 		if (!dst_vxlan) {
 			dev->stats.tx_errors++;
 			kfree_skb(skb);
@@ -2046,7 +2046,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	__be32 vni, label;
 	__u8 tos, ttl;
 	int err;
-	u32 flags = vxlan->flags;
+	u32 flags = vxlan->cfg.flags;
 	bool udp_sum = false;
 	bool xnet = !net_eq(vxlan->net, dev_net(vxlan->dev));
 
@@ -2228,7 +2228,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	skb_reset_mac_header(skb);
 
-	if (vxlan->flags & VXLAN_F_COLLECT_METADATA) {
+	if (vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA) {
 		if (info && info->mode & IP_TUNNEL_INFO_BRIDGE &&
 		    info->mode & IP_TUNNEL_INFO_TX) {
 			vni = tunnel_id_to_key32(info->key.tun_id);
@@ -2241,7 +2241,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 
-	if (vxlan->flags & VXLAN_F_PROXY) {
+	if (vxlan->cfg.flags & VXLAN_F_PROXY) {
 		eth = eth_hdr(skb);
 		if (ntohs(eth->h_proto) == ETH_P_ARP)
 			return arp_reduce(dev, skb, vni);
@@ -2261,7 +2261,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	f = vxlan_find_mac(vxlan, eth->h_dest, vni);
 	did_rsc = false;
 
-	if (f && (f->flags & NTF_ROUTER) && (vxlan->flags & VXLAN_F_RSC) &&
+	if (f && (f->flags & NTF_ROUTER) && (vxlan->cfg.flags & VXLAN_F_RSC) &&
 	    (ntohs(eth->h_proto) == ETH_P_IP ||
 	     ntohs(eth->h_proto) == ETH_P_IPV6)) {
 		did_rsc = route_shortcircuit(dev, skb);
@@ -2272,7 +2272,7 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (f == NULL) {
 		f = vxlan_find_mac(vxlan, all_zeros_mac, vni);
 		if (f == NULL) {
-			if ((vxlan->flags & VXLAN_F_L2MISS) &&
+			if ((vxlan->cfg.flags & VXLAN_F_L2MISS) &&
 			    !is_multicast_ether_addr(eth->h_dest))
 				vxlan_fdb_miss(vxlan, eth->h_dest);
 
@@ -2809,7 +2809,7 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
 	if (!vxlan->cfg.no_share) {
 		spin_lock(&vn->sock_lock);
 		vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
-				     vxlan->cfg.dst_port, vxlan->flags);
+				     vxlan->cfg.dst_port, vxlan->cfg.flags);
 		if (vs && !atomic_add_unless(&vs->refcnt, 1, 0)) {
 			spin_unlock(&vn->sock_lock);
 			return -EBUSY;
@@ -2818,7 +2818,7 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
 	}
 	if (!vs)
 		vs = vxlan_socket_create(vxlan->net, ipv6,
-					 vxlan->cfg.dst_port, vxlan->flags);
+					 vxlan->cfg.dst_port, vxlan->cfg.flags);
 	if (IS_ERR(vs))
 		return PTR_ERR(vs);
 #if IS_ENABLED(CONFIG_IPV6)
@@ -2833,8 +2833,8 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
 
 static int vxlan_sock_add(struct vxlan_dev *vxlan)
 {
-	bool ipv6 = vxlan->flags & VXLAN_F_IPV6;
-	bool metadata = vxlan->flags & VXLAN_F_COLLECT_METADATA;
+	bool ipv6 = vxlan->cfg.flags & VXLAN_F_IPV6;
+	bool metadata = vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA;
 	int ret = 0;
 
 	RCU_INIT_POINTER(vxlan->vn4_sock, NULL);
@@ -2935,7 +2935,7 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 		    (tmp->default_dst.remote_ip.sa.sa_family == AF_INET6 ||
 		     tmp->cfg.saddr.sa.sa_family == AF_INET6) == use_ipv6 &&
 		    tmp->cfg.dst_port == conf->dst_port &&
-		    (tmp->flags & VXLAN_F_RCV_FLAGS) ==
+		    (tmp->cfg.flags & VXLAN_F_RCV_FLAGS) ==
 		    (conf->flags & VXLAN_F_RCV_FLAGS)) {
 			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
 			return -EEXIST;
@@ -2991,7 +2991,6 @@ static void vxlan_config_apply(struct net_device *dev,
 	dev->needed_headroom = needed_headroom;
 
 	memcpy(&vxlan->cfg, conf, sizeof(*conf));
-	vxlan->flags |= conf->flags;
 }
 
 static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
@@ -3105,12 +3104,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			     IPV6_FLOWLABEL_MASK;
 
 	if (data[IFLA_VXLAN_LEARNING]) {
-		if (nla_get_u8(data[IFLA_VXLAN_LEARNING])) {
+		if (nla_get_u8(data[IFLA_VXLAN_LEARNING]))
 			conf->flags |= VXLAN_F_LEARN;
-		} else {
+		else
 			conf->flags &= ~VXLAN_F_LEARN;
-			vxlan->flags &= ~VXLAN_F_LEARN;
-		}
 	} else if (!changelink) {
 		/* default to learn on a new device */
 		conf->flags |= VXLAN_F_LEARN;
@@ -3399,43 +3396,44 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
 	    nla_put_u8(skb, IFLA_VXLAN_TOS, vxlan->cfg.tos) ||
 	    nla_put_be32(skb, IFLA_VXLAN_LABEL, vxlan->cfg.label) ||
 	    nla_put_u8(skb, IFLA_VXLAN_LEARNING,
-			!!(vxlan->flags & VXLAN_F_LEARN)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_LEARN)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_PROXY,
-			!!(vxlan->flags & VXLAN_F_PROXY)) ||
-	    nla_put_u8(skb, IFLA_VXLAN_RSC, !!(vxlan->flags & VXLAN_F_RSC)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_PROXY)) ||
+	    nla_put_u8(skb, IFLA_VXLAN_RSC,
+		       !!(vxlan->cfg.flags & VXLAN_F_RSC)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_L2MISS,
-			!!(vxlan->flags & VXLAN_F_L2MISS)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_L2MISS)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_L3MISS,
-			!!(vxlan->flags & VXLAN_F_L3MISS)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_L3MISS)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_COLLECT_METADATA,
-		       !!(vxlan->flags & VXLAN_F_COLLECT_METADATA)) ||
+		       !!(vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA)) ||
 	    nla_put_u32(skb, IFLA_VXLAN_AGEING, vxlan->cfg.age_interval) ||
 	    nla_put_u32(skb, IFLA_VXLAN_LIMIT, vxlan->cfg.addrmax) ||
 	    nla_put_be16(skb, IFLA_VXLAN_PORT, vxlan->cfg.dst_port) ||
 	    nla_put_u8(skb, IFLA_VXLAN_UDP_CSUM,
-			!(vxlan->flags & VXLAN_F_UDP_ZERO_CSUM_TX)) ||
+			!(vxlan->cfg.flags & VXLAN_F_UDP_ZERO_CSUM_TX)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
-			!!(vxlan->flags & VXLAN_F_UDP_ZERO_CSUM6_TX)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_UDP_ZERO_CSUM6_TX)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_UDP_ZERO_CSUM6_RX,
-			!!(vxlan->flags & VXLAN_F_UDP_ZERO_CSUM6_RX)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_UDP_ZERO_CSUM6_RX)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_REMCSUM_TX,
-			!!(vxlan->flags & VXLAN_F_REMCSUM_TX)) ||
+			!!(vxlan->cfg.flags & VXLAN_F_REMCSUM_TX)) ||
 	    nla_put_u8(skb, IFLA_VXLAN_REMCSUM_RX,
-			!!(vxlan->flags & VXLAN_F_REMCSUM_RX)))
+			!!(vxlan->cfg.flags & VXLAN_F_REMCSUM_RX)))
 		goto nla_put_failure;
 
 	if (nla_put(skb, IFLA_VXLAN_PORT_RANGE, sizeof(ports), &ports))
 		goto nla_put_failure;
 
-	if (vxlan->flags & VXLAN_F_GBP &&
+	if (vxlan->cfg.flags & VXLAN_F_GBP &&
 	    nla_put_flag(skb, IFLA_VXLAN_GBP))
 		goto nla_put_failure;
 
-	if (vxlan->flags & VXLAN_F_GPE &&
+	if (vxlan->cfg.flags & VXLAN_F_GPE &&
 	    nla_put_flag(skb, IFLA_VXLAN_GPE))
 		goto nla_put_failure;
 
-	if (vxlan->flags & VXLAN_F_REMCSUM_NOPARTIAL &&
+	if (vxlan->cfg.flags & VXLAN_F_REMCSUM_NOPARTIAL &&
 	    nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
 		goto nla_put_failure;
 
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 49a59202f85e..479bb75789ea 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -232,7 +232,6 @@ struct vxlan_dev {
 	struct net_device *dev;
 	struct net	  *net;		/* netns for packet i/o */
 	struct vxlan_rdst default_dst;	/* default destination */
-	u32		  flags;	/* VXLAN_F_* in vxlan.h */
 
 	struct timer_list age_timer;
 	spinlock_t	  hash_lock;
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index 7eb955e453e6..23594d93ad54 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -40,14 +40,14 @@ static int vxlan_get_options(const struct vport *vport, struct sk_buff *skb)
 	if (nla_put_u16(skb, OVS_TUNNEL_ATTR_DST_PORT, ntohs(dst_port)))
 		return -EMSGSIZE;
 
-	if (vxlan->flags & VXLAN_F_GBP) {
+	if (vxlan->cfg.flags & VXLAN_F_GBP) {
 		struct nlattr *exts;
 
 		exts = nla_nest_start(skb, OVS_TUNNEL_ATTR_EXTENSION);
 		if (!exts)
 			return -EMSGSIZE;
 
-		if (vxlan->flags & VXLAN_F_GBP &&
+		if (vxlan->cfg.flags & VXLAN_F_GBP &&
 		    nla_put_flag(skb, OVS_VXLAN_EXT_GBP))
 			return -EMSGSIZE;
 
-- 
2.12.2

[PATCH net-next v2 3/6] vxlan: improve validation of address family configuration

[Matthias Schiffer]

Address families of source and destination addresses must match, and
changelink operations can't change the address family.

In addition, always use the VXLAN_F_IPV6 to check if a VXLAN device uses
IPv4 or IPv6.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: new patch

 drivers/net/vxlan.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bdd19e5037b0..07f89b037681 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2459,10 +2459,7 @@ static int vxlan_change_mtu(struct net_device *dev, int new_mtu)
 	struct vxlan_rdst *dst = &vxlan->default_dst;
 	struct net_device *lowerdev = __dev_get_by_index(vxlan->net,
 							 dst->remote_ifindex);
-	bool use_ipv6 = false;
-
-	if (dst->remote_ip.sa.sa_family == AF_INET6)
-		use_ipv6 = true;
+	bool use_ipv6 = !!(vxlan->cfg.flags & VXLAN_F_IPV6);
 
 	/* This check is different than dev->max_mtu, because it looks at
 	 * the lowerdev->mtu, rather than the static dev->max_mtu
@@ -2871,11 +2868,20 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 		}
 	}
 
-	if (!conf->remote_ip.sa.sa_family)
+	if (!conf->remote_ip.sa.sa_family && !conf->saddr.sa.sa_family) {
+		/* Unless IPv6 is explicitly requested, assume IPv4 */
 		conf->remote_ip.sa.sa_family = AF_INET;
+		conf->saddr.sa.sa_family = AF_INET;
+	} else if (!conf->remote_ip.sa.sa_family) {
+		conf->remote_ip.sa.sa_family = conf->saddr.sa.sa_family;
+	} else if (!conf->saddr.sa.sa_family) {
+		conf->saddr.sa.sa_family = conf->remote_ip.sa.sa_family;
+	}
+
+	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
+		return -EINVAL;
 
-	if (conf->remote_ip.sa.sa_family == AF_INET6 ||
-	    conf->saddr.sa.sa_family == AF_INET6) {
+	if (conf->saddr.sa.sa_family == AF_INET6) {
 		if (!IS_ENABLED(CONFIG_IPV6))
 			return -EPFNOSUPPORT;
 		use_ipv6 = true;
@@ -2932,11 +2938,9 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 			continue;
 
 		if (tmp->cfg.vni == conf->vni &&
-		    (tmp->default_dst.remote_ip.sa.sa_family == AF_INET6 ||
-		     tmp->cfg.saddr.sa.sa_family == AF_INET6) == use_ipv6 &&
 		    tmp->cfg.dst_port == conf->dst_port &&
-		    (tmp->cfg.flags & VXLAN_F_RCV_FLAGS) ==
-		    (conf->flags & VXLAN_F_RCV_FLAGS)) {
+		    (tmp->cfg.flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6)) ==
+		    (conf->flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6))) {
 			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
 			return -EEXIST;
 		}
@@ -3069,22 +3073,35 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	}
 
 	if (data[IFLA_VXLAN_GROUP]) {
+		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET))
+			return -EOPNOTSUPP;
+
 		conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]);
+		conf->remote_ip.sa.sa_family = AF_INET;
 	} else if (data[IFLA_VXLAN_GROUP6]) {
 		if (!IS_ENABLED(CONFIG_IPV6))
 			return -EPFNOSUPPORT;
 
+		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6))
+			return -EOPNOTSUPP;
+
 		conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]);
 		conf->remote_ip.sa.sa_family = AF_INET6;
 	}
 
 	if (data[IFLA_VXLAN_LOCAL]) {
+		if (changelink && (conf->saddr.sa.sa_family != AF_INET))
+			return -EOPNOTSUPP;
+
 		conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]);
 		conf->saddr.sa.sa_family = AF_INET;
 	} else if (data[IFLA_VXLAN_LOCAL6]) {
 		if (!IS_ENABLED(CONFIG_IPV6))
 			return -EPFNOSUPPORT;
 
+		if (changelink && (conf->saddr.sa.sa_family != AF_INET6))
+			return -EOPNOTSUPP;
+
 		/* TODO: respect scope id */
 		conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
 		conf->saddr.sa.sa_family = AF_INET6;
-- 
2.12.2

[PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Matthias Schiffer]

* Multicast addresses are never valid as local address
* Link-local IPv6 unicast addresses may only be used as remote when the
  local address is link-local as well
* Don't allow link-local IPv6 local/remote addresses without interface

We also store in the flags field if link-local addresses are used for the
follow-up patches that actually make VXLAN over link-local IPv6 work.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: was "vxlan: don't allow link-local IPv6 local/remote addresses without
interface" before. v2 does a lot more checks and adds the
VXLAN_F_IPV6_LINKLOCAL flag.

 drivers/net/vxlan.c | 35 +++++++++++++++++++++++++++++++++++
 include/net/vxlan.h |  2 ++
 2 files changed, 37 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 07f89b037681..95a71546e8f2 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
 		return -EINVAL;
 
+	if (vxlan_addr_multicast(&conf->saddr))
+		return -EINVAL;
+
 	if (conf->saddr.sa.sa_family == AF_INET6) {
 		if (!IS_ENABLED(CONFIG_IPV6))
 			return -EPFNOSUPPORT;
 		use_ipv6 = true;
 		conf->flags |= VXLAN_F_IPV6;
+
+		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
+			int local_type =
+				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
+			int remote_type =
+				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
+
+			if (local_type & IPV6_ADDR_LINKLOCAL) {
+				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
+				    (remote_type != IPV6_ADDR_ANY)) {
+					pr_info("invalid combination of address scopes\n");
+					return -EINVAL;
+				}
+
+				conf->flags |= VXLAN_F_IPV6_LINKLOCAL;
+			} else {
+				if (remote_type ==
+				    (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
+					pr_info("invalid combination of address scopes\n");
+					return -EINVAL;
+				}
+
+				conf->flags &= ~VXLAN_F_IPV6_LINKLOCAL;
+			}
+		}
 	}
 
 	if (conf->label && !use_ipv6) {
@@ -2920,6 +2948,13 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 			return -EINVAL;
 		}
 
+#if IS_ENABLED(CONFIG_IPV6)
+		if (conf->flags & VXLAN_F_IPV6_LINKLOCAL) {
+			pr_info("link-local local/remote addresses require interface to be specified\n");
+			return -EINVAL;
+		}
+#endif
+
 		*lower = NULL;
 	}
 
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 479bb75789ea..b816a0a6686e 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -258,6 +258,7 @@ struct vxlan_dev {
 #define VXLAN_F_REMCSUM_NOPARTIAL	0x1000
 #define VXLAN_F_COLLECT_METADATA	0x2000
 #define VXLAN_F_GPE			0x4000
+#define VXLAN_F_IPV6_LINKLOCAL		0x8000
 
 /* Flags that are used in the receive path. These flags must match in
  * order for a socket to be shareable
@@ -272,6 +273,7 @@ struct vxlan_dev {
 /* Flags that can be set together with VXLAN_F_GPE. */
 #define VXLAN_F_ALLOWED_GPE		(VXLAN_F_GPE |			\
 					 VXLAN_F_IPV6 |			\
+					 VXLAN_F_IPV6_LINKLOCAL |	\
 					 VXLAN_F_UDP_ZERO_CSUM_TX |	\
 					 VXLAN_F_UDP_ZERO_CSUM6_TX |	\
 					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
-- 
2.12.2

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Sergei Shtylyov]

On 04/14/2017 07:44 PM, Matthias Schiffer wrote:

> * Multicast addresses are never valid as local address
> * Link-local IPv6 unicast addresses may only be used as remote when the
>   local address is link-local as well
> * Don't allow link-local IPv6 local/remote addresses without interface
>
> We also store in the flags field if link-local addresses are used for the
> follow-up patches that actually make VXLAN over link-local IPv6 work.
>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> ---
>
> v2: was "vxlan: don't allow link-local IPv6 local/remote addresses without
> interface" before. v2 does a lot more checks and adds the
> VXLAN_F_IPV6_LINKLOCAL flag.
>
>  drivers/net/vxlan.c | 35 +++++++++++++++++++++++++++++++++++
>  include/net/vxlan.h |  2 ++
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 07f89b037681..95a71546e8f2 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>  		return -EINVAL;
>
> +	if (vxlan_addr_multicast(&conf->saddr))
> +		return -EINVAL;
> +
>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>  		if (!IS_ENABLED(CONFIG_IPV6))
>  			return -EPFNOSUPPORT;
>  		use_ipv6 = true;
>  		conf->flags |= VXLAN_F_IPV6;
> +
> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> +			int local_type =
> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
> +			int remote_type =
> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
> +
> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
> +				    (remote_type != IPV6_ADDR_ANY)) {
> +					pr_info("invalid combination of address scopes\n");

    Maybe pr_err()?

> +					return -EINVAL;
> +				}
> +
> +				conf->flags |= VXLAN_F_IPV6_LINKLOCAL;
> +			} else {
> +				if (remote_type ==
> +				    (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
> +					pr_info("invalid combination of address scopes\n");

    Here as well...

> +					return -EINVAL;
> +				}
> +
> +				conf->flags &= ~VXLAN_F_IPV6_LINKLOCAL;
> +			}
> +		}
>  	}
>
>  	if (conf->label && !use_ipv6) {
[...]

MBR, Sergei

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Sergei Shtylyov]

On 04/14/2017 07:44 PM, Matthias Schiffer wrote:

> * Multicast addresses are never valid as local address
> * Link-local IPv6 unicast addresses may only be used as remote when the
>   local address is link-local as well
> * Don't allow link-local IPv6 local/remote addresses without interface
>
> We also store in the flags field if link-local addresses are used for the
> follow-up patches that actually make VXLAN over link-local IPv6 work.
>
> Signed-off-by: Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org>
> ---
>
> v2: was "vxlan: don't allow link-local IPv6 local/remote addresses without
> interface" before. v2 does a lot more checks and adds the
> VXLAN_F_IPV6_LINKLOCAL flag.
>
>  drivers/net/vxlan.c | 35 +++++++++++++++++++++++++++++++++++
>  include/net/vxlan.h |  2 ++
>  2 files changed, 37 insertions(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 07f89b037681..95a71546e8f2 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>  		return -EINVAL;
>
> +	if (vxlan_addr_multicast(&conf->saddr))
> +		return -EINVAL;
> +
>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>  		if (!IS_ENABLED(CONFIG_IPV6))
>  			return -EPFNOSUPPORT;
>  		use_ipv6 = true;
>  		conf->flags |= VXLAN_F_IPV6;
> +
> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> +			int local_type =
> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
> +			int remote_type =
> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
> +
> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
> +				    (remote_type != IPV6_ADDR_ANY)) {
> +					pr_info("invalid combination of address scopes\n");

    Maybe pr_err()?

> +					return -EINVAL;
> +				}
> +
> +				conf->flags |= VXLAN_F_IPV6_LINKLOCAL;
> +			} else {
> +				if (remote_type ==
> +				    (IPV6_ADDR_UNICAST | IPV6_ADDR_LINKLOCAL)) {
> +					pr_info("invalid combination of address scopes\n");

    Here as well...

> +					return -EINVAL;
> +				}
> +
> +				conf->flags &= ~VXLAN_F_IPV6_LINKLOCAL;
> +			}
> +		}
>  	}
>
>  	if (conf->label && !use_ipv6) {
[...]

MBR, Sergei

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Matthias Schiffer]

[-- Attachment #1.1: Type: text/plain, Size: 2700 bytes --]

On 04/14/2017 07:27 PM, Sergei Shtylyov wrote:
> On 04/14/2017 07:44 PM, Matthias Schiffer wrote:
> 
>> * Multicast addresses are never valid as local address
>> * Link-local IPv6 unicast addresses may only be used as remote when the
>>   local address is link-local as well
>> * Don't allow link-local IPv6 local/remote addresses without interface
>>
>> We also store in the flags field if link-local addresses are used for the
>> follow-up patches that actually make VXLAN over link-local IPv6 work.
>>
>> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
>> ---
>>
>> v2: was "vxlan: don't allow link-local IPv6 local/remote addresses without
>> interface" before. v2 does a lot more checks and adds the
>> VXLAN_F_IPV6_LINKLOCAL flag.
>>
>>  drivers/net/vxlan.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/net/vxlan.h |  2 ++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 07f89b037681..95a71546e8f2 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net
>> *src_net, struct vxlan_config *conf,
>>      if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>          return -EINVAL;
>>
>> +    if (vxlan_addr_multicast(&conf->saddr))
>> +        return -EINVAL;
>> +
>>      if (conf->saddr.sa.sa_family == AF_INET6) {
>>          if (!IS_ENABLED(CONFIG_IPV6))
>>              return -EPFNOSUPPORT;
>>          use_ipv6 = true;
>>          conf->flags |= VXLAN_F_IPV6;
>> +
>> +        if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>> +            int local_type =
>> +                ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>> +            int remote_type =
>> +                ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>> +
>> +            if (local_type & IPV6_ADDR_LINKLOCAL) {
>> +                if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>> +                    (remote_type != IPV6_ADDR_ANY)) {
>> +                    pr_info("invalid combination of address scopes\n");
> 
>    Maybe pr_err()?

Hmm, I mostly followed the style of the existing code, which uses pr_info
for such messages. Also, these messages can be triggered by userspace, as
they're diagnostics for the newlink/changelink operations; I'm not
convinced that their importance justifies pr_err().

Generally, it seems unusual to me to use the kernel log for configuration
diagnostics at all; just removing the messages would be another option.
Stephen also mentioned "extended netlink error reporting", but I guess that
can be done in another patchset.

Matthias


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Matthias Schiffer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2729 bytes --]

On 04/14/2017 07:27 PM, Sergei Shtylyov wrote:
> On 04/14/2017 07:44 PM, Matthias Schiffer wrote:
> 
>> * Multicast addresses are never valid as local address
>> * Link-local IPv6 unicast addresses may only be used as remote when the
>>   local address is link-local as well
>> * Don't allow link-local IPv6 local/remote addresses without interface
>>
>> We also store in the flags field if link-local addresses are used for the
>> follow-up patches that actually make VXLAN over link-local IPv6 work.
>>
>> Signed-off-by: Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org>
>> ---
>>
>> v2: was "vxlan: don't allow link-local IPv6 local/remote addresses without
>> interface" before. v2 does a lot more checks and adds the
>> VXLAN_F_IPV6_LINKLOCAL flag.
>>
>>  drivers/net/vxlan.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/net/vxlan.h |  2 ++
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 07f89b037681..95a71546e8f2 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net
>> *src_net, struct vxlan_config *conf,
>>      if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>          return -EINVAL;
>>
>> +    if (vxlan_addr_multicast(&conf->saddr))
>> +        return -EINVAL;
>> +
>>      if (conf->saddr.sa.sa_family == AF_INET6) {
>>          if (!IS_ENABLED(CONFIG_IPV6))
>>              return -EPFNOSUPPORT;
>>          use_ipv6 = true;
>>          conf->flags |= VXLAN_F_IPV6;
>> +
>> +        if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>> +            int local_type =
>> +                ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>> +            int remote_type =
>> +                ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>> +
>> +            if (local_type & IPV6_ADDR_LINKLOCAL) {
>> +                if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>> +                    (remote_type != IPV6_ADDR_ANY)) {
>> +                    pr_info("invalid combination of address scopes\n");
> 
>    Maybe pr_err()?

Hmm, I mostly followed the style of the existing code, which uses pr_info
for such messages. Also, these messages can be triggered by userspace, as
they're diagnostics for the newlink/changelink operations; I'm not
convinced that their importance justifies pr_err().

Generally, it seems unusual to me to use the kernel log for configuration
diagnostics at all; just removing the messages would be another option.
Stephen also mentioned "extended netlink error reporting", but I guess that
can be done in another patchset.

Matthias


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Stephen Hemminger]

On Fri, 14 Apr 2017 18:44:44 +0200
Matthias Schiffer <mschiffer@universe-factory.net> wrote:

> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 07f89b037681..95a71546e8f2 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>  		return -EINVAL;
>  
> +	if (vxlan_addr_multicast(&conf->saddr))
> +		return -EINVAL;
> +
>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>  		if (!IS_ENABLED(CONFIG_IPV6))
>  			return -EPFNOSUPPORT;
>  		use_ipv6 = true;
>  		conf->flags |= VXLAN_F_IPV6;
> +
> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> +			int local_type =
> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
> +			int remote_type =
> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
> +
> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
> +				    (remote_type != IPV6_ADDR_ANY)) {
> +					pr_info("invalid combination of address scopes\n");

It is always helpful to include device if possible in error message.
					netdev_notice(old->dev, " invalid combination of address scopes\n");
Also vxlan is good candidate for extended netlink error reporting.

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Stephen Hemminger]

On Fri, 14 Apr 2017 18:44:44 +0200
Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:

> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 07f89b037681..95a71546e8f2 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>  		return -EINVAL;
>  
> +	if (vxlan_addr_multicast(&conf->saddr))
> +		return -EINVAL;
> +
>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>  		if (!IS_ENABLED(CONFIG_IPV6))
>  			return -EPFNOSUPPORT;
>  		use_ipv6 = true;
>  		conf->flags |= VXLAN_F_IPV6;
> +
> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
> +			int local_type =
> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
> +			int remote_type =
> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
> +
> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
> +				    (remote_type != IPV6_ADDR_ANY)) {
> +					pr_info("invalid combination of address scopes\n");

It is always helpful to include device if possible in error message.
					netdev_notice(old->dev, " invalid combination of address scopes\n");
Also vxlan is good candidate for extended netlink error reporting.

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Matthias Schiffer]

[-- Attachment #1.1: Type: text/plain, Size: 1668 bytes --]

On 04/14/2017 07:36 PM, Stephen Hemminger wrote:
> On Fri, 14 Apr 2017 18:44:44 +0200
> Matthias Schiffer <mschiffer@universe-factory.net> wrote:
> 
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 07f89b037681..95a71546e8f2 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>  		return -EINVAL;
>>  
>> +	if (vxlan_addr_multicast(&conf->saddr))
>> +		return -EINVAL;
>> +
>>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>>  		if (!IS_ENABLED(CONFIG_IPV6))
>>  			return -EPFNOSUPPORT;
>>  		use_ipv6 = true;
>>  		conf->flags |= VXLAN_F_IPV6;
>> +
>> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>> +			int local_type =
>> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>> +			int remote_type =
>> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>> +
>> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
>> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>> +				    (remote_type != IPV6_ADDR_ANY)) {
>> +					pr_info("invalid combination of address scopes\n");
> 
> It is always helpful to include device if possible in error message.
> 					netdev_notice(old->dev, " invalid combination of address scopes\n");

That makes sense, I'll change it in v3.

> Also vxlan is good candidate for extended netlink error reporting.

Can you point me to a piece of code that does this? Unless you insist, I
wouldn't do it in this patchset, but I might implement the extended error
reporting later.

Matthias



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Matthias Schiffer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1697 bytes --]

On 04/14/2017 07:36 PM, Stephen Hemminger wrote:
> On Fri, 14 Apr 2017 18:44:44 +0200
> Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:
> 
>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>> index 07f89b037681..95a71546e8f2 100644
>> --- a/drivers/net/vxlan.c
>> +++ b/drivers/net/vxlan.c
>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>  		return -EINVAL;
>>  
>> +	if (vxlan_addr_multicast(&conf->saddr))
>> +		return -EINVAL;
>> +
>>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>>  		if (!IS_ENABLED(CONFIG_IPV6))
>>  			return -EPFNOSUPPORT;
>>  		use_ipv6 = true;
>>  		conf->flags |= VXLAN_F_IPV6;
>> +
>> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>> +			int local_type =
>> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>> +			int remote_type =
>> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>> +
>> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
>> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>> +				    (remote_type != IPV6_ADDR_ANY)) {
>> +					pr_info("invalid combination of address scopes\n");
> 
> It is always helpful to include device if possible in error message.
> 					netdev_notice(old->dev, " invalid combination of address scopes\n");

That makes sense, I'll change it in v3.

> Also vxlan is good candidate for extended netlink error reporting.

Can you point me to a piece of code that does this? Unless you insist, I
wouldn't do it in this patchset, but I might implement the extended error
reporting later.

Matthias



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Roopa Prabhu]

On 4/16/17, 8:03 AM, Matthias Schiffer wrote:
> On 04/14/2017 07:36 PM, Stephen Hemminger wrote:
>> On Fri, 14 Apr 2017 18:44:44 +0200
>> Matthias Schiffer <mschiffer@universe-factory.net> wrote:
>>
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>> index 07f89b037681..95a71546e8f2 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>>>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>>  		return -EINVAL;
>>>  
>>> +	if (vxlan_addr_multicast(&conf->saddr))
>>> +		return -EINVAL;
>>> +
>>>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>>>  		if (!IS_ENABLED(CONFIG_IPV6))
>>>  			return -EPFNOSUPPORT;
>>>  		use_ipv6 = true;
>>>  		conf->flags |= VXLAN_F_IPV6;
>>> +
>>> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>>> +			int local_type =
>>> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>>> +			int remote_type =
>>> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>>> +
>>> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
>>> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>>> +				    (remote_type != IPV6_ADDR_ANY)) {
>>> +					pr_info("invalid combination of address scopes\n");
>> It is always helpful to include device if possible in error message.
>> 					netdev_notice(old->dev, " invalid combination of address scopes\n");
> That makes sense, I'll change it in v3.

I think it should just return -EINVAL here since this is in response to a netlink call from user-space.
I dont think we should print anything but like stephen says use the extended ack mechanism to propagate more information
about the error.


>
>> Also vxlan is good candidate for extended netlink error reporting.
> Can you point me to a piece of code that does this? Unless you insist, I
> wouldn't do it in this patchset, but I might implement the extended error
> reporting later.
>
For rtnetlink users (vxlan is one of them) this is still in the works... see patch "net: rtnetlink: plumb extended ack to doit function"

Re: [PATCH net-next v2 4/6] vxlan: check valid combinations of address scopes

[Roopa Prabhu]

On 4/16/17, 8:03 AM, Matthias Schiffer wrote:
> On 04/14/2017 07:36 PM, Stephen Hemminger wrote:
>> On Fri, 14 Apr 2017 18:44:44 +0200
>> Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:
>>
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
>>> index 07f89b037681..95a71546e8f2 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -2881,11 +2881,39 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
>>>  	if (conf->saddr.sa.sa_family != conf->remote_ip.sa.sa_family)
>>>  		return -EINVAL;
>>>  
>>> +	if (vxlan_addr_multicast(&conf->saddr))
>>> +		return -EINVAL;
>>> +
>>>  	if (conf->saddr.sa.sa_family == AF_INET6) {
>>>  		if (!IS_ENABLED(CONFIG_IPV6))
>>>  			return -EPFNOSUPPORT;
>>>  		use_ipv6 = true;
>>>  		conf->flags |= VXLAN_F_IPV6;
>>> +
>>> +		if (!(conf->flags & VXLAN_F_COLLECT_METADATA)) {
>>> +			int local_type =
>>> +				ipv6_addr_type(&conf->saddr.sin6.sin6_addr);
>>> +			int remote_type =
>>> +				ipv6_addr_type(&conf->remote_ip.sin6.sin6_addr);
>>> +
>>> +			if (local_type & IPV6_ADDR_LINKLOCAL) {
>>> +				if (!(remote_type & IPV6_ADDR_LINKLOCAL) &&
>>> +				    (remote_type != IPV6_ADDR_ANY)) {
>>> +					pr_info("invalid combination of address scopes\n");
>> It is always helpful to include device if possible in error message.
>> 					netdev_notice(old->dev, " invalid combination of address scopes\n");
> That makes sense, I'll change it in v3.

I think it should just return -EINVAL here since this is in response to a netlink call from user-space.
I dont think we should print anything but like stephen says use the extended ack mechanism to propagate more information
about the error.


>
>> Also vxlan is good candidate for extended netlink error reporting.
> Can you point me to a piece of code that does this? Unless you insist, I
> wouldn't do it in this patchset, but I might implement the extended error
> reporting later.
>
For rtnetlink users (vxlan is one of them) this is still in the works... see patch "net: rtnetlink: plumb extended ack to doit function"

[PATCH net-next v2 5/6] vxlan: fix snooping for link-local IPv6 addresses

[Matthias Schiffer]

If VXLAN is run over link-local IPv6 addresses, it is necessary to store
the ifindex in the FDB entries. Otherwise, the used interface is undefined
and unicast communication will most likely fail.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: use u32 instead of __u32

 drivers/net/vxlan.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 95a71546e8f2..2995b57a1551 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -941,16 +941,24 @@ static int vxlan_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb,
  */
 static bool vxlan_snoop(struct net_device *dev,
 			union vxlan_addr *src_ip, const u8 *src_mac,
-			__be32 vni)
+			u32 src_ifindex, __be32 vni)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	struct vxlan_fdb *f;
+	u32 ifindex = 0;
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (src_ip->sa.sa_family == AF_INET6 &&
+	    (ipv6_addr_type(&src_ip->sin6.sin6_addr) & IPV6_ADDR_LINKLOCAL))
+		ifindex = src_ifindex;
+#endif
 
 	f = vxlan_find_mac(vxlan, src_mac, vni);
 	if (likely(f)) {
 		struct vxlan_rdst *rdst = first_remote_rcu(f);
 
-		if (likely(vxlan_addr_equal(&rdst->remote_ip, src_ip)))
+		if (likely(vxlan_addr_equal(&rdst->remote_ip, src_ip) &&
+			   rdst->remote_ifindex == ifindex))
 			return false;
 
 		/* Don't migrate static entries, drop packets */
@@ -977,7 +985,7 @@ static bool vxlan_snoop(struct net_device *dev,
 					 vxlan->cfg.dst_port,
 					 vni,
 					 vxlan->default_dst.remote_vni,
-					 0, NTF_SELF);
+					 ifindex, NTF_SELF);
 		spin_unlock(&vxlan->hash_lock);
 	}
 
@@ -1246,6 +1254,7 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
 			  struct sk_buff *skb, __be32 vni)
 {
 	union vxlan_addr saddr;
+	u32 ifindex = skb->dev->ifindex;
 
 	skb_reset_mac_header(skb);
 	skb->protocol = eth_type_trans(skb, vxlan->dev);
@@ -1267,7 +1276,7 @@ static bool vxlan_set_mac(struct vxlan_dev *vxlan,
 	}
 
 	if ((vxlan->cfg.flags & VXLAN_F_LEARN) &&
-	    vxlan_snoop(skb->dev, &saddr, eth_hdr(skb)->h_source, vni))
+	    vxlan_snoop(skb->dev, &saddr, eth_hdr(skb)->h_source, ifindex, vni))
 		return false;
 
 	return true;
@@ -1978,7 +1987,8 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 	}
 
 	if (dst_vxlan->cfg.flags & VXLAN_F_LEARN)
-		vxlan_snoop(skb->dev, &loopback, eth_hdr(skb)->h_source, vni);
+		vxlan_snoop(skb->dev, &loopback, eth_hdr(skb)->h_source, 0,
+			    vni);
 
 	u64_stats_update_begin(&tx_stats->syncp);
 	tx_stats->tx_packets++;
-- 
2.12.2

[PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Matthias Schiffer]

As link-local addresses are only valid for a single interface, we can allow
to use the same VNI for multiple independent VXLANs, as long as the used
interfaces are distinct. This way, VXLANs can always be used as a drop-in
replacement for VLANs with greater ID space.

This also extends VNI lookup to respect the ifindex when link-local IPv6
addresses are used, so using the same VNI on multiple interfaces can
actually work.

Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---

v2: use VXLAN_F_IPV6_LINKLOCAL in vxlan_vs_find_vni; rebase.

 drivers/net/vxlan.c | 73 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 2995b57a1551..9054245f0770 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -224,7 +224,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
 	return NULL;
 }
 
-static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs, __be32 vni)
+static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs, int ifindex,
+					   __be32 vni)
 {
 	struct vxlan_dev *vxlan;
 
@@ -233,17 +234,27 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs, __be32 vni)
 		vni = 0;
 
 	hlist_for_each_entry_rcu(vxlan, vni_head(vs, vni), hlist) {
-		if (vxlan->default_dst.remote_vni == vni)
-			return vxlan;
+		if (vxlan->default_dst.remote_vni != vni)
+			continue;
+
+		if (IS_ENABLED(CONFIG_IPV6)) {
+			const struct vxlan_config *cfg = &vxlan->cfg;
+
+			if ((cfg->flags & VXLAN_F_IPV6_LINKLOCAL) &&
+			    cfg->remote_ifindex != ifindex)
+				continue;
+		}
+
+		return vxlan;
 	}
 
 	return NULL;
 }
 
 /* Look up VNI in a per net namespace table */
-static struct vxlan_dev *vxlan_find_vni(struct net *net, __be32 vni,
-					sa_family_t family, __be16 port,
-					u32 flags)
+static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
+					__be32 vni, sa_family_t family,
+					__be16 port, u32 flags)
 {
 	struct vxlan_sock *vs;
 
@@ -251,7 +262,7 @@ static struct vxlan_dev *vxlan_find_vni(struct net *net, __be32 vni,
 	if (!vs)
 		return NULL;
 
-	return vxlan_vs_find_vni(vs, vni);
+	return vxlan_vs_find_vni(vs, ifindex, vni);
 }
 
 /* Fill in neighbour message in skbuff. */
@@ -1342,7 +1353,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
 
 	vni = vxlan_vni(vxlan_hdr(skb)->vx_vni);
 
-	vxlan = vxlan_vs_find_vni(vs, vni);
+	vxlan = vxlan_vs_find_vni(vs, skb->dev->ifindex, vni);
 	if (!vxlan)
 		goto drop;
 
@@ -2006,8 +2017,10 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
 }
 
 static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
-				 struct vxlan_dev *vxlan, union vxlan_addr *daddr,
-				 __be16 dst_port, __be32 vni, struct dst_entry *dst,
+				 struct vxlan_dev *vxlan,
+				 union vxlan_addr *daddr,
+				 __be16 dst_port, int dst_ifindex, __be32 vni,
+				 struct dst_entry *dst,
 				 u32 rt_flags)
 {
 #if IS_ENABLED(CONFIG_IPV6)
@@ -2023,7 +2036,7 @@ static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
 		struct vxlan_dev *dst_vxlan;
 
 		dst_release(dst);
-		dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+		dst_vxlan = vxlan_find_vni(vxlan->net, dst_ifindex, vni,
 					   daddr->sa.sa_family, dst_port,
 					   vxlan->cfg.flags);
 		if (!dst_vxlan) {
@@ -2055,6 +2068,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	struct dst_entry *ndst = NULL;
 	__be32 vni, label;
 	__u8 tos, ttl;
+	int ifindex;
 	int err;
 	u32 flags = vxlan->cfg.flags;
 	bool udp_sum = false;
@@ -2075,6 +2089,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 
 		dst_port = rdst->remote_port ? rdst->remote_port : vxlan->cfg.dst_port;
 		vni = (rdst->remote_vni) ? : default_vni;
+		ifindex = rdst->remote_ifindex;
 		local_ip = vxlan->cfg.saddr;
 		dst_cache = &rdst->dst_cache;
 		md->gbp = skb->mark;
@@ -2108,6 +2123,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		dst = &remote_ip;
 		dst_port = info->key.tp_dst ? : vxlan->cfg.dst_port;
 		vni = tunnel_id_to_key32(info->key.tun_id);
+		ifindex = 0;
 		dst_cache = &info->dst_cache;
 		if (info->options_len)
 			md = ip_tunnel_info_opts(info);
@@ -2125,8 +2141,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		struct rtable *rt;
 		__be16 df = 0;
 
-		rt = vxlan_get_route(vxlan, dev, sock4, skb,
-				     rdst ? rdst->remote_ifindex : 0, tos,
+		rt = vxlan_get_route(vxlan, dev, sock4, skb, ifindex, tos,
 				     dst->sin.sin_addr.s_addr,
 				     &local_ip.sin.sin_addr.s_addr,
 				     dst_port, src_port,
@@ -2139,8 +2154,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 		/* Bypass encapsulation if the destination is local */
 		if (!info) {
 			err = encap_bypass_if_local(skb, dev, vxlan, dst,
-						    dst_port, vni, &rt->dst,
-						    rt->rt_flags);
+						    dst_port, ifindex, vni,
+						    &rt->dst, rt->rt_flags);
 			if (err)
 				goto out_unlock;
 		} else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT) {
@@ -2162,8 +2177,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 	} else {
 		struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
 
-		ndst = vxlan6_get_route(vxlan, dev, sock6, skb,
-					rdst ? rdst->remote_ifindex : 0, tos,
+		ndst = vxlan6_get_route(vxlan, dev, sock6, skb, ifindex, tos,
 					label, &dst->sin6.sin6_addr,
 					&local_ip.sin6.sin6_addr,
 					dst_port, src_port,
@@ -2178,8 +2192,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
 			u32 rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
 
 			err = encap_bypass_if_local(skb, dev, vxlan, dst,
-						    dst_port, vni, ndst,
-						    rt6i_flags);
+						    dst_port, ifindex, vni,
+						    ndst, rt6i_flags);
 			if (err)
 				goto out_unlock;
 		}
@@ -2982,13 +2996,20 @@ static int vxlan_config_validate(struct net *src_net, struct vxlan_config *conf,
 		if (tmp == old)
 			continue;
 
-		if (tmp->cfg.vni == conf->vni &&
-		    tmp->cfg.dst_port == conf->dst_port &&
-		    (tmp->cfg.flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6)) ==
-		    (conf->flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6))) {
-			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
-			return -EEXIST;
-		}
+		if (tmp->cfg.vni != conf->vni)
+			continue;
+		if (tmp->cfg.dst_port != conf->dst_port)
+			continue;
+		if ((tmp->cfg.flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6)) !=
+		    (conf->flags & (VXLAN_F_RCV_FLAGS | VXLAN_F_IPV6)))
+			continue;
+
+		if ((conf->flags & VXLAN_F_IPV6_LINKLOCAL) &&
+		    tmp->cfg.remote_ifindex != conf->remote_ifindex)
+			continue;
+
+		pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
+		return -EEXIST;
 	}
 
 	return 0;
-- 
2.12.2

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Stephen Hemminger]

On Fri, 14 Apr 2017 18:44:46 +0200
Matthias Schiffer <mschiffer@universe-factory.net> wrote:

> As link-local addresses are only valid for a single interface, we can allow
> to use the same VNI for multiple independent VXLANs, as long as the used
> interfaces are distinct. This way, VXLANs can always be used as a drop-in
> replacement for VLANs with greater ID space.
> 
> This also extends VNI lookup to respect the ifindex when link-local IPv6
> addresses are used, so using the same VNI on multiple interfaces can
> actually work.
> 
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>

Why does this have to be IPv6 specific?

What about the case where VXLAN is not bound to an interface?
If that is used then that could be a problem.

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Stephen Hemminger]

On Fri, 14 Apr 2017 18:44:46 +0200
Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:

> As link-local addresses are only valid for a single interface, we can allow
> to use the same VNI for multiple independent VXLANs, as long as the used
> interfaces are distinct. This way, VXLANs can always be used as a drop-in
> replacement for VLANs with greater ID space.
> 
> This also extends VNI lookup to respect the ifindex when link-local IPv6
> addresses are used, so using the same VNI on multiple interfaces can
> actually work.
> 
> Signed-off-by: Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org>

Why does this have to be IPv6 specific?

What about the case where VXLAN is not bound to an interface?
If that is used then that could be a problem.

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Matthias Schiffer]

[-- Attachment #1.1: Type: text/plain, Size: 1332 bytes --]

On 04/14/2017 07:38 PM, Stephen Hemminger wrote:
> On Fri, 14 Apr 2017 18:44:46 +0200
> Matthias Schiffer <mschiffer@universe-factory.net> wrote:
> 
>> As link-local addresses are only valid for a single interface, we can allow
>> to use the same VNI for multiple independent VXLANs, as long as the used
>> interfaces are distinct. This way, VXLANs can always be used as a drop-in
>> replacement for VLANs with greater ID space.
>>
>> This also extends VNI lookup to respect the ifindex when link-local IPv6
>> addresses are used, so using the same VNI on multiple interfaces can
>> actually work.
>>
>> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> 
> Why does this have to be IPv6 specific?

I'm not familar with IPv4 link-local addresses and how route lookup works
for them. vxlan_get_route() sets flowi4_oif to the outgoing interface; does
__ip_route_output_key_hash() do the right thing for link-local addresses
when such addresses are used on multiple interfaces? I see some special
casing for multicast destinations, but none for link-local ones.

> 
> What about the case where VXLAN is not bound to an interface?
> If that is used then that could be a problem.
> 

With patch 4/6, link-local IPv6 addresses can't be configured without an
interface anymore.

Matthias


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Matthias Schiffer]

[-- Attachment #1.1.1: Type: text/plain, Size: 1390 bytes --]

On 04/14/2017 07:38 PM, Stephen Hemminger wrote:
> On Fri, 14 Apr 2017 18:44:46 +0200
> Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:
> 
>> As link-local addresses are only valid for a single interface, we can allow
>> to use the same VNI for multiple independent VXLANs, as long as the used
>> interfaces are distinct. This way, VXLANs can always be used as a drop-in
>> replacement for VLANs with greater ID space.
>>
>> This also extends VNI lookup to respect the ifindex when link-local IPv6
>> addresses are used, so using the same VNI on multiple interfaces can
>> actually work.
>>
>> Signed-off-by: Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org>
> 
> Why does this have to be IPv6 specific?

I'm not familar with IPv4 link-local addresses and how route lookup works
for them. vxlan_get_route() sets flowi4_oif to the outgoing interface; does
__ip_route_output_key_hash() do the right thing for link-local addresses
when such addresses are used on multiple interfaces? I see some special
casing for multicast destinations, but none for link-local ones.

> 
> What about the case where VXLAN is not bound to an interface?
> If that is used then that could be a problem.
> 

With patch 4/6, link-local IPv6 addresses can't be configured without an
interface anymore.

Matthias


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Matthias Schiffer]

[-- Attachment #1.1: Type: text/plain, Size: 2667 bytes --]

On 04/16/2017 05:15 PM, Matthias Schiffer wrote:
> On 04/14/2017 07:38 PM, Stephen Hemminger wrote:
>> On Fri, 14 Apr 2017 18:44:46 +0200
>> Matthias Schiffer <mschiffer@universe-factory.net> wrote:
>>
>>> As link-local addresses are only valid for a single interface, we can allow
>>> to use the same VNI for multiple independent VXLANs, as long as the used
>>> interfaces are distinct. This way, VXLANs can always be used as a drop-in
>>> replacement for VLANs with greater ID space.
>>>
>>> This also extends VNI lookup to respect the ifindex when link-local IPv6
>>> addresses are used, so using the same VNI on multiple interfaces can
>>> actually work.
>>>
>>> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
>>
>> Why does this have to be IPv6 specific?
> 
> I'm not familar with IPv4 link-local addresses and how route lookup works
> for them. vxlan_get_route() sets flowi4_oif to the outgoing interface; does
> __ip_route_output_key_hash() do the right thing for link-local addresses
> when such addresses are used on multiple interfaces? I see some special
> casing for multicast destinations, but none for link-local ones.
> 

Getting back to this (sorry for the delay, I got caught up in other
projects), I'm seeing the following pros and cons regarding the support of
VXLAN over IPv4 link-local addresses:

+ There should be no technical reason not to support it; as everything is
in the kernel, the usual problems with IPv4 LL (userspace APIs not
supporting passing a scope ID as part of the IP address) don't apply here

+ The code needed to support IPv4 LL should be easy to add

- IPv4 LL semantics aren't as well-defined as for IPv6. While IPv4 LL
addresses are usually in the 169.254.x.y range, the Linux kernel allows
setting the address scope independently of the range for IPv4. In contrast
to this, we need to judge the validity of the configuration based on
syntactic properties of the IP addresses (at least if we don't want to add
a lot of more compexity to the validation, and probably other parts of the
code.) Generally, code that checks for the 169.254.x.y range is uncommon in
the kernel (I think I only found a single instance, somewhere in the SCTP
implementation.)

- IPv4 LL addresses are mostly used for zeroconf; I don't really see a
usecase for zeroconf addresses + VXLANs

- Personally, I have no interest in IPv4


I probably forgot a few more arguments... All in all, I'd like the VXLAN
maintainers to decide if we do want IPv4 LL support or not, and if the
verdict is to support it, I'll implement it in the next revision of my
patchset.

Matthias


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH net-next v2 6/6] vxlan: allow multiple VXLANs with same VNI for IPv6 link-local addresses

[Matthias Schiffer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2725 bytes --]

On 04/16/2017 05:15 PM, Matthias Schiffer wrote:
> On 04/14/2017 07:38 PM, Stephen Hemminger wrote:
>> On Fri, 14 Apr 2017 18:44:46 +0200
>> Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org> wrote:
>>
>>> As link-local addresses are only valid for a single interface, we can allow
>>> to use the same VNI for multiple independent VXLANs, as long as the used
>>> interfaces are distinct. This way, VXLANs can always be used as a drop-in
>>> replacement for VLANs with greater ID space.
>>>
>>> This also extends VNI lookup to respect the ifindex when link-local IPv6
>>> addresses are used, so using the same VNI on multiple interfaces can
>>> actually work.
>>>
>>> Signed-off-by: Matthias Schiffer <mschiffer-Nyw9WiXk/RKXhJHkyCwd5uTW4wlIGRCZ@public.gmane.org>
>>
>> Why does this have to be IPv6 specific?
> 
> I'm not familar with IPv4 link-local addresses and how route lookup works
> for them. vxlan_get_route() sets flowi4_oif to the outgoing interface; does
> __ip_route_output_key_hash() do the right thing for link-local addresses
> when such addresses are used on multiple interfaces? I see some special
> casing for multicast destinations, but none for link-local ones.
> 

Getting back to this (sorry for the delay, I got caught up in other
projects), I'm seeing the following pros and cons regarding the support of
VXLAN over IPv4 link-local addresses:

+ There should be no technical reason not to support it; as everything is
in the kernel, the usual problems with IPv4 LL (userspace APIs not
supporting passing a scope ID as part of the IP address) don't apply here

+ The code needed to support IPv4 LL should be easy to add

- IPv4 LL semantics aren't as well-defined as for IPv6. While IPv4 LL
addresses are usually in the 169.254.x.y range, the Linux kernel allows
setting the address scope independently of the range for IPv4. In contrast
to this, we need to judge the validity of the configuration based on
syntactic properties of the IP addresses (at least if we don't want to add
a lot of more compexity to the validation, and probably other parts of the
code.) Generally, code that checks for the 169.254.x.y range is uncommon in
the kernel (I think I only found a single instance, somewhere in the SCTP
implementation.)

- IPv4 LL addresses are mostly used for zeroconf; I don't really see a
usecase for zeroconf addresses + VXLANs

- Personally, I have no interest in IPv4


I probably forgot a few more arguments... All in all, I'd like the VXLAN
maintainers to decide if we do want IPv4 LL support or not, and if the
verdict is to support it, I'll implement it in the next revision of my
patchset.

Matthias


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]