Re: svn commit: r364465 - in head/sys: conf net net/route
09.09.2020, 07:13, "Andriy Gapon" : > On 09/09/2020 00:50, Alexander V. Chernikov wrote: >> 08.09.2020, 21:03, "Andriy Gapon" : >>> On 22/08/2020 00:34, Alexander V. Chernikov wrote: Author: melifaro Date: Fri Aug 21 21:34:52 2020 New Revision: 364465 URL: https://svnweb.freebsd.org/changeset/base/364465 Log: Make net.fibs growable. Allow to dynamically grow the amount of fibs in each vnet. This change alters current behavior. Currently, if one defines ROUTETABLES > 1 in the kernel config, each vnet will be created with the number of fibs defined in the kernel config. After this commit vnets will be created with fibs=1. Dynamic net.fibs is not compatible with net.add_addr_allfibs. The plan is to deprecate the latter and make net.add_addr_allfibs=0 default behaviour. Reviewed by: glebius Relnotes: yes Differential Revision: https://reviews.freebsd.org/D26062 >>> >>> I wonder why no one reported a problem that I am seeing after upgrading >>> past >>> this revision. Maybe because I do have net.fibs=2 in my loader.conf? >> Hi Andriy, >> >> Does r365475 fix the problem for you? >> CTLFLAG_RWTUN flag got slipped through the cracks somewhere :-( > > I am not sure that it does, I haven't tried it, but I agree with Ryan's > comment. It should. > In general, I would keep CTLFLAG_RWTUN as the knob is a tunable indeed and > some > tools query that flag. Thanks for the suggestion, I've updated the params to include CTLFLAG_NOFETCH in r365517. > So, I would like to re-iterate my earlier suggestion to use CTLFLAG_NOFETCH > paired with explicit TUNABLE_INT_FETCH (which seems to be there already). > >>> The problem -- unfortunately I only have a screenshot -- but it's a page >>> fault >>> trap in this call chain: >>> sysctl_register_all -> sysctl_load_tunable_by_oid_locked -> sysctl_fibs -> >>> _sx_xlock. >>> >>> The crash is on the RTABLES_LOCK() line. >>> And it's kind of obvious why. >>> >>> The tunables, including net.fibs which is declared as RWTUN, are set at >>> SI_SUB_TUNABLES stage, but RTABLES_LOCK_INIT() is not called until >>> SI_SUB_PROTO_DOMAIN much later. In other words, sysctal_fibs can be called >>> earlier than vnet_rtables_init. >>> >>> I think that the best way to handle the problem would be to add >>> CTLFLAG_NOFETCH >>> to the sysctl declaration and then to add -- if necessary at all -- an >>> explicit >>> query of the kenv. >>> >>> -- >>> Andriy Gapon > > -- > Andriy Gapon ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
On 09/09/2020 00:50, Alexander V. Chernikov wrote: > 08.09.2020, 21:03, "Andriy Gapon" : >> On 22/08/2020 00:34, Alexander V. Chernikov wrote: >>> Author: melifaro >>> Date: Fri Aug 21 21:34:52 2020 >>> New Revision: 364465 >>> URL: https://svnweb.freebsd.org/changeset/base/364465 >>> >>> Log: >>> Make net.fibs growable. >>> >>> Allow to dynamically grow the amount of fibs in each vnet. >>> >>> This change alters current behavior. Currently, if one defines >>> ROUTETABLES > 1 in the kernel config, each vnet will be created >>> with the number of fibs defined in the kernel config. >>> After this commit vnets will be created with fibs=1. >>> >>> Dynamic net.fibs is not compatible with net.add_addr_allfibs. >>> The plan is to deprecate the latter and make >>> net.add_addr_allfibs=0 default behaviour. >>> >>> Reviewed by: glebius >>> Relnotes: yes >>> Differential Revision: https://reviews.freebsd.org/D26062 >> >> I wonder why no one reported a problem that I am seeing after upgrading past >> this revision. Maybe because I do have net.fibs=2 in my loader.conf? > Hi Andriy, > > Does r365475 fix the problem for you? > CTLFLAG_RWTUN flag got slipped through the cracks somewhere :-( I am not sure that it does, I haven't tried it, but I agree with Ryan's comment. In general, I would keep CTLFLAG_RWTUN as the knob is a tunable indeed and some tools query that flag. So, I would like to re-iterate my earlier suggestion to use CTLFLAG_NOFETCH paired with explicit TUNABLE_INT_FETCH (which seems to be there already). >> The problem -- unfortunately I only have a screenshot -- but it's a page >> fault >> trap in this call chain: >> sysctl_register_all -> sysctl_load_tunable_by_oid_locked -> sysctl_fibs -> >> _sx_xlock. >> >> The crash is on the RTABLES_LOCK() line. >> And it's kind of obvious why. >> >> The tunables, including net.fibs which is declared as RWTUN, are set at >> SI_SUB_TUNABLES stage, but RTABLES_LOCK_INIT() is not called until >> SI_SUB_PROTO_DOMAIN much later. In other words, sysctal_fibs can be called >> earlier than vnet_rtables_init. >> >> I think that the best way to handle the problem would be to add >> CTLFLAG_NOFETCH >> to the sysctl declaration and then to add -- if necessary at all -- an >> explicit >> query of the kenv. >> >> -- >> Andriy Gapon -- Andriy Gapon ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
08.09.2020, 21:03, "Andriy Gapon" : > On 22/08/2020 00:34, Alexander V. Chernikov wrote: >> Author: melifaro >> Date: Fri Aug 21 21:34:52 2020 >> New Revision: 364465 >> URL: https://svnweb.freebsd.org/changeset/base/364465 >> >> Log: >> Make net.fibs growable. >> >> Allow to dynamically grow the amount of fibs in each vnet. >> >> This change alters current behavior. Currently, if one defines >> ROUTETABLES > 1 in the kernel config, each vnet will be created >> with the number of fibs defined in the kernel config. >> After this commit vnets will be created with fibs=1. >> >> Dynamic net.fibs is not compatible with net.add_addr_allfibs. >> The plan is to deprecate the latter and make >> net.add_addr_allfibs=0 default behaviour. >> >> Reviewed by: glebius >> Relnotes: yes >> Differential Revision: https://reviews.freebsd.org/D26062 > > I wonder why no one reported a problem that I am seeing after upgrading past > this revision. Maybe because I do have net.fibs=2 in my loader.conf? Hi Andriy, Does r365475 fix the problem for you? CTLFLAG_RWTUN flag got slipped through the cracks somewhere :-( > The problem -- unfortunately I only have a screenshot -- but it's a page fault > trap in this call chain: > sysctl_register_all -> sysctl_load_tunable_by_oid_locked -> sysctl_fibs -> > _sx_xlock. > > The crash is on the RTABLES_LOCK() line. > And it's kind of obvious why. > > The tunables, including net.fibs which is declared as RWTUN, are set at > SI_SUB_TUNABLES stage, but RTABLES_LOCK_INIT() is not called until > SI_SUB_PROTO_DOMAIN much later. In other words, sysctal_fibs can be called > earlier than vnet_rtables_init. > > I think that the best way to handle the problem would be to add > CTLFLAG_NOFETCH > to the sysctl declaration and then to add -- if necessary at all -- an > explicit > query of the kenv. > > -- > Andriy Gapon ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
On 22/08/2020 00:34, Alexander V. Chernikov wrote: > Author: melifaro > Date: Fri Aug 21 21:34:52 2020 > New Revision: 364465 > URL: https://svnweb.freebsd.org/changeset/base/364465 > > Log: > Make net.fibs growable. > > Allow to dynamically grow the amount of fibs in each vnet. > > This change alters current behavior. Currently, if one defines >ROUTETABLES > 1 in the kernel config, each vnet will be created >with the number of fibs defined in the kernel config. >After this commit vnets will be created with fibs=1. > > Dynamic net.fibs is not compatible with net.add_addr_allfibs. >The plan is to deprecate the latter and make >net.add_addr_allfibs=0 default behaviour. > > Reviewed by:glebius > Relnotes: yes > Differential Revision: https://reviews.freebsd.org/D26062 I wonder why no one reported a problem that I am seeing after upgrading past this revision. Maybe because I do have net.fibs=2 in my loader.conf? The problem -- unfortunately I only have a screenshot -- but it's a page fault trap in this call chain: sysctl_register_all -> sysctl_load_tunable_by_oid_locked -> sysctl_fibs -> _sx_xlock. The crash is on the RTABLES_LOCK() line. And it's kind of obvious why. The tunables, including net.fibs which is declared as RWTUN, are set at SI_SUB_TUNABLES stage, but RTABLES_LOCK_INIT() is not called until SI_SUB_PROTO_DOMAIN much later. In other words, sysctal_fibs can be called earlier than vnet_rtables_init. I think that the best way to handle the problem would be to add CTLFLAG_NOFETCH to the sysctl declaration and then to add -- if necessary at all -- an explicit query of the kenv. -- Andriy Gapon ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
On 8/21/20 3:32 PM, Alexander V. Chernikov wrote: 21.08.2020, 23:21, "Julian Elischer" : On 8/21/20 2:34 PM, Alexander V. Chernikov wrote: Author: melifaro Date: Fri Aug 21 21:34:52 2020 New Revision: 364465 URL: https://svnweb.freebsd.org/changeset/base/364465 Log: Make net.fibs growable. Allow to dynamically grow the amount of fibs in each vnet. This change alters current behavior. Currently, if one defines ROUTETABLES > 1 in the kernel config, each vnet will be created with the number of fibs defined in the kernel config. After this commit vnets will be created with fibs=1. Dynamic net.fibs is not compatible with net.add_addr_allfibs. The plan is to deprecate the latter and make net.add_addr_allfibs=0 default behaviour. For a while, setting it to 1 should be very noisy. You mean that kernel should print warning when creating VNET && original net.fibs value is > 1 ? doing what you will soon be unable to do should warn you that you need to get ready to change your software to do it another way. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
22.08.2020, 00:31, "Jung-uk Kim" :
> On 20. 8. 21., Alexander V. Chernikov wrote:
>> Author: melifaro
>> Date: Fri Aug 21 21:34:52 2020
>> New Revision: 364465
>> URL: https://svnweb.freebsd.org/changeset/base/364465
>>
>> Log:
>> Make net.fibs growable.
>>
>> Allow to dynamically grow the amount of fibs in each vnet.
>>
>> This change alters current behavior. Currently, if one defines
>> ROUTETABLES > 1 in the kernel config, each vnet will be created
>> with the number of fibs defined in the kernel config.
>> After this commit vnets will be created with fibs=1.
>>
>> Dynamic net.fibs is not compatible with net.add_addr_allfibs.
>> The plan is to deprecate the latter and make
>> net.add_addr_allfibs=0 default behaviour.
>>
>> Reviewed by: glebius
>> Relnotes: yes
>> Differential Revision: https://reviews.freebsd.org/D26062
>>
>> Added:
>> head/sys/net/route/route_tables.c (contents, props changed)
>> Modified:
>> head/sys/conf/files
>> head/sys/net/route.c
>> head/sys/net/route.h
>> head/sys/net/route/route_var.h
>
> ...
>
> This commit broke the kernel build and the following patch fixed it for me.
Could you please name the architecture? ci.freebsd.org looks pretty green so
far.
>
> Index: sys/net/route/route_tables.c
> ===
> --- sys/net/route/route_tables.c (revision 364466)
> +++ sys/net/route/route_tables.c (working copy)
> @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
>
> Jung-uk Kim
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
On 20. 8. 21., Alexander V. Chernikov wrote:
> Author: melifaro
> Date: Fri Aug 21 21:34:52 2020
> New Revision: 364465
> URL: https://svnweb.freebsd.org/changeset/base/364465
>
> Log:
> Make net.fibs growable.
>
> Allow to dynamically grow the amount of fibs in each vnet.
>
> This change alters current behavior. Currently, if one defines
>ROUTETABLES > 1 in the kernel config, each vnet will be created
>with the number of fibs defined in the kernel config.
>After this commit vnets will be created with fibs=1.
>
> Dynamic net.fibs is not compatible with net.add_addr_allfibs.
>The plan is to deprecate the latter and make
>net.add_addr_allfibs=0 default behaviour.
>
> Reviewed by:glebius
> Relnotes: yes
> Differential Revision: https://reviews.freebsd.org/D26062
>
> Added:
> head/sys/net/route/route_tables.c (contents, props changed)
> Modified:
> head/sys/conf/files
> head/sys/net/route.c
> head/sys/net/route.h
> head/sys/net/route/route_var.h
...
This commit broke the kernel build and the following patch fixed it for me.
Index: sys/net/route/route_tables.c
===
--- sys/net/route/route_tables.c(revision 364466)
+++ sys/net/route/route_tables.c(working copy)
@@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
#include
#include
#include
+#include
#include
#include
#include
Jung-uk Kim
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
21.08.2020, 23:21, "Julian Elischer" : > On 8/21/20 2:34 PM, Alexander V. Chernikov wrote: >> Author: melifaro >> Date: Fri Aug 21 21:34:52 2020 >> New Revision: 364465 >> URL: https://svnweb.freebsd.org/changeset/base/364465 >> >> Log: >> Make net.fibs growable. >> >> Allow to dynamically grow the amount of fibs in each vnet. >> >> This change alters current behavior. Currently, if one defines >> ROUTETABLES > 1 in the kernel config, each vnet will be created >> with the number of fibs defined in the kernel config. >> After this commit vnets will be created with fibs=1. >> >> Dynamic net.fibs is not compatible with net.add_addr_allfibs. >> The plan is to deprecate the latter and make >> net.add_addr_allfibs=0 default behaviour. > > For a while, setting it to 1 should be very noisy. You mean that kernel should print warning when creating VNET && original net.fibs value is > 1 ? > >> ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r364465 - in head/sys: conf net net/route
On 8/21/20 2:34 PM, Alexander V. Chernikov wrote: Author: melifaro Date: Fri Aug 21 21:34:52 2020 New Revision: 364465 URL: https://svnweb.freebsd.org/changeset/base/364465 Log: Make net.fibs growable. Allow to dynamically grow the amount of fibs in each vnet. This change alters current behavior. Currently, if one defines ROUTETABLES > 1 in the kernel config, each vnet will be created with the number of fibs defined in the kernel config. After this commit vnets will be created with fibs=1. Dynamic net.fibs is not compatible with net.add_addr_allfibs. The plan is to deprecate the latter and make net.add_addr_allfibs=0 default behaviour. For a while, setting it to 1 should be very noisy. ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
svn commit: r364465 - in head/sys: conf net net/route
Author: melifaro
Date: Fri Aug 21 21:34:52 2020
New Revision: 364465
URL: https://svnweb.freebsd.org/changeset/base/364465
Log:
Make net.fibs growable.
Allow to dynamically grow the amount of fibs in each vnet.
This change alters current behavior. Currently, if one defines
ROUTETABLES > 1 in the kernel config, each vnet will be created
with the number of fibs defined in the kernel config.
After this commit vnets will be created with fibs=1.
Dynamic net.fibs is not compatible with net.add_addr_allfibs.
The plan is to deprecate the latter and make
net.add_addr_allfibs=0 default behaviour.
Reviewed by: glebius
Relnotes: yes
Differential Revision:https://reviews.freebsd.org/D26062
Added:
head/sys/net/route/route_tables.c (contents, props changed)
Modified:
head/sys/conf/files
head/sys/net/route.c
head/sys/net/route.h
head/sys/net/route/route_var.h
Modified: head/sys/conf/files
==
--- head/sys/conf/files Fri Aug 21 21:24:14 2020(r364464)
+++ head/sys/conf/files Fri Aug 21 21:34:52 2020(r364465)
@@ -4102,6 +4102,7 @@ net/route/nhop_utils.cstandard
net/route/route_ctl.c standard
net/route/route_ddb.c optional ddb
net/route/route_helpers.c standard
+net/route/route_tables.c standard
net/route/route_temporal.c standard
net/rss_config.c optional inet rss | inet6 rss
net/rtsock.c standard
Modified: head/sys/net/route.c
==
--- head/sys/net/route.cFri Aug 21 21:24:14 2020(r364464)
+++ head/sys/net/route.cFri Aug 21 21:34:52 2020(r364465)
@@ -74,29 +74,6 @@
#include
#include
-#include
-
-#defineRT_MAXFIBS UINT16_MAX
-
-/* Kernel config default option. */
-#ifdef ROUTETABLES
-#if ROUTETABLES <= 0
-#error "ROUTETABLES defined too low"
-#endif
-#if ROUTETABLES > RT_MAXFIBS
-#error "ROUTETABLES defined too big"
-#endif
-#defineRT_NUMFIBS ROUTETABLES
-#endif /* ROUTETABLES */
-/* Initialize to default if not otherwise set. */
-#ifndefRT_NUMFIBS
-#defineRT_NUMFIBS 1
-#endif
-
-/* This is read-only.. */
-u_int rt_numfibs = RT_NUMFIBS;
-SYSCTL_UINT(_net, OID_AUTO, fibs, CTLFLAG_RDTUN, _numfibs, 0, "");
-
/*
* By default add routes to all fibs for new interfaces.
* Once this is set to 0 then only allocate routes on interface
@@ -118,10 +95,6 @@ VNET_PCPUSTAT_SYSINIT(rtstat);
VNET_PCPUSTAT_SYSUNINIT(rtstat);
#endif
-VNET_DEFINE(struct rib_head *, rt_tables);
-#defineV_rt_tables VNET(rt_tables)
-
-
EVENTHANDLER_LIST_DEFINE(rt_addrmsg);
static int rt_ifdelroute(const struct rtentry *rt, const struct nhop_object *,
@@ -130,63 +103,6 @@ static int rt_exportinfo(struct rtentry *rt, struct rt
int flags);
/*
- * handler for net.my_fibnum
- */
-static int
-sysctl_my_fibnum(SYSCTL_HANDLER_ARGS)
-{
-int fibnum;
-int error;
-
-fibnum = curthread->td_proc->p_fibnum;
-error = sysctl_handle_int(oidp, , 0, req);
-return (error);
-}
-
-SYSCTL_PROC(_net, OID_AUTO, my_fibnum,
-CTLTYPE_INT | CTLFLAG_RD | CTLFLAG_MPSAFE, NULL, 0,
-_my_fibnum, "I",
-"default FIB of caller");
-
-static __inline struct rib_head **
-rt_tables_get_rnh_ptr(int table, int fam)
-{
- struct rib_head **rnh;
-
- KASSERT(table >= 0 && table < rt_numfibs,
- ("%s: table out of bounds (0 <= %d < %d)", __func__, table,
-rt_numfibs));
- KASSERT(fam >= 0 && fam < (AF_MAX + 1),
- ("%s: fam out of bounds (0 <= %d < %d)", __func__, fam, AF_MAX+1));
-
- /* rnh is [fib=0][af=0]. */
- rnh = (struct rib_head **)V_rt_tables;
- /* Get the offset to the requested table and fam. */
- rnh += table * (AF_MAX+1) + fam;
-
- return (rnh);
-}
-
-struct rib_head *
-rt_tables_get_rnh(int table, int fam)
-{
-
- return (*rt_tables_get_rnh_ptr(table, fam));
-}
-
-u_int
-rt_tables_get_gen(int table, int fam)
-{
- struct rib_head *rnh;
-
- rnh = *rt_tables_get_rnh_ptr(table, fam);
- KASSERT(rnh != NULL, ("%s: NULL rib_head pointer table %d fam %d",
- __func__, table, fam));
- return (rnh->rnh_gen);
-}
-
-
-/*
* route initialization must occur before ip6_init2(), which happenas at
* SI_ORDER_MIDDLE.
*/
@@ -194,89 +110,10 @@ static void
route_init(void)
{
- /* whack the tunable ints into line. */
- if (rt_numfibs > RT_MAXFIBS)
- rt_numfibs = RT_MAXFIBS;
- if (rt_numfibs == 0)
- rt_numfibs = 1;
nhops_init();
}
SYSINIT(route_init, SI_SUB_PROTO_DOMAIN, SI_ORDER_THIRD, route_init, NULL);
-static void
-vnet_route_init(const void *unused __unused)
-{
- struct domain *dom;
- struct rib_head **rnh;
- int table;
- int fam;
-
