| Message ID | 32de887afdc7d6851e7c53d27a21f1389bb0bd0f.1624604535.git.antony.antony@secunet.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] ipv6: Add sysctl for RA default route table number | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: idosch@OSS.NVIDIA.COM fw@strlen.de amcohen@nvidia.com edumazet@google.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 5415 this patch: 5415 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 5475 this patch: 5475 |
| netdev/header_inline | success | Link |
On 6/25/21 1:04 AM, Antony Antony wrote: > From: Christian Perle <christian.perle@secunet.com> > > Default routes learned from router advertisements(RA) are always placed > in main routing table. For policy based routing setups one may > want a different table for default routes. This commit adds a sysctl > to make table number for RA default routes configurable. > > examples: > sysctl net.ipv6.route.defrtr_table > sysctl -w net.ipv6.route.defrtr_table=42 > ip -6 route show table 42 How are the routing tables managed? If the netdevs are connected to a VRF this just works.
Hi David, On Fri, Jun 25, 2021 at 22:47:41 -0600, David Ahern wrote: > On 6/25/21 1:04 AM, Antony Antony wrote: > > From: Christian Perle <christian.perle@secunet.com> > > > > Default routes learned from router advertisements(RA) are always placed > > in main routing table. For policy based routing setups one may > > want a different table for default routes. This commit adds a sysctl > > to make table number for RA default routes configurable. > > > > examples: > > sysctl net.ipv6.route.defrtr_table > > sysctl -w net.ipv6.route.defrtr_table=42 > > ip -6 route show table 42 > > How are the routing tables managed? If the netdevs are connected to a > VRF this just works. The main routing table has no default route. Our scripts add routing rules based on interfaces. These rules use the specific routing table where the RA (when using SLAAC) installs the default route. The rest just works. I noticed an improvement the patch I sent. I will sent a rebased v2 after the net-next tree is open. I guess the net-next is closed now? - .proc_handler = proc_dointvec, + .proc_handler = proc_douintvec, -antony
Hi Antony, On 29/06/2021 15:53, Antony Antony wrote: > Hi David, > > On Fri, Jun 25, 2021 at 22:47:41 -0600, David Ahern wrote: >> On 6/25/21 1:04 AM, Antony Antony wrote: >>> From: Christian Perle <christian.perle@secunet.com> >>> >>> Default routes learned from router advertisements(RA) are always placed >>> in main routing table. For policy based routing setups one may >>> want a different table for default routes. This commit adds a sysctl >>> to make table number for RA default routes configurable. >>> >>> examples: >>> sysctl net.ipv6.route.defrtr_table >>> sysctl -w net.ipv6.route.defrtr_table=42 >>> ip -6 route show table 42 >> >> How are the routing tables managed? If the netdevs are connected to a >> VRF this just works. > > The main routing table has no default route. Our scripts add routing rules > based on interfaces. These rules use the specific routing table where the RA > (when using SLAAC) installs the default route. The rest just works. Could this be a devconf property instead of a global property? seems like the difference would be minor to your patch but the benefit is that setups using different routing tables for different policies could benefit (as mentioned when not using vrfs). Eyal.
On 6/29/21 8:05 AM, Eyal Birger wrote: > Hi Antony, > > On 29/06/2021 15:53, Antony Antony wrote: >> Hi David, >> >> On Fri, Jun 25, 2021 at 22:47:41 -0600, David Ahern wrote: >>> On 6/25/21 1:04 AM, Antony Antony wrote: >>>> From: Christian Perle <christian.perle@secunet.com> >>>> >>>> Default routes learned from router advertisements(RA) are always placed >>>> in main routing table. For policy based routing setups one may >>>> want a different table for default routes. This commit adds a sysctl >>>> to make table number for RA default routes configurable. >>>> >>>> examples: >>>> sysctl net.ipv6.route.defrtr_table >>>> sysctl -w net.ipv6.route.defrtr_table=42 >>>> ip -6 route show table 42 >>> >>> How are the routing tables managed? If the netdevs are connected to a >>> VRF this just works. >> >> The main routing table has no default route. Our scripts add routing >> rules >> based on interfaces. These rules use the specific routing table where That's the VRF use case -- routing rules based on interfaces. Connect those devices to VRFs and the RA does the right thing. >> the RA >> (when using SLAAC) installs the default route. The rest just works. > > Could this be a devconf property instead of a global property? seems > like the difference would be minor to your patch but the benefit is that > setups using different routing tables for different policies could > benefit (as mentioned when not using vrfs). exactly. This is definitely not a global setting, but a per device setting if at all.
On Tue, Jun 29, 2021 at 09:34:22 -0600, David Ahern wrote: > On 6/29/21 8:05 AM, Eyal Birger wrote: > > Hi Antony, > > > > On 29/06/2021 15:53, Antony Antony wrote: > >> Hi David, > >> > >> On Fri, Jun 25, 2021 at 22:47:41 -0600, David Ahern wrote: > >>> On 6/25/21 1:04 AM, Antony Antony wrote: > >>>> From: Christian Perle <christian.perle@secunet.com> > >>>> > >>>> Default routes learned from router advertisements(RA) are always placed > >>>> in main routing table. For policy based routing setups one may > >>>> want a different table for default routes. This commit adds a sysctl > >>>> to make table number for RA default routes configurable. > >>>> > >>>> examples: > >>>> sysctl net.ipv6.route.defrtr_table > >>>> sysctl -w net.ipv6.route.defrtr_table=42 > >>>> ip -6 route show table 42 > >>> > >>> How are the routing tables managed? If the netdevs are connected to a > >>> VRF this just works. Ah! I figured it out what you were hinting at! Sorry, I didn't know about IFLA_VRF_TABLE attribute of link type vrf. I also found the Documentation/networking/vrf.rst and red the commits including the iproute2 commits. Thanks for the hint. It looks like the vrf->tb_id is used for both v4 and v6 routing? We are only looking at V6 routing, because the table only have v6 routes. > >> > >> The main routing table has no default route. Our scripts add routing > >> rules > >> based on interfaces. These rules use the specific routing table where > > That's the VRF use case -- routing rules based on interfaces. Connect > those devices to VRFs and the RA does the right thing. > > >> the RA > >> (when using SLAAC) installs the default route. The rest just works. > > > > Could this be a devconf property instead of a global property? seems yes adding to ipv6_devconf.cnf.ra_defrtr_tble is interesting. I may propose a general solution that can replaces vrf_fib_table(retrun vrf->tb_id). Do we need two? one for v6(in ipv6_devconf) and one for v4(bit map entry in ipv4_devconf.cnf.data)? there will be two ways to configure the defrtr_table option. Frst using current ip link vrf table, when creating the devince, and sysctl options. e.g ip link add vrf-blue type vrf table 10 would set set both ipv4_devconf.cnf.data[IPV4_DEVCONF_DEFRTR_TABLE] ipv6_devconf.cnf.ra_defrtr_tble sysctl will add additional 4 options net.ipv4.conf.all.defrtr_table net.ipv4.conf.eth0.defrtr_table net.ipv6.conf.all.defrtr_tabe net.ipv6.conf.eth0.defrtr_table I guess replacing l3mdev_fib_table_rcu() would be complicated. I will think about a generic l3mdev_fib_table(). Any tips how to add defrtr_table support for various link types? > > like the difference would be minor to your patch but the benefit is that > > setups using different routing tables for different policies could > > benefit (as mentioned when not using vrfs). > > exactly. This is definitely not a global setting, but a per device > setting if at all. A per device setting would work for our use case. thanks, -antony
On 6/29/21 11:34 PM, Antony Antony wrote: > On Tue, Jun 29, 2021 at 09:34:22 -0600, David Ahern wrote: >> On 6/29/21 8:05 AM, Eyal Birger wrote: >>> Hi Antony, >>> >>> On 29/06/2021 15:53, Antony Antony wrote: >>>> Hi David, >>>> >>>> On Fri, Jun 25, 2021 at 22:47:41 -0600, David Ahern wrote: >>>>> On 6/25/21 1:04 AM, Antony Antony wrote: >>>>>> From: Christian Perle <christian.perle@secunet.com> >>>>>> >>>>>> Default routes learned from router advertisements(RA) are always placed >>>>>> in main routing table. For policy based routing setups one may >>>>>> want a different table for default routes. This commit adds a sysctl >>>>>> to make table number for RA default routes configurable. >>>>>> >>>>>> examples: >>>>>> sysctl net.ipv6.route.defrtr_table >>>>>> sysctl -w net.ipv6.route.defrtr_table=42 >>>>>> ip -6 route show table 42 >>>>> >>>>> How are the routing tables managed? If the netdevs are connected to a >>>>> VRF this just works. > > Ah! I figured it out what you were hinting at! Sorry, I didn't know about > IFLA_VRF_TABLE attribute of link type vrf. > > I also found the Documentation/networking/vrf.rst and red the commits > including the iproute2 commits. Thanks for the hint. > > It looks like the vrf->tb_id is used for both v4 and v6 routing? > We are only looking at V6 routing, because the table only have v6 routes. That's fine. There is no requirement to use both. > >>>> >>>> The main routing table has no default route. Our scripts add routing >>>> rules >>>> based on interfaces. These rules use the specific routing table where >> >> That's the VRF use case -- routing rules based on interfaces. Connect >> those devices to VRFs and the RA does the right thing. >> >>>> the RA >>>> (when using SLAAC) installs the default route. The rest just works. >>> >>> Could this be a devconf property instead of a global property? seems > > yes adding to ipv6_devconf.cnf.ra_defrtr_tble is interesting. > > I may propose a general solution that can replaces > vrf_fib_table(retrun vrf->tb_id). no. You missed my point. No change is needed to VRF at all. No change is needed for this use case at all if you simply: ip link add VRF type vrf table TABLE ip link set VRF up ip link set dev <RA DEV> vrf VRF You are good. RAs are processed and added to TABLE. your proposed change is not needed.
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h index bde0b7adb4a3..0eb599ee621a 100644 --- a/include/net/netns/ipv6.h +++ b/include/net/netns/ipv6.h @@ -53,6 +53,7 @@ struct netns_sysctl_ipv6 { int seg6_flowlabel; bool skip_notify_on_dev_down; u8 fib_notify_on_flag_change; + u32 ip6_rt_defrtr_table; }; struct netns_ipv6 { diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 7b756a7dc036..5c561f5b7618 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -4336,7 +4336,7 @@ struct fib6_info *rt6_get_dflt_router(struct net *net, const struct in6_addr *addr, struct net_device *dev) { - u32 tb_id = l3mdev_fib_table(dev) ? : RT6_TABLE_DFLT; + u32 tb_id = l3mdev_fib_table(dev) ? : net->ipv6.sysctl.ip6_rt_defrtr_table; struct fib6_info *rt; struct fib6_table *table; @@ -4371,7 +4371,7 @@ struct fib6_info *rt6_add_dflt_router(struct net *net, u32 defrtr_usr_metric) { struct fib6_config cfg = { - .fc_table = l3mdev_fib_table(dev) ? : RT6_TABLE_DFLT, + .fc_table = l3mdev_fib_table(dev) ? : net->ipv6.sysctl.ip6_rt_defrtr_table, .fc_metric = defrtr_usr_metric, .fc_ifindex = dev->ifindex, .fc_flags = RTF_GATEWAY | RTF_ADDRCONF | RTF_DEFAULT | @@ -6391,6 +6391,13 @@ static struct ctl_table ipv6_route_table_template[] = { .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_ONE, }, + { + .procname = "defrtr_table", + .data = &init_net.ipv6.sysctl.ip6_rt_defrtr_table, + .maxlen = sizeof(u32), + .mode = 0644, + .proc_handler = proc_dointvec, + }, { } }; @@ -6415,6 +6422,7 @@ struct ctl_table * __net_init ipv6_route_sysctl_init(struct net *net) table[8].data = &net->ipv6.sysctl.ip6_rt_min_advmss; table[9].data = &net->ipv6.sysctl.ip6_rt_gc_min_interval; table[10].data = &net->ipv6.sysctl.skip_notify_on_dev_down; + table[11].data = &net->ipv6.sysctl.ip6_rt_defrtr_table; /* Don't export sysctls to unprivileged users */ if (net->user_ns != &init_user_ns) @@ -6486,6 +6494,7 @@ static int __net_init ip6_route_net_init(struct net *net) net->ipv6.sysctl.ip6_rt_mtu_expires = 10*60*HZ; net->ipv6.sysctl.ip6_rt_min_advmss = IPV6_MIN_MTU - 20 - 40; net->ipv6.sysctl.skip_notify_on_dev_down = 0; + net->ipv6.sysctl.ip6_rt_defrtr_table = RT6_TABLE_DFLT; net->ipv6.ip6_rt_gc_expire = 30*HZ;