From 9df64bd7329b8d73f9f1d6b3c0f232bd5f3ae735 Mon Sep 17 00:00:00 2001 From: Nadav Strahilevitz Date: Thu, 10 Oct 2024 08:45:56 +0000 Subject: [PATCH] chore(bufferdecoder): readability refactors Move some functions around, rename and improve documentation. --- pkg/bufferdecoder/decoder.go | 28 +++++++-- pkg/bufferdecoder/decoder_test.go | 2 +- pkg/bufferdecoder/eventsreader.go | 61 +++++++++----------- pkg/bufferdecoder/eventsreader_bench_test.go | 8 +-- pkg/bufferdecoder/eventsreader_test.go | 6 +- pkg/ebpf/c/common/buffer.h | 2 +- pkg/ebpf/capture.go | 2 +- 7 files changed, 60 insertions(+), 49 deletions(-) diff --git a/pkg/bufferdecoder/decoder.go b/pkg/bufferdecoder/decoder.go index ef5f5f2d3c8b..f0e831615530 100644 --- a/pkg/bufferdecoder/decoder.go +++ b/pkg/bufferdecoder/decoder.go @@ -39,8 +39,14 @@ func (decoder *EbpfDecoder) BuffLen() int { return len(decoder.buffer) } -// ReadAmountBytes returns the total amount of bytes that decoder has read from its buffer up until now. -func (decoder *EbpfDecoder) ReadAmountBytes() int { +// BytesRead returns the total amount of bytes that decoder has read from its buffer up until now. +func (decoder *EbpfDecoder) BytesRead() int { + return decoder.cursor +} + +// MoveCursor moves the buffer cursor over n bytes. Returns the new cursor position. +func (decoder *EbpfDecoder) MoveCursor(n int) int { + decoder.cursor += n return decoder.cursor } @@ -107,7 +113,7 @@ func (decoder *EbpfDecoder) DecodeArguments(args []trace.Argument, argnum int, e args[idx] = arg } - // Fill missing arguments metadata + // Fill missing arguments for i := 0; i < len(evtParams); i++ { if args[i].Value == nil { args[i].ArgMeta = evtParams[i] @@ -260,8 +266,20 @@ func (decoder *EbpfDecoder) DecodeBytes(msg []byte, size int) error { return nil } -// DecodeIntArray translate from the decoder buffer, starting from the decoder cursor, to msg, size * 4 bytes (in order to get int32). -func (decoder *EbpfDecoder) DecodeIntArray(msg []int32, size int) error { +// ReadBytesLen is a helper which allocates a known size bytes buffer and decodes +// the bytes from the buffer into it. +func (decoder *EbpfDecoder) ReadBytesLen(len int) ([]byte, error) { + var err error + res := make([]byte, len) + err = decoder.DecodeBytes(res[:], len) + if err != nil { + return nil, errfmt.Errorf("error reading byte array: %v", err) + } + return res, nil +} + +// DecodeInt32Array translate from the decoder buffer, starting from the decoder cursor, to msg, size * 4 bytes (in order to get int32). +func (decoder *EbpfDecoder) DecodeInt32Array(msg []int32, size int) error { offset := decoder.cursor if len(decoder.buffer[offset:]) < size*4 { return ErrBufferTooShort diff --git a/pkg/bufferdecoder/decoder_test.go b/pkg/bufferdecoder/decoder_test.go index 6d7900b6ff3c..78beb51dbaf3 100644 --- a/pkg/bufferdecoder/decoder_test.go +++ b/pkg/bufferdecoder/decoder_test.go @@ -354,7 +354,7 @@ func TestDecodeIntArray(t *testing.T) { raw = append(raw, 1, 2, 3, 4, 5, 6, 7, 8) decoder := New(raw) var obtained [2]int32 - err := decoder.DecodeIntArray(obtained[:], 2) + err := decoder.DecodeInt32Array(obtained[:], 2) assert.Equal(t, nil, err) rawcp := append(raw, 1, 2, 3, 4, 5, 6, 7, 8) dataBuff := bytes.NewBuffer(rawcp) diff --git a/pkg/bufferdecoder/eventsreader.go b/pkg/bufferdecoder/eventsreader.go index 4e666cc97bd8..22492161bbb6 100644 --- a/pkg/bufferdecoder/eventsreader.go +++ b/pkg/bufferdecoder/eventsreader.go @@ -32,7 +32,7 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, params []trace.A return 0, arg, errfmt.Errorf("invalid arg index %d", argIdx) } arg.ArgMeta = params[argIdx] - argType := GetParamType(arg.Type) + argType := GetDecodeType(arg.Type) switch argType { case trace.U8_T: @@ -76,23 +76,22 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, params []trace.A case trace.STR_T: res, err = readStringFromBuff(ebpfMsgDecoder) case trace.STR_ARR_T: - // TODO optimization: create slice after getting arrLen - var ss []string var arrLen uint8 err = ebpfMsgDecoder.DecodeUint8(&arrLen) if err != nil { return uint(argIdx), arg, errfmt.Errorf("error reading string array number of elements: %v", err) } + strSlice := make([]string, 0, arrLen) for i := 0; i < int(arrLen); i++ { s, err := readStringFromBuff(ebpfMsgDecoder) if err != nil { return uint(argIdx), arg, errfmt.Errorf("error reading string element: %v", err) } - ss = append(ss, s) + strSlice = append(strSlice, s) } - res = ss + res = strSlice case trace.ARGS_ARR_T: - var ss []string + var strSlice []string var arrLen uint32 var argNum uint32 @@ -104,18 +103,18 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, params []trace.A if err != nil { return uint(argIdx), arg, errfmt.Errorf("error reading args number: %v", err) } - resBytes, err := ReadByteSliceFromBuff(ebpfMsgDecoder, int(arrLen)) + resBytes, err := ebpfMsgDecoder.ReadBytesLen(int(arrLen)) if err != nil { return uint(argIdx), arg, errfmt.Errorf("error reading args array: %v", err) } - ss = strings.Split(string(resBytes), "\x00") - if ss[len(ss)-1] == "" { - ss = ss[:len(ss)-1] + strSlice = strings.Split(string(resBytes), "\x00") + if strSlice[len(strSlice)-1] == "" { + strSlice = strSlice[:len(strSlice)-1] } - for int(argNum) > len(ss) { - ss = append(ss, "?") + for int(argNum) > len(strSlice) { + strSlice = append(strSlice, "?") } - res = ss + res = strSlice case trace.BYTES_T: var size uint32 err = ebpfMsgDecoder.DecodeUint32(&size) @@ -126,10 +125,10 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, params []trace.A if size > 4096 && (id < events.NetPacketBase || id > events.MaxNetID) { return uint(argIdx), arg, errfmt.Errorf("byte array size too big: %d", size) } - res, err = ReadByteSliceFromBuff(ebpfMsgDecoder, int(size)) + res, err = ebpfMsgDecoder.ReadBytesLen(int(size)) case trace.INT_ARR_2_T: var intArray [2]int32 - err = ebpfMsgDecoder.DecodeIntArray(intArray[:], 2) + err = ebpfMsgDecoder.DecodeInt32Array(intArray[:], 2) if err != nil { return uint(argIdx), arg, errfmt.Errorf("error reading int elements: %v", err) } @@ -162,7 +161,7 @@ func readArgFromBuff(id events.ID, ebpfMsgDecoder *EbpfDecoder, params []trace.A return uint(argIdx), arg, nil } -func GetParamType(paramType string) trace.DecodeAs { +func GetDecodeType(paramType string) trace.DecodeAs { switch paramType { case "int": return trace.INT_T @@ -225,7 +224,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error char sun_path[108]; // Pathname }; */ - sunPath, err := readStringVarFromBuff(ebpfMsgDecoder, 108) + sunPath, err := readVarStringFromBuffer(ebpfMsgDecoder, 108) if err != nil { return nil, errfmt.Errorf("error parsing sockaddr_un: %v", err) } @@ -255,7 +254,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error return nil, errfmt.Errorf("error parsing sockaddr_in: %v", err) } res["sin_addr"] = PrintUint32IP(addr) - _, err := ReadByteSliceFromBuff(ebpfMsgDecoder, 8) + _, err := ebpfMsgDecoder.ReadBytesLen(8) if err != nil { return nil, errfmt.Errorf("error parsing sockaddr_in: %v", err) } @@ -286,7 +285,7 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error return nil, errfmt.Errorf("error parsing sockaddr_in6: %v", err) } res["sin6_flowinfo"] = strconv.Itoa(int(flowinfo)) - addr, err := ReadByteSliceFromBuff(ebpfMsgDecoder, 16) + addr, err := ebpfMsgDecoder.ReadBytesLen(16) if err != nil { return nil, errfmt.Errorf("error parsing sockaddr_in6: %v", err) } @@ -301,6 +300,9 @@ func readSockaddrFromBuff(ebpfMsgDecoder *EbpfDecoder) (map[string]string, error return res, nil } +// readStringFromBuff reads strings from the event buffer using the following format: +// +// [32bit:string_size][string_size-1:byte_buffer][8bit:null_terminator] func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) { var err error var size uint32 @@ -311,7 +313,7 @@ func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) { if size > 4096 { return "", errfmt.Errorf("string size too big: %d", size) } - res, err := ReadByteSliceFromBuff(ebpfMsgDecoder, int(size-1)) // last byte is string terminating null + res, err := ebpfMsgDecoder.ReadBytesLen(int(size - 1)) // last byte is string terminating null defer func() { var dummy int8 err := ebpfMsgDecoder.DecodeInt8(&dummy) // discard last byte which is string terminating null @@ -325,9 +327,10 @@ func readStringFromBuff(ebpfMsgDecoder *EbpfDecoder) (string, error) { return string(res), nil } -// readStringVarFromBuff reads a null-terminated string from `buff` -// max length can be passed as `max` to optimize memory allocation, otherwise pass 0 -func readStringVarFromBuff(decoder *EbpfDecoder, max int) (string, error) { +// readVarStringFromBuffer reads a null-terminated string from the ebpf buffer where the size is not +// known. The helper will read from the buffer char-by-char until it hits the null terminator +// or the given max length. The cursor will then skip to the point in the buffer after the max length. +func readVarStringFromBuffer(decoder *EbpfDecoder, max int) (string, error) { var err error var char int8 res := make([]byte, 0, max) @@ -352,20 +355,10 @@ func readStringVarFromBuff(decoder *EbpfDecoder, max int) (string, error) { // The exact reason for this Trim is not known, so remove it for now, // since it increases processing time. // res = bytes.TrimLeft(res[:], "\000") - decoder.cursor += max - count // move cursor to the end of the buffer + decoder.MoveCursor(max - count) // skip the cursor to the desired endpoint return string(res), nil } -func ReadByteSliceFromBuff(ebpfMsgDecoder *EbpfDecoder, len int) ([]byte, error) { - var err error - res := make([]byte, len) - err = ebpfMsgDecoder.DecodeBytes(res[:], len) - if err != nil { - return nil, errfmt.Errorf("error reading byte array: %v", err) - } - return res, nil -} - // PrintUint32IP prints the IP address encoded as a uint32 func PrintUint32IP(in uint32) string { ip := make(net.IP, net.IPv4len) diff --git a/pkg/bufferdecoder/eventsreader_bench_test.go b/pkg/bufferdecoder/eventsreader_bench_test.go index 305b0c3f9d50..bd1136a69ceb 100644 --- a/pkg/bufferdecoder/eventsreader_bench_test.go +++ b/pkg/bufferdecoder/eventsreader_bench_test.go @@ -12,7 +12,7 @@ func BenchmarkReadStringVarFromBuff_ShortString(b *testing.B) { for i := 0; i < b.N; i++ { decoder := New(buffer) - str, _ = readStringVarFromBuff(decoder, max) + str, _ = readVarStringFromBuffer(decoder, max) } _ = str } @@ -24,7 +24,7 @@ func BenchmarkReadStringVarFromBuff_MediumString(b *testing.B) { for i := 0; i < b.N; i++ { decoder := New(buffer) - str, _ = readStringVarFromBuff(decoder, max) + str, _ = readVarStringFromBuffer(decoder, max) } _ = str } @@ -37,7 +37,7 @@ func BenchmarkReadStringVarFromBuff_LongString(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { decoder := New(buffer) - str, _ = readStringVarFromBuff(decoder, max) + str, _ = readVarStringFromBuffer(decoder, max) } _ = str } @@ -50,7 +50,7 @@ func BenchmarkReadStringVarFromBuff_LongStringLowMax(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { decoder := New(buffer) - str, _ = readStringVarFromBuff(decoder, max) + str, _ = readVarStringFromBuffer(decoder, max) } _ = str } diff --git a/pkg/bufferdecoder/eventsreader_test.go b/pkg/bufferdecoder/eventsreader_test.go index 5ffd7a207fbc..efbc6d28f3da 100644 --- a/pkg/bufferdecoder/eventsreader_test.go +++ b/pkg/bufferdecoder/eventsreader_test.go @@ -183,7 +183,7 @@ func TestReadArgFromBuff(t *testing.T) { if tc.name == "unknown" { return } - assert.Empty(t, decoder.BuffLen()-decoder.ReadAmountBytes(), tc.name) // passed in buffer should be emptied out + assert.Empty(t, decoder.BuffLen()-decoder.BytesRead(), tc.name) // passed in buffer should be emptied out }) } } @@ -258,13 +258,13 @@ func TestReadStringVarFromBuff(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { decoder := New(tt.buffer) - actual, err := readStringVarFromBuff(decoder, tt.max) + actual, err := readVarStringFromBuffer(decoder, tt.max) if tt.expectError { assert.Error(t, err) } else { assert.NoError(t, err) assert.Equal(t, tt.expected, actual) - assert.Equal(t, tt.expectedCursor, decoder.ReadAmountBytes()) + assert.Equal(t, tt.expectedCursor, decoder.BytesRead()) } }) } diff --git a/pkg/ebpf/c/common/buffer.h b/pkg/ebpf/c/common/buffer.h index 45b3cffbd716..9d988ac7e6ad 100644 --- a/pkg/ebpf/c/common/buffer.h +++ b/pkg/ebpf/c/common/buffer.h @@ -273,7 +273,7 @@ statfunc int save_args_str_arr_to_buf( args_buffer_t *buf, const char *start, const char *end, int elem_num, u8 index) { // Data saved to submit buf: [index][len][arg_len][arg #][array of null delimited string] - // Note: This helper saves null (0x00) delimited string array into buf in the format: + // Note: This helper saves null (0x00) delimited string array into buf in the format: // [[str1][\n][str2][\n][...][strn][\n])] if (start >= end) diff --git a/pkg/ebpf/capture.go b/pkg/ebpf/capture.go index 296c8f35bbc3..f17a81bcdc88 100644 --- a/pkg/ebpf/capture.go +++ b/pkg/ebpf/capture.go @@ -170,7 +170,7 @@ func (t *Tracee) handleFileCaptures(ctx context.Context) { } } - dataBytes, err := bufferdecoder.ReadByteSliceFromBuff(ebpfMsgDecoder, int(meta.Size)) + dataBytes, err := ebpfMsgDecoder.ReadBytesLen(int(meta.Size)) if err != nil { if err := f.Close(); err != nil { t.handleError(err)