Skip to content

Commit

Permalink
refactor: removed usages of unsafe package in plugins (#457)
Browse files Browse the repository at this point in the history
# Description

Partially closes #95.
#443 will remove the usage of
`unsafe` in `utils_linux`

## Related Issue

If this pull request is related to any issue, please mention it here.
Additionally, make sure that the issue is assigned to you before
submitting this pull request.

## Checklist

- [ ] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [ ] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [ ] I have correctly attributed the author(s) of the code.
- [ ] I have tested the changes locally.
- [ ] I have followed the project's style guidelines.
- [ ] I have updated the documentation, if necessary.
- [ ] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed


![image](https://github.com/microsoft/retina/assets/28567936/b2bd8aa5-e659-44e6-85d7-e67b3b1a43fa)
## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.
  • Loading branch information
nddq authored Jun 12, 2024
1 parent e2d5f99 commit 5fc2fcc
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 11 deletions.
14 changes: 12 additions & 2 deletions pkg/plugin/dropreason/dropreason_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
package dropreason

import (
"bytes"
"context"
"encoding/binary"
"errors"
"fmt"
"os"
"path"
"runtime"
"time"
"unsafe"

"github.com/cilium/cilium/api/v1/flow"
hubblev1 "github.com/cilium/cilium/pkg/hubble/api/v1"
Expand Down Expand Up @@ -344,7 +345,16 @@ func (dr *dropReason) processRecord(ctx context.Context, id int) {
dr.l.Info("Context is done, dropreason worker will stop running", zap.Int("id", id))
return
case record := <-dr.recordsChannel:
bpfEvent := (*kprobePacket)(unsafe.Pointer(&record.RawSample[0])) //nolint:typecheck
var bpfEvent kprobePacket
err := binary.Read(bytes.NewReader(record.RawSample), binary.LittleEndian, &bpfEvent)
if err != nil {
if binary.Size(bpfEvent) != len(record.RawSample) {
dr.l.Error("Error reading bpf event due to size mismatch", zap.Error(err), zap.Int("expected", binary.Size(bpfEvent)), zap.Int("actual", len(record.RawSample)))
} else {
dr.l.Error("Error reading bpf event", zap.Error(err))
}
continue
}
sourcePortShort := uint32(utils.HostToNetShort(bpfEvent.SrcPort))
destinationPortShort := uint32(utils.HostToNetShort(bpfEvent.DstPort))
dropKey := (dropMetricKey)(bpfEvent.Key)
Expand Down
37 changes: 30 additions & 7 deletions pkg/plugin/dropreason/dropreason_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,16 @@ func TestDropReasonRun(t *testing.T) {
mockedMapIterator.EXPECT().Err().Return(nil).MinTimes(1)
mockedMapIterator.EXPECT().Next(gomock.Any(), gomock.Any()).Return(false).MinTimes(1)

// create a rawSample slice and fill it with 56 bytes of data. 56 is the size of the rawSample in perf.Record (kprobePacket)
rawSample := make([]byte, 56)
for i := range rawSample {
rawSample[i] = byte(i)
}

// TODO(nddq) : test an actual kprobePacket similar to what we are doing with packetparserPacket in packetparser
mockedPerfRecord := perf.Record{
CPU: 0,
RawSample: []byte{0x01, 0x02, 0x03},
RawSample: rawSample,
LostSamples: 0,
}
mockedPerfReader.EXPECT().Read().Return(mockedPerfRecord, nil).MinTimes(1)
Expand Down Expand Up @@ -253,12 +260,18 @@ func TestDropReasonReadDataPodLevelEnabled(t *testing.T) {
mockedPerfReader := mocks.NewMockIPerfReader(ctrl)
menricher := enricher.NewMockEnricherInterface(ctrl) //nolint:typecheck

// mock perf reader record
// create a rawSample slice and fill it with 56 bytes of data. 56 is the size of the rawSample in perf.Record (kprobePacket)
rawSample := make([]byte, 56)
for i := range rawSample {
rawSample[i] = byte(i)
}

mockedPerfRecord := perf.Record{
CPU: 0,
RawSample: []byte{0x01, 0x02, 0x03},
RawSample: rawSample,
LostSamples: 0,
}

mockedPerfReader.EXPECT().Read().Return(mockedPerfRecord, nil).MinTimes(1)
menricher.EXPECT().Write(gomock.Any()).MinTimes(1)

Expand Down Expand Up @@ -336,10 +349,15 @@ func TestDropReasonReadData_WithPerfArrayLostSamples(t *testing.T) {
mockedMap := mocks.NewMockIMap(ctrl)
mockedPerfReader := mocks.NewMockIPerfReader(ctrl)

// mock perf reader record
// create a rawSample slice and fill it with 56 bytes of data. 56 is the size of the rawSample in perf.Record (kprobePacket)
rawSample := make([]byte, 56)
for i := range rawSample {
rawSample[i] = byte(i)
}

mockedPerfRecord := perf.Record{
CPU: 0,
RawSample: []byte{0x01, 0x02, 0x03},
RawSample: rawSample,
LostSamples: 3,
}
mockedPerfReader.EXPECT().Read().Return(mockedPerfRecord, nil).MinTimes(1)
Expand Down Expand Up @@ -373,10 +391,15 @@ func TestDropReasonReadData_WithUnknownError(t *testing.T) {
mockedMap := mocks.NewMockIMap(ctrl)
mockedPerfReader := mocks.NewMockIPerfReader(ctrl)

// mock perf reader record
// create a rawSample slice and fill it with 56 bytes of data. 56 is the size of the rawSample in perf.Record (kprobePacket)
rawSample := make([]byte, 56)
for i := range rawSample {
rawSample[i] = byte(i)
}

mockedPerfRecord := perf.Record{
CPU: 0,
RawSample: []byte{0x01, 0x02, 0x03},
RawSample: rawSample,
LostSamples: 3,
}
mockedPerfReader.EXPECT().Read().Return(mockedPerfRecord, errors.New("Unknown Error")).MinTimes(1)
Expand Down
10 changes: 8 additions & 2 deletions pkg/plugin/packetparser/packetparser_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
package packetparser

import (
"bytes"
"context"
"encoding/binary"
"errors"
"fmt"
"os"
"path"
"runtime"
"sync"
"unsafe"

"github.com/cilium/cilium/api/v1/flow"
v1 "github.com/cilium/cilium/pkg/hubble/api/v1"
Expand Down Expand Up @@ -531,7 +532,12 @@ func (p *packetParser) processRecord(ctx context.Context, id int) {
zap.Int("worker_id", id),
)

bpfEvent := (*packetparserPacket)(unsafe.Pointer(&record.RawSample[0])) //nolint:typecheck
var bpfEvent packetparserPacket
err := binary.Read(bytes.NewReader(record.RawSample), binary.LittleEndian, &bpfEvent)
if err != nil {
p.l.Error("Error reading bpfEvent", zap.Error(err))
continue
}

// Post processing of the bpfEvent.
// Anything after this is required only for Pod level metrics.
Expand Down

0 comments on commit 5fc2fcc

Please sign in to comment.