policy counters #4074
policy counters #4074
Conversation
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
kkourt
added
the
release-note/minor
kkourt
force-pushed
the
pr/kkourt/policy-counters
branch
4 times, most recently
from
47d8e5b to
06afe8d
Compare
Add a per-policy bpf map to maintain action statistics. For now, we keep counters for the following events: POLICY_POST: policy posted an event POLICY_SIGNAL: policy sent a signal POLICY_MONITOR_SIGNAL: policy did not sent a signal because it was in monitor mode POLICY_OVERRIDE: policy overrode a return value POLICY_MONITOR_OVERRIDE: policy did not overrode a return value because it was in monitor mode POLICY_NOTIFY_ENFORCER: policy notified the enforcer POLICY_MONITOR_NOTIFY_ENFORCER: policy did not notify the enforcer because it was in monitor mode Note that the enforcing actions have two variants: one where the enforcement action happened, and one when the enforcement did not happen because the policy was in monitor mode. Signed-off-by: Kornilios Kourtis <[email protected]>
Add user-space code to read policystats from the bpf map introduced in the previous commit. Signed-off-by: Kornilios Kourtis <[email protected]>
This is a minor fix: use the constant for the name. Signed-off-by: Kornilios Kourtis <[email protected]>
This commit intializes the policy stats map when creating the tracing sensor. Signed-off-by: Kornilios Kourtis <[email protected]>
Add tracing policy statistics. For now, they only include action counters. Signed-off-by: Kornilios Kourtis <[email protected]>
Signed-off-by: Kornilios Kourtis <[email protected]>
Add policy stats to tracingpolicy list commands. Signed-off-by: Kornilios Kourtis <[email protected]>
Note that the printed stats are aggregated into post, enforcement, and monitor counters. Signed-off-by: Kornilios Kourtis <[email protected]>
kkourt
force-pushed
the
pr/kkourt/policy-counters
branch
from
06afe8d to
091a40d
Compare
| PolicyActionsNr = 8 | ||
| ) | ||
|
|
||
| type PolicyStats struct { |
There was a problem hiding this comment.
Nit: It could make sense to compare struct policy_stats and PolicyStats in alignchecker. This could help to avoid adding things and forget to update one of the structs.
There was a problem hiding this comment.
+1 this needs to be added to alignchecker imo
| }; | ||
|
|
||
| struct { | ||
| __uint(type, BPF_MAP_TYPE_ARRAY); |
There was a problem hiding this comment.
Thinking out loud: Could it make sense to user a per-cpu array instead to avoid contention with lock_add? Possibly not as generating events is not the common path?
| PolicyActionsNr = 8 | ||
| ) | ||
|
|
||
| type PolicyStats struct { |
There was a problem hiding this comment.
+1 this needs to be added to alignchecker imo
| type PolicyAction uint8 | ||
|
|
||
| const ( | ||
| // NB: values below should match the ones in bpf/lib/policy_conf.h |
There was a problem hiding this comment.
This comment says they need to match policy_conf.h, but the actual file where they're defined is policy_stats.h
| @@ -71,7 +71,7 @@ func (pi *policyInfo) policyConfMap(prog *program.Program) *program.Map { | |||
| if pi.policyConf != nil { | |||
| return program.MapUserFrom(pi.policyConf) | |||
| } | |||
| pi.policyConf = program.MapBuilderPolicy("policy_conf", prog) | |||
| pi.policyConf = program.MapBuilderPolicy(policyconf.PolicyConfMapName, prog) | |||
There was a problem hiding this comment.
Minor nit (not related to this line but to the commit itself). The commit has a typo in the title. policyfonf -> policyconf
| message TracingPolicyActionCounters { | ||
| // number of post events generated from the policy | ||
| uint64 post = 1; | ||
| // number of signals sent from the policy | ||
| uint64 signal = 2; | ||
| // number of signals that were not sent because the policy was in monitor mode | ||
| uint64 monitor_signal = 3; | ||
| // number of return overrides | ||
| uint64 override = 4; | ||
| // number of return overrides that did not occur because the policy was in monitor mode | ||
| uint64 monitor_override = 5; | ||
| // number of enforcer notifications triggered from the policy | ||
| uint64 notify_enforcer = 6; | ||
| // number of enforcer notifications that did not occur because the policy was in monitor mode | ||
| uint64 monitor_notify_enforcer = 7; | ||
| } | ||
|
|
||
| message TracingPolicyStats { | ||
| TracingPolicyActionCounters action_counters = 1; | ||
| } |
There was a problem hiding this comment.
Just thinking out loud here, but maybe we want a comment somewhere in the BPF header instructing developers to make sure they update protobuf in addition to the userspace map reader when adding a new action counter.
| @@ -70,7 +71,8 @@ func PrintTracingPolicies(output io.Writer, policies []*tetragon.TracingPolicySt | |||
| } | |||
| } | |||
|
|
|||
| fmt.Fprintf(w, "%d\t%s\t%s\t%d\t%s\t%s\t%s\t%s\t\n", | |||
| counters := pol.GetStats().GetActionCounters() | |||
There was a problem hiding this comment.
Do we need nil checks here?
See commits.