Skip to content

Commit

Permalink
tetragon: Remove user space return filter
Browse files Browse the repository at this point in the history 
Now that we have return argument filter support in kernel, we no longer
need the user space support, removing it.

Due to bpf filter program complexity we can't load GT/LT filters in 4.19
kernels, so I'm disabling GT/LT tests for 4.19 kernels. Let's see if that's
a real problem before we get to fun of mixing user and kernel filtering
for 4.19 kernels and newer ones.

Signed-off-by: Jiri Olsa <[email protected]>
  • Loading branch information
@olsajiri
olsajiri committed Dec 4, 2023
1 parent ab56a18 commit 58a36a4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 125 deletions.
129 changes: 4 additions & 125 deletions pkg/sensors/tracing/generickprobe.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"net"
"net/http"
"path"
"strconv"
"strings"

"github.com/cilium/ebpf"
Expand Down Expand Up @@ -108,11 +107,6 @@ type genericKprobe struct {
argReturnPrinters []argPrinters
funcName string

// userReturnFilters are filter specs implemented in userspace after
// receiving events on the return value. We currently use this for return
// arg filtering.
userReturnFilters []v1alpha1.ArgSelector

// for kprobes that have a retprobe, we maintain the enter events in
// the map, so that we can merge them when the return event is
// generated. The events are maintained in the map below, using
Expand Down Expand Up @@ -528,14 +522,6 @@ func flagsString(flags uint32) string {
return s
}

func isGTOperator(op string) bool {
return op == "GT" || op == "GreaterThan"
}

func isLTOperator(op string) bool {
return op == "LT" || op == "LessThan"
}

type addKprobeIn struct {
useMulti bool
sensorPath string
Expand Down Expand Up @@ -790,27 +776,6 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt
}
}

// Copy over userspace return filters
var userReturnFilters []v1alpha1.ArgSelector
for _, s := range f.Selectors {
for _, returnArg := range s.MatchReturnArgs {
// we allow integer values so far
for _, v := range returnArg.Values {
if _, err := strconv.Atoi(v); err != nil {
return errFn(fmt.Errorf("ReturnArg value supports only integer values, got %s", v))
}
}
// only single value for GT,LT operators
if isGTOperator(returnArg.Operator) || isLTOperator(returnArg.Operator) {
if len(returnArg.Values) > 1 {
return errFn(fmt.Errorf("ReturnArg operater '%s' supports only single value, got %d",
returnArg.Operator, len(returnArg.Values)))
}
}
userReturnFilters = append(userReturnFilters, returnArg)
}
}

// Write attributes into BTF ptr for use with load
if !setRetprobe {
setRetprobe = f.Return
Expand All @@ -836,7 +801,6 @@ func addKprobe(funcName string, f *v1alpha1.KProbeSpec, in *addKprobeIn) (id idt
},
argSigPrinters: argSigPrinters,
argReturnPrinters: argReturnPrinters,
userReturnFilters: userReturnFilters,
funcName: funcName,
pendingEvents: nil,
tableId: idtable.UninitializedEntryID,
Expand Down Expand Up @@ -1609,7 +1573,6 @@ func handleMsgGenericKprobe(m *api.MsgGenericKprobe, gk *genericKprobe, r *bytes

// Cache return value on merge and run return filters below before
// passing up to notify hooks.
var retArg *api.MsgGenericKprobeArg

// there are two events for this probe (entry and return)
if gk.loadArgs.retprobe {
Expand All @@ -1620,7 +1583,7 @@ func handleMsgGenericKprobe(m *api.MsgGenericKprobe, gk *genericKprobe, r *bytes

if prev, exists := gk.pendingEvents.Get(key); exists {
gk.pendingEvents.Remove(key)
unix, retArg = retprobeMerge(prev, curr)
unix = retprobeMerge(prev, curr)
} else {
gk.pendingEvents.Add(key, curr)
kprobemetrics.MergePushedInc()
Expand All @@ -1636,92 +1599,10 @@ func handleMsgGenericKprobe(m *api.MsgGenericKprobe, gk *genericKprobe, r *bytes
// a filter we wouldn't be able to cleanup initial event from entry.
// Alternatively, some actions have no kernel analog, such as pause
// pod.
if filterReturnArg(gk.userReturnFilters, retArg) {
return []observer.Event{}, err
}

return []observer.Event{unix}, err
}

func filterReturnArg(userReturnFilters []v1alpha1.ArgSelector, retArg *api.MsgGenericKprobeArg) bool {
// Short circuit, returnFilter indicates we should eat this event.
if retArg == nil {
return false
}

// If no filters are specified default to allow.
if len(userReturnFilters) == 0 {
return false
}

// Multiple selectors will be logical OR together.
for _, uFilter := range userReturnFilters {
// MatchPIDs only supported in kernel space because we have
// full support back to 4.14 kernels.

// MatchArgs handlers, uFilters only necessary for return
// arg filters at the moment. Also we simply assume its an
// int which is naive, but good enough someone should devote
// more time to make this amazing tech(tm).
switch uFilter.Operator {
case "Equal":
// If retarg Equals any value in the set {Values} accept event
for _, v := range uFilter.Values {
if vint, err := strconv.Atoi(v); err == nil {
switch compare := (*retArg).(type) {
case api.MsgGenericKprobeArgInt:
if vint == int(compare.Value) {
return false
}
}
}
}
case "NotEqual":
inSet := false
for _, v := range uFilter.Values {
if vint, err := strconv.Atoi(v); err == nil {
switch compare := (*retArg).(type) {
case api.MsgGenericKprobeArgInt:
if vint == int(compare.Value) {
inSet = true
}
}
}
}
// If retarg was not in set {Values} accept event
if !inSet {
return false
}
}
if isGTOperator(uFilter.Operator) {
for _, v := range uFilter.Values {
if vint, err := strconv.Atoi(v); err == nil {
switch compare := (*retArg).(type) {
case api.MsgGenericKprobeArgInt:
if vint < int(compare.Value) {
return false
}
}
}
}
}
if isLTOperator(uFilter.Operator) {
for _, v := range uFilter.Values {
if vint, err := strconv.Atoi(v); err == nil {
switch compare := (*retArg).(type) {
case api.MsgGenericKprobeArgInt:
if vint > int(compare.Value) {
return false
}
}
}
}
}
}
// We walked all selectors and no selectors matched, eat the event.
return true
}

func reportMergeError(curr pendingEvent, prev pendingEvent) {
currFn := "UNKNOWN"
if curr.ev != nil {
Expand Down Expand Up @@ -1751,9 +1632,8 @@ func reportMergeError(curr pendingEvent, prev pendingEvent) {
}

// retprobeMerge merges the two events: the one from the entry probe with the one from the return probe
func retprobeMerge(prev pendingEvent, curr pendingEvent) (*tracing.MsgGenericKprobeUnix, *api.MsgGenericKprobeArg) {
func retprobeMerge(prev pendingEvent, curr pendingEvent) *tracing.MsgGenericKprobeUnix {
var retEv, enterEv *tracing.MsgGenericKprobeUnix
var ret *api.MsgGenericKprobeArg

if prev.returnEvent && !curr.returnEvent {
retEv = prev.ev
Expand All @@ -1763,7 +1643,7 @@ func retprobeMerge(prev pendingEvent, curr pendingEvent) (*tracing.MsgGenericKpr
enterEv = prev.ev
} else {
reportMergeError(curr, prev)
return nil, nil
return nil
}

kprobemetrics.MergeOkTotalInc()
Expand All @@ -1774,10 +1654,9 @@ func retprobeMerge(prev pendingEvent, curr pendingEvent) (*tracing.MsgGenericKpr
enterEv.Args[index] = retArg
} else {
enterEv.Args = append(enterEv.Args, retArg)
ret = &retArg
}
}
return enterEv, ret
return enterEv
}

func (k *observerKprobeSensor) LoadProbe(args sensors.LoadProbeArgs) error {
Expand Down
12 changes: 12 additions & 0 deletions pkg/sensors/tracing/kprobe_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1491,6 +1491,9 @@ func testKprobeObjectFilteredReturnValue(t *testing.T,
}

func TestKprobeObjectFilterReturnValueGTOk(t *testing.T) {
if !kernels.EnableLargeProgs() {
t.Skip("Older kernels do not support GT/LT matching")
}
pidStr := strconv.Itoa(int(observertesthelper.GetMyPid()))
dir := t.TempDir()
path := dir + "/testfile"
Expand Down Expand Up @@ -1524,6 +1527,9 @@ func TestKprobeObjectFilterReturnValueGTOk(t *testing.T) {
}

func TestKprobeObjectFilterReturnValueGTFail(t *testing.T) {
if !kernels.EnableLargeProgs() {
t.Skip("Older kernels do not support GT/LT matching")
}
pidStr := strconv.Itoa(int(observertesthelper.GetMyPid()))
dir := t.TempDir()
path := dir + "/testfile"
Expand All @@ -1535,6 +1541,9 @@ func TestKprobeObjectFilterReturnValueGTFail(t *testing.T) {
}

func TestKprobeObjectFilterReturnValueLTOk(t *testing.T) {
if !kernels.EnableLargeProgs() {
t.Skip("Older kernels do not support GT/LT matching")
}
pidStr := strconv.Itoa(int(observertesthelper.GetMyPid()))
dir := t.TempDir()
path := dir + "/testfile"
Expand All @@ -1559,6 +1568,9 @@ func TestKprobeObjectFilterReturnValueLTOk(t *testing.T) {
}

func TestKprobeObjectFilterReturnValueLTFail(t *testing.T) {
if !kernels.EnableLargeProgs() {
t.Skip("Older kernels do not support GT/LT matching")
}
pidStr := strconv.Itoa(int(observertesthelper.GetMyPid()))
dir := t.TempDir()
path := dir + "/testfile"
Expand Down

0 comments on commit 58a36a4

Please sign in to comment.