route: Initialize V_rt_numfibs earlier during boot

[markj]

Summary

V_rt_numfibs can be set at compile time (with the ROUTETABLES kernel
config option) or boot time (with the net.fibs tunable).
vnet_rtables_init(), running during SI_PROTO_DOMAIN, was checking the
tunable and updating V_rt_numfibs accordingly, but that means that
earlier SYSINITs, such as vnet_mroute_init(), see the compile-time value
for V_rt_numfibs before it gets corrected in vnet_rtables_init().

Fix this by initializing V_rt_numfibs earlier, so that SYSINITs are less
likely to use the wrong value.

PR: 294510

[glebius]

IMHO, every use of IS_DEFAULT_VNET() is a mark of incorrect code :) Ideally the function shall be split into two: one for SYSINIT() and other for VNET_SYSINIT().

[markj]

In D56473#1292728, @glebius wrote:

IMHO, every use of IS_DEFAULT_VNET() is a mark of incorrect code :) Ideally the function shall be split into two: one for SYSINIT() and other for VNET_SYSINIT().

Why, what's the advantage?

[glebius]

To be more verbose, in this particular case, an equivalent but better change would be:

// at line 86
VNET_DEFINE(uint32_t, _rt_numfibs) = 1;
...
static void
rtnumfibs_init(const void *unused __unused)
{
        int num_rtables_base;

        num_rtables_base = RT_NUMFIBS;
        TUNABLE_INT_FETCH("net.fibs", &num_rtables_base);
        V_rt_numfibs = normalize_num_rtables(num_rtables_base);
}
SYSINIT(rtnumfibs_init, SI_SUB_PROTO_BEGIN, SI_ORDER_FIRST, rtnumfibs_init, NULL);

I would question the idea that the non-default vnets always get 1 instead of configured number, but that's a different topic.

[glebius]

In D56473#1292731, @markj wrote:

IMHO, every use of IS_DEFAULT_VNET() is a mark of incorrect code :) Ideally the function shall be split into two: one for SYSINIT() and other for VNET_SYSINIT().

Why, what's the advantage?

With SYSINIT() we initialize all truly global things and with VNET_SYSINIT() vnet-specific. At minimum it makes code cleaner, as you don't mix two unrelated branches into one function. You also can specify different subsystems if needed.

[zlei]

sys/net/route/route_tables.c

67

Given the FIB number can now be increased at runtime, is it time to retire RT_NUMFIBS and default to 1, so we can avoid a big compile time V_rt_numfibs ?

Also CC @melifaro .

[pouria]

sys/net/route/route_tables.c

67

I'm already working on it.
I believe the whole ROUTE_TABLE compile option should be retired.
I'll upload the patches after metric implementation.

[markj]

Apply Gleb's suggestion.

[markj]

sys/net/route/route_tables.c

67

Yes, this change makes V_rt_numfibs default to 1, so that the compile-time option and loader tunable behave the same.

I don't really see a reason to remove the ROUTETABLES option?

[pouria]

sys/net/route/route_tables.c

67

It won't be in a single revision like MPATH.
I have many upcoming changes to the routing stack that will gradually make ROUTE_TABLES absence insignificant.
Of course, that would take time.
I'll explain the details in those revisions, but TL;DR: there will be no user benefit to want it disabled.
One of the underlying reasons of it is netlink itself.

[glebius]

Thanks! Maybe provide an XXX comment that we intentionally (but why?) make non-default vnets to always have it set to 1? Make it a documented strangeness.

[pouria]

LGTM, I suggest to use routing: xxx instead of route for commit message to indicate the change is for the kernel part and not for route(8).

[zlei]

Looks good to me.