netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/3] vxlan: a few minor cleanups
@ 2018-11-28 22:27 Roopa Prabhu
  2018-11-28 22:27 ` [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes Roopa Prabhu
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-28 22:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Roopa Prabhu (3):
  vxlan: support changelink for a few more attributes
  vxlan: extack support for some changelink cases
  vxlan: move flag sets to use a helper func

 drivers/net/vxlan.c | 199 +++++++++++++++++++++++++++-------------------------
 1 file changed, 102 insertions(+), 97 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes
  2018-11-28 22:27 [PATCH net-next v2 0/3] vxlan: a few minor cleanups Roopa Prabhu
@ 2018-11-28 22:27 ` Roopa Prabhu
  2018-11-29 13:56   ` Sabrina Dubroca
  2018-11-28 22:27 ` [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases Roopa Prabhu
  2018-11-28 22:27 ` [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func Roopa Prabhu
  2 siblings, 1 reply; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-28 22:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

We started very conservative when supporting changelink
especially because not all attribute changes could be
tested. This patch opens up a few more attributes for
changelink. The reason for choosing this set of attributes
is based on code references for these attributes. I have
tested TTL changes and did some changelink api testing
to sanity test the others.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 drivers/net/vxlan.c | 36 ++++--------------------------------
 1 file changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 9110662..73caa65 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3438,11 +3438,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	if (data[IFLA_VXLAN_TTL])
 		conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
 
-	if (data[IFLA_VXLAN_TTL_INHERIT]) {
-		if (changelink)
-			return -EOPNOTSUPP;
+	if (data[IFLA_VXLAN_TTL_INHERIT])
 		conf->flags |= VXLAN_F_TTL_INHERIT;
-	}
 
 	if (data[IFLA_VXLAN_LABEL])
 		conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) &
@@ -3462,29 +3459,21 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 		conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
 
 	if (data[IFLA_VXLAN_PROXY]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_PROXY]))
 			conf->flags |= VXLAN_F_PROXY;
 	}
 
 	if (data[IFLA_VXLAN_RSC]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_RSC]))
 			conf->flags |= VXLAN_F_RSC;
 	}
 
 	if (data[IFLA_VXLAN_L2MISS]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_L2MISS]))
 			conf->flags |= VXLAN_F_L2MISS;
 	}
 
 	if (data[IFLA_VXLAN_L3MISS]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_L3MISS]))
 			conf->flags |= VXLAN_F_L3MISS;
 	}
@@ -3527,50 +3516,33 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	}
 
 	if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]))
 			conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
 	}
 
 	if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
 			conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
 	}
 
 	if (data[IFLA_VXLAN_REMCSUM_TX]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
 			conf->flags |= VXLAN_F_REMCSUM_TX;
 	}
 
 	if (data[IFLA_VXLAN_REMCSUM_RX]) {
-		if (changelink)
-			return -EOPNOTSUPP;
 		if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
 			conf->flags |= VXLAN_F_REMCSUM_RX;
 	}
 
-	if (data[IFLA_VXLAN_GBP]) {
-		if (changelink)
-			return -EOPNOTSUPP;
+	if (data[IFLA_VXLAN_GBP])
 		conf->flags |= VXLAN_F_GBP;
-	}
 
-	if (data[IFLA_VXLAN_GPE]) {
-		if (changelink)
-			return -EOPNOTSUPP;
+	if (data[IFLA_VXLAN_GPE])
 		conf->flags |= VXLAN_F_GPE;
-	}
 
-	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
-		if (changelink)
-			return -EOPNOTSUPP;
+	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
 		conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
-	}
 
 	if (tb[IFLA_MTU]) {
 		if (changelink)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases
  2018-11-28 22:27 [PATCH net-next v2 0/3] vxlan: a few minor cleanups Roopa Prabhu
  2018-11-28 22:27 ` [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes Roopa Prabhu
@ 2018-11-28 22:27 ` Roopa Prabhu
  2018-12-01  0:31   ` kbuild test robot
  2018-11-28 22:27 ` [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func Roopa Prabhu
  2 siblings, 1 reply; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-28 22:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 drivers/net/vxlan.c | 76 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 59 insertions(+), 17 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 73caa65..4cb6b50 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3376,7 +3376,7 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
 
 static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			 struct net_device *dev, struct vxlan_config *conf,
-			 bool changelink)
+			 bool changelink, struct netlink_ext_ack *extack)
 {
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 
@@ -3389,40 +3389,60 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	if (data[IFLA_VXLAN_ID]) {
 		__be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
 
-		if (changelink && (vni != conf->vni))
+		if (changelink && (vni != conf->vni)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID],
+					    "Cannot change vni");
 			return -EOPNOTSUPP;
+		}
 		conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
 	}
 
 	if (data[IFLA_VXLAN_GROUP]) {
-		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET))
+		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP],
+					    "New group addr family does not match old group");
 			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))
+		if (!IS_ENABLED(CONFIG_IPV6)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6],
+					    "IPv6 support not enabled in the kernel");
 			return -EPFNOSUPPORT;
+		}
 
-		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6))
+		if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6],
+					    "New group addr family does not match old group");
 			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))
+		if (changelink && (conf->saddr.sa.sa_family != AF_INET)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL],
+					    "New local addr family does not match old");
 			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))
+		if (!IS_ENABLED(CONFIG_IPV6)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6],
+					    "IPv6 support not enabled in the kernel");
 			return -EPFNOSUPPORT;
+		}
 
-		if (changelink && (conf->saddr.sa.sa_family != AF_INET6))
+		if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6],
+					    "New local6 addr family does not match old");
 			return -EOPNOTSUPP;
+		}
 
 		/* TODO: respect scope id */
 		conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
@@ -3479,14 +3499,21 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	}
 
 	if (data[IFLA_VXLAN_LIMIT]) {
-		if (changelink)
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT],
+					    "Cannot change limit");
 			return -EOPNOTSUPP;
+		}
 		conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]);
 	}
 
 	if (data[IFLA_VXLAN_COLLECT_METADATA]) {
-		if (changelink)
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[IFLA_VXLAN_COLLECT_METADATA],
+					    "Cannot change metadata flag");
 			return -EOPNOTSUPP;
+		}
 		if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA]))
 			conf->flags |= VXLAN_F_COLLECT_METADATA;
 	}
@@ -3497,20 +3524,31 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 				= nla_data(data[IFLA_VXLAN_PORT_RANGE]);
 			conf->port_min = ntohs(p->low);
 			conf->port_max = ntohs(p->high);
-		} else {
+		else {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[IFLA_VXLAN_PORT_RANGE],
+					    "Cannot change port range");
 			return -EOPNOTSUPP;
 		}
 	}
 
 	if (data[IFLA_VXLAN_PORT]) {
-		if (changelink)
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[IFLA_VXLAN_PORT],
+					    "Cannot change port");
 			return -EOPNOTSUPP;
+		}
 		conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]);
 	}
 
 	if (data[IFLA_VXLAN_UDP_CSUM]) {
-		if (changelink)
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[IFLA_VXLAN_UDP_CSUM],
+					    "Cannot change udp csum");
 			return -EOPNOTSUPP;
+		}
 		if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM]))
 			conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX;
 	}
@@ -3545,8 +3583,12 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 		conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
 
 	if (tb[IFLA_MTU]) {
-		if (changelink)
+		if (changelink) {
+			NL_SET_ERR_MSG_ATTR(extack,
+					    tb[IFLA_MTU],
+					    "Cannot change mtu");
 			return -EOPNOTSUPP;
+		}
 		conf->mtu = nla_get_u32(tb[IFLA_MTU]);
 	}
 
@@ -3563,7 +3605,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 	struct vxlan_config conf;
 	int err;
 
-	err = vxlan_nl2conf(tb, data, dev, &conf, false);
+	err = vxlan_nl2conf(tb, data, dev, &conf, false, extack);
 	if (err)
 		return err;
 
@@ -3583,7 +3625,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
 	int err;
 
 	err = vxlan_nl2conf(tb, data,
-			    dev, &conf, true);
+			    dev, &conf, true, extack);
 	if (err)
 		return err;
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func
  2018-11-28 22:27 [PATCH net-next v2 0/3] vxlan: a few minor cleanups Roopa Prabhu
  2018-11-28 22:27 ` [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes Roopa Prabhu
  2018-11-28 22:27 ` [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases Roopa Prabhu
@ 2018-11-28 22:27 ` Roopa Prabhu
  2018-11-29 14:19   ` Sabrina Dubroca
  2 siblings, 1 reply; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-28 22:27 UTC (permalink / raw)
  To: davem; +Cc: netdev

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 drivers/net/vxlan.c | 95 ++++++++++++++++++++++++-----------------------------
 1 file changed, 43 insertions(+), 52 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 4cb6b50..47671fd 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3374,6 +3374,23 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
 	return err;
 }
 
+/* Set/clear flags based on attribute */
+static void vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
+			  int attrtype, unsigned long mask)
+{
+	unsigned long flags;
+
+	if (!tb[attrtype])
+		return;
+
+	if (nla_get_u8(tb[attrtype]))
+		flags = conf->flags | mask;
+	else
+		flags = conf->flags & ~mask;
+
+	conf->flags = flags;
+}
+
 static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			 struct net_device *dev, struct vxlan_config *conf,
 			 bool changelink, struct netlink_ext_ack *extack)
@@ -3458,45 +3475,27 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	if (data[IFLA_VXLAN_TTL])
 		conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
 
-	if (data[IFLA_VXLAN_TTL_INHERIT])
-		conf->flags |= VXLAN_F_TTL_INHERIT;
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
+		      VXLAN_F_TTL_INHERIT);
 
 	if (data[IFLA_VXLAN_LABEL])
 		conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) &
 			     IPV6_FLOWLABEL_MASK;
 
-	if (data[IFLA_VXLAN_LEARNING]) {
-		if (nla_get_u8(data[IFLA_VXLAN_LEARNING]))
-			conf->flags |= VXLAN_F_LEARN;
-		else
-			conf->flags &= ~VXLAN_F_LEARN;
-	} else if (!changelink) {
+	if (data[IFLA_VXLAN_LEARNING])
+		vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING,
+			      VXLAN_F_LEARN);
+	else if (!changelink)
 		/* default to learn on a new device */
 		conf->flags |= VXLAN_F_LEARN;
-	}
 
 	if (data[IFLA_VXLAN_AGEING])
 		conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
 
-	if (data[IFLA_VXLAN_PROXY]) {
-		if (nla_get_u8(data[IFLA_VXLAN_PROXY]))
-			conf->flags |= VXLAN_F_PROXY;
-	}
-
-	if (data[IFLA_VXLAN_RSC]) {
-		if (nla_get_u8(data[IFLA_VXLAN_RSC]))
-			conf->flags |= VXLAN_F_RSC;
-	}
-
-	if (data[IFLA_VXLAN_L2MISS]) {
-		if (nla_get_u8(data[IFLA_VXLAN_L2MISS]))
-			conf->flags |= VXLAN_F_L2MISS;
-	}
-
-	if (data[IFLA_VXLAN_L3MISS]) {
-		if (nla_get_u8(data[IFLA_VXLAN_L3MISS]))
-			conf->flags |= VXLAN_F_L3MISS;
-	}
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY, VXLAN_F_PROXY);
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC, VXLAN_F_RSC);
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS, VXLAN_F_L2MISS);
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS, VXLAN_F_L3MISS);
 
 	if (data[IFLA_VXLAN_LIMIT]) {
 		if (changelink) {
@@ -3514,8 +3513,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 					    "Cannot change metadata flag");
 			return -EOPNOTSUPP;
 		}
-		if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA]))
-			conf->flags |= VXLAN_F_COLLECT_METADATA;
+		vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA,
+			      VXLAN_F_COLLECT_METADATA);
 	}
 
 	if (data[IFLA_VXLAN_PORT_RANGE]) {
@@ -3553,34 +3552,26 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 			conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX;
 	}
 
-	if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
-		if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]))
-			conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
-	}
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
+		      VXLAN_F_UDP_ZERO_CSUM6_TX);
 
-	if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
-		if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
-			conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
-	}
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX,
+		      VXLAN_F_UDP_ZERO_CSUM6_RX);
 
-	if (data[IFLA_VXLAN_REMCSUM_TX]) {
-		if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
-			conf->flags |= VXLAN_F_REMCSUM_TX;
-	}
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX,
+		      VXLAN_F_REMCSUM_TX);
 
-	if (data[IFLA_VXLAN_REMCSUM_RX]) {
-		if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
-			conf->flags |= VXLAN_F_REMCSUM_RX;
-	}
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX,
+		      VXLAN_F_REMCSUM_RX);
 
-	if (data[IFLA_VXLAN_GBP])
-		conf->flags |= VXLAN_F_GBP;
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP,
+		      VXLAN_F_GBP);
 
-	if (data[IFLA_VXLAN_GPE])
-		conf->flags |= VXLAN_F_GPE;
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE,
+		      VXLAN_F_GPE);
 
-	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
-		conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
+	vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
+		      VXLAN_F_REMCSUM_NOPARTIAL);
 
 	if (tb[IFLA_MTU]) {
 		if (changelink) {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes
  2018-11-28 22:27 ` [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes Roopa Prabhu
@ 2018-11-29 13:56   ` Sabrina Dubroca
  2018-11-29 15:33     ` Roopa Prabhu
  0 siblings, 1 reply; 12+ messages in thread
From: Sabrina Dubroca @ 2018-11-29 13:56 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: davem, netdev

2018-11-28, 14:27:57 -0800, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> We started very conservative when supporting changelink
> especially because not all attribute changes could be
> tested. This patch opens up a few more attributes for
> changelink. The reason for choosing this set of attributes
> is based on code references for these attributes. I have
> tested TTL changes and did some changelink api testing
> to sanity test the others.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
>  drivers/net/vxlan.c | 36 ++++--------------------------------
>  1 file changed, 4 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 9110662..73caa65 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -3438,11 +3438,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  	if (data[IFLA_VXLAN_TTL])
>  		conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
>  
> -	if (data[IFLA_VXLAN_TTL_INHERIT]) {
> -		if (changelink)
> -			return -EOPNOTSUPP;
> +	if (data[IFLA_VXLAN_TTL_INHERIT])
>  		conf->flags |= VXLAN_F_TTL_INHERIT;
> -	}

This doesn't give us an option to disable TTL_INHERIT after it was
enabled once. Same thing with GBP, GPE, REMCSUM_NOPARTIAL.

> -	if (data[IFLA_VXLAN_GBP]) {
> -		if (changelink)
> -			return -EOPNOTSUPP;
> +	if (data[IFLA_VXLAN_GBP])
>  		conf->flags |= VXLAN_F_GBP;
> -	}
>  
> -	if (data[IFLA_VXLAN_GPE]) {
> -		if (changelink)
> -			return -EOPNOTSUPP;
> +	if (data[IFLA_VXLAN_GPE])
>  		conf->flags |= VXLAN_F_GPE;
> -	}

GPE implies running a different setup function (vxlan_raw_setup() vs
vxlan_ether_setup()), that vxlan_config_apply() only calls for
!changelink. I think this is incomplete.

I think we'd also end up with mixed tunnel types (GPE/!GPE) on the
same socket, I'm not sure how that would work. Normally, they each try
to create a separate socket, and pass the GPE flag on to the
associated vxlan_sock. I suspect that's also a problem with rx
offload.

> -	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
> -		if (changelink)
> -			return -EOPNOTSUPP;
> +	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
>  		conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
> -	}
>  
>  	if (tb[IFLA_MTU]) {
>  		if (changelink)
> -- 
> 2.1.4
> 

-- 
Sabrina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func
  2018-11-28 22:27 ` [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func Roopa Prabhu
@ 2018-11-29 14:19   ` Sabrina Dubroca
  2018-11-29 15:27     ` Roopa Prabhu
  0 siblings, 1 reply; 12+ messages in thread
From: Sabrina Dubroca @ 2018-11-29 14:19 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: davem, netdev

2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote:
> +/* Set/clear flags based on attribute */
> +static void vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
> +			  int attrtype, unsigned long mask)
> +{
> +	unsigned long flags;
> +
> +	if (!tb[attrtype])
> +		return;
> +
> +	if (nla_get_u8(tb[attrtype]))
> +		flags = conf->flags | mask;
> +	else
> +		flags = conf->flags & ~mask;
> +
> +	conf->flags = flags;
> +}
> +
>  static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  			 struct net_device *dev, struct vxlan_config *conf,
>  			 bool changelink, struct netlink_ext_ack *extack)
> @@ -3458,45 +3475,27 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  	if (data[IFLA_VXLAN_TTL])
>  		conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
>  
> -	if (data[IFLA_VXLAN_TTL_INHERIT])
> -		conf->flags |= VXLAN_F_TTL_INHERIT;
> +	vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
> +		      VXLAN_F_TTL_INHERIT);

IFLA_VXLAN_TTL_INHERIT is a NLA_FLAG. It doesn't have any u8 data to
get. Same for:

	[IFLA_VXLAN_GBP]	= { .type = NLA_FLAG, },
	[IFLA_VXLAN_GPE]	= { .type = NLA_FLAG, },
	[IFLA_VXLAN_REMCSUM_NOPARTIAL]	= { .type = NLA_FLAG },
	[IFLA_VXLAN_TTL_INHERIT]	= { .type = NLA_FLAG },



nit: This patch would have been easier to review if it came first in
the series. Converting:

    if (nla_get_u8(data[IFLA_VXLAN_FOO]))
        conf->flags |= VXLAN_F_FOO;

to this new helper, which in effect does

    if (nla_get_u8(data[IFLA_VXLAN_FOO]))
        conf->flags |= VXLAN_F_FOO;
    else
        conf->flags &= ~VXLAN_F_FOO;

seems to change behavior, but since you're (unless I missed one) only
applying it to flags that couldn't be changed before patch 1, it looks
fine.

-- 
Sabrina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func
  2018-11-29 14:19   ` Sabrina Dubroca
@ 2018-11-29 15:27     ` Roopa Prabhu
  2018-11-29 15:55       ` Sabrina Dubroca
  0 siblings, 1 reply; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-29 15:27 UTC (permalink / raw)
  To: sd; +Cc: David Miller, netdev

On Thu, Nov 29, 2018 at 6:19 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote:
> > +/* Set/clear flags based on attribute */
> > +static void vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
> > +                       int attrtype, unsigned long mask)
> > +{
> > +     unsigned long flags;
> > +
> > +     if (!tb[attrtype])
> > +             return;
> > +
> > +     if (nla_get_u8(tb[attrtype]))
> > +             flags = conf->flags | mask;
> > +     else
> > +             flags = conf->flags & ~mask;
> > +
> > +     conf->flags = flags;
> > +}
> > +
> >  static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> >                        struct net_device *dev, struct vxlan_config *conf,
> >                        bool changelink, struct netlink_ext_ack *extack)
> > @@ -3458,45 +3475,27 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> >       if (data[IFLA_VXLAN_TTL])
> >               conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> >
> > -     if (data[IFLA_VXLAN_TTL_INHERIT])
> > -             conf->flags |= VXLAN_F_TTL_INHERIT;
> > +     vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
> > +                   VXLAN_F_TTL_INHERIT);
>
> IFLA_VXLAN_TTL_INHERIT is a NLA_FLAG. It doesn't have any u8 data to
> get. Same for:
>
>         [IFLA_VXLAN_GBP]        = { .type = NLA_FLAG, },
>         [IFLA_VXLAN_GPE]        = { .type = NLA_FLAG, },
>         [IFLA_VXLAN_REMCSUM_NOPARTIAL]  = { .type = NLA_FLAG },
>         [IFLA_VXLAN_TTL_INHERIT]        = { .type = NLA_FLAG },
>
>

good catch. i see it, some flags are NLA_U8 and some NLA_FLAG. will fix.


>
> nit: This patch would have been easier to review if it came first in
> the series. Converting:

I considered that. But the first patch really removes some checks
making it easier for the follow-on patches...and that mostly dictated
the order.

>
>     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
>         conf->flags |= VXLAN_F_FOO;
>
> to this new helper, which in effect does
>
>     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
>         conf->flags |= VXLAN_F_FOO;
>     else
>         conf->flags &= ~VXLAN_F_FOO;
>
> seems to change behavior, but since you're (unless I missed one) only
> applying it to flags that couldn't be changed before patch 1, it looks
> fine.

It does not change behavior...the earlier code only supported the
flags during create time and did not support changing of the flag with
changelink.
this patch adds changelink support. But if you do see any change in
behavior, pls report. thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes
  2018-11-29 13:56   ` Sabrina Dubroca
@ 2018-11-29 15:33     ` Roopa Prabhu
  2018-11-29 15:51       ` Sabrina Dubroca
  0 siblings, 1 reply; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-29 15:33 UTC (permalink / raw)
  To: sd; +Cc: David Miller, netdev

On Thu, Nov 29, 2018 at 5:56 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2018-11-28, 14:27:57 -0800, Roopa Prabhu wrote:
> > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >
> > We started very conservative when supporting changelink
> > especially because not all attribute changes could be
> > tested. This patch opens up a few more attributes for
> > changelink. The reason for choosing this set of attributes
> > is based on code references for these attributes. I have
> > tested TTL changes and did some changelink api testing
> > to sanity test the others.
> >
> > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> > ---
> >  drivers/net/vxlan.c | 36 ++++--------------------------------
> >  1 file changed, 4 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index 9110662..73caa65 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -3438,11 +3438,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> >       if (data[IFLA_VXLAN_TTL])
> >               conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> >
> > -     if (data[IFLA_VXLAN_TTL_INHERIT]) {
> > -             if (changelink)
> > -                     return -EOPNOTSUPP;
> > +     if (data[IFLA_VXLAN_TTL_INHERIT])
> >               conf->flags |= VXLAN_F_TTL_INHERIT;
> > -     }
>
> This doesn't give us an option to disable TTL_INHERIT after it was
> enabled once. Same thing with GBP, GPE, REMCSUM_NOPARTIAL.

that is provided by patch3, with the changelink patch. I can squash
them, if thats easier.


>
> > -     if (data[IFLA_VXLAN_GBP]) {
> > -             if (changelink)
> > -                     return -EOPNOTSUPP;
> > +     if (data[IFLA_VXLAN_GBP])
> >               conf->flags |= VXLAN_F_GBP;
> > -     }
> >
> > -     if (data[IFLA_VXLAN_GPE]) {
> > -             if (changelink)
> > -                     return -EOPNOTSUPP;
> > +     if (data[IFLA_VXLAN_GPE])
> >               conf->flags |= VXLAN_F_GPE;
> > -     }
>
> GPE implies running a different setup function (vxlan_raw_setup() vs
> vxlan_ether_setup()), that vxlan_config_apply() only calls for
> !changelink. I think this is incomplete.
>
> I think we'd also end up with mixed tunnel types (GPE/!GPE) on the
> same socket, I'm not sure how that would work. Normally, they each try
> to create a separate socket, and pass the GPE flag on to the
> associated vxlan_sock. I suspect that's also a problem with rx
> offload.

that is good to know. I will drop the change to GPE and also the rx
offload flag and let somebody else using it
open it up for changelink. thanks for the review


> > -     if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
> > -             if (changelink)
> > -                     return -EOPNOTSUPP;
> > +     if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
> >               conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
> > -     }
> >
> >       if (tb[IFLA_MTU]) {
> >               if (changelink)
> > --

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes
  2018-11-29 15:33     ` Roopa Prabhu
@ 2018-11-29 15:51       ` Sabrina Dubroca
  0 siblings, 0 replies; 12+ messages in thread
From: Sabrina Dubroca @ 2018-11-29 15:51 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: David Miller, netdev

2018-11-29, 07:33:11 -0800, Roopa Prabhu wrote:
> On Thu, Nov 29, 2018 at 5:56 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
> >
> > 2018-11-28, 14:27:57 -0800, Roopa Prabhu wrote:
> > > From: Roopa Prabhu <roopa@cumulusnetworks.com>
> > >
> > > We started very conservative when supporting changelink
> > > especially because not all attribute changes could be
> > > tested. This patch opens up a few more attributes for
> > > changelink. The reason for choosing this set of attributes
> > > is based on code references for these attributes. I have
> > > tested TTL changes and did some changelink api testing
> > > to sanity test the others.
> > >
> > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> > > ---
> > >  drivers/net/vxlan.c | 36 ++++--------------------------------
> > >  1 file changed, 4 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > > index 9110662..73caa65 100644
> > > --- a/drivers/net/vxlan.c
> > > +++ b/drivers/net/vxlan.c
> > > @@ -3438,11 +3438,8 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
> > >       if (data[IFLA_VXLAN_TTL])
> > >               conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
> > >
> > > -     if (data[IFLA_VXLAN_TTL_INHERIT]) {
> > > -             if (changelink)
> > > -                     return -EOPNOTSUPP;
> > > +     if (data[IFLA_VXLAN_TTL_INHERIT])
> > >               conf->flags |= VXLAN_F_TTL_INHERIT;
> > > -     }
> >
> > This doesn't give us an option to disable TTL_INHERIT after it was
> > enabled once. Same thing with GBP, GPE, REMCSUM_NOPARTIAL.
> 
> that is provided by patch3, with the changelink patch. I can squash
> them, if thats easier.

Yes, I see it now. Sequential review got me :(
I think you can just add a note in the commit message, "will be fixed
in a further patch introducing the vxlan_nl2flag() helper" or similar.

> > > -     if (data[IFLA_VXLAN_GBP]) {
> > > -             if (changelink)
> > > -                     return -EOPNOTSUPP;
> > > +     if (data[IFLA_VXLAN_GBP])
> > >               conf->flags |= VXLAN_F_GBP;
> > > -     }
> > >
> > > -     if (data[IFLA_VXLAN_GPE]) {
> > > -             if (changelink)
> > > -                     return -EOPNOTSUPP;
> > > +     if (data[IFLA_VXLAN_GPE])
> > >               conf->flags |= VXLAN_F_GPE;
> > > -     }
> >
> > GPE implies running a different setup function (vxlan_raw_setup() vs
> > vxlan_ether_setup()), that vxlan_config_apply() only calls for
> > !changelink. I think this is incomplete.
> >
> > I think we'd also end up with mixed tunnel types (GPE/!GPE) on the
> > same socket, I'm not sure how that would work. Normally, they each try
> > to create a separate socket, and pass the GPE flag on to the
> > associated vxlan_sock. I suspect that's also a problem with rx
> > offload.
> 
> that is good to know. I will drop the change to GPE and also the rx
> offload flag and let somebody else using it
> open it up for changelink. thanks for the review

Looking at vxlan_rcv(), there's also a possible issue with the GBP
flag:

	if (vs->flags & VXLAN_F_GBP)
		vxlan_parse_gbp_hdr(&unparsed, skb, vs->flags, md);

I don't know why that flag must match on all tunnels for a socket, but
given that it was the reason for commit ac5132d1a03f ("vxlan: Only
bind to sockets with compatible flags enabled"), I'd leave it alone as
well.

> > > -     if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
> > > -             if (changelink)
> > > -                     return -EOPNOTSUPP;
> > > +     if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
> > >               conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
> > > -     }
> > >
> > >       if (tb[IFLA_MTU]) {
> > >               if (changelink)
> > > --

-- 
Sabrina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func
  2018-11-29 15:27     ` Roopa Prabhu
@ 2018-11-29 15:55       ` Sabrina Dubroca
  2018-11-29 16:05         ` Roopa Prabhu
  0 siblings, 1 reply; 12+ messages in thread
From: Sabrina Dubroca @ 2018-11-29 15:55 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: David Miller, netdev

2018-11-29, 07:27:17 -0800, Roopa Prabhu wrote:
> On Thu, Nov 29, 2018 at 6:19 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
> > 2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote:
> > nit: This patch would have been easier to review if it came first in
> > the series. Converting:
> 
> I considered that. But the first patch really removes some checks
> making it easier for the follow-on patches...and that mostly dictated
> the order.

ok.

> >     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
> >         conf->flags |= VXLAN_F_FOO;
> >
> > to this new helper, which in effect does
> >
> >     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
> >         conf->flags |= VXLAN_F_FOO;
> >     else
> >         conf->flags &= ~VXLAN_F_FOO;
> >
> > seems to change behavior, but since you're (unless I missed one) only
> > applying it to flags that couldn't be changed before patch 1, it looks
> > fine.
> 
> It does not change behavior...the earlier code only supported the
> flags during create time and did not support changing of the flag with
> changelink.
> this patch adds changelink support. But if you do see any change in
> behavior, pls report. thanks.

Yeah, looks correct. I just had to go back to patch 1 and check.
A note "only flags [list] are changed, and they could not be modified
after creation of the device prior to patch 1" in the cover and/or in
this patch's commit message would have spared reviewers a bit of a
scare :)

-- 
Sabrina

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func
  2018-11-29 15:55       ` Sabrina Dubroca
@ 2018-11-29 16:05         ` Roopa Prabhu
  0 siblings, 0 replies; 12+ messages in thread
From: Roopa Prabhu @ 2018-11-29 16:05 UTC (permalink / raw)
  To: sd; +Cc: David Miller, netdev

On Thu, Nov 29, 2018 at 7:55 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
>
> 2018-11-29, 07:27:17 -0800, Roopa Prabhu wrote:
> > On Thu, Nov 29, 2018 at 6:19 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
> > > 2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote:
> > > nit: This patch would have been easier to review if it came first in
> > > the series. Converting:
> >
> > I considered that. But the first patch really removes some checks
> > making it easier for the follow-on patches...and that mostly dictated
> > the order.
>
> ok.
>
> > >     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
> > >         conf->flags |= VXLAN_F_FOO;
> > >
> > > to this new helper, which in effect does
> > >
> > >     if (nla_get_u8(data[IFLA_VXLAN_FOO]))
> > >         conf->flags |= VXLAN_F_FOO;
> > >     else
> > >         conf->flags &= ~VXLAN_F_FOO;
> > >
> > > seems to change behavior, but since you're (unless I missed one) only
> > > applying it to flags that couldn't be changed before patch 1, it looks
> > > fine.
> >
> > It does not change behavior...the earlier code only supported the
> > flags during create time and did not support changing of the flag with
> > changelink.
> > this patch adds changelink support. But if you do see any change in
> > behavior, pls report. thanks.
>
> Yeah, looks correct. I just had to go back to patch 1 and check.
> A note "only flags [list] are changed, and they could not be modified
> after creation of the device prior to patch 1" in the cover and/or in
> this patch's commit message would have spared reviewers a bit of a
> scare :)

ack, sounds good. I am also going to reduce the scope of the series a
bit as i am only interested in support for TTL changelink right now.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases
  2018-11-28 22:27 ` [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases Roopa Prabhu
@ 2018-12-01  0:31   ` kbuild test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2018-12-01  0:31 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: kbuild-all, davem, netdev

[-- Attachment #1: Type: text/plain, Size: 8462 bytes --]

Hi Roopa,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Roopa-Prabhu/vxlan-a-few-minor-cleanups/20181130-184658
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/net/vxlan.c: In function 'vxlan_nl2conf':
>> drivers/net/vxlan.c:3527:3: error: expected '}' before 'else'
      else {
      ^~~~

vim +3527 drivers/net/vxlan.c

  3376	
  3377	static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
  3378				 struct net_device *dev, struct vxlan_config *conf,
  3379				 bool changelink, struct netlink_ext_ack *extack)
  3380	{
  3381		struct vxlan_dev *vxlan = netdev_priv(dev);
  3382	
  3383		memset(conf, 0, sizeof(*conf));
  3384	
  3385		/* if changelink operation, start with old existing cfg */
  3386		if (changelink)
  3387			memcpy(conf, &vxlan->cfg, sizeof(*conf));
  3388	
  3389		if (data[IFLA_VXLAN_ID]) {
  3390			__be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
  3391	
  3392			if (changelink && (vni != conf->vni)) {
  3393				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID],
  3394						    "Cannot change vni");
  3395				return -EOPNOTSUPP;
  3396			}
  3397			conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
  3398		}
  3399	
  3400		if (data[IFLA_VXLAN_GROUP]) {
  3401			if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) {
  3402				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP],
  3403						    "New group addr family does not match old group");
  3404				return -EOPNOTSUPP;
  3405			}
  3406			conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]);
  3407			conf->remote_ip.sa.sa_family = AF_INET;
  3408		} else if (data[IFLA_VXLAN_GROUP6]) {
  3409			if (!IS_ENABLED(CONFIG_IPV6)) {
  3410				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6],
  3411						    "IPv6 support not enabled in the kernel");
  3412				return -EPFNOSUPPORT;
  3413			}
  3414	
  3415			if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) {
  3416				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6],
  3417						    "New group addr family does not match old group");
  3418				return -EOPNOTSUPP;
  3419			}
  3420	
  3421			conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]);
  3422			conf->remote_ip.sa.sa_family = AF_INET6;
  3423		}
  3424	
  3425		if (data[IFLA_VXLAN_LOCAL]) {
  3426			if (changelink && (conf->saddr.sa.sa_family != AF_INET)) {
  3427				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL],
  3428						    "New local addr family does not match old");
  3429				return -EOPNOTSUPP;
  3430			}
  3431	
  3432			conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]);
  3433			conf->saddr.sa.sa_family = AF_INET;
  3434		} else if (data[IFLA_VXLAN_LOCAL6]) {
  3435			if (!IS_ENABLED(CONFIG_IPV6)) {
  3436				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6],
  3437						    "IPv6 support not enabled in the kernel");
  3438				return -EPFNOSUPPORT;
  3439			}
  3440	
  3441			if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) {
  3442				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6],
  3443						    "New local6 addr family does not match old");
  3444				return -EOPNOTSUPP;
  3445			}
  3446	
  3447			/* TODO: respect scope id */
  3448			conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
  3449			conf->saddr.sa.sa_family = AF_INET6;
  3450		}
  3451	
  3452		if (data[IFLA_VXLAN_LINK])
  3453			conf->remote_ifindex = nla_get_u32(data[IFLA_VXLAN_LINK]);
  3454	
  3455		if (data[IFLA_VXLAN_TOS])
  3456			conf->tos  = nla_get_u8(data[IFLA_VXLAN_TOS]);
  3457	
  3458		if (data[IFLA_VXLAN_TTL])
  3459			conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
  3460	
  3461		if (data[IFLA_VXLAN_TTL_INHERIT])
  3462			conf->flags |= VXLAN_F_TTL_INHERIT;
  3463	
  3464		if (data[IFLA_VXLAN_LABEL])
  3465			conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) &
  3466				     IPV6_FLOWLABEL_MASK;
  3467	
  3468		if (data[IFLA_VXLAN_LEARNING]) {
  3469			if (nla_get_u8(data[IFLA_VXLAN_LEARNING]))
  3470				conf->flags |= VXLAN_F_LEARN;
  3471			else
  3472				conf->flags &= ~VXLAN_F_LEARN;
  3473		} else if (!changelink) {
  3474			/* default to learn on a new device */
  3475			conf->flags |= VXLAN_F_LEARN;
  3476		}
  3477	
  3478		if (data[IFLA_VXLAN_AGEING])
  3479			conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
  3480	
  3481		if (data[IFLA_VXLAN_PROXY]) {
  3482			if (nla_get_u8(data[IFLA_VXLAN_PROXY]))
  3483				conf->flags |= VXLAN_F_PROXY;
  3484		}
  3485	
  3486		if (data[IFLA_VXLAN_RSC]) {
  3487			if (nla_get_u8(data[IFLA_VXLAN_RSC]))
  3488				conf->flags |= VXLAN_F_RSC;
  3489		}
  3490	
  3491		if (data[IFLA_VXLAN_L2MISS]) {
  3492			if (nla_get_u8(data[IFLA_VXLAN_L2MISS]))
  3493				conf->flags |= VXLAN_F_L2MISS;
  3494		}
  3495	
  3496		if (data[IFLA_VXLAN_L3MISS]) {
  3497			if (nla_get_u8(data[IFLA_VXLAN_L3MISS]))
  3498				conf->flags |= VXLAN_F_L3MISS;
  3499		}
  3500	
  3501		if (data[IFLA_VXLAN_LIMIT]) {
  3502			if (changelink) {
  3503				NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT],
  3504						    "Cannot change limit");
  3505				return -EOPNOTSUPP;
  3506			}
  3507			conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]);
  3508		}
  3509	
  3510		if (data[IFLA_VXLAN_COLLECT_METADATA]) {
  3511			if (changelink) {
  3512				NL_SET_ERR_MSG_ATTR(extack,
  3513						    tb[IFLA_VXLAN_COLLECT_METADATA],
  3514						    "Cannot change metadata flag");
  3515				return -EOPNOTSUPP;
  3516			}
  3517			if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA]))
  3518				conf->flags |= VXLAN_F_COLLECT_METADATA;
  3519		}
  3520	
  3521		if (data[IFLA_VXLAN_PORT_RANGE]) {
  3522			if (!changelink) {
  3523				const struct ifla_vxlan_port_range *p
  3524					= nla_data(data[IFLA_VXLAN_PORT_RANGE]);
  3525				conf->port_min = ntohs(p->low);
  3526				conf->port_max = ntohs(p->high);
> 3527			else {
  3528				NL_SET_ERR_MSG_ATTR(extack,
  3529						    tb[IFLA_VXLAN_PORT_RANGE],
  3530						    "Cannot change port range");
  3531				return -EOPNOTSUPP;
  3532			}
  3533		}
  3534	
  3535		if (data[IFLA_VXLAN_PORT]) {
  3536			if (changelink) {
  3537				NL_SET_ERR_MSG_ATTR(extack,
  3538						    tb[IFLA_VXLAN_PORT],
  3539						    "Cannot change port");
  3540				return -EOPNOTSUPP;
  3541			}
  3542			conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]);
  3543		}
  3544	
  3545		if (data[IFLA_VXLAN_UDP_CSUM]) {
  3546			if (changelink) {
  3547				NL_SET_ERR_MSG_ATTR(extack,
  3548						    tb[IFLA_VXLAN_UDP_CSUM],
  3549						    "Cannot change udp csum");
  3550				return -EOPNOTSUPP;
  3551			}
  3552			if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM]))
  3553				conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX;
  3554		}
  3555	
  3556		if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
  3557			if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]))
  3558				conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
  3559		}
  3560	
  3561		if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
  3562			if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
  3563				conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
  3564		}
  3565	
  3566		if (data[IFLA_VXLAN_REMCSUM_TX]) {
  3567			if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
  3568				conf->flags |= VXLAN_F_REMCSUM_TX;
  3569		}
  3570	
  3571		if (data[IFLA_VXLAN_REMCSUM_RX]) {
  3572			if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
  3573				conf->flags |= VXLAN_F_REMCSUM_RX;
  3574		}
  3575	
  3576		if (data[IFLA_VXLAN_GBP])
  3577			conf->flags |= VXLAN_F_GBP;
  3578	
  3579		if (data[IFLA_VXLAN_GPE])
  3580			conf->flags |= VXLAN_F_GPE;
  3581	
  3582		if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
  3583			conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
  3584	
  3585		if (tb[IFLA_MTU]) {
  3586			if (changelink) {
  3587				NL_SET_ERR_MSG_ATTR(extack,
  3588						    tb[IFLA_MTU],
  3589						    "Cannot change mtu");
  3590				return -EOPNOTSUPP;
  3591			}
  3592			conf->mtu = nla_get_u32(tb[IFLA_MTU]);
  3593		}
  3594	
  3595		if (data[IFLA_VXLAN_DF])
  3596			conf->df = nla_get_u8(data[IFLA_VXLAN_DF]);
  3597	
  3598		return 0;
  3599	}
  3600	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 66023 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-12-01 11:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 22:27 [PATCH net-next v2 0/3] vxlan: a few minor cleanups Roopa Prabhu
2018-11-28 22:27 ` [PATCH net-next v2 1/3] vxlan: support changelink for a few more attributes Roopa Prabhu
2018-11-29 13:56   ` Sabrina Dubroca
2018-11-29 15:33     ` Roopa Prabhu
2018-11-29 15:51       ` Sabrina Dubroca
2018-11-28 22:27 ` [PATCH net-next v2 2/3] vxlan: extack support for some changelink cases Roopa Prabhu
2018-12-01  0:31   ` kbuild test robot
2018-11-28 22:27 ` [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func Roopa Prabhu
2018-11-29 14:19   ` Sabrina Dubroca
2018-11-29 15:27     ` Roopa Prabhu
2018-11-29 15:55       ` Sabrina Dubroca
2018-11-29 16:05         ` Roopa Prabhu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).