netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next PATCH] net: ipv6: use common fib_default_rule_pref
@ 2015-09-02 13:03 Phil Sutter
  2015-09-02 18:34 ` David Miller
  2015-09-03 11:10 ` [net-next PATCH] net: kill default_pref field of struct fib_rules_ops Phil Sutter
  0 siblings, 2 replies; 12+ messages in thread
From: Phil Sutter @ 2015-09-02 13:03 UTC (permalink / raw)
  To: netdev; +Cc: tgraf, davem

This switches IPv6 policy routing to use the shared
fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
multicast routing for IPv4 as well as IPv6.

The motivation for this patch is a complaint about iproute2 behaving
inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
assigned priority value was decreased with each rule added.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 net/ipv6/fib6_rules.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 2367a16..a859ad2 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -258,11 +258,6 @@ nla_put_failure:
 	return -ENOBUFS;
 }
 
-static u32 fib6_rule_default_pref(struct fib_rules_ops *ops)
-{
-	return 0x3FFF;
-}
-
 static size_t fib6_rule_nlmsg_payload(struct fib_rule *rule)
 {
 	return nla_total_size(16) /* dst */
@@ -279,7 +274,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = {
 	.configure		= fib6_rule_configure,
 	.compare		= fib6_rule_compare,
 	.fill			= fib6_rule_fill,
-	.default_pref		= fib6_rule_default_pref,
+	.default_pref		= fib_default_rule_pref,
 	.nlmsg_payload		= fib6_rule_nlmsg_payload,
 	.nlgroup		= RTNLGRP_IPV6_RULE,
 	.policy			= fib6_rule_policy,
-- 
2.1.2

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

* Re: [net-next PATCH] net: ipv6: use common fib_default_rule_pref
  2015-09-02 13:03 [net-next PATCH] net: ipv6: use common fib_default_rule_pref Phil Sutter
@ 2015-09-02 18:34 ` David Miller
  2015-09-02 18:46   ` Thomas Graf
  2015-09-03 11:10 ` [net-next PATCH] net: kill default_pref field of struct fib_rules_ops Phil Sutter
  1 sibling, 1 reply; 12+ messages in thread
From: David Miller @ 2015-09-02 18:34 UTC (permalink / raw)
  To: phil; +Cc: netdev, tgraf

From: Phil Sutter <phil@nwl.cc>
Date: Wed,  2 Sep 2015 15:03:12 +0200

> This switches IPv6 policy routing to use the shared
> fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
> multicast routing for IPv4 as well as IPv6.
> 
> The motivation for this patch is a complaint about iproute2 behaving
> inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
> IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
> assigned priority value was decreased with each rule added.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

All ->default_pref() methods are therefore going to be set to the
default, so just kill off the method entirely and call
fib_default_rule_pref() directly.

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

* Re: [net-next PATCH] net: ipv6: use common fib_default_rule_pref
  2015-09-02 18:34 ` David Miller
@ 2015-09-02 18:46   ` Thomas Graf
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Graf @ 2015-09-02 18:46 UTC (permalink / raw)
  To: David Miller; +Cc: phil, netdev

On 09/02/15 at 11:34am, David Miller wrote:
> From: Phil Sutter <phil@nwl.cc>
> Date: Wed,  2 Sep 2015 15:03:12 +0200
> 
> > This switches IPv6 policy routing to use the shared
> > fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
> > multicast routing for IPv4 as well as IPv6.
> > 
> > The motivation for this patch is a complaint about iproute2 behaving
> > inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
> > IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
> > assigned priority value was decreased with each rule added.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> All ->default_pref() methods are therefore going to be set to the
> default, so just kill off the method entirely and call
> fib_default_rule_pref() directly.

How strict are we with regard to compatibility here? New IPv6 rules
with no pref specified currently get appended at the end of the list
whereas this would start inserting at the head.

I'm absolutely in favour of the new behaviour but this could break
scripts which do not have proper prefs specified.

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

* [net-next PATCH] net: kill default_pref field of struct fib_rules_ops
  2015-09-02 13:03 [net-next PATCH] net: ipv6: use common fib_default_rule_pref Phil Sutter
  2015-09-02 18:34 ` David Miller
@ 2015-09-03 11:10 ` Phil Sutter
  2015-09-06  5:00   ` David Miller
  1 sibling, 1 reply; 12+ messages in thread
From: Phil Sutter @ 2015-09-03 11:10 UTC (permalink / raw)
  To: netdev; +Cc: tgraf, davem

Since now all users of that field have been converted to use the generic
function fib_default_rule_pref() when assigning to it, fib_nl_newrule()
may just use it directly instead.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/net/fib_rules.h |  1 -
 net/core/fib_rules.c    | 10 +++-------
 net/decnet/dn_rules.c   |  1 -
 net/ipv4/fib_rules.c    |  1 -
 net/ipv4/ipmr.c         |  1 -
 net/ipv6/fib6_rules.c   |  1 -
 net/ipv6/ip6mr.c        |  1 -
 7 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 4e8f804..75cda93 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -66,7 +66,6 @@ struct fib_rules_ops {
 					   struct nlattr **);
 	int			(*fill)(struct fib_rule *, struct sk_buff *,
 					struct fib_rule_hdr *);
-	u32			(*default_pref)(struct fib_rules_ops *ops);
 	size_t			(*nlmsg_payload)(struct fib_rule *);
 
 	/* Called after modifications to the rules set, must flush
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index ae8306e..bf77e36 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -44,7 +44,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
 }
 EXPORT_SYMBOL(fib_default_rule_add);
 
-u32 fib_default_rule_pref(struct fib_rules_ops *ops)
+static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 {
 	struct list_head *pos;
 	struct fib_rule *rule;
@@ -60,7 +60,6 @@ u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 
 	return 0;
 }
-EXPORT_SYMBOL(fib_default_rule_pref);
 
 static void notify_rule_change(int event, struct fib_rule *rule,
 			       struct fib_rules_ops *ops, struct nlmsghdr *nlh,
@@ -299,8 +298,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	}
 	rule->fr_net = net;
 
-	if (tb[FRA_PRIORITY])
-		rule->pref = nla_get_u32(tb[FRA_PRIORITY]);
+	rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
+	                              : fib_default_rule_pref(ops);
 
 	if (tb[FRA_IIFNAME]) {
 		struct net_device *dev;
@@ -350,9 +349,6 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	else
 		rule->suppress_ifgroup = -1;
 
-	if (!tb[FRA_PRIORITY] && ops->default_pref)
-		rule->pref = ops->default_pref(ops);
-
 	err = -EINVAL;
 	if (tb[FRA_GOTO]) {
 		if (rule->action != FR_ACT_GOTO)
diff --git a/net/decnet/dn_rules.c b/net/decnet/dn_rules.c
index 9d66a0f..295bbd6 100644
--- a/net/decnet/dn_rules.c
+++ b/net/decnet/dn_rules.c
@@ -229,7 +229,6 @@ static const struct fib_rules_ops __net_initconst dn_fib_rules_ops_template = {
 	.configure	= dn_fib_rule_configure,
 	.compare	= dn_fib_rule_compare,
 	.fill		= dn_fib_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.flush_cache	= dn_fib_rule_flush_cache,
 	.nlgroup	= RTNLGRP_DECnet_RULE,
 	.policy		= dn_fib_rule_policy,
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 18123d5..f2bda9e 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -318,7 +318,6 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = {
 	.delete		= fib4_rule_delete,
 	.compare	= fib4_rule_compare,
 	.fill		= fib4_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.nlmsg_payload	= fib4_rule_nlmsg_payload,
 	.flush_cache	= fib4_rule_flush_cache,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3a2c016..866ee89 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -233,7 +233,6 @@ static const struct fib_rules_ops __net_initconst ipmr_rules_ops_template = {
 	.match		= ipmr_rule_match,
 	.configure	= ipmr_rule_configure,
 	.compare	= ipmr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ipmr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
 	.policy		= ipmr_rule_policy,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index a859ad2..9f777ec 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -274,7 +274,6 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = {
 	.configure		= fib6_rule_configure,
 	.compare		= fib6_rule_compare,
 	.fill			= fib6_rule_fill,
-	.default_pref		= fib_default_rule_pref,
 	.nlmsg_payload		= fib6_rule_nlmsg_payload,
 	.nlgroup		= RTNLGRP_IPV6_RULE,
 	.policy			= fib6_rule_policy,
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 74ceb73..774c95e 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -217,7 +217,6 @@ static const struct fib_rules_ops __net_initconst ip6mr_rules_ops_template = {
 	.match		= ip6mr_rule_match,
 	.configure	= ip6mr_rule_configure,
 	.compare	= ip6mr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ip6mr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV6_RULE,
 	.policy		= ip6mr_rule_policy,
-- 
2.1.2

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

* Re: [net-next PATCH] net: kill default_pref field of struct fib_rules_ops
  2015-09-03 11:10 ` [net-next PATCH] net: kill default_pref field of struct fib_rules_ops Phil Sutter
@ 2015-09-06  5:00   ` David Miller
  2015-09-06  9:54     ` Phil Sutter
  2015-09-06 10:20     ` [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref Phil Sutter
  0 siblings, 2 replies; 12+ messages in thread
From: David Miller @ 2015-09-06  5:00 UTC (permalink / raw)
  To: phil; +Cc: netdev, tgraf

From: Phil Sutter <phil@nwl.cc>
Date: Thu,  3 Sep 2015 13:10:22 +0200

> Since now all users of that field have been converted to use the generic
> function fib_default_rule_pref() when assigning to it, fib_nl_newrule()
> may just use it directly instead.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

I did not instruct you to submit this change assuming I had applied
your ipv6 one.  Which I did not and will not do.

Instead, I want you to do it all in one go.

Thanks.

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

* Re: [net-next PATCH] net: kill default_pref field of struct fib_rules_ops
  2015-09-06  5:00   ` David Miller
@ 2015-09-06  9:54     ` Phil Sutter
  2015-09-06 10:20     ` [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref Phil Sutter
  1 sibling, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2015-09-06  9:54 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, tgraf

On Sat, Sep 05, 2015 at 10:00:40PM -0700, David Miller wrote:
> From: Phil Sutter <phil@nwl.cc>
> Date: Thu,  3 Sep 2015 13:10:22 +0200
> 
> > Since now all users of that field have been converted to use the generic
> > function fib_default_rule_pref() when assigning to it, fib_nl_newrule()
> > may just use it directly instead.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> I did not instruct you to submit this change assuming I had applied
> your ipv6 one.  Which I did not and will not do.
> 
> Instead, I want you to do it all in one go.

Ah. No biggie, I'll fold them together then. Just found it useful to
separate the logically unrelated changes in different commits.

Cheers, Phil

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

* [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref
  2015-09-06  5:00   ` David Miller
  2015-09-06  9:54     ` Phil Sutter
@ 2015-09-06 10:20     ` Phil Sutter
  2015-09-09  5:37       ` David Miller
  1 sibling, 1 reply; 12+ messages in thread
From: Phil Sutter @ 2015-09-06 10:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, tgraf

This switches IPv6 policy routing to use the shared
fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
multicast routing for IPv4 as well as IPv6.

The motivation for this patch is a complaint about iproute2 behaving
inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
assigned priority value was decreased with each rule added.

Since then all users of the default_pref field have been converted to
assign the generic function fib_default_rule_pref(), fib_nl_newrule()
may just use it directly instead. Therefore get rid of the function
pointer altogether and make fib_default_rule_pref() static, as it's not
used outside fib_rules.c anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Folded together with API change and adjusted commit message accordingly.
---
 include/net/fib_rules.h |  1 -
 net/core/fib_rules.c    | 10 +++-------
 net/decnet/dn_rules.c   |  1 -
 net/ipv4/fib_rules.c    |  1 -
 net/ipv4/ipmr.c         |  1 -
 net/ipv6/fib6_rules.c   |  6 ------
 net/ipv6/ip6mr.c        |  1 -
 7 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 4e8f804..75cda93 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -66,7 +66,6 @@ struct fib_rules_ops {
 					   struct nlattr **);
 	int			(*fill)(struct fib_rule *, struct sk_buff *,
 					struct fib_rule_hdr *);
-	u32			(*default_pref)(struct fib_rules_ops *ops);
 	size_t			(*nlmsg_payload)(struct fib_rule *);
 
 	/* Called after modifications to the rules set, must flush
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index ae8306e..bf77e36 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -44,7 +44,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
 }
 EXPORT_SYMBOL(fib_default_rule_add);
 
-u32 fib_default_rule_pref(struct fib_rules_ops *ops)
+static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 {
 	struct list_head *pos;
 	struct fib_rule *rule;
@@ -60,7 +60,6 @@ u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 
 	return 0;
 }
-EXPORT_SYMBOL(fib_default_rule_pref);
 
 static void notify_rule_change(int event, struct fib_rule *rule,
 			       struct fib_rules_ops *ops, struct nlmsghdr *nlh,
@@ -299,8 +298,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	}
 	rule->fr_net = net;
 
-	if (tb[FRA_PRIORITY])
-		rule->pref = nla_get_u32(tb[FRA_PRIORITY]);
+	rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
+	                              : fib_default_rule_pref(ops);
 
 	if (tb[FRA_IIFNAME]) {
 		struct net_device *dev;
@@ -350,9 +349,6 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	else
 		rule->suppress_ifgroup = -1;
 
-	if (!tb[FRA_PRIORITY] && ops->default_pref)
-		rule->pref = ops->default_pref(ops);
-
 	err = -EINVAL;
 	if (tb[FRA_GOTO]) {
 		if (rule->action != FR_ACT_GOTO)
diff --git a/net/decnet/dn_rules.c b/net/decnet/dn_rules.c
index 9d66a0f..295bbd6 100644
--- a/net/decnet/dn_rules.c
+++ b/net/decnet/dn_rules.c
@@ -229,7 +229,6 @@ static const struct fib_rules_ops __net_initconst dn_fib_rules_ops_template = {
 	.configure	= dn_fib_rule_configure,
 	.compare	= dn_fib_rule_compare,
 	.fill		= dn_fib_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.flush_cache	= dn_fib_rule_flush_cache,
 	.nlgroup	= RTNLGRP_DECnet_RULE,
 	.policy		= dn_fib_rule_policy,
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 18123d5..f2bda9e 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -318,7 +318,6 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = {
 	.delete		= fib4_rule_delete,
 	.compare	= fib4_rule_compare,
 	.fill		= fib4_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.nlmsg_payload	= fib4_rule_nlmsg_payload,
 	.flush_cache	= fib4_rule_flush_cache,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3a2c016..866ee89 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -233,7 +233,6 @@ static const struct fib_rules_ops __net_initconst ipmr_rules_ops_template = {
 	.match		= ipmr_rule_match,
 	.configure	= ipmr_rule_configure,
 	.compare	= ipmr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ipmr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
 	.policy		= ipmr_rule_policy,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 2367a16..9f777ec 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -258,11 +258,6 @@ nla_put_failure:
 	return -ENOBUFS;
 }
 
-static u32 fib6_rule_default_pref(struct fib_rules_ops *ops)
-{
-	return 0x3FFF;
-}
-
 static size_t fib6_rule_nlmsg_payload(struct fib_rule *rule)
 {
 	return nla_total_size(16) /* dst */
@@ -279,7 +274,6 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = {
 	.configure		= fib6_rule_configure,
 	.compare		= fib6_rule_compare,
 	.fill			= fib6_rule_fill,
-	.default_pref		= fib6_rule_default_pref,
 	.nlmsg_payload		= fib6_rule_nlmsg_payload,
 	.nlgroup		= RTNLGRP_IPV6_RULE,
 	.policy			= fib6_rule_policy,
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 74ceb73..774c95e 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -217,7 +217,6 @@ static const struct fib_rules_ops __net_initconst ip6mr_rules_ops_template = {
 	.match		= ip6mr_rule_match,
 	.configure	= ip6mr_rule_configure,
 	.compare	= ip6mr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ip6mr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV6_RULE,
 	.policy		= ip6mr_rule_policy,
-- 
2.1.2

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

* Re: [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref
  2015-09-06 10:20     ` [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref Phil Sutter
@ 2015-09-09  5:37       ` David Miller
  2015-09-09  5:38         ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2015-09-09  5:37 UTC (permalink / raw)
  To: phil; +Cc: netdev, tgraf

From: Phil Sutter <phil@nwl.cc>
Date: Sun,  6 Sep 2015 12:20:58 +0200

> This switches IPv6 policy routing to use the shared
> fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
> multicast routing for IPv4 as well as IPv6.
> 
> The motivation for this patch is a complaint about iproute2 behaving
> inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
> IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
> assigned priority value was decreased with each rule added.
> 
> Since then all users of the default_pref field have been converted to
> assign the generic function fib_default_rule_pref(), fib_nl_newrule()
> may just use it directly instead. Therefore get rid of the function
> pointer altogether and make fib_default_rule_pref() static, as it's not
> used outside fib_rules.c anymore.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Folded together with API change and adjusted commit message accordingly.

Applied, thanks.

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

* Re: [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref
  2015-09-09  5:37       ` David Miller
@ 2015-09-09  5:38         ` David Miller
  2015-09-09 11:45           ` Phil Sutter
  0 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2015-09-09  5:38 UTC (permalink / raw)
  To: phil; +Cc: netdev, tgraf

From: David Miller <davem@davemloft.net>
Date: Tue, 08 Sep 2015 22:37:05 -0700 (PDT)

> Applied, thanks.

Actually, reverted, this doesn't even compile.

net/core/fib_rules.c:47:12: error: static declaration of ‘fib_default_rule_pref’ follows non-static declaration
 static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
            ^
In file included from net/core/fib_rules.c:18:0:
include/net/fib_rules.h:120:5: note: previous declaration of ‘fib_default_rule_pref’ was here
 u32 fib_default_rule_pref(struct fib_rules_ops *ops);
     ^

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

* Re: [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref
  2015-09-09  5:38         ` David Miller
@ 2015-09-09 11:45           ` Phil Sutter
  2015-09-09 12:20             ` [net-next PATCH v3] " Phil Sutter
  0 siblings, 1 reply; 12+ messages in thread
From: Phil Sutter @ 2015-09-09 11:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, tgraf

On Tue, Sep 08, 2015 at 10:38:09PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 08 Sep 2015 22:37:05 -0700 (PDT)
> 
> > Applied, thanks.
> 
> Actually, reverted, this doesn't even compile.
> 
> net/core/fib_rules.c:47:12: error: static declaration of ‘fib_default_rule_pref’ follows non-static declaration
>  static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
>             ^
> In file included from net/core/fib_rules.c:18:0:
> include/net/fib_rules.h:120:5: note: previous declaration of ‘fib_default_rule_pref’ was here
>  u32 fib_default_rule_pref(struct fib_rules_ops *ops);
>      ^

What the heck? I should throw away the optimizer, it seems to make me
skip testing sometimes. Sorry for the mess, I'll follow up with a fixed
and *tested* version.

Cheers, Phil

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

* [net-next PATCH v3] net: ipv6: use common fib_default_rule_pref
  2015-09-09 11:45           ` Phil Sutter
@ 2015-09-09 12:20             ` Phil Sutter
  2015-09-09 21:22               ` David Miller
  0 siblings, 1 reply; 12+ messages in thread
From: Phil Sutter @ 2015-09-09 12:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, tgraf

This switches IPv6 policy routing to use the shared
fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
multicast routing for IPv4 as well as IPv6.

The motivation for this patch is a complaint about iproute2 behaving
inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
assigned priority value was decreased with each rule added.

Since then all users of the default_pref field have been converted to
assign the generic function fib_default_rule_pref(), fib_nl_newrule()
may just use it directly instead. Therefore get rid of the function
pointer altogether and make fib_default_rule_pref() static, as it's not
used outside fib_rules.c anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Folded together with API change and adjusted commit message accordingly.

Changes since v2:
- Dropped leftover function declaration in fib_rules.h
- Tested for IPv4 and IPv6 to correctly apply default prefs while still
  respecting user-defined preference.
---
 include/net/fib_rules.h |  2 --
 net/core/fib_rules.c    | 10 +++-------
 net/decnet/dn_rules.c   |  1 -
 net/ipv4/fib_rules.c    |  1 -
 net/ipv4/ipmr.c         |  1 -
 net/ipv6/fib6_rules.c   |  6 ------
 net/ipv6/ip6mr.c        |  1 -
 7 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 4e8f804..59160de 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -66,7 +66,6 @@ struct fib_rules_ops {
 					   struct nlattr **);
 	int			(*fill)(struct fib_rule *, struct sk_buff *,
 					struct fib_rule_hdr *);
-	u32			(*default_pref)(struct fib_rules_ops *ops);
 	size_t			(*nlmsg_payload)(struct fib_rule *);
 
 	/* Called after modifications to the rules set, must flush
@@ -118,5 +117,4 @@ int fib_rules_lookup(struct fib_rules_ops *, struct flowi *, int flags,
 		     struct fib_lookup_arg *);
 int fib_default_rule_add(struct fib_rules_ops *, u32 pref, u32 table,
 			 u32 flags);
-u32 fib_default_rule_pref(struct fib_rules_ops *ops);
 #endif
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index ae8306e..bf77e36 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -44,7 +44,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
 }
 EXPORT_SYMBOL(fib_default_rule_add);
 
-u32 fib_default_rule_pref(struct fib_rules_ops *ops)
+static u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 {
 	struct list_head *pos;
 	struct fib_rule *rule;
@@ -60,7 +60,6 @@ u32 fib_default_rule_pref(struct fib_rules_ops *ops)
 
 	return 0;
 }
-EXPORT_SYMBOL(fib_default_rule_pref);
 
 static void notify_rule_change(int event, struct fib_rule *rule,
 			       struct fib_rules_ops *ops, struct nlmsghdr *nlh,
@@ -299,8 +298,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	}
 	rule->fr_net = net;
 
-	if (tb[FRA_PRIORITY])
-		rule->pref = nla_get_u32(tb[FRA_PRIORITY]);
+	rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
+	                              : fib_default_rule_pref(ops);
 
 	if (tb[FRA_IIFNAME]) {
 		struct net_device *dev;
@@ -350,9 +349,6 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh)
 	else
 		rule->suppress_ifgroup = -1;
 
-	if (!tb[FRA_PRIORITY] && ops->default_pref)
-		rule->pref = ops->default_pref(ops);
-
 	err = -EINVAL;
 	if (tb[FRA_GOTO]) {
 		if (rule->action != FR_ACT_GOTO)
diff --git a/net/decnet/dn_rules.c b/net/decnet/dn_rules.c
index 9d66a0f..295bbd6 100644
--- a/net/decnet/dn_rules.c
+++ b/net/decnet/dn_rules.c
@@ -229,7 +229,6 @@ static const struct fib_rules_ops __net_initconst dn_fib_rules_ops_template = {
 	.configure	= dn_fib_rule_configure,
 	.compare	= dn_fib_rule_compare,
 	.fill		= dn_fib_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.flush_cache	= dn_fib_rule_flush_cache,
 	.nlgroup	= RTNLGRP_DECnet_RULE,
 	.policy		= dn_fib_rule_policy,
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index 18123d5..f2bda9e 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -318,7 +318,6 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = {
 	.delete		= fib4_rule_delete,
 	.compare	= fib4_rule_compare,
 	.fill		= fib4_rule_fill,
-	.default_pref	= fib_default_rule_pref,
 	.nlmsg_payload	= fib4_rule_nlmsg_payload,
 	.flush_cache	= fib4_rule_flush_cache,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 3a2c016..866ee89 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -233,7 +233,6 @@ static const struct fib_rules_ops __net_initconst ipmr_rules_ops_template = {
 	.match		= ipmr_rule_match,
 	.configure	= ipmr_rule_configure,
 	.compare	= ipmr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ipmr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV4_RULE,
 	.policy		= ipmr_rule_policy,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index 2367a16..9f777ec 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -258,11 +258,6 @@ nla_put_failure:
 	return -ENOBUFS;
 }
 
-static u32 fib6_rule_default_pref(struct fib_rules_ops *ops)
-{
-	return 0x3FFF;
-}
-
 static size_t fib6_rule_nlmsg_payload(struct fib_rule *rule)
 {
 	return nla_total_size(16) /* dst */
@@ -279,7 +274,6 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = {
 	.configure		= fib6_rule_configure,
 	.compare		= fib6_rule_compare,
 	.fill			= fib6_rule_fill,
-	.default_pref		= fib6_rule_default_pref,
 	.nlmsg_payload		= fib6_rule_nlmsg_payload,
 	.nlgroup		= RTNLGRP_IPV6_RULE,
 	.policy			= fib6_rule_policy,
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 74ceb73..774c95e 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -217,7 +217,6 @@ static const struct fib_rules_ops __net_initconst ip6mr_rules_ops_template = {
 	.match		= ip6mr_rule_match,
 	.configure	= ip6mr_rule_configure,
 	.compare	= ip6mr_rule_compare,
-	.default_pref	= fib_default_rule_pref,
 	.fill		= ip6mr_rule_fill,
 	.nlgroup	= RTNLGRP_IPV6_RULE,
 	.policy		= ip6mr_rule_policy,
-- 
2.1.2

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

* Re: [net-next PATCH v3] net: ipv6: use common fib_default_rule_pref
  2015-09-09 12:20             ` [net-next PATCH v3] " Phil Sutter
@ 2015-09-09 21:22               ` David Miller
  0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2015-09-09 21:22 UTC (permalink / raw)
  To: phil; +Cc: netdev, tgraf

From: Phil Sutter <phil@nwl.cc>
Date: Wed,  9 Sep 2015 14:20:56 +0200

> This switches IPv6 policy routing to use the shared
> fib_default_rule_pref() function of IPv4 and DECnet. It is also used in
> multicast routing for IPv4 as well as IPv6.
> 
> The motivation for this patch is a complaint about iproute2 behaving
> inconsistent between IPv4 and IPv6 when adding policy rules: Formerly,
> IPv6 rules were assigned a fixed priority of 0x3FFF whereas for IPv4 the
> assigned priority value was decreased with each rule added.
> 
> Since then all users of the default_pref field have been converted to
> assign the generic function fib_default_rule_pref(), fib_nl_newrule()
> may just use it directly instead. Therefore get rid of the function
> pointer altogether and make fib_default_rule_pref() static, as it's not
> used outside fib_rules.c anymore.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

This looks a lot better, applied, thanks.

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

end of thread, other threads:[~2015-09-09 21:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-02 13:03 [net-next PATCH] net: ipv6: use common fib_default_rule_pref Phil Sutter
2015-09-02 18:34 ` David Miller
2015-09-02 18:46   ` Thomas Graf
2015-09-03 11:10 ` [net-next PATCH] net: kill default_pref field of struct fib_rules_ops Phil Sutter
2015-09-06  5:00   ` David Miller
2015-09-06  9:54     ` Phil Sutter
2015-09-06 10:20     ` [net-next PATCH v2] net: ipv6: use common fib_default_rule_pref Phil Sutter
2015-09-09  5:37       ` David Miller
2015-09-09  5:38         ` David Miller
2015-09-09 11:45           ` Phil Sutter
2015-09-09 12:20             ` [net-next PATCH v3] " Phil Sutter
2015-09-09 21:22               ` David Miller

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).