Add FIB_ALGO to GENERIC on amd64/arm64.

[melifaro]

Summary

Option FIB_ALGO gates new modular fib lookup functionality, enabling more performant routing table lookups.

The initial support was added in D27401, back in December.

A number of issues have been fixed since then: 1, 2, 3.

At this point, I believe it should be ready for the wider audience, e.g. all -CURRENT users.

[melifaro]

Going to commit this on Saturday, April 24

[arichardson]

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

[melifaro]

In D28434#672782, @arichardson wrote:

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

Ack. Interesting, local test run worked. Will take a look in the evening.

[markj]

In D28434#672782, @arichardson wrote:

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

It's yet another problem related to type definitions and CTF, causing dtrace to bail. In this case, struct rib_head has a conditionally defined member, rnh_gen_rib. FIB_ALGO is defined only when opt_route.h is included, so unless everything that includes route.h also includes opt_route.h, we end up with different definitions for struct rib_head depending on whether opt_route.h is included. I can see two solutions: move FIB_ALGO to opt_global.h so that FIB_ALGO is consistently defined, or make the definition of rnh_gen_fib unconditional. I prefer the second solution since out-of-tree modules could be compiled without FIB_ALGO defined.

Even if not for the CTF limits causing problems here, I think it's good to address these kinds of problems as otherwise debuggers can end up using the wrong definition when it's ambiguous.

[markj]

In D28434#672871, @markj wrote:

In D28434#672782, @arichardson wrote:

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

It's yet another problem related to type definitions and CTF, causing dtrace to bail. In this case, struct rib_head has a conditionally defined member, rnh_gen_rib. FIB_ALGO is defined only when opt_route.h is included, so unless everything that includes route.h also includes opt_route.h, we end up with different definitions for struct rib_head depending on whether opt_route.h is included. I can see two solutions: move FIB_ALGO to opt_global.h so that FIB_ALGO is consistently defined, or make the definition of rnh_gen_fib unconditional. I prefer the second solution since out-of-tree modules could be compiled without FIB_ALGO defined.

I misread a bit, the problematic definition is in route_var.h, not route.h, and so is not so widely included in the kernel. Indeed, some files in sys/net/route fail to include opt_route.h. It may be sufficient to just fix those, as a third solution.

[melifaro]

In D28434#672895, @markj wrote:

In D28434#672871, @markj wrote:

In D28434#672782, @arichardson wrote:

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

It's yet another problem related to type definitions and CTF, causing dtrace to bail. In this case, struct rib_head has a conditionally defined member, rnh_gen_rib. FIB_ALGO is defined only when opt_route.h is included, so unless everything that includes route.h also includes opt_route.h, we end up with different definitions for struct rib_head depending on whether opt_route.h is included. I can see two solutions: move FIB_ALGO to opt_global.h so that FIB_ALGO is consistently defined, or make the definition of rnh_gen_fib unconditional. I prefer the second solution since out-of-tree modules could be compiled without FIB_ALGO defined.

Option 2 landed as bc5ef45aec3fa8acf2dd3408cebd207317543a8b. Thank you!

I misread a bit, the problematic definition is in route_var.h, not route.h, and so is not so widely included in the kernel. Indeed, some files in sys/net/route fail to include opt_route.h. It may be sufficient to just fix those, as a third solution.

[melifaro]

In D28434#672787, @melifaro wrote:

In D28434#672782, @arichardson wrote:

This may be the cause of CI failures starting in build 18011: https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/
The only changes are the following the commits (https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18011/changes):
bddae5c8a64dc6b292198945cbe676bb2158d438, 5d1403a79a3e56403fb63c062252a23fce81e5f1, and 6993187a8c30e83a408aad631a8d8629d8273c9d

Ack. Interesting, local test run worked. Will take a look in the evening.

Should be better with 439d087d0b55.

[jrtc27]

Is there a reason why this was only enabled on amd64 and arm64? I'd expect this kind of thing to either be for all kernels or for all 64-bit kernels (maybe excluding mips), but powerpcp64(le) doesn't enable it despite having big iron servers, nor does riscv (though performance sucks on all shipping hardware currently anyway so hardly a big deal). Normally the only reason to limit something to amd64+arm64 is if it's not supported anywhere else.