linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement.
@ 2021-01-13  1:50 Praveen Chaudhary
  2021-01-13  1:50 ` [PATCH v1 net-next 1/1] " Praveen Chaudhary
  0 siblings, 1 reply; 4+ messages in thread
From: Praveen Chaudhary @ 2021-01-13  1:50 UTC (permalink / raw)
  To: davem, kuba, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel

Allow user to set metric on default route learned via Router Advertisement.

Note: RFC 4191 does not say anything for metric for IPv6 default route.

Fix:
For IPv4, default route is learned via DHCPv4 and user is allowed to change
metric using config in etc/network/interfaces. But for IPv6, default route can
be learned via RA, for which, currently a fixed metric value 1024 is used.

Ideally, user should be able to configure metric on default route for IPv6
similar to IPv4. This fix adds sysctl for the same.

Logs:
----------------------------------------------------------------
For IPv4:
----------------------------------------------------------------

Config in etc/network/interfaces
----------------------------------------------------------------
```
auto eth0
iface eth0 inet dhcp
    metric 4261413864
```

IPv4 Kernel Route Table:
----------------------------------------------------------------
```
$ sudo route -n
Kernel IP routing table
Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
0.0.0.0         172.11.44.1     0.0.0.0         UG    -33553432 0        0 eth0
```

FRR Table, if a static route is configured. [In real scenario, it is useful to prefer BGP learned default route over DHCPv4 default route.]
----------------------------------------------------------------
```
Codes: K - kernel route, C - connected, S - static, R - RIP,
       O - OSPF, I - IS-IS, B - BGP, P - PIM, E - EIGRP, N - NHRP,
       T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
       > - selected route, * - FIB route

S>* 0.0.0.0/0 [20/0] is directly connected, eth0, 00:00:03
K   0.0.0.0/0 [254/1000] via 172.21.47.1, eth0, 6d08h51m
```

----------------------------------------------------------------
i.e. User can prefer Default Router learned via Routing Protocol,
Similar behavior is not possible for IPv6, without this fix.


----------------------------------------------------------------
After fix [for IPv6]:
----------------------------------------------------------------
```
sudo sysctl -w net.ipv6.conf.eth0.net.ipv6.conf.eth0.accept_ra_defrtr_metric=0x770003e9
```

IP monitor:
----------------------------------------------------------------
```
default via fe80::xx16:xxxx:feb3:ce8e dev eth0 proto ra metric 1996489705  pref high
```

Kernel IPv6 routing table
----------------------------------------------------------------
```
Destination                    Next Hop                   Flag Met Ref Use If
::/0                           fe80::xx16:xxxx:feb3:ce8e  UGDAe 1996489705 0
 0 eth0
```

FRR Table, if a static route is configured. [In real scenario, it is useful to prefer BGP learned default route over IPv6 RA default route.]
```
----------------------------------------------------------------
Codes: K - kernel route, C - connected, S - static, R - RIPng,
       O - OSPFv3, I - IS-IS, B - BGP, N - NHRP, T - Table,
       v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
       > - selected route, * - FIB route

S>* ::/0 [20/0] is directly connected, eth0, 00:00:06
K   ::/0 [119/1001] via fe80::xx16:xxxx:feb3:ce8e, eth0, 6d07h43m
----------------------------------------------------------------
```

If the metric is changed later, the effect will be seen only when IPv6 RA is received, because the default route must be fully controlled by RA msg.
```
admin@lnos-x1-a-asw03:~$ sudo sysctl -w net.ipv6.conf.eth0.accept_ra_defrtr_metric=0x770003e8
net.ipv6.conf.eth0.accept_ra_defrtr_metric = 0x770003e8

```

IP monitor: when metric is changed after learning Default Route from previous IPv6 RA msg:
```
Deleted default via fe80::xx16:xxxx:feb3:ce8e dev eth0 proto ra metric 1996489705  expires 3sec hoplimit 64 pref high
default via fe80::xx16:xxxx:feb3:ce8e dev eth0 proto ra metric 1996489704  pref high
```

Praveen Chaudhary (1):
  Allow user to set metric on default route learned via Router
    Advertisement.

 Documentation/networking/ip-sysctl.rst | 18 ++++++++++++++++++
 include/linux/ipv6.h                   |  1 +
 include/net/ip6_route.h                |  3 ++-
 include/uapi/linux/ipv6.h              |  1 +
 include/uapi/linux/sysctl.h            |  1 +
 net/ipv6/addrconf.c                    | 10 ++++++++++
 net/ipv6/ndisc.c                       | 14 ++++++++++----
 net/ipv6/route.c                       |  5 +++--
 8 files changed, 46 insertions(+), 7 deletions(-)


base-commit: 139711f033f636cc78b6aaf7363252241b9698ef
-- 
2.29.0


^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-13  1:50 [PATCH v1 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement Praveen Chaudhary
@ 2021-01-13  1:50 ` Praveen Chaudhary
  2021-01-15  2:27   ` David Ahern
  0 siblings, 1 reply; 4+ messages in thread
From: Praveen Chaudhary @ 2021-01-13  1:50 UTC (permalink / raw)
  To: davem, kuba, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel
  Cc: Zhenggen Xu

For IPv4, default route is learned via DHCPv4 and user is allowed to change
metric using config etc/network/interfaces. But for IPv6, default route can
be learned via RA, for which, currently a fixed metric value 1024 is used.

Ideally, user should be able to configure metric on default route for IPv6
similar to IPv4. This fix adds sysctl for the same.

Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
Signed-off-by: Zhenggen Xu<zxu@linkedin.com>

Changes in v1.
---
1.) Correct the call to rt6_add_dflt_router.
---

---
 Documentation/networking/ip-sysctl.rst | 18 ++++++++++++++++++
 include/linux/ipv6.h                   |  1 +
 include/net/ip6_route.h                |  3 ++-
 include/uapi/linux/ipv6.h              |  1 +
 include/uapi/linux/sysctl.h            |  1 +
 net/ipv6/addrconf.c                    | 10 ++++++++++
 net/ipv6/ndisc.c                       | 14 ++++++++++----
 net/ipv6/route.c                       |  5 +++--
 8 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
index dd2b12a32b73..384159081d91 100644
--- a/Documentation/networking/ip-sysctl.rst
+++ b/Documentation/networking/ip-sysctl.rst
@@ -1871,6 +1871,24 @@ accept_ra_defrtr - BOOLEAN
 		- enabled if accept_ra is enabled.
 		- disabled if accept_ra is disabled.
 
+accept_ra_defrtr_metric - INTEGER
+	Route metric for default route learned in Router Advertisement. This
+	value will be assigned as metric for the route learned via IPv6 Router
+	Advertisement.
+
+	Possible values are:
+		0:
+			Use default value i.e. IP6_RT_PRIO_USER	1024.
+		0xFFFFFFFF to -1:
+			-ve values represent high route metric, value will be treated as
+			unsigned value. This behaviour is inline with current IPv4 metric
+			shown with commands such as "route -n" or "ip route list".
+		1 to 0x7FFFFFF:
+			+ve values will be used as is for route metric.
+
+	Functional default: enabled if accept_ra_defrtr is enabled.
+				disabled if accept_ra_defrtr is disabled.
+
 accept_ra_from_local - BOOLEAN
 	Accept RA with source-address that is found on local machine
 	if the RA is otherwise proper and able to be accepted.
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index dda61d150a13..19af90c77200 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -31,6 +31,7 @@ struct ipv6_devconf {
 	__s32		max_desync_factor;
 	__s32		max_addresses;
 	__s32		accept_ra_defrtr;
+	__s32		accept_ra_defrtr_metric;
 	__s32		accept_ra_min_hop_limit;
 	__s32		accept_ra_pinfo;
 	__s32		ignore_routes_with_linkdown;
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 2a5277758379..a470bdab2420 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -174,7 +174,8 @@ struct fib6_info *rt6_get_dflt_router(struct net *net,
 				     struct net_device *dev);
 struct fib6_info *rt6_add_dflt_router(struct net *net,
 				     const struct in6_addr *gwaddr,
-				     struct net_device *dev, unsigned int pref);
+				     struct net_device *dev, unsigned int pref,
+				     unsigned int defrtr_usr_metric);
 
 void rt6_purge_dflt_routers(struct net *net);
 
diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
index 13e8751bf24a..945de5de5144 100644
--- a/include/uapi/linux/ipv6.h
+++ b/include/uapi/linux/ipv6.h
@@ -189,6 +189,7 @@ enum {
 	DEVCONF_ACCEPT_RA_RT_INFO_MIN_PLEN,
 	DEVCONF_NDISC_TCLASS,
 	DEVCONF_RPL_SEG_ENABLED,
+	DEVCONF_ACCEPT_RA_DEFRTR_METRIC,
 	DEVCONF_MAX
 };
 
diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
index 458179df9b27..5e79c196e33c 100644
--- a/include/uapi/linux/sysctl.h
+++ b/include/uapi/linux/sysctl.h
@@ -571,6 +571,7 @@ enum {
 	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
 	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
 	NET_IPV6_ACCEPT_RA_RT_INFO_MIN_PLEN=27,
+	NET_IPV6_ACCEPT_RA_DEFRTR_METRIC=28,
 	__NET_IPV6_MAX
 };
 
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index eff2cacd5209..702ec4a33936 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -205,6 +205,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
+	.accept_ra_defrtr_metric = 0,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_pinfo	= 1,
@@ -260,6 +261,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
 	.max_desync_factor	= MAX_DESYNC_FACTOR,
 	.max_addresses		= IPV6_MAX_ADDRESSES,
 	.accept_ra_defrtr	= 1,
+	.accept_ra_defrtr_metric = 0,
 	.accept_ra_from_local	= 0,
 	.accept_ra_min_hop_limit= 1,
 	.accept_ra_pinfo	= 1,
@@ -5475,6 +5477,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
 	array[DEVCONF_MAX_DESYNC_FACTOR] = cnf->max_desync_factor;
 	array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
 	array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
+	array[DEVCONF_ACCEPT_RA_DEFRTR_METRIC] = cnf->accept_ra_defrtr_metric;
 	array[DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT] = cnf->accept_ra_min_hop_limit;
 	array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
 #ifdef CONFIG_IPV6_ROUTER_PREF
@@ -6667,6 +6670,13 @@ static const struct ctl_table addrconf_sysctl[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec,
 	},
+	{
+		.procname	= "accept_ra_defrtr_metric",
+		.data		= &ipv6_devconf.accept_ra_defrtr_metric,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
 	{
 		.procname	= "accept_ra_min_hop_limit",
 		.data		= &ipv6_devconf.accept_ra_min_hop_limit,
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 76717478f173..96ab202e95f9 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1180,6 +1180,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 	unsigned int pref = 0;
 	__u32 old_if_flags;
 	bool send_ifinfo_notify = false;
+	unsigned int defrtr_usr_metric = 0;
 
 	__u8 *opt = (__u8 *)(ra_msg + 1);
 
@@ -1303,18 +1304,23 @@ static void ndisc_router_discovery(struct sk_buff *skb)
 			return;
 		}
 	}
-	if (rt && lifetime == 0) {
+	/* Set default route metric if specified by user */
+	defrtr_usr_metric = in6_dev->cnf.accept_ra_defrtr_metric;
+	if (defrtr_usr_metric == 0)
+		defrtr_usr_metric = IP6_RT_PRIO_USER;
+	/* delete the route if lifetime is 0 or if metric needs change */
+	if (rt && ((lifetime == 0) || (rt->fib6_metric != defrtr_usr_metric)))  {
 		ip6_del_rt(net, rt, false);
 		rt = NULL;
 	}
 
-	ND_PRINTK(3, info, "RA: rt: %p  lifetime: %d, for dev: %s\n",
-		  rt, lifetime, skb->dev->name);
+	ND_PRINTK(3, info, "RA: rt: %p  lifetime: %d, metric: %d, for dev: %s\n",
+		  rt, lifetime, defrtr_usr_metric, skb->dev->name);
 	if (!rt && lifetime) {
 		ND_PRINTK(3, info, "RA: adding default router\n");
 
 		rt = rt6_add_dflt_router(net, &ipv6_hdr(skb)->saddr,
-					 skb->dev, pref);
+					 skb->dev, pref, defrtr_usr_metric);
 		if (!rt) {
 			ND_PRINTK(0, err,
 				  "RA: %s failed to add default route\n",
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 188e114b29b4..5f177ae97e42 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4252,11 +4252,12 @@ struct fib6_info *rt6_get_dflt_router(struct net *net,
 struct fib6_info *rt6_add_dflt_router(struct net *net,
 				     const struct in6_addr *gwaddr,
 				     struct net_device *dev,
-				     unsigned int pref)
+				     unsigned int pref,
+				     unsigned int defrtr_usr_metric)
 {
 	struct fib6_config cfg = {
 		.fc_table	= l3mdev_fib_table(dev) ? : RT6_TABLE_DFLT,
-		.fc_metric	= IP6_RT_PRIO_USER,
+		.fc_metric	= defrtr_usr_metric ? defrtr_usr_metric : IP6_RT_PRIO_USER,
 		.fc_ifindex	= dev->ifindex,
 		.fc_flags	= RTF_GATEWAY | RTF_ADDRCONF | RTF_DEFAULT |
 				  RTF_UP | RTF_EXPIRES | RTF_PREF(pref),
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-13  1:50 ` [PATCH v1 net-next 1/1] " Praveen Chaudhary
@ 2021-01-15  2:27   ` David Ahern
  2021-01-15  8:08     ` Praveen Chaudhary
  0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2021-01-15  2:27 UTC (permalink / raw)
  To: Praveen Chaudhary, davem, kuba, corbet, kuznet, yoshfuji, netdev,
	linux-doc, linux-kernel
  Cc: Zhenggen Xu

On 1/12/21 6:50 PM, Praveen Chaudhary wrote:
> For IPv4, default route is learned via DHCPv4 and user is allowed to change
> metric using config etc/network/interfaces. But for IPv6, default route can
> be learned via RA, for which, currently a fixed metric value 1024 is used.
> 
> Ideally, user should be able to configure metric on default route for IPv6
> similar to IPv4. This fix adds sysctl for the same.

This is a single patch set, so the details you have in the cover letter
should be in this description here. Also, please just 'ip' commands in
the patch description; 'route' command is a dinosaur that needs to be
retired.

> 
> Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
> Signed-off-by: Zhenggen Xu<zxu@linkedin.com>
> 
> Changes in v1.
> ---
> 1.) Correct the call to rt6_add_dflt_router.
> ---
> 
> ---
>  Documentation/networking/ip-sysctl.rst | 18 ++++++++++++++++++
>  include/linux/ipv6.h                   |  1 +
>  include/net/ip6_route.h                |  3 ++-
>  include/uapi/linux/ipv6.h              |  1 +
>  include/uapi/linux/sysctl.h            |  1 +
>  net/ipv6/addrconf.c                    | 10 ++++++++++
>  net/ipv6/ndisc.c                       | 14 ++++++++++----
>  net/ipv6/route.c                       |  5 +++--
>  8 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index dd2b12a32b73..384159081d91 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -1871,6 +1871,24 @@ accept_ra_defrtr - BOOLEAN
>  		- enabled if accept_ra is enabled.
>  		- disabled if accept_ra is disabled.
>  
> +accept_ra_defrtr_metric - INTEGER

Drop the 'accept' part; ra_defrtr_metric is sufficiently long. Since the
value is not from the RA, it is not really about accepting data from the RA.

> +	Route metric for default route learned in Router Advertisement. This
> +	value will be assigned as metric for the route learned via IPv6 Router
> +	Advertisement.
> +
> +	Possible values are:
> +		0:
> +			Use default value i.e. IP6_RT_PRIO_USER	1024.
> +		0xFFFFFFFF to -1:
> +			-ve values represent high route metric, value will be treated as
> +			unsigned value. This behaviour is inline with current IPv4 metric
> +			shown with commands such as "route -n" or "ip route list".
> +		1 to 0x7FFFFFF:
> +			+ve values will be used as is for route metric.

route metric is a u32, so these ranges should not be needed. 'ip route
list' shows metric as a positive number only.


> +
> +	Functional default: enabled if accept_ra_defrtr is enabled.
> +				disabled if accept_ra_defrtr is disabled.

Alignment problem, but I think this can be moved above to the
description and changed to something like 'only takes affect if
accept_ra_defrtr' is enabled.

> +
>  accept_ra_from_local - BOOLEAN
>  	Accept RA with source-address that is found on local machine
>  	if the RA is otherwise proper and able to be accepted.
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index dda61d150a13..19af90c77200 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -31,6 +31,7 @@ struct ipv6_devconf {
>  	__s32		max_desync_factor;
>  	__s32		max_addresses;
>  	__s32		accept_ra_defrtr;
> +	__s32		accept_ra_defrtr_metric;

__u32 and drop the 'accept_' prefix.


>  	__s32		accept_ra_min_hop_limit;
>  	__s32		accept_ra_pinfo;
>  	__s32		ignore_routes_with_linkdown;
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 2a5277758379..a470bdab2420 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -174,7 +174,8 @@ struct fib6_info *rt6_get_dflt_router(struct net *net,
>  				     struct net_device *dev);
>  struct fib6_info *rt6_add_dflt_router(struct net *net,
>  				     const struct in6_addr *gwaddr,
> -				     struct net_device *dev, unsigned int pref);
> +				     struct net_device *dev, unsigned int pref,
> +				     unsigned int defrtr_usr_metric);

u32 for consistency

>  
>  void rt6_purge_dflt_routers(struct net *net);
>  
> diff --git a/include/uapi/linux/ipv6.h b/include/uapi/linux/ipv6.h
> index 13e8751bf24a..945de5de5144 100644
> --- a/include/uapi/linux/ipv6.h
> +++ b/include/uapi/linux/ipv6.h
> @@ -189,6 +189,7 @@ enum {
>  	DEVCONF_ACCEPT_RA_RT_INFO_MIN_PLEN,
>  	DEVCONF_NDISC_TCLASS,
>  	DEVCONF_RPL_SEG_ENABLED,
> +	DEVCONF_ACCEPT_RA_DEFRTR_METRIC,

Drop 'ACCEPT_'

>  	DEVCONF_MAX
>  };
>  
> diff --git a/include/uapi/linux/sysctl.h b/include/uapi/linux/sysctl.h
> index 458179df9b27..5e79c196e33c 100644
> --- a/include/uapi/linux/sysctl.h
> +++ b/include/uapi/linux/sysctl.h
> @@ -571,6 +571,7 @@ enum {
>  	NET_IPV6_ACCEPT_SOURCE_ROUTE=25,
>  	NET_IPV6_ACCEPT_RA_FROM_LOCAL=26,
>  	NET_IPV6_ACCEPT_RA_RT_INFO_MIN_PLEN=27,
> +	NET_IPV6_ACCEPT_RA_DEFRTR_METRIC=28,

Drop 'ACCEPT_'

>  	__NET_IPV6_MAX
>  };
>  
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index eff2cacd5209..702ec4a33936 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -205,6 +205,7 @@ static struct ipv6_devconf ipv6_devconf __read_mostly = {
>  	.max_desync_factor	= MAX_DESYNC_FACTOR,
>  	.max_addresses		= IPV6_MAX_ADDRESSES,
>  	.accept_ra_defrtr	= 1,
> +	.accept_ra_defrtr_metric = 0,
>  	.accept_ra_from_local	= 0,
>  	.accept_ra_min_hop_limit= 1,
>  	.accept_ra_pinfo	= 1,
> @@ -260,6 +261,7 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
>  	.max_desync_factor	= MAX_DESYNC_FACTOR,
>  	.max_addresses		= IPV6_MAX_ADDRESSES,
>  	.accept_ra_defrtr	= 1,
> +	.accept_ra_defrtr_metric = 0,
>  	.accept_ra_from_local	= 0,
>  	.accept_ra_min_hop_limit= 1,
>  	.accept_ra_pinfo	= 1,
> @@ -5475,6 +5477,7 @@ static inline void ipv6_store_devconf(struct ipv6_devconf *cnf,
>  	array[DEVCONF_MAX_DESYNC_FACTOR] = cnf->max_desync_factor;
>  	array[DEVCONF_MAX_ADDRESSES] = cnf->max_addresses;
>  	array[DEVCONF_ACCEPT_RA_DEFRTR] = cnf->accept_ra_defrtr;
> +	array[DEVCONF_ACCEPT_RA_DEFRTR_METRIC] = cnf->accept_ra_defrtr_metric;
>  	array[DEVCONF_ACCEPT_RA_MIN_HOP_LIMIT] = cnf->accept_ra_min_hop_limit;
>  	array[DEVCONF_ACCEPT_RA_PINFO] = cnf->accept_ra_pinfo;
>  #ifdef CONFIG_IPV6_ROUTER_PREF
> @@ -6667,6 +6670,13 @@ static const struct ctl_table addrconf_sysctl[] = {
>  		.mode		= 0644,
>  		.proc_handler	= proc_dointvec,
>  	},
> +	{
> +		.procname	= "accept_ra_defrtr_metric",
> +		.data		= &ipv6_devconf.accept_ra_defrtr_metric,
> +		.maxlen		= sizeof(int),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec,

proc_douintvec


> +	},
>  	{
>  		.procname	= "accept_ra_min_hop_limit",
>  		.data		= &ipv6_devconf.accept_ra_min_hop_limit,
> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index 76717478f173..96ab202e95f9 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c
> @@ -1180,6 +1180,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
>  	unsigned int pref = 0;
>  	__u32 old_if_flags;
>  	bool send_ifinfo_notify = false;
> +	unsigned int defrtr_usr_metric = 0;

net coding style is reverse xmas tree - ie., longest to shortest. yes,
this function is old school, but new entries can be added in the right
place. Since you set before use, the initialization to 0 is not
necessary. And u32 for consistency.

>  
>  	__u8 *opt = (__u8 *)(ra_msg + 1);
>  
> @@ -1303,18 +1304,23 @@ static void ndisc_router_discovery(struct sk_buff *skb)
>  			return;
>  		}
>  	}
> -	if (rt && lifetime == 0) {
> +	/* Set default route metric if specified by user */
> +	defrtr_usr_metric = in6_dev->cnf.accept_ra_defrtr_metric;
> +	if (defrtr_usr_metric == 0)
> +		defrtr_usr_metric = IP6_RT_PRIO_USER;
> +	/* delete the route if lifetime is 0 or if metric needs change */
> +	if (rt && ((lifetime == 0) || (rt->fib6_metric != defrtr_usr_metric)))  {
>  		ip6_del_rt(net, rt, false);
>  		rt = NULL;
>  	}
>  
> -	ND_PRINTK(3, info, "RA: rt: %p  lifetime: %d, for dev: %s\n",
> -		  rt, lifetime, skb->dev->name);
> +	ND_PRINTK(3, info, "RA: rt: %p  lifetime: %d, metric: %d, for dev: %s\n",
> +		  rt, lifetime, defrtr_usr_metric, skb->dev->name);
>  	if (!rt && lifetime) {
>  		ND_PRINTK(3, info, "RA: adding default router\n");
>  
>  		rt = rt6_add_dflt_router(net, &ipv6_hdr(skb)->saddr,
> -					 skb->dev, pref);
> +					 skb->dev, pref, defrtr_usr_metric);
>  		if (!rt) {
>  			ND_PRINTK(0, err,
>  				  "RA: %s failed to add default route\n",
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 188e114b29b4..5f177ae97e42 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4252,11 +4252,12 @@ struct fib6_info *rt6_get_dflt_router(struct net *net,
>  struct fib6_info *rt6_add_dflt_router(struct net *net,
>  				     const struct in6_addr *gwaddr,
>  				     struct net_device *dev,
> -				     unsigned int pref)
> +				     unsigned int pref,
> +				     unsigned int defrtr_usr_metric)

u32

>  {
>  	struct fib6_config cfg = {
>  		.fc_table	= l3mdev_fib_table(dev) ? : RT6_TABLE_DFLT,
> -		.fc_metric	= IP6_RT_PRIO_USER,
> +		.fc_metric	= defrtr_usr_metric ? defrtr_usr_metric : IP6_RT_PRIO_USER,

you can abbreviate that as:
		defrtr_usr_metric ? : IP6_RT_PRIO_USER,

>  		.fc_ifindex	= dev->ifindex,
>  		.fc_flags	= RTF_GATEWAY | RTF_ADDRCONF | RTF_DEFAULT |
>  				  RTF_UP | RTF_EXPIRES | RTF_PREF(pref),
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread
* RE: [PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-15  2:27   ` David Ahern
@ 2021-01-15  8:08     ` Praveen Chaudhary
  0 siblings, 0 replies; 4+ messages in thread
From: Praveen Chaudhary @ 2021-01-15  8:08 UTC (permalink / raw)
  To: davem, kuba, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel

Hi David

Thanks a lot for Review,

I have raised v2 after addressing your review comments from
https://lkml.org/lkml/2021/1/14/1400.

List of changes in v2:
---
1.) Replace accept_ra_defrtr_metric with ra_defrtr_metric.
2.) Change Type to __u32 instead of __s32.
3.) Change description in Documentation/networking/ip-sysctl.rst.
4.) Use proc_douintvec instead of proc_dointvec.
5.) Code style in ndisc_router_discovery().
6.) Change Type to u32 instead of unsigned int.
---

Thanks a lot again for help.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-15  8:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13  1:50 [PATCH v1 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement Praveen Chaudhary
2021-01-13  1:50 ` [PATCH v1 net-next 1/1] " Praveen Chaudhary
2021-01-15  2:27   ` David Ahern
2021-01-15  8:08     ` Praveen Chaudhary

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