From 57bba301baeac01fbbe994326d64a082d9367841 Mon Sep 17 00:00:00 2001 From: gavinz-ship-it Date: Tue, 27 Oct 2026 16:16:47 -0700 Subject: [PATCH 1/5] Create common_test.go Add unit tests for common.go --- .../forwarding/fwdport/ports/common_test.go | 385 ++++++++++++++++++ 1 file changed, 385 insertions(+) create mode 100644 dataplane/forwarding/fwdport/ports/common_test.go diff --git a/dataplane/forwarding/fwdport/ports/common_test.go b/dataplane/forwarding/fwdport/ports/common_test.go new file mode 100644 index 00000000..4747d1d1 --- /dev/null +++ b/dataplane/forwarding/fwdport/ports/common_test.go @@ -0,0 +1,385 @@ +// Copyright 2023 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build linux + +package ports + +import ( + "fmt" + "net" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/openconfig/gnmi/errdiff" + "github.com/vishvananda/netlink" + "google.golang.org/protobuf/testing/protocmp" + + "github.com/openconfig/lemming/dataplane/forwarding/fwdport/porttestutil" + "github.com/openconfig/lemming/dataplane/forwarding/infra/fwdcontext" + fwdpb "github.com/openconfig/lemming/proto/forwarding" +) + +type fakeStateIfaceMgr struct { + link netlink.Link + linkErr error + setStateErr error + gotName string + gotUp *bool +} + +func (f *fakeStateIfaceMgr) LinkByName(string) (netlink.Link, error) { + return f.link, f.linkErr +} + +func (f *fakeStateIfaceMgr) SetState(name string, up bool) error { + f.gotName = name + f.gotUp = &up + return f.setStateErr +} + +func boolPtr(b bool) *bool { + return &b +} + +func TestGetAndSetState(t *testing.T) { + tests := []struct { + desc string + ifMgr *fakeStateIfaceMgr + pi *fwdpb.PortInfo + want *fwdpb.PortStateReply + wantSetUp *bool + wantErr string + }{{ + desc: "link by name error", + ifMgr: &fakeStateIfaceMgr{ + linkErr: fmt.Errorf("no such link"), + }, + wantErr: "no such link", + }, { + desc: "read state admin down oper down", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: 0, + OperState: netlink.OperDown, + }, + }, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + }, + }, { + desc: "read state admin up oper lower layer down", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperLowerLayerDown, + }, + }, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + }, + }, { + desc: "read state admin up oper unknown", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperUnknown, + }, + }, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + }, + }, { + desc: "read state admin up oper up", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + }, + }, { + desc: "set state unchanged admin state", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + }, + pi: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + }, + }, { + desc: "set state enable up", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: 0, + OperState: netlink.OperDown, + }, + }, + }, + pi: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + }, + wantSetUp: boolPtr(true), + }, { + desc: "set state disable down", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + }, + pi: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + }, + wantSetUp: boolPtr(false), + }, { + desc: "set state unspecified admin status", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + }, + pi: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_UNSPECIFIED, + }, + want: &fwdpb.PortStateReply{ + Status: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + }, + }, { + desc: "set state error", + ifMgr: &fakeStateIfaceMgr{ + link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Flags: 0, + OperState: netlink.OperDown, + }, + }, + setStateErr: fmt.Errorf("set state failed"), + }, + pi: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + wantSetUp: boolPtr(true), + wantErr: "set state failed", + }} + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + got, err := getAndSetState("eth0", tt.ifMgr, tt.pi) + if d := errdiff.Check(err, tt.wantErr); d != "" { + t.Fatalf("getAndSetState() unexpected error: %s", d) + } + if d := cmp.Diff(got, tt.want, protocmp.Transform()); d != "" { + t.Errorf("getAndSetState() unexpected diff (-got, +want):\n%s", d) + } + if d := cmp.Diff(tt.ifMgr.gotUp, tt.wantSetUp); d != "" { + t.Errorf("SetState() unexpected up value (-got, +want):\n%s", d) + } + }) + } +} + +type fakeLinkSubscriber struct { + updCh chan<- netlink.LinkUpdate + doneCh <-chan struct{} + subErr error + subCalls int +} + +func (f *fakeLinkSubscriber) LinkSubscribe(ch chan<- netlink.LinkUpdate, done <-chan struct{}) error { + f.subCalls++ + if f.subErr != nil { + return f.subErr + } + f.updCh = ch + f.doneCh = done + return nil +} + +func TestLinkStateWatcher(t *testing.T) { + t.Run("watch notify and unwatch", func(t *testing.T) { + sub := &fakeLinkSubscriber{} + w := &linkStateWatcher{ + ifaceMgr: sub, + ports: make(map[string]portTarget), + } + ctx := fwdcontext.New("test-ctx", "fwd") + eventCh := make(chan *fwdpb.EventDesc, 10) + if err := ctx.SetNotification(func(event *fwdpb.EventDesc) { + eventCh <- event + }); err != nil { + t.Fatalf("SetNotification() failed: %v", err) + } + + p1 := porttestutil.CreateTestPort(t, ctx, "port1") + p2 := porttestutil.CreateTestPort(t, ctx, "port2") + + if err := w.watch("eth0", p1, ctx); err != nil { + t.Fatalf("watch(eth0) unexpected error: %v", err) + } + if err := w.watch("eth1", p2, ctx); err != nil { + t.Fatalf("watch(eth1) unexpected error: %v", err) + } + if sub.subCalls != 1 { + t.Fatalf("LinkSubscribe() calls = %d, want 1", sub.subCalls) + } + + // Update for unwatched interface should be ignored. + sub.updCh <- netlink.LinkUpdate{ + Link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: "unwatched0", + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + } + + // Update for eth0 (admin up, oper up). + sub.updCh <- netlink.LinkUpdate{ + Link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: "eth0", + Flags: net.FlagUp, + OperState: netlink.OperUp, + }, + }, + } + + got := <-eventCh + want := &fwdpb.EventDesc{ + Event: fwdpb.Event_EVENT_PORT, + Desc: &fwdpb.EventDesc_Port{ + Port: &fwdpb.PortEventDesc{ + Context: &fwdpb.ContextId{Id: "test-ctx"}, + PortId: &fwdpb.PortId{ObjectId: &fwdpb.ObjectId{Id: "port1"}}, + PortInfo: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + }, + }, + }, + } + if d := cmp.Diff(got, want, protocmp.Transform(), protocmp.IgnoreFields(&fwdpb.EventDesc{}, "sequence_number")); d != "" { + t.Errorf("received event diff (-got, +want):\n%s", d) + } + + // Unwatch eth0; watcher should remain active because eth1 is still watched. + w.unwatch("eth0") + select { + case <-sub.doneCh: + t.Fatal("doneCh closed before all ports were unwatched") + default: + } + + // Subsequent update for eth0 should be ignored, while eth1 update should notify. + sub.updCh <- netlink.LinkUpdate{ + Link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: "eth0", + Flags: 0, + OperState: netlink.OperDown, + }, + }, + } + sub.updCh <- netlink.LinkUpdate{ + Link: &netlink.Dummy{ + LinkAttrs: netlink.LinkAttrs{ + Name: "eth1", + Flags: net.FlagUp, + OperState: netlink.OperLowerLayerDown, + }, + }, + } + + got = <-eventCh + want = &fwdpb.EventDesc{ + Event: fwdpb.Event_EVENT_PORT, + Desc: &fwdpb.EventDesc_Port{ + Port: &fwdpb.PortEventDesc{ + Context: &fwdpb.ContextId{Id: "test-ctx"}, + PortId: &fwdpb.PortId{ObjectId: &fwdpb.ObjectId{Id: "port2"}}, + PortInfo: &fwdpb.PortInfo{ + AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, + OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, + }, + }, + }, + } + if d := cmp.Diff(got, want, protocmp.Transform(), protocmp.IgnoreFields(&fwdpb.EventDesc{}, "sequence_number")); d != "" { + t.Errorf("received event diff (-got, +want):\n%s", d) + } + + // Unwatch last port; doneCh is closed synchronously in unwatch. + w.unwatch("eth1") + select { + case <-sub.doneCh: + default: + t.Fatal("doneCh was not closed after unwatching all ports") + } + }) +} From e879fdfb10dadf02b2d0192207fb7c3308b899ba Mon Sep 17 00:00:00 2001 From: gavinz-ship-it Date: Tue, 27 Oct 2026 16:18:53 -0700 Subject: [PATCH 2/5] Update BUILD Update BUILD file for common_test.go --- dataplane/forwarding/fwdport/ports/BUILD | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/dataplane/forwarding/fwdport/ports/BUILD b/dataplane/forwarding/fwdport/ports/BUILD index d1b89615..a8bf9a23 100644 --- a/dataplane/forwarding/fwdport/ports/BUILD +++ b/dataplane/forwarding/fwdport/ports/BUILD @@ -23,34 +23,25 @@ go_library( "//dataplane/forwarding/infra/fwdpacket", "//dataplane/forwarding/util/hash/crc16", "//dataplane/forwarding/util/queue", + "//dataplane/kernel", "//dataplane/proto/packetio", "//internal/debug", "//proto/forwarding", "@com_github_golang_glog//:glog", "@com_github_google_gopacket//:gopacket", "@com_github_google_gopacket//layers", + "@com_github_google_gopacket//pcap", "@com_github_google_gopacket//pcapgo", - ] + select({ - "@io_bazel_rules_go//go/platform:android": [ - "//dataplane/kernel", - "@com_github_google_gopacket//pcap", - "@com_github_vishvananda_netlink//:netlink", - "@org_golang_x_sys//unix", - ], - "@io_bazel_rules_go//go/platform:linux": [ - "//dataplane/kernel", - "@com_github_google_gopacket//pcap", - "@com_github_vishvananda_netlink//:netlink", - "@org_golang_x_sys//unix", - ], - "//conditions:default": [], - }), + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_x_sys//unix", + ], ) go_test( name = "ports_test", size = "small", srcs = [ + "common_test.go", "cpu_test.go", "fake_test.go", "group_test.go", @@ -77,6 +68,8 @@ go_test( "@com_github_google_gopacket//layers", "@com_github_google_gopacket//pcapgo", "@com_github_openconfig_gnmi//errdiff", + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_google_protobuf//testing/protocmp", "@org_uber_go_mock//gomock", ], ) From 75b341a0dbd29091c7aa6451815c6853a4ba6ffe Mon Sep 17 00:00:00 2001 From: gavinz-ship-it Date: Tue, 27 Oct 2026 16:35:24 -0700 Subject: [PATCH 3/5] Update common_test.go Minor changes. --- dataplane/forwarding/fwdport/ports/common_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dataplane/forwarding/fwdport/ports/common_test.go b/dataplane/forwarding/fwdport/ports/common_test.go index 4747d1d1..1acb2c41 100644 --- a/dataplane/forwarding/fwdport/ports/common_test.go +++ b/dataplane/forwarding/fwdport/ports/common_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/openconfig/gnmi/errdiff" "github.com/vishvananda/netlink" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/testing/protocmp" "github.com/openconfig/lemming/dataplane/forwarding/fwdport/porttestutil" @@ -49,10 +50,6 @@ func (f *fakeStateIfaceMgr) SetState(name string, up bool) error { return f.setStateErr } -func boolPtr(b bool) *bool { - return &b -} - func TestGetAndSetState(t *testing.T) { tests := []struct { desc string @@ -169,7 +166,7 @@ func TestGetAndSetState(t *testing.T) { OperStatus: fwdpb.PortState_PORT_STATE_DISABLED_DOWN, }, }, - wantSetUp: boolPtr(true), + wantSetUp: proto.Bool(true), }, { desc: "set state disable down", ifMgr: &fakeStateIfaceMgr{ @@ -189,7 +186,7 @@ func TestGetAndSetState(t *testing.T) { OperStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, }, }, - wantSetUp: boolPtr(false), + wantSetUp: proto.Bool(false), }, { desc: "set state unspecified admin status", ifMgr: &fakeStateIfaceMgr{ @@ -223,7 +220,7 @@ func TestGetAndSetState(t *testing.T) { pi: &fwdpb.PortInfo{ AdminStatus: fwdpb.PortState_PORT_STATE_ENABLED_UP, }, - wantSetUp: boolPtr(true), + wantSetUp: proto.Bool(true), wantErr: "set state failed", }} From 382a1ad630a891ec44fbf33e86c9287c61f5f629 Mon Sep 17 00:00:00 2001 From: gavinz-ship-it Date: Tue, 27 Oct 2026 16:35:41 -0700 Subject: [PATCH 4/5] Update BUILD minor changes --- dataplane/forwarding/fwdport/ports/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/dataplane/forwarding/fwdport/ports/BUILD b/dataplane/forwarding/fwdport/ports/BUILD index a8bf9a23..d00c019d 100644 --- a/dataplane/forwarding/fwdport/ports/BUILD +++ b/dataplane/forwarding/fwdport/ports/BUILD @@ -69,6 +69,7 @@ go_test( "@com_github_google_gopacket//pcapgo", "@com_github_openconfig_gnmi//errdiff", "@com_github_vishvananda_netlink//:netlink", + "@org_golang_google_protobuf//proto", "@org_golang_google_protobuf//testing/protocmp", "@org_uber_go_mock//gomock", ], From 91f8d4ee6c521067001eecec85c35bc6c17ac171 Mon Sep 17 00:00:00 2001 From: gavinz-ship-it Date: Tue, 27 Oct 2026 17:01:45 -0700 Subject: [PATCH 5/5] Update BUILD Fix gazelle errors. --- dataplane/forwarding/fwdport/ports/BUILD | 37 ++++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/dataplane/forwarding/fwdport/ports/BUILD b/dataplane/forwarding/fwdport/ports/BUILD index d00c019d..61ac66c0 100644 --- a/dataplane/forwarding/fwdport/ports/BUILD +++ b/dataplane/forwarding/fwdport/ports/BUILD @@ -23,18 +23,28 @@ go_library( "//dataplane/forwarding/infra/fwdpacket", "//dataplane/forwarding/util/hash/crc16", "//dataplane/forwarding/util/queue", - "//dataplane/kernel", "//dataplane/proto/packetio", "//internal/debug", "//proto/forwarding", "@com_github_golang_glog//:glog", "@com_github_google_gopacket//:gopacket", "@com_github_google_gopacket//layers", - "@com_github_google_gopacket//pcap", "@com_github_google_gopacket//pcapgo", - "@com_github_vishvananda_netlink//:netlink", - "@org_golang_x_sys//unix", - ], + ] + select({ + "@io_bazel_rules_go//go/platform:android": [ + "//dataplane/kernel", + "@com_github_google_gopacket//pcap", + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_x_sys//unix", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "//dataplane/kernel", + "@com_github_google_gopacket//pcap", + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_x_sys//unix", + ], + "//conditions:default": [], + }), ) go_test( @@ -68,9 +78,18 @@ go_test( "@com_github_google_gopacket//layers", "@com_github_google_gopacket//pcapgo", "@com_github_openconfig_gnmi//errdiff", - "@com_github_vishvananda_netlink//:netlink", - "@org_golang_google_protobuf//proto", - "@org_golang_google_protobuf//testing/protocmp", "@org_uber_go_mock//gomock", - ], + ] + select({ + "@io_bazel_rules_go//go/platform:android": [ + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_google_protobuf//proto", + "@org_golang_google_protobuf//testing/protocmp", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "@com_github_vishvananda_netlink//:netlink", + "@org_golang_google_protobuf//proto", + "@org_golang_google_protobuf//testing/protocmp", + ], + "//conditions:default": [], + }), )