new: driver selection logic #484

FedeDP · 2024-03-20T10:20:54Z

What type of PR is this?

/kind feature
when a
Any specific area of the project related to this PR?

/area library
/area cli

What this PR does / why we need it:

This PR expands the driver automatic selection logic, in this way:

--type CLI option/type config key/FALCOCTL_DRIVER_TYPE env var do now support multiple driver types to be specified
if a single type is specified, we retain the old behavior
when multiple types are passed, the automatic selection logic will choose the best-fit given:
- types are passed in decreasing priority order
- driver must be supported on the machine

New default is to enable all drivers in this order: modern_ebpf,ebpf,kmod.
COS target does now specialize PreferredDriver method since it does not support kmod.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

…ogic. `Driver.Type` will now be a slice in config. Then, cmd/driver_linux will take care of discovering, given the list of allowed driver types loaded from the config, the correct driver to be used. This has multple consequencies: * enforcing a single `--type` works like the existing behavior * allowed driver types are in descending priority order Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2024-03-20T11:06:56Z

/milestone v0.8.0

FedeDP · 2024-03-20T11:10:05Z

pkg/driver/type/modernbpf.go

+// type Tracing, attachType AttachTraceRawTp program availability.
+//
+//go:linkname probeProgram github.com/cilium/ebpf/features.probeProgram
+func probeProgram(spec *ebpf.ProgramSpec) error


To avoid fully rewriting the function, we "unsafely" import it using go:linkname.
I think this is better than copying the function locally, because then we get any change probeProgram might get with new versions, and if probeProgram changes signature/name, our build will fail while bumping the library dep.
This way, we always know if something has changed.

FedeDP · 2024-03-20T11:10:23Z

pkg/driver/type/modernbpf.go

+	// err := features.HaveProgramType(ebpf.Tracing)
+	// Therefore, we need to manually build a feature test.
+	// Empty tracing program that just returns 0
+	progSpec := &ebpf.ProgramSpec{


Pass a prog spec with correct AttachType type.

FedeDP · 2024-03-20T11:15:18Z

cmd/driver/driver_linux.go

@@ -162,11 +183,22 @@ func NewDriverCmd(ctx context.Context, opt *options.Common) *cobra.Command {
 		},
 	}

-	cmd.PersistentFlags().Var(driverTypes, "type", "Driver type to be used "+driverTypes.Allowed())
+	cmd.PersistentFlags().StringSliceVar(&driverTypesStr, "type", config.DefaultDriver.Type,


type is now a slice.

FedeDP · 2024-03-20T11:15:43Z

cmd/driver/driver_linux.go

 	cmd.PersistentFlags().StringVar(&driver.Version, "version", config.DefaultDriver.Version, "Driver version to be used.")
 	cmd.PersistentFlags().StringSliceVar(&driver.Repos, "repo", config.DefaultDriver.Repos, "Driver repo to be used.")
 	cmd.PersistentFlags().StringVar(&driver.Name, "name", config.DefaultDriver.Name, "Driver name to be used.")
 	cmd.PersistentFlags().StringVar(&driver.HostRoot, "host-root", config.DefaultDriver.HostRoot, "Driver host root to be used.")
+	cmd.PersistentFlags().StringVar(&driverKernelRelease,


Moved --kernelrelease and --kernelversion CLI options directly to driver root command.

FedeDP · 2024-03-20T11:16:44Z

cmd/driver/driver_linux.go

-					return fmt.Errorf("automatic driver selection failed")
+			// Step 2: fetch system info (kernel release/version and distro)
+			var err error
+			driver.Kr, err = driverkernel.FetchInfo(driverKernelRelease, driverKernelVersion)


options.Driver now holds run kernelrelease and distro too.
This way, we discover them just once, here, and then pass the driver options down to all driver related commands, without needing to discover distro or kernel infos twice.

FedeDP · 2024-03-20T11:17:36Z

cmd/driver/install/install.go

-		} else {
-			return "", fmt.Errorf("detected an unsupported target system, please get in touch with the Falco community")
-		}
+	if o.Distro.String() == driverdistro.UndeterminedDistro && o.Compile {


Check is now a bit different because we are not discovering the distro here anymore; instead we receive a distro from driver root command.

We need to check if ErrUnsupported was returned, ie: if the distro is UndeterminedDistro.

Signed-off-by: Federico Di Pierro <[email protected]>

…dern bpf support. Signed-off-by: Federico Di Pierro <[email protected]>

…md/driver_linux root command. Signed-off-by: Federico Di Pierro <[email protected]>

Signed-off-by: Federico Di Pierro <[email protected]>

…ogram type check. Signed-off-by: Federico Di Pierro <[email protected]>

…version. Moreover, added a version for others OS. Signed-off-by: Federico Di Pierro <[email protected]>

pkg/driver/distro/generic.go

FedeDP · 2024-03-22T10:22:39Z

pkg/driver/type/modernbpf.go

+func probeProgram(spec *ebpf.ProgramSpec) error
+
+//nolint:gocritic // the method shall not be able to modify kr
+func (m *modernBpf) Supported(_ kernelrelease.KernelRelease) bool {


Note: modern bpf will check directly against running kernel, to see if all required features are supported.
This means that we cannot check against "foreign" kernel releases.

leogr

First quick look. I really like the new UX! 🤩

It SGTM already, still I'd like to get a second opinion before approving.
cc @alacuku

pkg/driver/kernel/kernel_others.go

Also, allow `FetchInfo` to override kernelrelease/version if only one of them is enforced. Signed-off-by: Federico Di Pierro <[email protected]>

poiana · 2024-03-25T09:59:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [FedeDP,leogr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

poiana added kind/feature do-not-merge/work-in-progress dco-signoff: yes area/library labels Mar 20, 2024

poiana requested review from alacuku and loresuso March 20, 2024 10:20

poiana added area/cli approved size/L size/XL and removed size/L labels Mar 20, 2024

poiana added this to the v0.8.0 milestone Mar 20, 2024

FedeDP commented Mar 20, 2024

View reviewed changes

FedeDP changed the title ~~wip: new: driver selection logic~~ new: driver selection logic Mar 20, 2024

poiana removed the do-not-merge/work-in-progress label Mar 20, 2024

FedeDP commented Mar 20, 2024

View reviewed changes

FedeDP added 6 commits March 21, 2024 08:57

chore(pkg/driver): COS does only support ebpf probes.

67d03c7

Signed-off-by: Federico Di Pierro <[email protected]>

chore(cmd/driver,pkg/driver): use cilium/ebpf library to check for mo…

3047ce4

…dern bpf support. Signed-off-by: Federico Di Pierro <[email protected]>

chore(cmd/driver,pkg/driver): only load distro and kernelrelease in c…

73efa99

…md/driver_linux root command. Signed-off-by: Federico Di Pierro <[email protected]>

chore(cmd,pkg): fixed tests and linter.

b63c0f8

Signed-off-by: Federico Di Pierro <[email protected]>

chore(pkg/driver): less verbose ProgramSpec for modern bpf Tracing pr…

9458932

…ogram type check. Signed-off-by: Federico Di Pierro <[email protected]>

chore(pkg/driver/kernel): moved driverkernel FetchInfo to linux only …

2a56cf9

…version. Moreover, added a version for others OS. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP force-pushed the new/driver_selection_logic branch from 8f33dbc to 2a56cf9 Compare March 21, 2024 07:57

FedeDP commented Mar 21, 2024

View reviewed changes

pkg/driver/distro/generic.go Show resolved Hide resolved

FedeDP mentioned this pull request Mar 21, 2024

wip: new(cmd,pkg): leverage falcoctl to skip generating configs for kernels where modern bpf is supported falcosecurity/dbg-go#43

Closed

FedeDP commented Mar 22, 2024

View reviewed changes

leogr reviewed Mar 22, 2024

View reviewed changes

pkg/driver/kernel/kernel_others.go Outdated Show resolved Hide resolved

chore(cmdk,pkg,internal): bump copyright year for all touched files.

2f91849

Also, allow `FetchInfo` to override kernelrelease/version if only one of them is enforced. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP mentioned this pull request Mar 25, 2024

new(pkg): use driverkit local build processor instead of implementing drivers build #356

Merged

4 tasks

leogr approved these changes Mar 25, 2024

View reviewed changes

poiana assigned leogr Mar 25, 2024

poiana added the lgtm label Mar 25, 2024

poiana merged commit cf958dc into main Mar 25, 2024
16 of 17 checks passed

poiana deleted the new/driver_selection_logic branch March 25, 2024 09:59

FedeDP mentioned this pull request Mar 25, 2024

new(pkg/driver): try to fetch kernel headers leveraging driverkit library when building drivers #476

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new: driver selection logic #484

new: driver selection logic #484

FedeDP commented Mar 20, 2024

FedeDP commented Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 20, 2024

FedeDP Mar 22, 2024

leogr left a comment

poiana commented Mar 25, 2024

new: driver selection logic #484

new: driver selection logic #484

Conversation

FedeDP commented Mar 20, 2024

FedeDP commented Mar 20, 2024

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 20, 2024

Choose a reason for hiding this comment

FedeDP Mar 22, 2024

Choose a reason for hiding this comment

leogr left a comment

Choose a reason for hiding this comment

poiana commented Mar 25, 2024