All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v0 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement.
@ 2021-01-11 21:58 Praveen Chaudhary
  2021-01-11 21:58 ` [PATCH v0 net-next 1/1] " Praveen Chaudhary
  0 siblings, 1 reply; 6+ messages in thread
From: Praveen Chaudhary @ 2021-01-11 21:58 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                       | 12 +++++++++---
 net/ipv6/route.c                       |  5 +++--
 8 files changed, 45 insertions(+), 6 deletions(-)


base-commit: 139711f033f636cc78b6aaf7363252241b9698ef
-- 
2.29.0


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

* [PATCH v0 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-11 21:58 [PATCH v0 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement Praveen Chaudhary
@ 2021-01-11 21:58 ` Praveen Chaudhary
  2021-01-11 23:16   ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Praveen Chaudhary @ 2021-01-11 21:58 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>
---
 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                       | 12 +++++++++---
 net/ipv6/route.c                       |  5 +++--
 8 files changed, 45 insertions(+), 6 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..955dde8aad22 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,13 +1304,18 @@ 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");
 
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] 6+ messages in thread

* Re: [PATCH v0 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-11 21:58 ` [PATCH v0 net-next 1/1] " Praveen Chaudhary
@ 2021-01-11 23:16   ` Jakub Kicinski
  2021-01-12 19:55     ` Praveen Chaudhary
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2021-01-11 23:16 UTC (permalink / raw)
  To: Praveen Chaudhary
  Cc: davem, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel,
	Zhenggen Xu

On Mon, 11 Jan 2021 13:58:29 -0800 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.
> 
> Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
> Signed-off-by: Zhenggen Xu<zxu@linkedin.com>

Please put a space between the name and '<'.

I haven't looked at the code yet, but I can tell you this patch
triggers a few checkpatch --strict warnings, and breaks allmodconfig
build.

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

* RE: [PATCH v0 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-11 23:16   ` Jakub Kicinski
@ 2021-01-12 19:55     ` Praveen Chaudhary
  2021-01-12 23:43       ` Jakub Kicinski
  0 siblings, 1 reply; 6+ messages in thread
From: Praveen Chaudhary @ 2021-01-12 19:55 UTC (permalink / raw)
  To: davem, kuba, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel

Hi Jakub

Thanks for the review,

Sure, I will reraise the patch (again v0i, sonce no code changes) after adding space before '<'.

This patch adds lines in 'include/uapi/', that requires ABI version changes for debian build. I am not sure, if we need any such changes to avoid breaking allmodconfig. It will be really helpful, if you can look at the patch once 'https://lkml.org/lkml/2021/1/11/1668' and suggest on this. Thanks a lot again.

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

* Re: [PATCH v0 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-12 19:55     ` Praveen Chaudhary
@ 2021-01-12 23:43       ` Jakub Kicinski
  2021-01-13  2:14         ` Praveen Chaudhary
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Kicinski @ 2021-01-12 23:43 UTC (permalink / raw)
  To: Praveen Chaudhary
  Cc: davem, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel

On Tue, 12 Jan 2021 11:55:11 -0800 Praveen Chaudhary wrote:
> Hi Jakub
> 
> Thanks for the review,
> 
> Sure, I will reraise the patch (again v0i, sonce no code changes) after adding space before '<'.
> 
> This patch adds lines in 'include/uapi/', that requires ABI version changes for debian build. I am not sure, if we need any such changes to avoid breaking allmodconfig. It will be really helpful, if you can look at the patch once 'https://lkml.org/lkml/2021/1/11/1668' and suggest on this. Thanks a lot again.

The code doesn't build, AFAIK it's because:

net/ipv6/ndisc.c:1322:8: error: too few arguments to function ‘rt6_add_dflt_router’

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

* RE: [PATCH v0 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
  2021-01-12 23:43       ` Jakub Kicinski
@ 2021-01-13  2:14         ` Praveen Chaudhary
  0 siblings, 0 replies; 6+ messages in thread
From: Praveen Chaudhary @ 2021-01-13  2:14 UTC (permalink / raw)
  To: davem, kuba, corbet, kuznet, yoshfuji, netdev, linux-doc, linux-kernel

Thanks for the hint, Yeah I missed the call to rt6_add_dflt_router while 
applying patch to master branch.

I am developer of SONiC OS (https://azure.github.io/SONiC/) in LinkedIn. 
We are planning to move to IPv6 only network and I realise that IPv6 needs
capability to let administrator configure metric on default route 
learned via Router Advertisement in Linux. We support a fixed value 
1024 today in Linux.

Note for IPv4, administrator can configure metric on default route learned via
DHCPv4. 

Kindly Review the fix, this feature is useful for IPv6 and Thanks Again.

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

end of thread, other threads:[~2021-01-13  2:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-11 21:58 [PATCH v0 net-next 0/1] Allow user to set metric on default route learned via Router Advertisement Praveen Chaudhary
2021-01-11 21:58 ` [PATCH v0 net-next 1/1] " Praveen Chaudhary
2021-01-11 23:16   ` Jakub Kicinski
2021-01-12 19:55     ` Praveen Chaudhary
2021-01-12 23:43       ` Jakub Kicinski
2021-01-13  2:14         ` Praveen Chaudhary

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.