Page MenuHomeFreeBSD

Add FIB_ALGO to GENERIC on amd64/arm64.
ClosedPublic

Authored by melifaro on Jan 31 2021, 11:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 17, 6:17 AM
Unknown Object (File)
Thu, Mar 7, 2:23 AM
Unknown Object (File)
Thu, Mar 7, 2:23 AM
Unknown Object (File)
Thu, Mar 7, 2:23 AM
Unknown Object (File)
Thu, Mar 7, 2:13 AM
Unknown Object (File)
Fri, Mar 1, 1:37 PM
Unknown Object (File)
Feb 1 2024, 5:25 PM
Unknown Object (File)
Jan 15 2024, 11:47 PM

Details

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.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

melifaro added reviewers: network, olivier, ae.
melifaro changed the repository for this revision from rS FreeBSD src repository - subversion to rG FreeBSD src repository.
This revision is now accepted and ready to land.Feb 2 2021, 5:21 PM

Going to commit this on Saturday, April 24

This revision was automatically updated to reflect the committed changes.

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

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.

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.

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.

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

Should be better with 439d087d0b55.

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.