-
Notifications
You must be signed in to change notification settings - Fork 389
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
tetragon: map changes #3328
base: main
Are you sure you want to change the base?
tetragon: map changes #3328
Conversation
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
e6455a4
to
96eade9
Compare
96eade9
to
5070379
Compare
02a5e54
to
da0b16e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good! Just some nits or questions.
pkg/sensors/program/map.go
Outdated
@@ -105,7 +105,7 @@ type Map struct { | |||
} | |||
|
|||
func (m *Map) String() string { | |||
return fmt.Sprintf("Map{Name:%s PinPath:%s}", m.Name, m.PinPath) | |||
return fmt.Sprintf("Map{Name:%s PinPath:%s Owner: %t}", m.Name, m.PinPath, m.IsOwner()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, very tiny nit I just noticed:
return fmt.Sprintf("Map{Name:%s PinPath:%s Owner: %t}", m.Name, m.PinPath, m.IsOwner()) | |
return fmt.Sprintf("Map{Name:%s PinPath:%s Owner:%t}", m.Name, m.PinPath, m.IsOwner()) |
@@ -118,6 +118,8 @@ const ( | |||
KeyEventCacheRetryDelay = "event-cache-retry-delay" | |||
|
|||
KeyCompatibilitySyscall64SizeType = "enable-compatibility-syscall64-size-type" | |||
|
|||
KeyExecveMapEntries = "execve-map-entries" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regarding naming, fyi that we have something called --process-cache-size
and other size
related option, so maybe we should do --execve-map-size
or --execve-map-max-entries
. Unsure it's relevant.
pkg/option/flags.go
Outdated
@@ -419,4 +422,6 @@ func AddFlags(flags *pflag.FlagSet) { | |||
flags.Int(KeyEventCacheRetryDelay, defaults.DefaultEventCacheRetryDelay, "Delay in seconds between event cache retries") | |||
|
|||
flags.Bool(KeyCompatibilitySyscall64SizeType, false, "syscall64 type will produce output of type size (compatibility flag, will be removed in v1.4)") | |||
|
|||
flags.String(KeyExecveMapEntries, "32768", "Set entries for execve_map table (entries/size/max)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we reuse ExecveMapMaxEntries
here? Or put the values in defaults
and reuse it in pkg/sensors/base/base.go
?
Or put an empty string given your default is based on custom logic, idk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and not sure why you wrote ah I understand now seeing the code, could you maybe be a bit more explicit in the help doc? :)(entries/size/max)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add the extra option
pkg/option/flags.go
Outdated
@@ -252,6 +254,7 @@ func ReadAndSetFlags() error { | |||
|
|||
Config.CompatibilitySyscall64SizeType = viper.GetBool(KeyCompatibilitySyscall64SizeType) | |||
|
|||
Config.ExecveMapEntries = viper.GetString(KeyExecveMapEntries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proposition: maybe you could call GetExecveMapEntries
so that you do the flag parsing here and you have a number in Config.ExecveMapEntries?
Passing sensors maps to program loader so the loader has access to all sensor maps. Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding Maps array to loader LoadOpts object as additional source of user maps for loader maps resolving. This will allow program loader to use user maps pinned to another program and will ease up maps sharing for sensors in following changes. At the moment for simplicity we will pass all the sensors maps, so we filter out user maps before using the array. Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Now that the loader can access all sensor maps we no longer need to pin user maps to programs. Also for static maps we can add new program.MapUserFrom interface where it's enough just to pass map pointer to create its user map. Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding owner info when printing map object, like: Map{Name:m1 PinPath:m1 Owner: false} Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding support to allow to setup execve_map max entries, so we can control the size of this map. Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Adding --execve-map-entries and --execve-map-size options to setup entries of execve_map map. It's possible to setup execve_map entries directly with: --execve-map-entries 100 or just specify the size of the map with: --execve-map-size 100M Adding log line that shows on startup the execve_map entries setup: time="2025-02-11T10:57:06Z" level=info msg="Set execve_map entries 118082" size=99M Signed-off-by: Jiri Olsa <jolsa@kernel.org>
da0b16e
to
f52b3ee
Compare
Adding execve-map-entries option to setup entries of execve_map map, plus related loader changes.