From 3f5c60edc49d88750977b732e7c52cc850e4e158 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 01:04:53 -0400 Subject: [PATCH 1/7] chore: refactor packetparser userspace code --- go.mod | 2 +- pkg/plugin/packetparser/mocks/mock_types.go | 151 +++++++++-------- pkg/plugin/packetparser/packetparser_linux.go | 133 ++++++--------- .../packetparser/packetparser_linux_test.go | 160 ++++++++++-------- pkg/plugin/packetparser/types_linux.go | 48 +++--- 5 files changed, 252 insertions(+), 242 deletions(-) diff --git a/go.mod b/go.mod index 779189931e..50c8280bc8 100644 --- a/go.mod +++ b/go.mod @@ -159,7 +159,6 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.20 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect - github.com/mdlayher/netlink v1.7.2 // indirect github.com/mdlayher/socket v0.4.1 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect @@ -305,6 +304,7 @@ require ( github.com/inspektor-gadget/inspektor-gadget v0.27.0 github.com/jellydator/ttlcache/v3 v3.3.0 github.com/jsternberg/zap-logfmt v1.3.0 + github.com/mdlayher/netlink v1.7.2 github.com/microsoft/ApplicationInsights-Go v0.4.4 github.com/mitchellh/mapstructure v1.5.0 github.com/onsi/ginkgo/v2 v2.20.2 diff --git a/pkg/plugin/packetparser/mocks/mock_types.go b/pkg/plugin/packetparser/mocks/mock_types.go index 8f48f0fc6e..f632a0cf98 100644 --- a/pkg/plugin/packetparser/mocks/mock_types.go +++ b/pkg/plugin/packetparser/mocks/mock_types.go @@ -14,34 +14,35 @@ import ( perf "github.com/cilium/ebpf/perf" tc "github.com/florianl/go-tc" + netlink "github.com/mdlayher/netlink" gomock "go.uber.org/mock/gomock" ) -// MockIQdisc is a mock of IQdisc interface. -type MockIQdisc struct { +// Mockqdisc is a mock of qdisc interface. +type Mockqdisc struct { ctrl *gomock.Controller - recorder *MockIQdiscMockRecorder + recorder *MockqdiscMockRecorder } -// MockIQdiscMockRecorder is the mock recorder for MockIQdisc. -type MockIQdiscMockRecorder struct { - mock *MockIQdisc +// MockqdiscMockRecorder is the mock recorder for Mockqdisc. +type MockqdiscMockRecorder struct { + mock *Mockqdisc } -// NewMockIQdisc creates a new mock instance. -func NewMockIQdisc(ctrl *gomock.Controller) *MockIQdisc { - mock := &MockIQdisc{ctrl: ctrl} - mock.recorder = &MockIQdiscMockRecorder{mock} +// NewMockqdisc creates a new mock instance. +func NewMockqdisc(ctrl *gomock.Controller) *Mockqdisc { + mock := &Mockqdisc{ctrl: ctrl} + mock.recorder = &MockqdiscMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockIQdisc) EXPECT() *MockIQdiscMockRecorder { +func (m *Mockqdisc) EXPECT() *MockqdiscMockRecorder { return m.recorder } // Add mocks base method. -func (m *MockIQdisc) Add(info *tc.Object) error { +func (m *Mockqdisc) Add(info *tc.Object) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Add", info) ret0, _ := ret[0].(error) @@ -49,13 +50,13 @@ func (m *MockIQdisc) Add(info *tc.Object) error { } // Add indicates an expected call of Add. -func (mr *MockIQdiscMockRecorder) Add(info any) *gomock.Call { +func (mr *MockqdiscMockRecorder) Add(info any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockIQdisc)(nil).Add), info) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*Mockqdisc)(nil).Add), info) } // Delete mocks base method. -func (m *MockIQdisc) Delete(info *tc.Object) error { +func (m *Mockqdisc) Delete(info *tc.Object) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Delete", info) ret0, _ := ret[0].(error) @@ -63,36 +64,36 @@ func (m *MockIQdisc) Delete(info *tc.Object) error { } // Delete indicates an expected call of Delete. -func (mr *MockIQdiscMockRecorder) Delete(info any) *gomock.Call { +func (mr *MockqdiscMockRecorder) Delete(info any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockIQdisc)(nil).Delete), info) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*Mockqdisc)(nil).Delete), info) } -// MockIFilter is a mock of IFilter interface. -type MockIFilter struct { +// Mockfilter is a mock of filter interface. +type Mockfilter struct { ctrl *gomock.Controller - recorder *MockIFilterMockRecorder + recorder *MockfilterMockRecorder } -// MockIFilterMockRecorder is the mock recorder for MockIFilter. -type MockIFilterMockRecorder struct { - mock *MockIFilter +// MockfilterMockRecorder is the mock recorder for Mockfilter. +type MockfilterMockRecorder struct { + mock *Mockfilter } -// NewMockIFilter creates a new mock instance. -func NewMockIFilter(ctrl *gomock.Controller) *MockIFilter { - mock := &MockIFilter{ctrl: ctrl} - mock.recorder = &MockIFilterMockRecorder{mock} +// NewMockfilter creates a new mock instance. +func NewMockfilter(ctrl *gomock.Controller) *Mockfilter { + mock := &Mockfilter{ctrl: ctrl} + mock.recorder = &MockfilterMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockIFilter) EXPECT() *MockIFilterMockRecorder { +func (m *Mockfilter) EXPECT() *MockfilterMockRecorder { return m.recorder } // Add mocks base method. -func (m *MockIFilter) Add(info *tc.Object) error { +func (m *Mockfilter) Add(info *tc.Object) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Add", info) ret0, _ := ret[0].(error) @@ -100,36 +101,36 @@ func (m *MockIFilter) Add(info *tc.Object) error { } // Add indicates an expected call of Add. -func (mr *MockIFilterMockRecorder) Add(info any) *gomock.Call { +func (mr *MockfilterMockRecorder) Add(info any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*MockIFilter)(nil).Add), info) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Add", reflect.TypeOf((*Mockfilter)(nil).Add), info) } -// MockITc is a mock of ITc interface. -type MockITc struct { +// Mocknltc is a mock of nltc interface. +type Mocknltc struct { ctrl *gomock.Controller - recorder *MockITcMockRecorder + recorder *MocknltcMockRecorder } -// MockITcMockRecorder is the mock recorder for MockITc. -type MockITcMockRecorder struct { - mock *MockITc +// MocknltcMockRecorder is the mock recorder for Mocknltc. +type MocknltcMockRecorder struct { + mock *Mocknltc } -// NewMockITc creates a new mock instance. -func NewMockITc(ctrl *gomock.Controller) *MockITc { - mock := &MockITc{ctrl: ctrl} - mock.recorder = &MockITcMockRecorder{mock} +// NewMocknltc creates a new mock instance. +func NewMocknltc(ctrl *gomock.Controller) *Mocknltc { + mock := &Mocknltc{ctrl: ctrl} + mock.recorder = &MocknltcMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockITc) EXPECT() *MockITcMockRecorder { +func (m *Mocknltc) EXPECT() *MocknltcMockRecorder { return m.recorder } // Close mocks base method. -func (m *MockITc) Close() error { +func (m *Mocknltc) Close() error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Close") ret0, _ := ret[0].(error) @@ -137,13 +138,13 @@ func (m *MockITc) Close() error { } // Close indicates an expected call of Close. -func (mr *MockITcMockRecorder) Close() *gomock.Call { +func (mr *MocknltcMockRecorder) Close() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockITc)(nil).Close)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*Mocknltc)(nil).Close)) } // Filter mocks base method. -func (m *MockITc) Filter() *tc.Filter { +func (m *Mocknltc) Filter() *tc.Filter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Filter") ret0, _ := ret[0].(*tc.Filter) @@ -151,13 +152,13 @@ func (m *MockITc) Filter() *tc.Filter { } // Filter indicates an expected call of Filter. -func (mr *MockITcMockRecorder) Filter() *gomock.Call { +func (mr *MocknltcMockRecorder) Filter() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Filter", reflect.TypeOf((*MockITc)(nil).Filter)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Filter", reflect.TypeOf((*Mocknltc)(nil).Filter)) } // Qdisc mocks base method. -func (m *MockITc) Qdisc() *tc.Qdisc { +func (m *Mocknltc) Qdisc() *tc.Qdisc { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Qdisc") ret0, _ := ret[0].(*tc.Qdisc) @@ -165,36 +166,50 @@ func (m *MockITc) Qdisc() *tc.Qdisc { } // Qdisc indicates an expected call of Qdisc. -func (mr *MockITcMockRecorder) Qdisc() *gomock.Call { +func (mr *MocknltcMockRecorder) Qdisc() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Qdisc", reflect.TypeOf((*MockITc)(nil).Qdisc)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Qdisc", reflect.TypeOf((*Mocknltc)(nil).Qdisc)) } -// MockIPerf is a mock of IPerf interface. -type MockIPerf struct { +// SetOption mocks base method. +func (m *Mocknltc) SetOption(arg0 netlink.ConnOption, arg1 bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetOption", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetOption indicates an expected call of SetOption. +func (mr *MocknltcMockRecorder) SetOption(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetOption", reflect.TypeOf((*Mocknltc)(nil).SetOption), arg0, arg1) +} + +// MockperfReader is a mock of perfReader interface. +type MockperfReader struct { ctrl *gomock.Controller - recorder *MockIPerfMockRecorder + recorder *MockperfReaderMockRecorder } -// MockIPerfMockRecorder is the mock recorder for MockIPerf. -type MockIPerfMockRecorder struct { - mock *MockIPerf +// MockperfReaderMockRecorder is the mock recorder for MockperfReader. +type MockperfReaderMockRecorder struct { + mock *MockperfReader } -// NewMockIPerf creates a new mock instance. -func NewMockIPerf(ctrl *gomock.Controller) *MockIPerf { - mock := &MockIPerf{ctrl: ctrl} - mock.recorder = &MockIPerfMockRecorder{mock} +// NewMockperfReader creates a new mock instance. +func NewMockperfReader(ctrl *gomock.Controller) *MockperfReader { + mock := &MockperfReader{ctrl: ctrl} + mock.recorder = &MockperfReaderMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockIPerf) EXPECT() *MockIPerfMockRecorder { +func (m *MockperfReader) EXPECT() *MockperfReaderMockRecorder { return m.recorder } // Close mocks base method. -func (m *MockIPerf) Close() error { +func (m *MockperfReader) Close() error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Close") ret0, _ := ret[0].(error) @@ -202,13 +217,13 @@ func (m *MockIPerf) Close() error { } // Close indicates an expected call of Close. -func (mr *MockIPerfMockRecorder) Close() *gomock.Call { +func (mr *MockperfReaderMockRecorder) Close() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockIPerf)(nil).Close)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Close", reflect.TypeOf((*MockperfReader)(nil).Close)) } // Read mocks base method. -func (m *MockIPerf) Read() (perf.Record, error) { +func (m *MockperfReader) Read() (perf.Record, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Read") ret0, _ := ret[0].(perf.Record) @@ -217,7 +232,7 @@ func (m *MockIPerf) Read() (perf.Record, error) { } // Read indicates an expected call of Read. -func (mr *MockIPerfMockRecorder) Read() *gomock.Call { +func (mr *MockperfReaderMockRecorder) Read() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*MockIPerf)(nil).Read)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*MockperfReader)(nil).Read)) } diff --git a/pkg/plugin/packetparser/packetparser_linux.go b/pkg/plugin/packetparser/packetparser_linux.go index 4737ff62dd..d640fb3f58 100644 --- a/pkg/plugin/packetparser/packetparser_linux.go +++ b/pkg/plugin/packetparser/packetparser_linux.go @@ -23,6 +23,7 @@ import ( "github.com/cilium/ebpf/perf" "github.com/florianl/go-tc" helper "github.com/florianl/go-tc/core" + nl "github.com/mdlayher/netlink" "github.com/microsoft/retina/internal/ktime" "github.com/microsoft/retina/pkg/common" kcfg "github.com/microsoft/retina/pkg/config" @@ -307,8 +308,8 @@ func (p *packetParser) cleanAll() error { } p.tcMap.Range(func(key, value interface{}) bool { - v := value.(*val) - p.clean(v.tcnl, v.tcIngressObj, v.tcEgressObj) + v := value.(*tcValue) + p.clean(v.tc, v.qdisc) return true }) @@ -320,18 +321,12 @@ func (p *packetParser) cleanAll() error { return nil } -func (p *packetParser) clean(tcnl ITc, tcIngressObj *tc.Object, tcEgressObj *tc.Object) { +func (p *packetParser) clean(tcnl nltc, qdisc *tc.Object) { // Warning, not error. Clean is best effort. if tcnl != nil { - if err := getQdisc(tcnl).Delete(tcEgressObj); err != nil && !errors.Is(err, tc.ErrNoArg) { + if err := getQdisc(tcnl).Delete(qdisc); err != nil && !errors.Is(err, tc.ErrNoArg) { p.l.Debug("could not delete egress qdisc", zap.Error(err)) } - if err := getQdisc(tcnl).Delete(tcIngressObj); err != nil && !errors.Is(err, tc.ErrNoArg) { - p.l.Debug("could not delete ingress qdisc", zap.Error(err)) - } - if err := tcnl.Close(); err != nil { - p.l.Warn("could not close rtnetlink socket", zap.Error(err)) - } } } @@ -357,9 +352,9 @@ func (p *packetParser) endpointWatcherCallbackFn(obj interface{}) { case endpoint.EndpointDeleted: p.l.Debug("Endpoint deleted", zap.String("name", iface.Name)) // Clean. - if v, ok := p.tcMap.Load(ifaceKey); ok { - tcMapVal := v.(*val) - p.clean(tcMapVal.tcnl, tcMapVal.tcIngressObj, tcMapVal.tcEgressObj) + if value, ok := p.tcMap.Load(ifaceKey); ok { + v := value.(*tcValue) + p.clean(v.tc, v.qdisc) // Delete from map. p.tcMap.Delete(ifaceKey) } @@ -369,77 +364,78 @@ func (p *packetParser) endpointWatcherCallbackFn(obj interface{}) { } } -// This does the following: -// 1. Create a tunnel interface. -// 2. Create a qdisc and attach it to the tunnel interface. -// 3. Attach ingress program to the endpoint interface. -// 4. Create a qdisc and attach it to the endpoint interface. -// 5. Attach egress program to the endpoint interface. -// Inspired by https://github.com/mauriciovasquezbernal/talks/blob/1f2080afe731949a033330c0adc290be8f3fc06d/2022-ebpf-training/2022-10-13/drop/main.go . -// Supported ifaceTypes - device and veth. -func (p *packetParser) createQdiscAndAttach(iface netlink.LinkAttrs, ifaceType string) { +// createQdiscAndAttach creates a qdisc of type clsact on the interface and attaches the ingress and egress bpf filter programs to it. +// Only support interfaces of type veth and device. +func (p *packetParser) createQdiscAndAttach(iface netlink.LinkAttrs, ifaceType interfaceType) { p.l.Debug("Starting qdisc attachment", zap.String("interface", iface.Name)) - // Create tunnel interface. var ( - tcnl ITc - err error ingressProgram, egressProgram *ebpf.Program ingressInfo, egressInfo *ebpf.ProgramInfo + err error ) - if ifaceType == "device" { + switch ifaceType { + case Device: ingressProgram = p.objs.HostIngressFilter egressProgram = p.objs.HostEgressFilter - ingressInfo = p.hostIngressInfo egressInfo = p.hostEgressInfo - } else if ifaceType == "veth" { + case Veth: ingressProgram = p.objs.EndpointIngressFilter egressProgram = p.objs.EndpointEgressFilter - ingressInfo = p.endpointIngressInfo egressInfo = p.endpointEgressInfo - } else { - p.l.Error("Unknown ifaceType", zap.String("ifaceType", ifaceType)) + default: + p.l.Error("Unknown interface type", zap.String("interface type", string(ifaceType))) return } - tcnl, err = tcOpen(&tc.Config{}) + // open a rtnetlink socket + rtnl, err := tcOpen(&tc.Config{}) if err != nil { p.l.Error("could not open rtnetlink socket", zap.Int("NetNsID", iface.NetNsID), zap.Error(err)) return } - - var qdiscIngress, qdiscEgress *tc.Object + // set extended acknowledge option for more detailed error messages. + if err = rtnl.SetOption(nl.ExtendedAcknowledge, true); err != nil { + p.l.Warn("could not set extended acknowledge option", zap.Error(err)) + } // Create a qdisc of type clsact on the tunnel interface. - // We will attach the ingress bpf filter on this. - qdiscIngress = &tc.Object{ + // Even though the parameter we are setting is for ingress, clact is a special qdisc that have both ingress and egress hooks. + // https://lwn.net/Articles/671458/ + // We will attach the ingress and egress programs to this qdisc. + clsactQdisc := &tc.Object{ Msg: tc.Msg{ Family: unix.AF_UNSPEC, Ifindex: uint32(iface.Index), - Handle: helper.BuildHandle(0xFFFF, 0x0000), - Parent: tc.HandleIngress, + Handle: helper.BuildHandle(0xFFFF, 0x0000), // nolint:gomnd // special handle for ingress qdisc https://tldp.org/HOWTO/Traffic-Control-HOWTO/components.html + // we can actually be pedantic and create another qdisc for egress by setting the parent to tc.HandleRoot but it is not necessary for reasons stated above. + Parent: tc.HandleIngress, }, Attribute: tc.Attribute{ Kind: "clsact", }, } + defer func() { + if err != nil { + p.clean(rtnl, clsactQdisc) + } + }() // Install Qdisc on interface. - if err := getQdisc(tcnl).Add(qdiscIngress); err != nil && !errors.Is(err, os.ErrExist) { - p.l.Error("could not assign clsact ingress to ", zap.String("interface", iface.Name), zap.Error(err)) - p.clean(tcnl, qdiscIngress, qdiscEgress) + if err := getQdisc(rtnl).Add(clsactQdisc); err != nil && !errors.Is(err, os.ErrExist) { + p.l.Error("could not assign clsact to tunnel interface", zap.String("interface", iface.Name), zap.Error(err)) return } - // Create a filter of type bpf on the tunnel interface. - filterIngress := tc.Object{ + // Create an ingress filter of type bpf on the tunnel interface. + ingressFilter := tc.Object{ Msg: tc.Msg{ Family: unix.AF_UNSPEC, Ifindex: uint32(iface.Index), - Handle: 0, - Parent: 0xFFFFFFF2, - Info: 0x10300, + Handle: 0, // arbitrary handle to distinguish between ingress and egress filters + Parent: helper.BuildHandle(0xFFFF, tc.HandleMinIngress), // nolint:gomnd // same major component (0xffff) as clsact + Info: 0x10300, // nolint:gomnd // ignore, have not been able to find the exact meaning of this value }, Attribute: tc.Attribute{ Kind: "bpf", @@ -450,39 +446,18 @@ func (p *packetParser) createQdiscAndAttach(iface netlink.LinkAttrs, ifaceType s }, }, } - if err := getFilter(tcnl).Add(&filterIngress); err != nil && !errors.Is(err, os.ErrExist) { - p.l.Error("could not add bpf ingress to ", zap.String("interface", iface.Name), zap.Error(err)) - p.clean(tcnl, qdiscIngress, qdiscEgress) - return - } - - // Create a qdisc of type clsact on the endpoint interface. - qdiscEgress = &tc.Object{ - Msg: tc.Msg{ - Family: unix.AF_UNSPEC, - Ifindex: uint32(iface.Index), - Handle: helper.BuildHandle(0xFFFF, 0), - Parent: helper.BuildHandle(0xFFFF, 0xFFF1), - }, - Attribute: tc.Attribute{ - Kind: "clsact", - }, - } - - // Install Qdisc on interface. - if err := getQdisc(tcnl).Add(qdiscEgress); err != nil && !errors.Is(err, os.ErrExist) { - p.l.Error("could not assign clsact egress to ", zap.String("interface", iface.Name), zap.Error(err)) - p.clean(tcnl, qdiscIngress, qdiscEgress) + if err := getFilter(rtnl).Add(&ingressFilter); err != nil && !errors.Is(err, os.ErrExist) { + p.l.Error("could not add bpf ingress filter to qdisc", zap.String("interface", iface.Name), zap.Error(err)) return } - // Create a filter of type bpf on the endpoint interface. - filterEgress := tc.Object{ + // Create an egress filter of type bpf on the endpoint interface. + egressFilter := tc.Object{ Msg: tc.Msg{ Family: unix.AF_UNSPEC, Ifindex: uint32(iface.Index), Handle: 1, - Info: TC_H_MAKE(1<<16, uint32(utils.HostToNetShort(0x0003))), - Parent: TC_H_MAKE(0xFFFFFFF1, 0xFFF3), + Parent: helper.BuildHandle(0xFFFF, tc.HandleMinEgress), // nolint:gomnd // ignore + Info: 0x10300, // nolint:gomnd // ignore }, Attribute: tc.Attribute{ Kind: "bpf", @@ -493,16 +468,18 @@ func (p *packetParser) createQdiscAndAttach(iface netlink.LinkAttrs, ifaceType s }, }, } - if err := getFilter(tcnl).Add(&filterEgress); err != nil && !errors.Is(err, os.ErrExist) { - p.l.Error("could not add bpf egress to ", zap.String("interface", iface.Name), zap.Error(err)) - p.clean(tcnl, qdiscIngress, qdiscEgress) + if err := getFilter(rtnl).Add(&egressFilter); err != nil && !errors.Is(err, os.ErrExist) { + p.l.Error("could not add bpf egress filter to qdisc", zap.String("interface", iface.Name), zap.Error(err)) return } // Cache. ifaceKey := ifaceToKey(iface) - ifaceVal := &val{tcnl: tcnl, tcIngressObj: qdiscIngress, tcEgressObj: qdiscEgress} - p.tcMap.Store(ifaceKey, ifaceVal) + tcValue := &tcValue{ + tc: rtnl, + qdisc: clsactQdisc, + } + p.tcMap.Store(ifaceKey, tcValue) p.l.Debug("Successfully added bpf", zap.String("interface", iface.Name)) } diff --git a/pkg/plugin/packetparser/packetparser_linux_test.go b/pkg/plugin/packetparser/packetparser_linux_test.go index 3fd55b8fc2..6cce6571eb 100644 --- a/pkg/plugin/packetparser/packetparser_linux_test.go +++ b/pkg/plugin/packetparser/packetparser_linux_test.go @@ -66,34 +66,18 @@ func TestCleanAll(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mtcnl := mocks.NewMockITc(ctrl) - mtcnl.EXPECT().Close().Return(nil).AnyTimes() + mrtnl := mocks.NewMocknltc(ctrl) + mrtnl.EXPECT().Close().Return(nil).AnyTimes() - mq := mocks.NewMockIQdisc(ctrl) + mq := mocks.NewMockqdisc(ctrl) mq.EXPECT().Delete(gomock.Any()).Return(nil).AnyTimes() - getQdisc = func(tcnl ITc) IQdisc { + getQdisc = func(nltc) qdisc { return mq } - p.tcMap.Store(key{ - name: "test", - hardwareAddr: "test", - netNs: 1, - }, &val{ - tcnl: mtcnl, - tcIngressObj: &tc.Object{}, - tcEgressObj: &tc.Object{}, - }) - p.tcMap.Store(key{ - name: "test2", - hardwareAddr: "test2", - netNs: 2, - }, &val{ - tcnl: mtcnl, - tcIngressObj: &tc.Object{}, - tcEgressObj: &tc.Object{}, - }) + p.tcMap.Store(tcKey{"test", "test", 1}, &tcValue{mrtnl, &tc.Object{}}) + p.tcMap.Store(tcKey{"test2", "test2", 2}, &tcValue{mrtnl, &tc.Object{}}) assert.Nil(t, p.cleanAll()) @@ -117,23 +101,22 @@ func TestClean(t *testing.T) { cfg: cfgPodLevelEnabled, l: log.Logger().Named("test"), } - p.clean(nil, nil, nil) // Should not panic. + p.clean(nil, nil) // Should not panic. // Test tcnl calls. - mq := mocks.NewMockIQdisc(ctrl) - mq.EXPECT().Delete(gomock.Any()).Return(nil).Times(2) + mq := mocks.NewMockqdisc(ctrl) + mq.EXPECT().Delete(gomock.Any()).Return(nil).Times(1) - mtcnl := mocks.NewMockITc(ctrl) - mtcnl.EXPECT().Qdisc().Return(nil).Times(2) - mtcnl.EXPECT().Close().Return(nil).Times(1) + mrtnl := mocks.NewMocknltc(ctrl) + mrtnl.EXPECT().Qdisc().Return(nil).Times(1) - getQdisc = func(tcnl ITc) IQdisc { + getQdisc = func(tcnl nltc) qdisc { // Add this verify tcnl.Qdisc() is called twice tcnl.Qdisc() return mq } - p.clean(mtcnl, &tc.Object{}, &tc.Object{}) + p.clean(mrtnl, &tc.Object{}) } func TestCleanWithErrors(t *testing.T) { @@ -148,18 +131,17 @@ func TestCleanWithErrors(t *testing.T) { } // Test we try delete qdiscs even if we get errors. - mq := mocks.NewMockIQdisc(ctrl) - mq.EXPECT().Delete(gomock.Any()).Return(errors.New("error")).Times(2) + mq := mocks.NewMockqdisc(ctrl) + mq.EXPECT().Delete(gomock.Any()).Return(errors.New("error")).Times(1) //nolint:err113 // ignore - mtcnl := mocks.NewMockITc(ctrl) - mtcnl.EXPECT().Qdisc().Return(nil).AnyTimes() - mtcnl.EXPECT().Close().Return(nil).Times(1) + mrtnl := mocks.NewMocknltc(ctrl) + mrtnl.EXPECT().Qdisc().Return(nil).AnyTimes() - getQdisc = func(tcnl ITc) IQdisc { + getQdisc = func(nltc) qdisc { return mq } - p.clean(mtcnl, &tc.Object{}, &tc.Object{}) + p.clean(mrtnl, &tc.Object{}) } func TestEndpointWatcherCallbackFn_EndpointDeleted(t *testing.T) { @@ -179,11 +161,7 @@ func TestEndpointWatcherCallbackFn_EndpointDeleted(t *testing.T) { NetNsID: 1, } key := ifaceToKey(linkAttr) - p.tcMap.Store(key, &val{ - tcnl: nil, - tcIngressObj: &tc.Object{}, - tcEgressObj: &tc.Object{}, - }) + p.tcMap.Store(key, &tcValue{nil, &tc.Object{}}) // Create EndpointDeleted event. e := &endpoint.EndpointEvent{ @@ -202,25 +180,25 @@ func TestCreateQdiscAndAttach(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mfilter := mocks.NewMockIFilter(ctrl) + mfilter := mocks.NewMockfilter(ctrl) mfilter.EXPECT().Add(gomock.Any()).Return(nil).AnyTimes() - mq := mocks.NewMockIQdisc(ctrl) + mq := mocks.NewMockqdisc(ctrl) mq.EXPECT().Add(gomock.Any()).Return(nil).AnyTimes() - mtcnl := mocks.NewMockITc(ctrl) - mtcnl.EXPECT().Qdisc().Return(nil).AnyTimes() + mrtnl := mocks.NewMocknltc(ctrl) + mrtnl.EXPECT().Qdisc().Return(nil).AnyTimes() - getQdisc = func(tcnl ITc) IQdisc { + getQdisc = func(nltc) qdisc { return mq } - getFilter = func(tcnl ITc) IFilter { + getFilter = func(nltc) filter { return mfilter } - tcOpen = func(c *tc.Config) (ITc, error) { - return mtcnl, nil + tcOpen = func(*tc.Config) (nltc, error) { + return mrtnl, nil } getFD = func(e *ebpf.Program) int { @@ -282,7 +260,7 @@ func TestReadData_Error(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mperf := mocks.NewMockIPerf(ctrl) + mperf := mocks.NewMockperfReader(ctrl) mperf.EXPECT().Read().Return(perf.Record{}, errors.New("error")).AnyTimes() menricher := enricher.NewMockEnricherInterface(ctrl) //nolint:typecheck @@ -321,7 +299,7 @@ func TestReadDataPodLevelEnabled(t *testing.T) { RawSample: bytes, } - mperf := mocks.NewMockIPerf(ctrl) + mperf := mocks.NewMockperfReader(ctrl) mperf.EXPECT().Read().Return(record, nil).MinTimes(1) menricher := enricher.NewMockEnricherInterface(ctrl) //nolint:typecheck @@ -343,7 +321,7 @@ func TestReadDataPodLevelEnabled(t *testing.T) { exCh := make(chan *v1.Event, 10) p.SetupChannel(exCh) - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() p.run(ctx) @@ -371,28 +349,43 @@ func TestStartWithDataAggregationLevelLow(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockFilter := mocks.NewMockIFilter(ctrl) - mQdisc := mocks.NewMockIQdisc(ctrl) + mockFilter := mocks.NewMockfilter(ctrl) + mQdisc := mocks.NewMockqdisc(ctrl) - // We are expecting two calls to Add since we are invoking createQdiscAndAttach for eth0 + // We are expecting one call to Add since we are invoking createQdiscAndAttach for eth0 mockFilter.EXPECT().Add(gomock.Any()).Return(nil).Times(2) - mQdisc.EXPECT().Add(gomock.Any()).Return(nil).Times(2) + mQdisc.EXPECT().Add(gomock.Any()).Return(nil).Times(1) - mockTC := mocks.NewMockITc(ctrl) + mockRtnl := mocks.NewMocknltc(ctrl) + + bpfEvent := &packetparserPacket{ + SrcIp: uint32(83886272), // 192.0.0.5 + DstIp: uint32(16777226), // 10.0.0.1 + Proto: uint8(6), // TCP + ObservationPoint: uint8(1), // TO Endpoint + SrcPort: uint16(80), + DstPort: uint16(443), + } + bytes, err := json.Marshal(bpfEvent) // nolint:musttag // ignore + require.NoError(t, err) + record := perf.Record{ + LostSamples: 0, + RawSample: bytes, + } - mockReader := mocks.NewMockIPerf(ctrl) - mockReader.EXPECT().Read().Return(perf.Record{}, nil).AnyTimes() + mockReader := mocks.NewMockperfReader(ctrl) + mockReader.EXPECT().Read().Return(record, nil).MinTimes(1) - getQdisc = func(_ ITc) IQdisc { + getQdisc = func(_ nltc) qdisc { return mQdisc } - getFilter = func(_ ITc) IFilter { + getFilter = func(_ nltc) filter { return mockFilter } - tcOpen = func(_ *tc.Config) (ITc, error) { - return mockTC, nil + tcOpen = func(_ *tc.Config) (nltc, error) { + return mockRtnl, nil } getFD = func(_ *ebpf.Program) int { @@ -408,6 +401,7 @@ func TestStartWithDataAggregationLevelLow(t *testing.T) { l: log.Logger().Named("test"), objs: pObj, reader: mockReader, + recordsChannel: make(chan perf.Record, buffer), interfaceLockMap: &sync.Map{}, endpointIngressInfo: &ebpf.ProgramInfo{ Name: "ingress", @@ -425,7 +419,7 @@ func TestStartWithDataAggregationLevelLow(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - err := p.Start(ctx) + err = p.Start(ctx) require.NoError(t, err) } @@ -434,28 +428,43 @@ func TestStartWithDataAggregationLevelHigh(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() - mockFilter := mocks.NewMockIFilter(ctrl) - mQdisc := mocks.NewMockIQdisc(ctrl) + mockFilter := mocks.NewMockfilter(ctrl) + mQdisc := mocks.NewMockqdisc(ctrl) // We are not expecting any calls to Add since we are not invoking createQdiscAndAttach for eth0 mockFilter.EXPECT().Add(gomock.Any()).Return(nil).Times(0) mQdisc.EXPECT().Add(gomock.Any()).Return(nil).Times(0) - mockTC := mocks.NewMockITc(ctrl) + mockRtnl := mocks.NewMocknltc(ctrl) + + bpfEvent := &packetparserPacket{ + SrcIp: uint32(83886272), // 192.0.0.5 + DstIp: uint32(16777226), // 10.0.0.1 + Proto: uint8(6), // TCP + ObservationPoint: uint8(1), // TO Endpoint + SrcPort: uint16(80), + DstPort: uint16(443), + } + bytes, err := json.Marshal(bpfEvent) // nolint:musttag // ignore + require.NoError(t, err) + record := perf.Record{ + LostSamples: 0, + RawSample: bytes, + } - mockReader := mocks.NewMockIPerf(ctrl) - mockReader.EXPECT().Read().Return(perf.Record{}, nil).AnyTimes() + mockReader := mocks.NewMockperfReader(ctrl) + mockReader.EXPECT().Read().Return(record, nil).MinTimes(1) - getQdisc = func(_ ITc) IQdisc { + getQdisc = func(_ nltc) qdisc { return mQdisc } - getFilter = func(_ ITc) IFilter { + getFilter = func(_ nltc) filter { return mockFilter } - tcOpen = func(_ *tc.Config) (ITc, error) { - return mockTC, nil + tcOpen = func(_ *tc.Config) (nltc, error) { + return mockRtnl, nil } getFD = func(_ *ebpf.Program) int { @@ -471,6 +480,7 @@ func TestStartWithDataAggregationLevelHigh(t *testing.T) { l: log.Logger().Named("test"), objs: pObj, reader: mockReader, + recordsChannel: make(chan perf.Record, buffer), interfaceLockMap: &sync.Map{}, endpointIngressInfo: &ebpf.ProgramInfo{ Name: "ingress", @@ -488,7 +498,7 @@ func TestStartWithDataAggregationLevelHigh(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - err := p.Start(ctx) + err = p.Start(ctx) require.NoError(t, err) } diff --git a/pkg/plugin/packetparser/types_linux.go b/pkg/plugin/packetparser/types_linux.go index 27709f4ec7..0624f5c82a 100644 --- a/pkg/plugin/packetparser/types_linux.go +++ b/pkg/plugin/packetparser/types_linux.go @@ -12,6 +12,7 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/perf" "github.com/florianl/go-tc" + nl "github.com/mdlayher/netlink" "github.com/vishvananda/netlink" "github.com/microsoft/retina/pkg/enricher" @@ -34,8 +35,6 @@ const ( Name api.PluginName = "packetparser" toEndpoint string = "toEndpoint" fromEndpoint string = "fromEndpoint" - Veth string = "veth" - Device string = "device" workers int = 2 buffer int = 10000 bpfSourceDir string = "_cprog" @@ -44,14 +43,21 @@ const ( dynamicHeaderFileName string = "dynamic.h" ) +type interfaceType string + +const ( + Veth interfaceType = "veth" + Device interfaceType = "device" +) + var ( - getQdisc = func(tcnl ITc) IQdisc { + getQdisc = func(tcnl nltc) qdisc { return tcnl.Qdisc() } - getFilter = func(tcnl ITc) IFilter { + getFilter = func(tcnl nltc) filter { return tcnl.Filter() } - tcOpen = func(config *tc.Config) (ITc, error) { + tcOpen = func(config *tc.Config) (nltc, error) { return tc.Open(config) } getFD = func(e *ebpf.Program) int { @@ -62,41 +68,43 @@ var ( perCPUBuffer = 32 ) -type key struct { +type tcKey struct { name string hardwareAddr string netNs int } +type tcValue struct { + tc nltc + qdisc *tc.Object +} + //go:generate go run go.uber.org/mock/mockgen@v0.4.0 -source=types_linux.go -destination=mocks/mock_types.go -package=mocks -// Define the interfaces. -type IQdisc interface { +// tc qdisc interface +type qdisc interface { Add(info *tc.Object) error Delete(info *tc.Object) error } -type IFilter interface { +// tc filter interface +type filter interface { Add(info *tc.Object) error } -type ITc interface { +// netlink tc interface +type nltc interface { Qdisc() *tc.Qdisc Filter() *tc.Filter + SetOption(nl.ConnOption, bool) error Close() error } -type IPerf interface { +type perfReader interface { Read() (perf.Record, error) Close() error } -type val struct { - tcnl ITc - tcIngressObj *tc.Object - tcEgressObj *tc.Object -} - type packetParser struct { cfg *kcfg.Config l *log.ZapLogger @@ -104,7 +112,7 @@ type packetParser struct { objs *packetparserObjects //nolint:typecheck // tcMap is a map of key to *val. tcMap *sync.Map - reader IPerf + reader perfReader enricher enricher.EnricherInterface // interfaceLockMap is a map of key to *sync.Mutex. interfaceLockMap *sync.Map @@ -117,8 +125,8 @@ type packetParser struct { externalChannel chan *v1.Event } -func ifaceToKey(iface netlink.LinkAttrs) key { - return key{ +func ifaceToKey(iface netlink.LinkAttrs) tcKey { + return tcKey{ name: iface.Name, hardwareAddr: iface.HardwareAddr.String(), netNs: iface.NetNsID, From 727d1f223d35fe37f629fe15684a87803d2e6b2d Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 01:14:03 -0400 Subject: [PATCH 2/7] linter --- pkg/plugin/packetparser/packetparser_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/plugin/packetparser/packetparser_linux_test.go b/pkg/plugin/packetparser/packetparser_linux_test.go index 6cce6571eb..acdf8a079c 100644 --- a/pkg/plugin/packetparser/packetparser_linux_test.go +++ b/pkg/plugin/packetparser/packetparser_linux_test.go @@ -18,7 +18,7 @@ import ( v1 "github.com/cilium/cilium/pkg/hubble/api/v1" "github.com/cilium/ebpf" "github.com/cilium/ebpf/perf" - "github.com/florianl/go-tc" + tc "github.com/florianl/go-tc" kcfg "github.com/microsoft/retina/pkg/config" "github.com/microsoft/retina/pkg/enricher" "github.com/microsoft/retina/pkg/log" From 1288c6d3b576030587a0cabc78e4cc265e6db9e2 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 01:26:51 -0400 Subject: [PATCH 3/7] linter --- pkg/plugin/packetparser/packetparser_linux.go | 2 +- pkg/plugin/packetparser/types_linux.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/plugin/packetparser/packetparser_linux.go b/pkg/plugin/packetparser/packetparser_linux.go index d640fb3f58..ef766ee0e2 100644 --- a/pkg/plugin/packetparser/packetparser_linux.go +++ b/pkg/plugin/packetparser/packetparser_linux.go @@ -21,7 +21,7 @@ import ( v1 "github.com/cilium/cilium/pkg/hubble/api/v1" "github.com/cilium/ebpf" "github.com/cilium/ebpf/perf" - "github.com/florianl/go-tc" + tc "github.com/florianl/go-tc" helper "github.com/florianl/go-tc/core" nl "github.com/mdlayher/netlink" "github.com/microsoft/retina/internal/ktime" diff --git a/pkg/plugin/packetparser/types_linux.go b/pkg/plugin/packetparser/types_linux.go index 0624f5c82a..4977f65207 100644 --- a/pkg/plugin/packetparser/types_linux.go +++ b/pkg/plugin/packetparser/types_linux.go @@ -11,7 +11,7 @@ import ( v1 "github.com/cilium/cilium/pkg/hubble/api/v1" "github.com/cilium/ebpf" "github.com/cilium/ebpf/perf" - "github.com/florianl/go-tc" + tc "github.com/florianl/go-tc" nl "github.com/mdlayher/netlink" "github.com/vishvananda/netlink" From 88b80b28c83247ee35a9bcf1a0f4f90f7fefda48 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 10:33:39 -0400 Subject: [PATCH 4/7] ut --- pkg/plugin/packetparser/packetparser_linux_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/plugin/packetparser/packetparser_linux_test.go b/pkg/plugin/packetparser/packetparser_linux_test.go index acdf8a079c..79916e8b05 100644 --- a/pkg/plugin/packetparser/packetparser_linux_test.go +++ b/pkg/plugin/packetparser/packetparser_linux_test.go @@ -19,6 +19,7 @@ import ( "github.com/cilium/ebpf" "github.com/cilium/ebpf/perf" tc "github.com/florianl/go-tc" + nl "github.com/mdlayher/netlink" kcfg "github.com/microsoft/retina/pkg/config" "github.com/microsoft/retina/pkg/enricher" "github.com/microsoft/retina/pkg/log" @@ -188,6 +189,7 @@ func TestCreateQdiscAndAttach(t *testing.T) { mrtnl := mocks.NewMocknltc(ctrl) mrtnl.EXPECT().Qdisc().Return(nil).AnyTimes() + mrtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).AnyTimes() getQdisc = func(nltc) qdisc { return mq @@ -357,6 +359,7 @@ func TestStartWithDataAggregationLevelLow(t *testing.T) { mQdisc.EXPECT().Add(gomock.Any()).Return(nil).Times(1) mockRtnl := mocks.NewMocknltc(ctrl) + mockRtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).Times(1) bpfEvent := &packetparserPacket{ SrcIp: uint32(83886272), // 192.0.0.5 From fd695d2eb0869f4b59deb0dca2103ff55e94aa2b Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 14:42:30 -0400 Subject: [PATCH 5/7] Update packetparser_linux.go Signed-off-by: Quang Nguyen --- pkg/plugin/packetparser/packetparser_linux.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/plugin/packetparser/packetparser_linux.go b/pkg/plugin/packetparser/packetparser_linux.go index ef766ee0e2..c4748164fe 100644 --- a/pkg/plugin/packetparser/packetparser_linux.go +++ b/pkg/plugin/packetparser/packetparser_linux.go @@ -321,12 +321,15 @@ func (p *packetParser) cleanAll() error { return nil } -func (p *packetParser) clean(tcnl nltc, qdisc *tc.Object) { +func (p *packetParser) clean(rtnl nltc, qdisc *tc.Object) { // Warning, not error. Clean is best effort. - if tcnl != nil { - if err := getQdisc(tcnl).Delete(qdisc); err != nil && !errors.Is(err, tc.ErrNoArg) { + if rtnl != nil { + if err := getQdisc(rtnl).Delete(qdisc); err != nil && !errors.Is(err, tc.ErrNoArg) { p.l.Debug("could not delete egress qdisc", zap.Error(err)) } + if err := rtnl.Close(); err != nil { + p.l.Warn("could not close rtnetlink socket", zap.Error(err)) + } } } From 138d592bf3a1300da8ca9f2c1a94eef1d3a63147 Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Thu, 19 Sep 2024 18:00:53 -0400 Subject: [PATCH 6/7] update UT --- pkg/plugin/packetparser/packetparser_linux_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/plugin/packetparser/packetparser_linux_test.go b/pkg/plugin/packetparser/packetparser_linux_test.go index 79916e8b05..19ccdee962 100644 --- a/pkg/plugin/packetparser/packetparser_linux_test.go +++ b/pkg/plugin/packetparser/packetparser_linux_test.go @@ -69,6 +69,7 @@ func TestCleanAll(t *testing.T) { mrtnl := mocks.NewMocknltc(ctrl) mrtnl.EXPECT().Close().Return(nil).AnyTimes() + mrtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).AnyTimes() mq := mocks.NewMockqdisc(ctrl) mq.EXPECT().Delete(gomock.Any()).Return(nil).AnyTimes() @@ -110,6 +111,7 @@ func TestClean(t *testing.T) { mrtnl := mocks.NewMocknltc(ctrl) mrtnl.EXPECT().Qdisc().Return(nil).Times(1) + mrtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).AnyTimes() getQdisc = func(tcnl nltc) qdisc { // Add this verify tcnl.Qdisc() is called twice From 4fb94c11f0197383c2b62456d22bfbeff078b65d Mon Sep 17 00:00:00 2001 From: Quang Nguyen Date: Fri, 20 Sep 2024 00:25:49 -0400 Subject: [PATCH 7/7] add expect close call --- pkg/plugin/packetparser/packetparser_linux_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/plugin/packetparser/packetparser_linux_test.go b/pkg/plugin/packetparser/packetparser_linux_test.go index 19ccdee962..80c010da56 100644 --- a/pkg/plugin/packetparser/packetparser_linux_test.go +++ b/pkg/plugin/packetparser/packetparser_linux_test.go @@ -111,6 +111,7 @@ func TestClean(t *testing.T) { mrtnl := mocks.NewMocknltc(ctrl) mrtnl.EXPECT().Qdisc().Return(nil).Times(1) + mrtnl.EXPECT().Close().Return(nil).AnyTimes() mrtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).AnyTimes() getQdisc = func(tcnl nltc) qdisc { @@ -138,6 +139,8 @@ func TestCleanWithErrors(t *testing.T) { mq.EXPECT().Delete(gomock.Any()).Return(errors.New("error")).Times(1) //nolint:err113 // ignore mrtnl := mocks.NewMocknltc(ctrl) + mrtnl.EXPECT().Close().Return(nil).AnyTimes() + mrtnl.EXPECT().SetOption(nl.ExtendedAcknowledge, true).Return(nil).AnyTimes() mrtnl.EXPECT().Qdisc().Return(nil).AnyTimes() getQdisc = func(nltc) qdisc {