Skip to contentSkip to conversation

Added NLM_F_EXCL support to fib_nl_newrule

PATCH Superseded Mateusz Bajorski 2 participants 2 messages May 20, 2016 – May 23, 2016

Subsystems (3)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Added NLM_F_EXCL support to fib_nl_newrule

When adding rule with NLM_F_EXCL flag then check if the same rule exist. If yes then exit with -EEXIST.

This is already implemented in iproute2:

	if (cmd == RTM_NEWRULE) {
		req.n.nlmsg_flags |= NLM_F_CREATE|NLM_F_EXCL;
		req.r.rtm_type = RTN_UNICAST;
	}

Tested ipv4 and ipv6 with net-next linux on qemu x86

expected behavior after patch:
localhost ~ # ip rule
0: from all lookup local
32766: from all lookup main
32767: from all lookup default
localhost ~ # ip rule add from 10.46.177.97 lookup 104 pref 1005 localhost ~ # ip rule add from 10.46.177.97 lookup 104 pref 1005 RTNETLINK answers: File exists
localhost ~ # ip rule
0: from all lookup local
1005: from 10.46.177.97 lookup 104
32766: from all lookup main
32767: from all lookup default

There was already topic regarding this but I don't see any changes merged and problem still occurs.
https://lkml.kernel.org/r/1135778809.5944.7.camel+%28%29+localhost+%21+localdomain

---
 net/core/fib_rules.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
net/core/fib_rules.c +41 -0
index 840aceb..b9816a3 100644
@@ -291,6 +291,47 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	if (err < 0)
 		goto errout;
 
+	if (nlh->nlmsg_flags & NLM_F_EXCL) {
+		list_for_each_entry(rule, &ops->rules_list, list) {
+			if (frh->action && (frh->action != rule->action))
+				continue;
+
+			if (frh_get_table(frh, tb) &&
+				frh_get_table(frh, tb) != rule->table)
+				continue;
+
+			if (tb[FRA_PRIORITY] &&
+				rule->pref != nla_get_u32(tb[FRA_PRIORITY]))
+				continue;
+
+			if (tb[FRA_IIFNAME] &&
+				nla_strcmp(tb[FRA_IIFNAME], rule->iifname))
+				continue;
+
+			if (tb[FRA_OIFNAME] &&
+				nla_strcmp(tb[FRA_OIFNAME], rule->oifname))
+				continue;
+
+			if (tb[FRA_FWMARK] &&
+				rule->mark != nla_get_u32(tb[FRA_FWMARK]))
+				continue;
+
+			if (tb[FRA_FWMASK] &&
+				rule->mark_mask != nla_get_u32(tb[FRA_FWMASK]))
+				continue;
+
+			if (tb[FRA_TUN_ID] &&
+				rule->tun_id != nla_get_be64(tb[FRA_TUN_ID]))
+				continue;
+
+			if (!ops->compare(rule, frh, tb))
+				continue;
+
+			err = -EEXIST;
+			goto errout;
+		}
+	}
+
 	rule = kzalloc(ops->rule_size, GFP_KERNEL);
 	if (rule == NULL) {
 		err = -ENOMEM;
Signature · 1 line
2.6.4
maintainer
#1 lore raw

From: Mateusz Bajorski <[email protected]>
Date: Fri, 20 May 2016 17:58:23 +0200

Please format your Subject line properly, it should be of the form:

[PATCH] $SUBSYSTEM: Description.

In this case "fib_fules: " would be an appropriate subsystem specification.

Quoted 6 lines
net/core/fib_rules.c +0 -0
index 840aceb..b9816a3 100644
@@ -291,6 +291,47 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	if (err < 0)

net/core/fib_rules.c static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) · line 301

Quoted 10 lines
net/core/fib_rules.c +8 -0
 		goto errout;
 
+	if (nlh->nlmsg_flags & NLM_F_EXCL) {
+		list_for_each_entry(rule, &ops->rules_list, list) {
+			if (frh->action && (frh->action != rule->action))
+				continue;
+
+			if (frh_get_table(frh, tb) &&
+				frh_get_table(frh, tb) != rule->table)
+				continue;

First of all, this is not indented properly.

When a conditional, or function call, spans multiple lines, the second and subsequent lines must be indented precisely to the first column after the openning parenthesis of the first line. You must use the appropriate number of TAB and SPACE characters necessary to do so.

Second of all, this is so messy having to check the 'tb' pointers over and over again each iteration of the loop.

It's therefore much better to put this check later in the function long after we've built the whole new rule, right before we do the priority ordering loop.