Skip to contentSkip to conversation

vxlan: fix error reporting

PATCH In review Jiri Benc 3 participants 6 messages Sep 2, 2016 – Sep 4, 2016

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jiri Benc

author
#0 lore raw

cover vxlan: fix error reporting

This patchset improves checking for invalid configuration in VXLAN and fixes problems with duplicated and inappropriate error messages.

Jiri Benc (2):
vxlan: reject multicast destination without an interface
vxlan: fix duplicated and wrong error messages

 drivers/net/vxlan.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH 1/2 vxlan: reject multicast destination without an interface

Currently, kernel accepts configurations such as:

ip l a type vxlan dstport 4789 id 1 group 239.192.0.1 ip l a type vxlan dstport 4789 id 1 group ff0e::110

However, neither of those really works. In the IPv4 case, the interface cannot be brought up ("RTNETLINK answers: No such device"). This is because multicast join will be rejected without the interface being specified.

In the IPv6 case, multicast wil be joined on the first interface found. This is not what the user wants as it depends on random factors (order of interfaces).

Note that it's possible to add a local address but it doesn't solve anything. For IPv4, it's not considered in the multicast join (thus the same error as above is returned on ifup). This could be added but it wouldn't help for IPv6 anyway. For IPv6, we do need the interface.

Just reject a configuration that sets multicast address and does not provide an interface. Nobody can depend on the previous behavior as it never worked.

Signed-off-by Jiri Benc <[email protected]>
---
 drivers/net/vxlan.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/net/vxlan.c +3 -0
index c0dda6fc0921..6358e35d74b7 100644
@@ -2842,6 +2842,9 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 			dev->mtu = lowerdev->mtu - (use_ipv6 ? VXLAN6_HEADROOM : VXLAN_HEADROOM);
 
 		needed_headroom = lowerdev->hard_header_len;
+	} else if (vxlan_addr_multicast(&dst->remote_ip)) {
+		pr_info("multicast destination requires interface to be specified\n");
+		return -EINVAL;
 	}
 
 	if (conf->mtu) {
Signature · 1 line
1.8.3.1
author
#2 lore raw

PATCH 2/2 vxlan: fix duplicated and wrong error messages

In reply to Jiri Benc

vxlan_dev_configure outputs error messages before returning, no need to print again the same mesages in vxlan_newlink. Also, vxlan_dev_configure may return a particular error code for a different reason than vxlan_newlink thinks.

Move the remaining error messages into vxlan_dev_configure and let vxlan_newlink just pass on the error code.

Signed-off-by Jiri Benc <[email protected]>
---
 drivers/net/vxlan.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)
drivers/net/vxlan.c +9 -26
index 6358e35d74b7..6e65832051d6 100644
@@ -2782,14 +2782,15 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 	struct net_device *lowerdev = NULL;
 
 	if (conf->flags & VXLAN_F_GPE) {
-		if (conf->flags & ~VXLAN_F_ALLOWED_GPE)
-			return -EINVAL;
 		/* 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_COLLECT_METADATA))
+		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 {

@@ -2877,8 +2878,10 @@ static int vxlan_dev_configure(struct net *src_net, struct net_device *dev,
 		     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))
-		return -EEXIST;
+		    (vxlan->flags & VXLAN_F_RCV_FLAGS)) {
+			pr_info("duplicate VNI %u\n", be32_to_cpu(conf->vni));
+			return -EEXIST;
+		}
 	}
 
 	dev->ethtool_ops = &vxlan_ethtool_ops;

@@ -2912,7 +2915,6 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 			 struct nlattr *tb[], struct nlattr *data[])
 {
 	struct vxlan_config conf;
-	int err;
 
 	memset(&conf, 0, sizeof(conf));
 

@@ -3021,26 +3023,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 	if (tb[IFLA_MTU])
 		conf.mtu = nla_get_u32(tb[IFLA_MTU]);
 
-	err = vxlan_dev_configure(src_net, dev, &conf);
-	switch (err) {
-	case -ENODEV:
-		pr_info("ifindex %d does not exist\n", conf.remote_ifindex);
-		break;
-
-	case -EPERM:
-		pr_info("IPv6 is disabled via sysctl\n");
-		break;
-
-	case -EEXIST:
-		pr_info("duplicate VNI %u\n", be32_to_cpu(conf.vni));
-		break;
-
-	case -EINVAL:
-		pr_info("unsupported combination of extensions\n");
-		break;
-	}
-
-	return err;
+	return vxlan_dev_configure(src_net, dev, &conf);
 }
 
 static void vxlan_dellink(struct net_device *dev, struct list_head *head)
Signature · 1 line
1.8.3.1
#3 lore raw
In reply to Jiri Benc
Quoted 10 lines · On Fri, 2 Sep 2016 13:37:10 +0200 Jiri Benc <[email protected]> wrote:

This patchset improves checking for invalid configuration in VXLAN and fixes problems with duplicated and inappropriate error messages.

Jiri Benc (2):
vxlan: reject multicast destination without an interface
vxlan: fix duplicated and wrong error messages

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

These should also be detected and rejected in iproute2 errors in kernel log are user unfriendly api.

author
#4 lore raw
Quoted 10 lines · Stephen Hemminger
Quoted 7 lines · On Fri, 2 Sep 2016 13:37:10 +0200 Jiri Benc <[email protected]> wrote:

This patchset improves checking for invalid configuration in VXLAN and fixes problems with duplicated and inappropriate error messages.

Jiri Benc (2):
vxlan: reject multicast destination without an interface
vxlan: fix duplicated and wrong error messages

Quoted 6 lines
Quoted 3 lines
 drivers/net/vxlan.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

These should also be detected and rejected in iproute2 errors in kernel log are user unfriendly api.

I agree and I intend to send a patch once this is accepted.

Jiri

maintainer
#5 lore raw
In reply to Jiri Benc

From: Jiri Benc <[email protected]>
Date: Fri, 2 Sep 2016 13:37:10 +0200

Quoted 2 lines

This patchset improves checking for invalid configuration in VXLAN and fixes problems with duplicated and inappropriate error messages.

Series applied, thanks.