Make reverse policy filter mapping optional

tpapagian · tpapagian · commit 0de3361113a1 · 2024-12-16T18:14:29.000+02:00
By adding a command line argument (and the appropriate configmap option).

Signed-off-by: Anastasios Papagiannis &lt;anastasios.papagiannis@isovalent.com&gt;
diff --git a/cmd/tetra/debug/dump.go b/cmd/tetra/debug/dump.go
@@ -200,18 +200,20 @@ func PolicyfilterState(fname string) {
 		fmt.Printf("%d: %s\n", polId, strings.Join(ids, ","))
 	}
 
-	fmt.Println("--- Reverse Map ---")
+	if data.Reverse != nil {
+		fmt.Println("--- Reverse Map ---")
 
-	if len(data.Reverse) == 0 {
-		fmt.Printf("(empty)\n")
-	}
+		if len(data.Reverse) == 0 {
+			fmt.Printf("(empty)\n")
+		}
 
-	for cgIDs, polIds := range data.Reverse {
-		ids := make([]string, 0, len(polIds))
-		for id := range polIds {
-			ids = append(ids, strconv.FormatUint(uint64(id), 10))
+		for cgIDs, polIds := range data.Reverse {
+			ids := make([]string, 0, len(polIds))
+			for id := range polIds {
+				ids = append(ids, strconv.FormatUint(uint64(id), 10))
+			}
+			fmt.Printf("%d: %s\n", cgIDs, strings.Join(ids, ","))
 		}
-		fmt.Printf("%d: %s\n", cgIDs, strings.Join(ids, ","))
 	}
 }
 
diff --git a/docs/content/en/docs/reference/helm-chart.md b/docs/content/en/docs/reference/helm-chart.md
diff --git a/docs/data/tetragon_flags.yaml b/docs/data/tetragon_flags.yaml
diff --git a/install/kubernetes/tetragon/README.md b/install/kubernetes/tetragon/README.md
diff --git a/install/kubernetes/tetragon/templates/tetragon_configmap.yaml b/install/kubernetes/tetragon/templates/tetragon_configmap.yaml
@@ -59,6 +59,9 @@ data:
 {{- if .Values.tetragon.enablePolicyFilter }}
   enable-policy-filter: "true"
 {{- end }}
+{{- if .Values.tetragon.enablePolicyFilterReverseMap }}
+  enable-policy-filter-reverse-map: "true"
+{{- end }}
 {{- if .Values.tetragon.enablePolicyFilterDebug }}
   enable-policy-filter-debug: "true"
 {{- end }}
diff --git a/install/kubernetes/tetragon/values.yaml b/install/kubernetes/tetragon/values.yaml
@@ -193,6 +193,8 @@ tetragon:
     port: 6060
   # -- Enable policy filter. This is required for K8s namespace and pod-label filtering.
   enablePolicyFilter: True
+  # -- Enable policy filter reverse map.
+  enablePolicyFilterReverseMap: false
   # -- Enable policy filter debug messages.
   enablePolicyFilterDebug: false
   # -- Enable latency monitoring in message handling
diff --git a/pkg/option/config.go b/pkg/option/config.go
@@ -77,8 +77,9 @@ type config struct {
 
 	ReleasePinned bool
 
-	EnablePolicyFilter      bool
-	EnablePolicyFilterDebug bool
+	EnablePolicyFilter           bool
+	EnablePolicyFilterReverseMap bool
+	EnablePolicyFilterDebug      bool
 
 	EnablePidSetFilter bool
 
diff --git a/pkg/option/flags.go b/pkg/option/flags.go
@@ -81,8 +81,9 @@ const (
 
 	KeyReleasePinnedBPF = "release-pinned-bpf"
 
-	KeyEnablePolicyFilter      = "enable-policy-filter"
-	KeyEnablePolicyFilterDebug = "enable-policy-filter-debug"
+	KeyEnablePolicyFilter           = "enable-policy-filter"
+	KeyEnablePolicyFilterReverseMap = "enable-policy-filter-reverse-map"
+	KeyEnablePolicyFilterDebug      = "enable-policy-filter-debug"
 
 	KeyEnablePidSetFilter = "enable-pid-set-filter"
 
@@ -202,6 +203,7 @@ func ReadAndSetFlags() error {
 
 	Config.ReleasePinned = viper.GetBool(KeyReleasePinnedBPF)
 	Config.EnablePolicyFilter = viper.GetBool(KeyEnablePolicyFilter)
+	Config.EnablePolicyFilterReverseMap = viper.GetBool(KeyEnablePolicyFilterReverseMap)
 	Config.EnablePolicyFilterDebug = viper.GetBool(KeyEnablePolicyFilterDebug)
 	Config.EnableMsgHandlingLatency = viper.GetBool(KeyEnableMsgHandlingLatency)
 
@@ -378,6 +380,7 @@ func AddFlags(flags *pflag.FlagSet) {
 	// Provide option to enable policy filtering. Because the code is new,
 	// this is set to false by default.
 	flags.Bool(KeyEnablePolicyFilter, false, "Enable policy filter code")
+	flags.Bool(KeyEnablePolicyFilterReverseMap, false, "Enable reverse mappings for policy filter maps")
 	flags.Bool(KeyEnablePolicyFilterDebug, false, "Enable policy filter debug messages")
 
 	// Provide option to enable the pidSet export filters.
diff --git a/pkg/policyfilter/k8s_test.go b/pkg/policyfilter/k8s_test.go
@@ -739,7 +739,7 @@ func TestK8s(t *testing.T) {
 
 	// testState implements cgFinder
 	ts := newTestState(client)
-	st, err := newState(log, ts)
+	st, err := newState(log, ts, true)
 	if err != nil {
 		t.Skipf("failed to initialize policy filter state: %s", err)
 	}
diff --git a/pkg/policyfilter/map.go b/pkg/policyfilter/map.go
@@ -59,7 +59,7 @@ func openMap(spec *ebpf.CollectionSpec, mapName string, innerMaxEntries uint32)
 }
 
 // newMap returns a new policy filter map.
-func newPfMap() (PfMap, error) {
+func newPfMap(enableReverseMap bool) (PfMap, error) {
 	// use the generic kprobe program, to find the policy filter map spec
 	objName, _ := kernels.GenericKprobeObjs()
 	objPath := path.Join(option.Config.HubbleLib, objName)
@@ -72,13 +72,22 @@ func newPfMap() (PfMap, error) {
 	if ret.dir, err = openMap(spec, MapName, polMapSize); err != nil {
 		return PfMap{}, fmt.Errorf("opening map %s failed: %w", MapName, err)
 	}
-	if ret.rev, err = openMap(spec, RevMapName, polMaxPolicies); err != nil {
-		return PfMap{}, fmt.Errorf("opening reverse map %s failed: %w", MapName, err)
+
+	if enableReverseMap {
+		if ret.rev, err = openMap(spec, RevMapName, polMaxPolicies); err != nil {
+			return PfMap{}, fmt.Errorf("opening reverse map %s failed: %w", MapName, err)
+		}
 	}
+
 	return ret, nil
 }
 
 func releaseMap(m *ebpf.Map) error {
+	// this may happen in the case where the reverse map is not enabled
+	if m == nil {
+		return nil
+	}
+
 	if err := m.Close(); err != nil {
 		return err
 	}
@@ -114,6 +123,11 @@ func (m polMap) addCgroupIDsReverse(polID PolicyID, cgIDs []CgroupID) error {
 }
 
 func addReverseMapping(m *ebpf.Map, polID PolicyID, cgID CgroupID) error {
+	// reverse map does not exist, so nothing to do here
+	if m == nil {
+		return nil
+	}
+
 	var id uint32
 	err := m.Lookup(cgID, &id)
 	if err == nil { // inner map exists
@@ -221,6 +235,11 @@ func getMapSize(m *ebpf.Map) (uint32, error) {
 }
 
 func (m PfMap) deletePolicyIdInReverse(polID PolicyID) error {
+	// reverse map does not exist, so nothing to do here
+	if m.rev == nil {
+		return nil
+	}
+
 	var key CgroupID
 	var id uint32
 
@@ -318,9 +337,12 @@ func (m PfMap) readAll() (PfMapDump, error) {
 		return PfMapDump{}, fmt.Errorf("error reading direct map: %w", err)
 	}
 
-	r, err := readAll[CgroupID, PolicyID](m.rev)
-	if err != nil {
-		return PfMapDump{}, fmt.Errorf("error reading reverse map: %w", err)
+	var r map[CgroupID]map[PolicyID]struct{}
+	if m.rev != nil {
+		r, err = readAll[CgroupID, PolicyID](m.rev)
+		if err != nil {
+			return PfMapDump{}, fmt.Errorf("error reading reverse map: %w", err)
+		}
 	}
 
 	return PfMapDump{Direct: d, Reverse: r}, nil
@@ -365,6 +387,11 @@ func (m polMap) addCgroupIDs(cgIDs []CgroupID) error {
 // delCgroupIDs delete cgroups ids from the policy map
 // todo: use batch operations when supported
 func (m polMap) delCgroupIDs(polID PolicyID, cgIDs []CgroupID) error {
+	// reverse map does not exist, so nothing to do here
+	if m.Reverse == nil {
+		return nil
+	}
+
 	rmRevCgIDs := []CgroupID{}
 	for i, cgID := range cgIDs {
 		if err := m.Inner.Delete(&cgID); err != nil {
@@ -428,21 +455,29 @@ func OpenMap(fname string) (PfMap, error) {
 
 	dir := filepath.Dir(fname)
 	reverseMapPath := filepath.Join(dir, RevMapName)
-	r, err := ebpf.LoadPinnedMap(reverseMapPath, &ebpf.LoadPinOptions{
-		ReadOnly: true,
-	})
 
-	if err != nil {
-		d.Close()
-		return PfMap{}, err
+	// check if the reverse map exists
+	// the reverse map may not exist in the case where
+	// enable-policy-filter-reverse-map is false
+	var r *ebpf.Map
+	if _, err := os.Stat(reverseMapPath); err == nil {
+		r, err = ebpf.LoadPinnedMap(reverseMapPath, &ebpf.LoadPinOptions{
+			ReadOnly: true,
+		})
+		if err != nil {
+			d.Close()
+			return PfMap{}, err
+		}
 	}
 
 	return PfMap{dir: d, rev: r}, err
 }
 
 func (m PfMap) Close() {
 	m.dir.Close()
-	m.rev.Close()
+	if m.rev != nil {
+		m.rev.Close()
+	}
 }
 
 func (m PfMap) Dump() (PfMapDump, error) {
diff --git a/pkg/policyfilter/map_test.go b/pkg/policyfilter/map_test.go
@@ -40,7 +40,7 @@ func TestPfMapOps(t *testing.T) {
 	if !bpffsReady {
 		t.Skip("failed to initialize bpffs")
 	}
-	pfm, err := newPfMap()
+	pfm, err := newPfMap(true)
 	require.NoError(t, err)
 	defer pfm.release()
 
diff --git a/pkg/policyfilter/policyfilter.go b/pkg/policyfilter/policyfilter.go
@@ -25,7 +25,7 @@ func GetState() (State, error) {
 	setGlobalPf.Do(func() {
 		if option.Config.EnablePolicyFilter {
 			logger.GetLogger().Info("Enabling policy filtering")
-			glblState, glblError = New()
+			glblState, glblError = New(option.Config.EnablePolicyFilterReverseMap)
 		} else {
 			glblState = &disabled{}
 			glblError = nil
@@ -45,7 +45,7 @@ func resetStateOnlyForTesting() {
 	}
 	if option.Config.EnablePolicyFilter {
 		logger.GetLogger().Info("Enabling policy filtering")
-		glblState, glblError = New()
+		glblState, glblError = New(true)
 	} else {
 		glblState = &disabled{}
 		glblError = nil
diff --git a/pkg/policyfilter/state.go b/pkg/policyfilter/state.go
@@ -269,17 +269,19 @@ type state struct {
 // allocated resources (namely the bpf map).
 //
 //revive:disable:unexported-return
-func New() (*state, error) {
+func New(enableReverseMap bool) (*state, error) {
 	log := logger.GetLogger().WithField("subsystem", "policy-filter")
 	return newState(
 		log,
 		&cgfsFinder{fsscan.New(), log},
+		enableReverseMap,
 	)
 }
 
 func newState(
 	log logrus.FieldLogger,
 	cgidFinder cgidFinder,
+	enableReverseMap bool,
 ) (*state, error) {
 	var err error
 	ret := &state{
@@ -288,7 +290,7 @@ func newState(
 		DebugLogger: logger.NewDebugLogger(log, option.Config.EnablePolicyFilterDebug),
 	}
 
-	ret.pfMap, err = newPfMap()
+	ret.pfMap, err = newPfMap(enableReverseMap)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/policyfilter/state_test.go b/pkg/policyfilter/state_test.go
@@ -11,7 +11,7 @@ import (
 )
 
 func TestState(t *testing.T) {
-	s, err := New()
+	s, err := New(true)
 	if err != nil {
 		t.Skipf("failed to inialize policy filter state: %s", err)
 	}

Original file line number	Diff line number	Diff line change
`@@ -739,7 +739,7 @@ func TestK8s(t *testing.T) {`
`739`	`739`
`740`	`740`	`// testState implements cgFinder`
`741`	`741`	`ts := newTestState(client)`
`742`		`- st, err := newState(log, ts)`
	`742`	`+ st, err := newState(log, ts, true)`
`743`	`743`	`if err != nil {`
`744`	`744`	`t.Skipf("failed to initialize policy filter state: %s", err)`
`745`	`745`	`}`