RE: [PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.

2021-01-15 Thread Praveen Chaudhary
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.


Re: [PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.

2021-01-14 Thread David Ahern
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
> Signed-off-by: Zhenggen Xu
> 
> 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.
> + 0x 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 0x7FF:
> + +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
> --- 

[PATCH v1 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.

2021-01-12 Thread Praveen Chaudhary
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
Signed-off-by: Zhenggen Xu

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.
+   0x 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 0x7FF:
+   +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 
= {