Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new(docker,pkg/driverbuilder): use cmake instead of makefile template to build kmod and bpf #302

Merged
merged 10 commits into from
Feb 28, 2024
Merged
8 changes: 8 additions & 0 deletions .github/workflows/reusable_build_test_driverkit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ jobs:

- name: Test
run: make test

- name: Set integration tests DRIVERVERSIONS env
if: inputs.arch == 'amd64'
run: echo "DRIVERVERSIONS=master 6.0.1+driver 2.0.0+driver 17f5df52a7d9ed6bb12d3b1768460def8439936d" >> $GITHUB_ENV

- name: Set integration tests DRIVERVERSIONS env
if: inputs.arch == 'arm64'
run: echo "DRIVERVERSIONS=master 6.0.1+driver 2.0.0+driver" >> $GITHUB_ENV

- name: Integration tests
run: make integration_test
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ ifeq ($(COMMITS_FROM_GIT_TAG),0)
endif
endif

DRIVERVERSIONS ?= master

DOCKER_ORG ?= falcosecurity

ARCH := $(shell uname -m)
Expand Down Expand Up @@ -105,7 +107,9 @@ integration_test: $(test_configs)

.PHONY: $(test_configs)
$(test_configs): ${driverkit}
${driverkit} docker -c $@ --builderimage auto:master -l debug --timeout 600
$(foreach d,$(DRIVERVERSIONS),\
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now run integration tests against multiple driver versions.

Copy link
Contributor Author

@FedeDP FedeDP Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way, we can ensure backward compatibility with driver versions for changes.

${driverkit} docker -c $@ --builderimage auto:master -l debug --timeout 600 --driverversion $d; \
)

.PHONY: ${driverkit_docgen}
${driverkit_docgen}: ${PWD}/docgen
Expand Down
52 changes: 38 additions & 14 deletions pkg/driverbuilder/builder/builders.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,25 @@ import (
)

// DriverDirectory is the directory the processor uses to store the driver.
const DriverDirectory = "/tmp/driver"

// ModuleFileName is the standard file name for the kernel module.
const ModuleFileName = "module.ko"

// ProbeFileName is the standard file name for the eBPF probe.
const ProbeFileName = "probe.o"

// ModuleFullPath is the standard path for the kernel module. Builders must place the compiled module at this location.
var ModuleFullPath = path.Join(DriverDirectory, ModuleFileName)

// ProbeFullPath is the standard path for the eBPF probe. Builders must place the compiled probe at this location.
var ProbeFullPath = path.Join(DriverDirectory, "bpf", ProbeFileName)
const (
DriverDirectory = "/tmp/driver"
cmakeCmdFmt = `cmake -Wno-dev \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the cmake command being run.
In the future we might want to allow users to override it perhaps? I don't think it will be needed anyway.

Btw it spots some unused CMake variable to keep compatibility with old driver versions, eg: PROBE_NAME, PROBE_VERSION...

-DUSE_BUNDLED_DEPS=On \
-DCREATE_TEST_TARGETS=Off \
-DBUILD_LIBSCAP_GVISOR=Off \
-DBUILD_LIBSCAP_MODERN_BPF=Off \
-DENABLE_DRIVERS_TESTS=Off \
-DDRIVER_NAME=%s \
-DPROBE_NAME=%s \
-DBUILD_BPF=On \
-DDRIVER_VERSION=%s \
-DPROBE_VERSION=%s \
-DGIT_COMMIT=%s \
-DDRIVER_DEVICE_NAME=%s \
-DPROBE_DEVICE_NAME=%s \
.. && \
sed -i s/'DRIVER_COMMIT ""'/'DRIVER_COMMIT "%s"'/g driver/src/driver_config.h`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the end, we also manually set the DRIVER_COMMIT because currently libs do not honor -DGIT_COMMIT being set by external user: https://github.com/falcosecurity/libs/blob/master/cmake/modules/compute_versions.cmake#L35
Indeed, GIT_COMMIT is always overridden right now, therefore we force-set it in the driver config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could save all of this if we used git to download the repo and then checkout the needed version; but it would hurt download performances (ie: we would need to download the full repo history to allow git to compute versions for us, as we do in the libs CI: https://github.com/falcosecurity/libs/blob/master/.github/workflows/ci.yml#L61)

)

var HeadersNotFoundErr = errors.New("kernel headers not found")

Expand All @@ -55,6 +61,14 @@ type Config struct {
*Build
}

func (c Config) ToDriverFullPath() string {
return path.Join(DriverDirectory, "build", "driver", fmt.Sprintf("%s.ko", c.DriverName))
Copy link
Contributor Author

@FedeDP FedeDP Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expect the built kmod with cmake to be under /tmp/driver/build/driver/$drivername.ko.

}

func (c Config) ToProbeFullPath() string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expect the built probe with cmake to be under /tmp/driver/build/driver/bpf/probe.o.

return path.Join(DriverDirectory, "build", "driver", "bpf", "probe.o")
}

type commonTemplateData struct {
DriverBuildDir string
ModuleDownloadURL string
Expand All @@ -63,6 +77,7 @@ type commonTemplateData struct {
BuildModule bool
BuildProbe bool
GCCVersion string
CmakeCmd string
}

// Builder represents a builder capable of generating a script for a driverkit target.
Expand Down Expand Up @@ -293,10 +308,19 @@ func (c Config) toTemplateData(b Builder, kr kernelrelease.KernelRelease) common
DriverBuildDir: DriverDirectory,
ModuleDownloadURL: fmt.Sprintf("%s/%s.tar.gz", c.DownloadBaseURL, c.DriverVersion),
ModuleDriverName: c.DriverName,
ModuleFullPath: ModuleFullPath,
ModuleFullPath: c.ToDriverFullPath(),
BuildModule: len(c.ModuleFilePath) > 0,
BuildProbe: len(c.ProbeFilePath) > 0,
GCCVersion: c.GCCVersion,
CmakeCmd: fmt.Sprintf(cmakeCmdFmt,
c.DriverName,
c.DriverName,
c.DriverVersion,
c.DriverVersion,
c.DriverVersion,
c.DeviceName,
c.DeviceName,
c.DriverVersion),
}
}

Expand Down
27 changes: 23 additions & 4 deletions pkg/driverbuilder/builder/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
_ "embed"
"fmt"
"github.com/falcosecurity/driverkit/pkg/kernelrelease"
"path/filepath"
)

// NOTE: since this is only used by local build,
Expand Down Expand Up @@ -54,6 +55,15 @@ func (l *LocalBuilder) TemplateData(c Config, kr kernelrelease.KernelRelease, _
BuildModule: len(c.ModuleFilePath) > 0,
BuildProbe: len(c.ProbeFilePath) > 0,
GCCVersion: l.GccPath,
CmakeCmd: fmt.Sprintf(cmakeCmdFmt,
c.DriverName,
c.DriverName,
c.DriverVersion,
c.DriverVersion,
c.DriverVersion,
c.DeviceName,
c.DeviceName,
c.DriverVersion),
},
UseDKMS: l.UseDKMS,
DownloadSrc: len(l.SrcDir) == 0, // if no srcdir is provided, download src!
Expand All @@ -63,17 +73,26 @@ func (l *LocalBuilder) TemplateData(c Config, kr kernelrelease.KernelRelease, _
}

func (l *LocalBuilder) GetModuleFullPath(c Config, kr kernelrelease.KernelRelease) string {
moduleFullPath := ModuleFullPath
if l.UseDKMS {
// When using dkms, we will use a GLOB to match the pattern; ModuleFullPath won't be used in the templated script anyway.
moduleFullPath = fmt.Sprintf("/var/lib/dkms/%s/%s/%s/%s/module/%s.*", c.DriverName, c.DriverVersion, kr.String(), kr.Architecture.ToNonDeb(), c.DriverName)
return fmt.Sprintf("/var/lib/dkms/%s/%s/%s/%s/module/%s.*", c.DriverName, c.DriverVersion, kr.String(), kr.Architecture.ToNonDeb(), c.DriverName)
}
return moduleFullPath
if l.SrcDir != "" {
return filepath.Join(l.SrcDir, fmt.Sprintf("%s.ko", c.DriverName))
}
return c.ToDriverFullPath()
}

func (l *LocalBuilder) GetProbeFullPath(c Config) string {
if l.SrcDir != "" {
return filepath.Join(l.SrcDir, "bpf", "probe.o")
}
return c.ToProbeFullPath()
}

func (l *LocalBuilder) GetDriverBuildDir() string {
driverBuildDir := DriverDirectory
if len(l.SrcDir) > 0 {
if l.SrcDir != "" {
driverBuildDir = l.SrcDir
}
return driverBuildDir
Expand Down
18 changes: 8 additions & 10 deletions pkg/driverbuilder/builder/templates/alinux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ rm -Rf /tmp/module-download
mkdir -p /tmp/module-download

curl --silent -SL {{ .ModuleDownloadURL }} | tar -xzf - -C /tmp/module-download
mv /tmp/module-download/*/driver/* {{ .DriverBuildDir }}

cp /driverkit/module-Makefile {{ .DriverBuildDir }}/Makefile
bash /driverkit/fill-driver-config.sh {{ .DriverBuildDir }}
mv /tmp/module-download/*/* {{ .DriverBuildDir }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scripts are a bit different now:

  • we move all the stuff under the download libs repo to{{ .DriverBuildDir }} now, not only the driver subfolder
  • we run cmake with lots of things disabled. Unfortunately, given that libs only recently gained the ability to build the bpf probe in MINIMAL_BUILD mode, we need to configure the project in full mode, and this requires git because grpc and protobuf deps need to be fetched/cloned.
  • Then, building is just a matter of calling make driver and make bpf with the correct CC and KERNELDIR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually really like how much this all simplifies the code, and I think the trade offs are worth it.


# Fetch the kernel
mkdir /tmp/kernel-download
Expand All @@ -42,19 +39,20 @@ rm -Rf /tmp/kernel
mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

cd {{ .DriverBuildDir }}
mkdir -p build && cd build
{{ .CmakeCmd }}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we always use the same CmakeCmd, make it easily customizable (at least during development) by using a single templated variable for it.


{{ if .BuildModule }}
# Build the module
cd {{ .DriverBuildDir }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel
mv {{ .ModuleDriverName }}.ko {{ .ModuleFullPath }}
Copy link
Contributor Author

@FedeDP FedeDP Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need the mv anymore since ModuleFullPath is already the correct path.

make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel driver
strip -g {{ .ModuleFullPath }}
# Print results
modinfo {{ .ModuleFullPath }}
{{ end }}

{{ if .BuildProbe }}
# Build the eBPF probe
cd {{ .DriverBuildDir }}/bpf
make KERNELDIR=/tmp/kernel
ls -l probe.o
make KERNELDIR=/tmp/kernel bpf
ls -l driver/bpf/probe.o
{{ end }}
18 changes: 8 additions & 10 deletions pkg/driverbuilder/builder/templates/almalinux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ rm -Rf /tmp/module-download
mkdir -p /tmp/module-download

curl --silent -SL {{ .ModuleDownloadURL }} | tar -xzf - -C /tmp/module-download
mv /tmp/module-download/*/driver/* {{ .DriverBuildDir }}

cp /driverkit/module-Makefile {{ .DriverBuildDir }}/Makefile
bash /driverkit/fill-driver-config.sh {{ .DriverBuildDir }}
mv /tmp/module-download/*/* {{ .DriverBuildDir }}

# Fetch the kernel
mkdir /tmp/kernel-download
Expand All @@ -42,19 +39,20 @@ rm -Rf /tmp/kernel
mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

cd {{ .DriverBuildDir }}
mkdir -p build && cd build
{{ .CmakeCmd }}

{{ if .BuildModule }}
# Build the module
cd {{ .DriverBuildDir }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel
mv {{ .ModuleDriverName }}.ko {{ .ModuleFullPath }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel driver
strip -g {{ .ModuleFullPath }}
# Print results
modinfo {{ .ModuleFullPath }}
{{ end }}

{{ if .BuildProbe }}
# Build the eBPF probe
cd {{ .DriverBuildDir }}/bpf
make KERNELDIR=/tmp/kernel
ls -l probe.o
make KERNELDIR=/tmp/kernel bpf
ls -l driver/bpf/probe.o
{{ end }}
20 changes: 9 additions & 11 deletions pkg/driverbuilder/builder/templates/amazonlinux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ rm -Rf /tmp/module-download
mkdir -p /tmp/module-download

curl --silent -SL {{ .ModuleDownloadURL }} | tar -xzf - -C /tmp/module-download
mv /tmp/module-download/*/driver/* {{ .DriverBuildDir }}

cp /driverkit/module-Makefile {{ .DriverBuildDir }}/Makefile
bash /driverkit/fill-driver-config.sh {{ .DriverBuildDir }}
mv /tmp/module-download/*/* {{ .DriverBuildDir }}

# Fetch the kernel
mkdir /tmp/kernel-download
Expand All @@ -45,19 +42,20 @@ rm -Rf /tmp/kernel
mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

{{ if .BuildModule }}
# Build the kernel module
cd {{ .DriverBuildDir }}
mkdir -p build && cd build
{{ .CmakeCmd }}

make KERNELDIR=/tmp/kernel CC=/usr/bin/gcc-{{ .GCCVersion }} LD=/usr/bin/ld.bfd CROSS_COMPILE=""
mv {{ .ModuleDriverName }}.ko {{ .ModuleFullPath }}
{{ if .BuildModule }}
# Build the module
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel LD=/usr/bin/ld.bfd CROSS_COMPILE="" driver
strip -g {{ .ModuleFullPath }}
# Print results
modinfo {{ .ModuleFullPath }}
{{ end }}

{{ if .BuildProbe }}
# Build the eBPF probe
cd {{ .DriverBuildDir }}/bpf
make KERNELDIR=/tmp/kernel
ls -l probe.o
make KERNELDIR=/tmp/kernel bpf
ls -l driver/bpf/probe.o
{{ end }}
18 changes: 8 additions & 10 deletions pkg/driverbuilder/builder/templates/archlinux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ rm -Rf /tmp/module-download
mkdir -p /tmp/module-download

curl --silent -SL {{ .ModuleDownloadURL }} | tar -xzf - -C /tmp/module-download
mv /tmp/module-download/*/driver/* {{ .DriverBuildDir }}

cp /driverkit/module-Makefile {{ .DriverBuildDir }}/Makefile
bash /driverkit/fill-driver-config.sh {{ .DriverBuildDir }}
mv /tmp/module-download/*/* {{ .DriverBuildDir }}

# Fetch the kernel
mkdir /tmp/kernel-download
Expand All @@ -42,19 +39,20 @@ rm -Rf /tmp/kernel
mkdir -p /tmp/kernel
mv usr/lib/modules/*/build/* /tmp/kernel

cd {{ .DriverBuildDir }}
mkdir -p build && cd build
{{ .CmakeCmd }}

{{ if .BuildModule }}
# Build the module
cd {{ .DriverBuildDir }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel
mv {{ .ModuleDriverName }}.ko {{ .ModuleFullPath }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel driver
strip -g {{ .ModuleFullPath }}
# Print results
modinfo {{ .ModuleFullPath }}
{{ end }}

{{ if .BuildProbe }}
# Build the eBPF probe
cd {{ .DriverBuildDir }}/bpf
make KERNELDIR=/tmp/kernel
ls -l probe.o
make KERNELDIR=/tmp/kernel bpf
ls -l driver/bpf/probe.o
{{ end }}
20 changes: 9 additions & 11 deletions pkg/driverbuilder/builder/templates/centos.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ rm -Rf /tmp/module-download
mkdir -p /tmp/module-download

curl --silent -SL {{ .ModuleDownloadURL }} | tar -xzf - -C /tmp/module-download
mv /tmp/module-download/*/driver/* {{ .DriverBuildDir }}

cp /driverkit/module-Makefile {{ .DriverBuildDir }}/Makefile
bash /driverkit/fill-driver-config.sh {{ .DriverBuildDir }}
mv /tmp/module-download/*/* {{ .DriverBuildDir }}

# Fetch the kernel
mkdir /tmp/kernel-download
Expand All @@ -42,20 +39,21 @@ rm -Rf /tmp/kernel
mkdir -p /tmp/kernel
mv usr/src/kernels/*/* /tmp/kernel

cd {{ .DriverBuildDir }}
sed -i 's/$(MAKE) -C $(KERNELDIR)/$(MAKE) KCFLAGS="-Wno-incompatible-pointer-types" -C $(KERNELDIR)/g' driver/Makefile.in
mkdir -p build && cd build
{{ .CmakeCmd }}

{{ if .BuildModule }}
# Build the module
cd {{ .DriverBuildDir }}
sed -i 's/make -C $(KERNELDIR)/make KCFLAGS="-Wno-incompatible-pointer-types" -C $(KERNELDIR)/g' Makefile
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel
mv {{ .ModuleDriverName }}.ko {{ .ModuleFullPath }}
make CC=/usr/bin/gcc-{{ .GCCVersion }} KERNELDIR=/tmp/kernel driver
strip -g {{ .ModuleFullPath }}
# Print results
modinfo {{ .ModuleFullPath }}
{{ end }}

{{ if .BuildProbe }}
# Build the eBPF probe
cd {{ .DriverBuildDir }}/bpf
make KERNELDIR=/tmp/kernel
ls -l probe.o
make KERNELDIR=/tmp/kernel bpf
ls -l driver/bpf/probe.o
{{ end }}
Loading
Loading