[PATCH 5/7 net-next] vxlan: add VXLAN_NL2FLAG macro

Subsystems: networking drivers, networking [general], the rest

STALE2204d

3 messages, 3 authors, 2020-08-27 · open the first message on its own page

[PATCH 5/7 net-next] vxlan: add VXLAN_NL2FLAG macro

From: Fabian Frederick <hidden>
Date: 2020-08-27 06:50:43 

Replace common flag assignment with a macro.
This could yet be simplified with changelink/supported but it would
remove clarity

Signed-off-by: Fabian Frederick <redacted>
---
 drivers/net/vxlan.c | 113 +++++---------------------------------------
 include/net/vxlan.h |  10 ++++
 2 files changed, 23 insertions(+), 100 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1e9ab1002281c..e9b561b9d23e1 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -4034,14 +4034,7 @@ 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]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
-				    VXLAN_F_TTL_INHERIT, changelink, false,
-				    extack);
-		if (err)
-			return err;
-
-	}
+	VXLAN_NL2FLAG(IFLA_VXLAN_TTL_INHERIT, VXLAN_F_TTL_INHERIT, changelink, false);
 
 	if (data[IFLA_VXLAN_LABEL])
 		conf->label = nla_get_be32(data[IFLA_VXLAN_LABEL]) &
@@ -4061,37 +4054,10 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 	if (data[IFLA_VXLAN_AGEING])
 		conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
 
-	if (data[IFLA_VXLAN_PROXY]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY,
-				    VXLAN_F_PROXY, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_RSC]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC,
-				    VXLAN_F_RSC, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_L2MISS]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS,
-				    VXLAN_F_L2MISS, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_L3MISS]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS,
-				    VXLAN_F_L3MISS, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
+	VXLAN_NL2FLAG(IFLA_VXLAN_PROXY, VXLAN_F_PROXY, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_RSC, VXLAN_F_RSC, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_L2MISS, VXLAN_F_L2MISS, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_L3MISS, VXLAN_F_L3MISS, changelink, false);
 
 	if (data[IFLA_VXLAN_LIMIT]) {
 		if (changelink) {
@@ -4102,13 +4068,7 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
 		conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]);
 	}
 
-	if (data[IFLA_VXLAN_COLLECT_METADATA]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA,
-				    VXLAN_F_COLLECT_METADATA, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
+	VXLAN_NL2FLAG(IFLA_VXLAN_COLLECT_METADATA, VXLAN_F_COLLECT_METADATA, changelink, false);
 
 	if (data[IFLA_VXLAN_PORT_RANGE]) {
 		if (!changelink) {
@@ -4142,60 +4102,13 @@ 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]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
-				    VXLAN_F_UDP_ZERO_CSUM6_TX, changelink,
-				    false, extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX,
-				    VXLAN_F_UDP_ZERO_CSUM6_RX, changelink,
-				    false, extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_REMCSUM_TX]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX,
-				    VXLAN_F_REMCSUM_TX, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_REMCSUM_RX]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX,
-				    VXLAN_F_REMCSUM_RX, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_GBP]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP,
-				    VXLAN_F_GBP, changelink, false, extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_GPE]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE,
-				    VXLAN_F_GPE, changelink, false,
-				    extack);
-		if (err)
-			return err;
-	}
-
-	if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
-		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
-				    VXLAN_F_REMCSUM_NOPARTIAL, changelink,
-				    false, extack);
-		if (err)
-			return err;
-	}
+	VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_TX, VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_UDP_ZERO_CSUM6_RX, VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_TX, IFLA_VXLAN_REMCSUM_TX, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_RX, VXLAN_F_REMCSUM_RX, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_GBP, VXLAN_F_GBP, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_GPE, VXLAN_F_GPE, changelink, false);
+	VXLAN_NL2FLAG(IFLA_VXLAN_REMCSUM_NOPARTIAL, VXLAN_F_REMCSUM_NOPARTIAL, changelink, false);
 
 	if (tb[IFLA_MTU]) {
 		if (changelink) {
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 3a41627cbdfe5..8a56b7a0f75f9 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -290,6 +290,16 @@ struct vxlan_dev {
 					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
 					 VXLAN_F_COLLECT_METADATA)
 
+
+#define VXLAN_NL2FLAG(iflag, flag, changelink, changelink_supported) {   \
+	if (data[iflag]) {						 \
+		err = vxlan_nl2flag(conf, data, iflag, flag, changelink, \
+				    changelink_supported, extack);       \
+		if (err)						 \
+			return err;					 \
+	}								 \
+}
+
 struct net_device *vxlan_dev_create(struct net *net, const char *name,
 				    u8 name_assign_type, struct vxlan_config *conf);
 
-- 
2.27.0

Re: [PATCH 5/7 net-next] vxlan: add VXLAN_NL2FLAG macro

From: Michal Kubecek <hidden>
Date: 2020-08-27 09:50:29 

On Thu, Aug 27, 2020 at 08:50:19AM +0200, Fabian Frederick wrote:
Replace common flag assignment with a macro.
This could yet be simplified with changelink/supported but it would
remove clarity

Signed-off-by: Fabian Frederick <redacted>
---
[...]
quoted hunk
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 3a41627cbdfe5..8a56b7a0f75f9 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -290,6 +290,16 @@ struct vxlan_dev {
 					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
 					 VXLAN_F_COLLECT_METADATA)
 
+
+#define VXLAN_NL2FLAG(iflag, flag, changelink, changelink_supported) {   \
+	if (data[iflag]) {						 \
+		err = vxlan_nl2flag(conf, data, iflag, flag, changelink, \
+				    changelink_supported, extack);       \
+		if (err)						 \
+			return err;					 \
+	}								 \
+}
+
Hiding a goto or return in a macro is generally discouraged as it may
confuse people reading or updating the code. See e.g. commit
94f826b8076e ("net: fix a potential rcu_read_lock() imbalance in
rt6_fill_node()") for an example of such problem - which was likely the
trigger for removal of NLA_PUT() and related macros shortly after.

Michal

 struct net_device *vxlan_dev_create(struct net *net, const char *name,
 				    u8 name_assign_type, struct vxlan_config *conf);
 
-- 
2.27.0

Re: [PATCH 5/7 net-next] vxlan: add VXLAN_NL2FLAG macro

From: David Miller <davem@davemloft.net>
Date: 2020-08-27 14:29:53 

From: Michal Kubecek <redacted>
Date: Thu, 27 Aug 2020 11:50:25 +0200

On Thu, Aug 27, 2020 at 08:50:19AM +0200, Fabian Frederick wrote:
quoted
Replace common flag assignment with a macro.
This could yet be simplified with changelink/supported but it would
remove clarity

Signed-off-by: Fabian Frederick <redacted>
---
[...]
quoted
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 3a41627cbdfe5..8a56b7a0f75f9 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -290,6 +290,16 @@ struct vxlan_dev {
 					 VXLAN_F_UDP_ZERO_CSUM6_RX |	\
 					 VXLAN_F_COLLECT_METADATA)
 
+
+#define VXLAN_NL2FLAG(iflag, flag, changelink, changelink_supported) {   \
+	if (data[iflag]) {						 \
+		err = vxlan_nl2flag(conf, data, iflag, flag, changelink, \
+				    changelink_supported, extack);       \
+		if (err)						 \
+			return err;					 \
+	}								 \
+}
+
Hiding a goto or return in a macro is generally discouraged as it may
confuse people reading or updating the code.
Agreed, please don't do this.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help