Skip to content

pkg/option: allow policy-filter-map-entries configurable via flag #4331

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.

Sign up for GitHub

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

Open
kyledong-suse wants to merge 2 commits into
base: main
Choose a base branch
from
Open

pkg/option: allow policy-filter-map-entries configurable via flag #4331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9dc9735
pkg/option: allow policy-filter-map-entries configurable via flag
kyledong-suse Nov 11, 2025
53bbe52
fix: fix error message typo
kyledong-suse Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bpf/process/policy_filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ struct {

struct {
__uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
__uint(max_entries, POLICY_FILTER_MAX_POLICIES);
__uint(max_entries, 1); // will be resized by agent when needed
Copy link
Member

@mtardy mtardy Nov 20, 2025

Choose a reason for hiding this comment

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

This is used by policy_filter_cgroup_maps just nearby. Maybe we would need to update both and remove that const?

Copy link
Contributor Author

@kyledong-suse kyledong-suse Nov 21, 2025

Choose a reason for hiding this comment

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

Sure, I think it makes sense to me. I'll update both of them and remove that const.

Copy link
Contributor Author

@kyledong-suse kyledong-suse Dec 1, 2025

Choose a reason for hiding this comment

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

@mtardy Given another think, the POLICY_FILTER_MAX_POLICIES in policy_filter_cgroup_maps represents how many policies can apply to a single cgroup (inner map size of policy_filter_cgroup_maps).
I’m fine to simplify and drive both policy_filter_maps and policy_filter_cgroup_maps from the same policy-filter-map-entries setting and remove that const. However, I just want you to aware that if we want to update both, it would be mixing a global capacity knob with a per-cgroup limit, which could waste memory if we config more policies globally.

Copy link
Contributor Author

@kyledong-suse kyledong-suse Dec 10, 2025

Choose a reason for hiding this comment

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

@mtardy Please feel free to add you comment. Thanks

Copy link
Contributor

@kkourt kkourt Dec 16, 2025

Choose a reason for hiding this comment

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

I’m fine to simplify and drive both policy_filter_maps and policy_filter_cgroup_maps from the same policy-filter-map-entries setting and remove that const. However, I just want you to aware that if we want to update both, it would be mixing a global capacity knob with a per-cgroup limit, which could waste memory if we config more policies globally.

My reading is that that policy_filter_cgroup_maps uses POLICY_FILTER_MAX_POLICIES since it's the most pessimistic approach. This means, that there will be no capacity issues (i.e., a failure to insert) in policy_filter_cgroup_maps without hitting a capacity issue in policy_filter_maps.

I think there are three options here:

  1. Have the run-time size to determine the size of policy_filter_maps and policy_filter_cgroup_maps. That is, have the run-time size instead of POLICY_FILTER_MAX_POLICIES.
  2. Have a different run-time size for both. In this case, we would need to check the error path of when the capacity of policy_filter_cgroup_maps is not enough and do something reasonable. This can be a follow-up PR.
  3. Leave it as is. In this case, we would still need to check the error path since we might end up with a case where the capacity of policy_filter_cgroup_maps is not enough.

Note that option (2.) can be done in a followup PR after (1.).

CCing @tpapagian who I believe introduced this change.

Copy link
Contributor Author

@kyledong-suse kyledong-suse Dec 18, 2025

Choose a reason for hiding this comment

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

My preference would be to leave it as-is for now, and keep policy_filter_cgroup_maps sized using POLICY_FILTER_MAX_POLICIES.

A few reasons for this:

  • Using POLICY_FILTER_MAX_POLICIES for policy_filter_cgroup_maps is intentionally pessimistic and gives us a safety guarantee.
  • While driving both maps from a single run-time knob is appealing, it mixes two different semantics: a global policy capacity, and a per-cgroup upper bound. This coupling can indeed lead to wasted memory when the global configuration is large, and I’m not fully convinced that trade-off is worth it yet.
  • Introducing separate run-time sizing (option 2) would require us to define and carefully handle a new failure mode (partial success where the global map accepts entries but the cgroup map does not). That feels like a larger behavioural change than this PR intends.

I agree that we should eventually harden the error path around policy_filter_cgroup_maps, but I think that can be done independently of changing sizing semantics, and safely as a follow-up.

__type(key, u32); /* policy id */
__array(
values, struct {
Expand Down
4 changes: 4 additions & 0 deletions docs/data/tetragon_flags.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/defaults/defaults.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ const (

// defaults for the {k,u}retprobes lru cache
DefaultRetprobesCacheSize = 4096

// defaults for the policy filter map
DefaultPolicyFilterMapEntries = 128
)

var (
Expand Down
3 changes: 3 additions & 0 deletions pkg/defaults/defaults_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,7 @@ const (

// defaults for the {k,u}retprobes lru cache
DefaultRetprobesCacheSize = 4096

// defaults for the policy filter map
DefaultPolicyFilterMapEntries = 128
)
5 changes: 5 additions & 0 deletions pkg/option/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ type config struct {
ExecveMapSize string

RetprobesCacheSize int

PolicyFilterMapEntries int
}

var (
Expand All @@ -159,6 +161,9 @@ var (

// Set default value for {k,u}retprobes lru events cache
RetprobesCacheSize: defaults.DefaultRetprobesCacheSize,

// set default value for the policy filter map
PolicyFilterMapEntries: defaults.DefaultPolicyFilterMapEntries,
}
)

Expand Down
6 changes: 6 additions & 0 deletions pkg/option/flags.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ const (
KeyExecveMapSize = "execve-map-size"

KeyRetprobesCacheSize = "retprobes-cache-size"

KeyPolicyFilterMapEntries = "policy-filter-map-entries"
)

type UsernameMetadaCode int
Expand Down Expand Up @@ -305,6 +307,8 @@ func ReadAndSetFlags() error {
Config.ExecveMapSize = viper.GetString(KeyExecveMapSize)

Config.RetprobesCacheSize = viper.GetInt(KeyRetprobesCacheSize)

Config.PolicyFilterMapEntries = viper.GetInt(KeyPolicyFilterMapEntries)
Copy link
Contributor

@kkourt kkourt Jan 7, 2026

Choose a reason for hiding this comment

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

Given the discussion in 9dc9735#r2629371514, should we reject values that are larger than larger than POLICY_FILTER_MAX_POLICIES?

Copy link
Contributor Author

@kyledong-suse kyledong-suse Jan 15, 2026

Choose a reason for hiding this comment

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

I don’t think we should reject values larger than POLICY_FILTER_MAX_POLICIES. The goal of this PR is to allow users to configure the number of entries in the policy filter maps. The original limit of 128 is sometimes too small, and users may legitimately have more policies in their maps.

Copy link
Contributor

@kkourt kkourt Jan 23, 2026

Choose a reason for hiding this comment

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

OK, but given that we have:

// This map keeps exactly the same information as policy_filter_maps
// but keeps the reverse mappings. i.e.
// policy_filter_maps maps policy_id to cgroup_ids
// policy_filter_cgroup_maps maps cgroup_id to policy_ids
struct {
	__uint(type, BPF_MAP_TYPE_HASH_OF_MAPS);
	__uint(max_entries, POLICY_FILTER_MAX_CGROUP_IDS);
	__type(key, __u64); /* cgroup id */
	__array(
		values, struct {
			__uint(type, BPF_MAP_TYPE_HASH);
			__uint(max_entries, POLICY_FILTER_MAX_POLICIES);
			__type(key, __u32); /* policy id */
			__type(value, __u8); /* empty  */
		});
} policy_filter_cgroup_maps SEC(".maps");

Doesn't this mean that for a value larger than POLICY_FILTER_MAX_POLICIES, things will not work as expected when considering the policy_filter_cgoup_maps?

return nil
}

Expand Down Expand Up @@ -504,4 +508,6 @@ func AddFlags(flags *pflag.FlagSet) {
flags.String(KeyExecveMapSize, "", "Set size for execve_map table (allows K/M/G suffix)")

flags.Int(KeyRetprobesCacheSize, defaults.DefaultRetprobesCacheSize, "Set {k,u}retprobes events cache maximum size")

flags.Int(KeyPolicyFilterMapEntries, defaults.DefaultPolicyFilterMapEntries, "Set maximum number of policies in policy_filter_maps. This map restricts tracing policies to specific pods/containers. Increase if you have many policies, decrease to save memory if you have few policies.")
}
7 changes: 6 additions & 1 deletion pkg/policyfilter/map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (

"github.com/cilium/tetragon/pkg/bpf"
"github.com/cilium/tetragon/pkg/config"
"github.com/cilium/tetragon/pkg/option"
)

const (
Expand Down Expand Up @@ -72,6 +73,10 @@ func newPfMap(enableCgroupMap bool) (PfMap, error) {
return PfMap{}, fmt.Errorf("loading spec for %s failed: %w", objPath, err)
}

if _, ok := spec.Maps["policy_filter_maps"]; ok {
spec.Maps["policy_filter_maps"].MaxEntries = uint32(option.Config.PolicyFilterMapEntries)
}

var ret PfMap
if ret.policyMap, err = openMap(spec, MapName, polMapSize); err != nil {
return PfMap{}, fmt.Errorf("opening map %s failed: %w", MapName, err)
Expand All @@ -80,7 +85,7 @@ func newPfMap(enableCgroupMap bool) (PfMap, error) {
if enableCgroupMap {
if ret.cgroupMap, err = openMap(spec, CgroupMapName, polMaxPolicies); err != nil {
releaseMap(ret.policyMap)
return PfMap{}, fmt.Errorf("opening cgroup map %s failed: %w", MapName, err)
return PfMap{}, fmt.Errorf("opening cgroup map %s failed: %w", CgroupMapName, err)
}
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/sensors/program/loader_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
cachedbtf "github.com/cilium/tetragon/pkg/btf"
"github.com/cilium/tetragon/pkg/logger"
"github.com/cilium/tetragon/pkg/logger/logfields"
"github.com/cilium/tetragon/pkg/option"
"github.com/cilium/tetragon/pkg/sensors/unloader"
)

Expand Down Expand Up @@ -961,6 +962,11 @@ func doLoadProgram(
}
}

// TODO: remove this special case handling (see #4398)
Copy link
Contributor

@olsajiri olsajiri Jan 16, 2026

Choose a reason for hiding this comment

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

this is bad.. would it help moving this all under program.Map ? there's draft PR for that already #4501 @sayboras

Copy link
Member

@mtardy mtardy Jan 16, 2026

Choose a reason for hiding this comment

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

Yeah, it was discussed here for info #4331 (comment)

Copy link
Member

@sayboras sayboras Jan 27, 2026

Choose a reason for hiding this comment

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

oh right, I was waiting for this to land in first, just to avoid any potential conflict here.

if ms, ok := spec.Maps["policy_filter_maps"]; ok {
ms.MaxEntries = uint32(option.Config.PolicyFilterMapEntries)
}

// Find all the maps referenced by the program, so we'll rewrite only
// the ones used.
var progSpec *ebpf.ProgramSpec
Expand Down
Loading