From 8f10473e67ccdda5d470edab984d5f0aa70cdc67 Mon Sep 17 00:00:00 2001 From: Guo-shiuan Wang Date: Thu, 28 Mar 2024 16:48:14 +0000 Subject: [PATCH 1/4] More tests with debugging. --- go.mod | 2 +- go.sum | 4 +- .../basictraffic/basic_traffic_test.go | 15 +++++ .../dataplane/mymac/mymac_test.go | 67 ++++++++++++------- repositories.bzl | 16 ++--- 5 files changed, 69 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index f1235419..6a0d0e4f 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/openconfig/gribi v1.0.0 github.com/openconfig/gribigo v0.0.0-20240314002941-b59d04db23bd github.com/openconfig/kne v0.1.16 - github.com/openconfig/magna v0.0.0-20240125181018-b59ccfe781f8 + github.com/openconfig/magna v0.0.0-20240326180454-518e16696c84 github.com/openconfig/ondatra v0.5.2 github.com/openconfig/ygnmi v0.11.1 github.com/openconfig/ygot v0.29.18 diff --git a/go.sum b/go.sum index fec128b8..137d943b 100644 --- a/go.sum +++ b/go.sum @@ -1560,8 +1560,8 @@ github.com/openconfig/kne v0.1.16 h1:3bvkinEoT4xo0c5kcqnotdboq3ZfFHoVRP1YZja97W0 github.com/openconfig/kne v0.1.16/go.mod h1:WROHtukCSPVMFEE8dfIec9RneqLDNqaFf92wTAVuAvk= github.com/openconfig/lemming/operator v0.2.0 h1:dovZnR6lQkOHXcODli1NDOr/GVYrBY05KS5X11jxVbw= github.com/openconfig/lemming/operator v0.2.0/go.mod h1:LKgEXSR5VK2CAeh2uKijKAXFj42uQuwakrCHVPF0iII= -github.com/openconfig/magna v0.0.0-20240125181018-b59ccfe781f8 h1:Li3oruOjubVSr+7UAc6ohpWeD0FKk+7t7SzEkMN66Qk= -github.com/openconfig/magna v0.0.0-20240125181018-b59ccfe781f8/go.mod h1:3B8JDwmq2vuR0J1JB4+XtpxC+FrVvtZa1129LJ+7AIw= +github.com/openconfig/magna v0.0.0-20240326180454-518e16696c84 h1:tiy1WDRRFuWXCA9o0We8x3an3vywDVowyertHgjEUXo= +github.com/openconfig/magna v0.0.0-20240326180454-518e16696c84/go.mod h1:HxoDKtQaYwLhAQCAGpq5F16kdtxTKD4Vj2NikVn8+Gw= github.com/openconfig/ondatra v0.5.2 h1:AUBT+KlPzP/bgiCbdOY+PrJdmK4o/Upc1aD20VKWx20= github.com/openconfig/ondatra v0.5.2/go.mod h1:Ez1STBlzUo+Zq5bONkE2R0lhnGArevnrXpoak8MiWAE= github.com/openconfig/testt v0.0.0-20220311054427-efbb1a32ec07 h1:X631iD/B0ximGFb5P9LY5wHju4SiedxUhc5UZEo7VSw= diff --git a/integration_tests/dataplane/basictraffic/basic_traffic_test.go b/integration_tests/dataplane/basictraffic/basic_traffic_test.go index 91779fd9..7535e88a 100644 --- a/integration_tests/dataplane/basictraffic/basic_traffic_test.go +++ b/integration_tests/dataplane/basictraffic/basic_traffic_test.go @@ -193,6 +193,21 @@ func TestTraffic(t *testing.T) { } } +func TestTrafficAgain(t *testing.T) { + ate := ondatra.ATE(t, "ate") + ateTop := configureATE(t, ate) + ate.OTG().PushConfig(t, ateTop) + ate.OTG().StartProtocols(t) + + dut := ondatra.DUT(t, "dut") + configureDUT(t, dut) + + loss := testTraffic(t, ate, ateTop, atePort1, dutPort1, atePort2) + if loss > 1 { + t.Errorf("loss %f, greater than 1", loss) + } +} + // configureATE configures port1 and port2 on the ATE. func configureATE(t *testing.T, ate *ondatra.ATEDevice) gosnappi.Config { top := gosnappi.NewConfig() diff --git a/integration_tests/dataplane/mymac/mymac_test.go b/integration_tests/dataplane/mymac/mymac_test.go index f5bd85c5..30c55c65 100644 --- a/integration_tests/dataplane/mymac/mymac_test.go +++ b/integration_tests/dataplane/mymac/mymac_test.go @@ -227,27 +227,51 @@ func TestMyMac(t *testing.T) { macAddrMask []byte passed bool }{{ + desc: "Traffic passed", + clearMyMac: false, + passed: true, + }, { desc: "Traffic dropped", // Remove the default entry that allows all traffic to L3. clearMyMac: true, passed: false, + }, { + desc: "Traffic passed again", // Remove the default entry that allows all traffic to L3. + clearMyMac: false, + passed: true, + // }, { + // desc: "Traffic passed again", + // clearMyMac: false, + // macAddr: net.HardwareAddr(atePort1.MAC), + // macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + // passed: true, }} - ate := ondatra.ATE(t, "ate") - ateTop := configureATE(t, ate) - ate.OTG().PushConfig(t, ateTop) - ate.OTG().StartProtocols(t) + // ate := ondatra.ATE(t, "ate") + // ateTop := configureATE(t, ate) + // ate.OTG().PushConfig(t, ateTop) + // ate.OTG().StartProtocols(t) - dut := ondatra.DUT(t, "dut") - configureDUT(t, dut) + // dut := ondatra.DUT(t, "dut") + // configureDUT(t, dut) for _, tt := range tests { + ate := ondatra.ATE(t, "ate") + ateTop := configureATE(t, ate) + ate.OTG().PushConfig(t, ateTop) + ate.OTG().StartProtocols(t) + + dut := ondatra.DUT(t, "dut") + configureDUT(t, dut) + tx, rx := testTraffic(t, ate, ateTop, atePort1, dutPort1, atePort2, 10*time.Second, dut, tt.clearMyMac) t.Logf("[%s] Got TX: %d, RX: %d", tt.desc, tx, rx) if tx == 0 { t.Fatalf("No packet sent") } switch { - case tt.passed && rx != tx, !tt.passed && rx != 0: + case tt.passed && rx != tx: t.Errorf("got %d, expect %d", rx, tx) + case !tt.passed && rx != 0: + t.Errorf("got %d, expect 0", rx) } } } @@ -275,36 +299,31 @@ func testTraffic(t *testing.T, ate *ondatra.ATEDevice, top gosnappi.Config, srcE otg := ate.OTG() top.Flows().Clear().Items() - ipFLow := top.Flows().Add().SetName("Flow") - ipFLow.Metrics().SetEnable(true) - ipFLow.TxRx().Port().SetTxName(srcEndPoint.Name).SetRxNames([]string{dstEndPoint.Name}) - - ipFLow.Rate().SetPps(10) + ipFlow := top.Flows().Add().SetName("Flow") + ipFlow.Metrics().SetEnable(true) + ipFlow.TxRx().Port().SetTxName(srcEndPoint.Name).SetRxNames([]string{dstEndPoint.Name}) - // OTG specifies that the order of the .Packet().Add() calls determines - // the stack of headers that are to be used, starting at the outer-most and - // ending with the inner-most. + txPkts := uint64(1) + ipFlow.Rate().SetPps(1) + ipFlow.Duration().FixedPackets().SetPackets(uint32(txPkts)) // Set up ethernet layer. - eth := ipFLow.Packet().Add().Ethernet() + eth := ipFlow.Packet().Add().Ethernet() eth.Src().SetValue(srcEndPoint.MAC) eth.Dst().SetValue(srcPeerEndpoint.MAC) - ip4 := ipFLow.Packet().Add().Ipv4() + ip4 := ipFlow.Packet().Add().Ipv4() ip4.Src().SetValue(srcEndPoint.IPv4) ip4.Dst().SetValue(dstEndPoint.IPv4) ip4.Version().SetValue(4) otg.PushConfig(t, top) - + t.Logf(">>>>>> Sending traffic...") otg.StartTraffic(t) - time.Sleep(dur) - t.Logf("Stop traffic") - otg.StopTraffic(t) - - time.Sleep(5 * time.Second) + defer otg.StopTraffic(t) - txPkts := gnmi.Get(t, otg, gnmi.OTG().Flow("Flow").Counters().OutPkts().State()) + gnmi.Await(t, otg, gnmi.OTG().Flow("Flow").Counters().OutPkts().State(), 5*time.Second, txPkts) + t.Logf(">>>>>> Traffic received.") rxPkts := gnmi.Get(t, otg, gnmi.OTG().Flow("Flow").Counters().InPkts().State()) return txPkts, rxPkts } diff --git a/repositories.bzl b/repositories.bzl index 70255573..6bb82a4d 100644 --- a/repositories.bzl +++ b/repositories.bzl @@ -983,8 +983,8 @@ def go_repositories(): go_repository( name = "com_github_grpc_ecosystem_go_grpc_middleware", importpath = "github.com/grpc-ecosystem/go-grpc-middleware", - sum = "h1:+9834+KizmvFV7pXQGSXQTsaWhq2GjuNUt0aUU0YBYw=", - version = "v1.3.0", + sum = "h1:UH//fgunKIs4JdUbpDl1VZCDaL56wXCB/5+wF6uHfaI=", + version = "v1.4.0", ) go_repository( name = "com_github_grpc_ecosystem_go_grpc_middleware_v2", @@ -1615,8 +1615,8 @@ def go_repositories(): go_repository( name = "com_github_open_traffic_generator_ixia_c_operator", importpath = "github.com/open-traffic-generator/ixia-c-operator", - sum = "h1:xH0hLVWf2wuVUT9ovFdh/WwwBf1oGkBu5YEWD61igck=", - version = "v0.3.4", + sum = "h1:dablUs6FAToVDFaoIo2M+Z9UCa93KAwlj7HJqNwLwTQ=", + version = "v0.3.6", ) go_repository( name = "com_github_open_traffic_generator_keng_operator", @@ -1648,8 +1648,8 @@ def go_repositories(): go_repository( name = "com_github_openconfig_featureprofiles", importpath = "github.com/openconfig/featureprofiles", - sum = "h1:3GlQ1wjWqTN+w0kLHtGxACCJ3CRnHjJ8McYwSkWpZXY=", - version = "v0.0.0-20230629171650-1057c66c4e52", + sum = "h1:InOTVH2wHIvMedgOmDwzrzTJo12JaEmKwdDtckhpb/Y=", + version = "v0.0.0-20240122173202-7718e50374f2", ) go_repository( @@ -1745,8 +1745,8 @@ def go_repositories(): go_repository( name = "com_github_openconfig_magna", importpath = "github.com/openconfig/magna", - sum = "h1:Li3oruOjubVSr+7UAc6ohpWeD0FKk+7t7SzEkMN66Qk=", - version = "v0.0.0-20240125181018-b59ccfe781f8", + sum = "h1:tiy1WDRRFuWXCA9o0We8x3an3vywDVowyertHgjEUXo=", + version = "v0.0.0-20240326180454-518e16696c84", ) go_repository( From ddf3ccffc625173d19853e48ee4d492ef9d0d84d Mon Sep 17 00:00:00 2001 From: Guo-shiuan Wang Date: Fri, 29 Mar 2024 00:10:14 +0000 Subject: [PATCH 2/4] Workaround the Magna bug by using different flowID for MyMac tests. --- dataplane/saiserver/acl.go | 89 ++++---- .../dataplane/mymac/mymac_test.go | 194 ++++++++++++------ 2 files changed, 182 insertions(+), 101 deletions(-) diff --git a/dataplane/saiserver/acl.go b/dataplane/saiserver/acl.go index 3a78d5aa..e59efbcc 100644 --- a/dataplane/saiserver/acl.go +++ b/dataplane/saiserver/acl.go @@ -293,36 +293,28 @@ func (a *acl) CreateAclEntry(ctx context.Context, req *saipb.CreateAclEntryReque return &saipb.CreateAclEntryResponse{Oid: id}, nil } -type myMacInfo struct { - priority *uint32 - portID *uint64 // wildcard if not specified - vlanID *uint16 // wildcard if not specified - macAddress []byte - macAddressMask []byte -} - -// ToEntryDesc returns the EntryDesc. -func (mi *myMacInfo) ToEntryDesc(m *myMac) (*fwdpb.EntryDesc, error) { - if mi.priority == nil { +// entryDescFromReq returns the EntryDesc based on req. +func entryDescFromReq(m *myMac, req *saipb.CreateMyMacRequest) (*fwdpb.EntryDesc, error) { + if req.Priority == nil { return nil, fmt.Errorf("priority needs to be specified") } fields := []*fwdconfig.PacketFieldMaskedBytesBuilder{ fwdconfig.PacketFieldMaskedBytes(fwdpb.PacketFieldNum_PACKET_FIELD_NUM_ETHER_MAC_DST). - WithBytes(mi.macAddress, mi.macAddressMask), + WithBytes(req.GetMacAddress(), req.GetMacAddressMask()), } - if mi.vlanID != nil { + if req.VlanId != nil { fields = append(fields, fwdconfig.PacketFieldMaskedBytes(fwdpb.PacketFieldNum_PACKET_FIELD_NUM_VLAN_TAG). - WithBytes(binary.BigEndian.AppendUint16(nil, *mi.vlanID), binary.BigEndian.AppendUint16(nil, 0x0FFF))) + WithBytes(binary.BigEndian.AppendUint16(nil, uint16(req.GetVlanId())), binary.BigEndian.AppendUint16(nil, 0x0FFF))) } - if mi.portID != nil { + if req.PortId != nil { fwdCtx, err := m.dataplane.FindContext(&fwdpb.ContextId{Id: m.dataplane.ID()}) if err != nil { return nil, err } - obj, err := fwdCtx.Objects.FindID(&fwdpb.ObjectId{Id: fmt.Sprint(*mi.portID)}) + obj, err := fwdCtx.Objects.FindID(&fwdpb.ObjectId{Id: fmt.Sprint(req.GetPortId())}) if err != nil { return nil, err } @@ -334,7 +326,7 @@ func (mi *myMacInfo) ToEntryDesc(m *myMac) (*fwdpb.EntryDesc, error) { ed := &fwdpb.EntryDesc{ Entry: &fwdpb.EntryDesc_Flow{ Flow: &fwdpb.FlowEntryDesc{ - Priority: *mi.priority, + Priority: req.GetPriority(), Bank: 1, }, }, @@ -347,37 +339,25 @@ func (mi *myMacInfo) ToEntryDesc(m *myMac) (*fwdpb.EntryDesc, error) { type myMac struct { saipb.UnimplementedMyMacServer - mgr *attrmgr.AttrMgr - dataplane switchDataplaneAPI - myMacTable map[uint64]*myMacInfo + mgr *attrmgr.AttrMgr + dataplane switchDataplaneAPI } func newMyMac(mgr *attrmgr.AttrMgr, dataplane switchDataplaneAPI, s *grpc.Server) *myMac { m := &myMac{ - mgr: mgr, - dataplane: dataplane, - myMacTable: map[uint64]*myMacInfo{}, + mgr: mgr, + dataplane: dataplane, } saipb.RegisterMyMacServer(s, m) return m } func (m *myMac) CreateMyMac(ctx context.Context, req *saipb.CreateMyMacRequest) (*saipb.CreateMyMacResponse, error) { - mi := &myMacInfo{ - priority: req.Priority, - portID: req.PortId, - macAddress: req.GetMacAddress(), - macAddressMask: req.GetMacAddressMask(), - } - if req.VlanId != nil { - vid := (uint16)(req.GetVlanId()) - mi.vlanID = &vid - } - if mi.macAddress == nil || mi.macAddressMask == nil { + if req.MacAddress == nil || req.MacAddressMask == nil { return nil, status.Errorf(codes.InvalidArgument, "MAC address and MAC address mask cannot be empty") } id := m.mgr.NextID() - ed, err := mi.ToEntryDesc(m) + ed, err := entryDescFromReq(m, req) if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "failed to create entry descriptor: %v", err) } @@ -390,7 +370,6 @@ func (m *myMac) CreateMyMac(ctx context.Context, req *saipb.CreateMyMacRequest) if _, err := m.dataplane.TableEntryAdd(ctx, mReq); err != nil { return nil, err } - m.myMacTable[id] = mi // Populate the switch attribute. saReq := &saipb.GetSwitchAttributeRequest{ Oid: switchID, @@ -410,22 +389,50 @@ func (m *myMac) CreateMyMac(ctx context.Context, req *saipb.CreateMyMacRequest) } func (m *myMac) RemoveMyMac(ctx context.Context, req *saipb.RemoveMyMacRequest) (*saipb.RemoveMyMacResponse, error) { - mi, ok := m.myMacTable[req.GetOid()] - if !ok { - return nil, status.Errorf(codes.FailedPrecondition, "%d does not exist", req.GetOid()) + cReq := &saipb.CreateMyMacRequest{} + if err := m.mgr.PopulateAllAttributes(fmt.Sprint(req.GetOid()), cReq); err != nil { + return nil, err } - ed, err := mi.ToEntryDesc(m) + ed, err := entryDescFromReq(m, cReq) if err != nil { return nil, status.Errorf(codes.FailedPrecondition, "failed to create entry descriptor: %v", err) } + mReq := &fwdpb.TableEntryRemoveRequest{ ContextId: &fwdpb.ContextId{Id: m.dataplane.ID()}, TableId: &fwdpb.TableId{ObjectId: &fwdpb.ObjectId{Id: MyMacTable}}, EntryDesc: ed, } - if _, err := m.dataplane.TableEntryRemove(ctx, mReq); err != nil { return nil, err } + + // Populate the switch attribute. + saReq := &saipb.GetSwitchAttributeRequest{ + Oid: switchID, + AttrType: []saipb.SwitchAttr{ + saipb.SwitchAttr_SWITCH_ATTR_MY_MAC_LIST, + }, + } + saResp := &saipb.GetSwitchAttributeResponse{} + if err := m.mgr.PopulateAttributes(saReq, saResp); err != nil { + return nil, fmt.Errorf("Failed to populate switch attributes: %v", err) + } + locate := func(uint64) int { + for i := range saResp.GetAttr().MyMacList { + if saResp.GetAttr().MyMacList[i] == req.GetOid() { + return i + } + } + return -1 + } + if loc := locate(req.GetOid()); loc != -1 { + m.mgr.StoreAttributes(switchID, &saipb.SwitchAttribute{ + MyMacList: append(saResp.GetAttr().MyMacList[:loc], saResp.GetAttr().MyMacList[loc+1:]...), + }) + } else { + return nil, fmt.Errorf("Failed to store switch attributes: %v", err) + } + return &saipb.RemoveMyMacResponse{}, nil } diff --git a/integration_tests/dataplane/mymac/mymac_test.go b/integration_tests/dataplane/mymac/mymac_test.go index 30c55c65..2bc86e54 100644 --- a/integration_tests/dataplane/mymac/mymac_test.go +++ b/integration_tests/dataplane/mymac/mymac_test.go @@ -99,9 +99,8 @@ func dataplaneConn(t testing.TB, dut *ondatra.DUTDevice) *grpc.ClientConn { return conn } -func configureDUT(t testing.TB, dut *ondatra.DUTDevice) { +func configureDUT(t testing.TB, conn *grpc.ClientConn, dut *ondatra.DUTDevice) { t.Helper() - conn := dataplaneConn(t, dut) ric := saipb.NewRouterInterfaceClient(conn) port1ID, err := strconv.ParseUint(dut.Port(t, "port1").Name(), 10, 64) if err != nil { @@ -171,16 +170,14 @@ func configureDUT(t testing.TB, dut *ondatra.DUTDevice) { } // clearMyMac removes all entities in MyMac table. -func clearMyMac(t *testing.T, dut *ondatra.DUTDevice) error { +func clearMyMac(t *testing.T, sc saipb.SwitchClient, mmc saipb.MyMacClient) error { // Try to get the MyMac list, and there should be only one entry. - conn := dataplaneConn(t, dut) - mmc := saipb.NewSwitchClient(conn) var err error req := &saipb.GetSwitchAttributeRequest{ Oid: 1, AttrType: []saipb.SwitchAttr{saipb.SwitchAttr_SWITCH_ATTR_MY_MAC_LIST}, } - resp, err := mmc.GetSwitchAttribute(context.Background(), req) + resp, err := sc.GetSwitchAttribute(context.Background(), req) if err != nil { return err } @@ -189,83 +186,141 @@ func clearMyMac(t *testing.T, dut *ondatra.DUTDevice) error { return nil } for _, oid := range oids { - mmc := saipb.NewMyMacClient(conn) if _, err := mmc.RemoveMyMac(context.Background(), &saipb.RemoveMyMacRequest{ Oid: oid, }); err != nil { return fmt.Errorf("Failed to remove MyMac: %+v", err) } + t.Logf("Removed OID: %d", oid) } t.Logf("MyMac table cleared.") return nil } // restoreMyMac restores the rule to allow all traffic to send to L3. -func restoreMyMac(t *testing.T, dut *ondatra.DUTDevice) error { +func restoreMyMac(t *testing.T, mmc saipb.MyMacClient) (uint64, error) { // Allow all traffic to L3 processing. - conn := dataplaneConn(t, dut) - mmc := saipb.NewMyMacClient(conn) - _, err := mmc.CreateMyMac(context.Background(), &saipb.CreateMyMacRequest{ + resp, err := mmc.CreateMyMac(context.Background(), &saipb.CreateMyMacRequest{ Switch: 1, Priority: proto.Uint32(1), MacAddress: []byte{0, 0, 0, 0, 0, 0}, MacAddressMask: []byte{0, 0, 0, 0, 0, 0}, }) if err != nil { - return err + return 0, err } t.Logf("MyMac table restored.") + return resp.GetOid(), nil +} + +// printMyMacEntries prints the entries in the MyMac table. +func printMyMacEntries(t *testing.T, sc saipb.SwitchClient, mmc saipb.MyMacClient) error { + var err error + req := &saipb.GetSwitchAttributeRequest{ + Oid: 1, + AttrType: []saipb.SwitchAttr{saipb.SwitchAttr_SWITCH_ATTR_MY_MAC_LIST}, + } + resp, err := sc.GetSwitchAttribute(context.Background(), req) + if err != nil { + return err + } + oids := resp.GetAttr().MyMacList + if oids == nil { + return nil + } + for _, oid := range oids { + resp2, err := mmc.GetMyMacAttribute(context.Background(), &saipb.GetMyMacAttributeRequest{ + Oid: oid, + }) + if err != nil { + return err + } + t.Logf(">>>>>> MyMac[%d]: %+v", oid, resp2.GetAttr()) + } return nil } +// entry contains the MyMac request information. +type entry struct { + priority uint32 + macAddr string + macAddrMask []byte +} + +type myMacTest struct { + desc string + clearMyMac bool + dstMAC string + entries []entry + passed bool +} + func TestMyMac(t *testing.T) { - // TODO: add more sub-tests when the Magna issue is resolved. - tests := []struct { - desc string - clearMyMac bool - macAddr []byte - macAddrMask []byte - passed bool - }{{ - desc: "Traffic passed", + specialMAC := "00:1A:11:17:5F:80" + tests := []myMacTest{{ + desc: "Traffic passed by default", clearMyMac: false, passed: true, }, { - desc: "Traffic dropped", // Remove the default entry that allows all traffic to L3. + desc: "Traffic dropped with clearing MyMac table", // Remove the default entry that allows all traffic to L3. clearMyMac: true, passed: false, }, { - desc: "Traffic passed again", // Remove the default entry that allows all traffic to L3. - clearMyMac: false, - passed: true, - // }, { - // desc: "Traffic passed again", - // clearMyMac: false, - // macAddr: net.HardwareAddr(atePort1.MAC), - // macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, - // passed: true, + desc: "Traffic passed with specific allowed MAC address", + clearMyMac: true, + entries: []entry{{ + priority: 2010, + macAddr: dutPort1.MAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + priority: 2000, + macAddr: specialMAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + passed: true, + }, { + desc: "Traffic dropped with specific allowed MAC address", + clearMyMac: true, + entries: []entry{{ + priority: 2010, + macAddr: dutPort2.MAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + priority: 2000, + macAddr: specialMAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + passed: false, + }, { + desc: "Traffic passed with special MAC address", + clearMyMac: true, + dstMAC: specialMAC, // special MAC other than the port MAC. + entries: []entry{{ + priority: 2010, + macAddr: dutPort2.MAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + priority: 2000, + macAddr: specialMAC, + macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + passed: true, }} - // ate := ondatra.ATE(t, "ate") - // ateTop := configureATE(t, ate) - // ate.OTG().PushConfig(t, ateTop) - // ate.OTG().StartProtocols(t) - - // dut := ondatra.DUT(t, "dut") - // configureDUT(t, dut) - - for _, tt := range tests { - ate := ondatra.ATE(t, "ate") - ateTop := configureATE(t, ate) - ate.OTG().PushConfig(t, ateTop) - ate.OTG().StartProtocols(t) + ate := ondatra.ATE(t, "ate") + ateTop := configureATE(t, ate) + ate.OTG().PushConfig(t, ateTop) + ate.OTG().StartProtocols(t) - dut := ondatra.DUT(t, "dut") - configureDUT(t, dut) - - tx, rx := testTraffic(t, ate, ateTop, atePort1, dutPort1, atePort2, 10*time.Second, dut, tt.clearMyMac) + dut := ondatra.DUT(t, "dut") + conn := dataplaneConn(t, dut) + configureDUT(t, conn, dut) + sc := saipb.NewSwitchClient(conn) + mmc := saipb.NewMyMacClient(conn) + for i, tt := range tests { + tx, rx := testTraffic(t, tt, sc, mmc, fmt.Sprintf("Flow%d", i), ate, ateTop, atePort1, dutPort1, atePort2, 10*time.Second, dut) t.Logf("[%s] Got TX: %d, RX: %d", tt.desc, tx, rx) if tx == 0 { - t.Fatalf("No packet sent") + t.Fatalf("no packet sent") } switch { case tt.passed && rx != tx: @@ -291,26 +346,47 @@ func configureATE(t *testing.T, ate *ondatra.ATEDevice) gosnappi.Config { // testTraffic generates traffic flow from source network to // destination network via srcEndPoint to dstEndPoint and checks for // packet loss and returns the number of tx and rx packets. -func testTraffic(t *testing.T, ate *ondatra.ATEDevice, top gosnappi.Config, srcEndPoint, srcPeerEndpoint, dstEndPoint attrs.Attributes, dur time.Duration, dut *ondatra.DUTDevice, clearMyMacTable bool) (uint64, uint64) { - if clearMyMacTable { - clearMyMac(t, dut) - defer restoreMyMac(t, dut) +func testTraffic(t *testing.T, tt myMacTest, sc saipb.SwitchClient, mmc saipb.MyMacClient, flowID string, ate *ondatra.ATEDevice, top gosnappi.Config, srcEndPoint, srcPeerEndpoint, dstEndPoint attrs.Attributes, dur time.Duration, dut *ondatra.DUTDevice) (uint64, uint64) { + if tt.clearMyMac { + clearMyMac(t, sc, mmc) + defer restoreMyMac(t, mmc) } + for _, ent := range tt.entries { + maddr, err := net.ParseMAC(ent.macAddr) + if err != nil { + t.Fatalf("failed to parse MAC address: %v", err) + } + resp, err := mmc.CreateMyMac(context.Background(), &saipb.CreateMyMacRequest{ + Switch: 1, + Priority: &ent.priority, + MacAddress: maddr, + MacAddressMask: ent.macAddrMask, + }) + if err != nil { + t.Fatalf("failed to create MyMac entry: %v", err) + } + t.Logf("[%s] Added MyMac Entry %d: %s with mask %v", tt.desc, resp.GetOid(), ent.macAddr, ent.macAddrMask) + } + otg := ate.OTG() top.Flows().Clear().Items() - ipFlow := top.Flows().Add().SetName("Flow") + ipFlow := top.Flows().Add().SetName(flowID) ipFlow.Metrics().SetEnable(true) ipFlow.TxRx().Port().SetTxName(srcEndPoint.Name).SetRxNames([]string{dstEndPoint.Name}) - txPkts := uint64(1) - ipFlow.Rate().SetPps(1) + txPkts := uint64(100) + ipFlow.Rate().SetPps(100) ipFlow.Duration().FixedPackets().SetPackets(uint32(txPkts)) // Set up ethernet layer. eth := ipFlow.Packet().Add().Ethernet() eth.Src().SetValue(srcEndPoint.MAC) eth.Dst().SetValue(srcPeerEndpoint.MAC) + // Change the traffic's dst MAC if specified. + if tt.dstMAC != "" { + eth.Dst().SetValue(tt.dstMAC) + } ip4 := ipFlow.Packet().Add().Ipv4() ip4.Src().SetValue(srcEndPoint.IPv4) @@ -318,12 +394,10 @@ func testTraffic(t *testing.T, ate *ondatra.ATEDevice, top gosnappi.Config, srcE ip4.Version().SetValue(4) otg.PushConfig(t, top) - t.Logf(">>>>>> Sending traffic...") otg.StartTraffic(t) defer otg.StopTraffic(t) - gnmi.Await(t, otg, gnmi.OTG().Flow("Flow").Counters().OutPkts().State(), 5*time.Second, txPkts) - t.Logf(">>>>>> Traffic received.") - rxPkts := gnmi.Get(t, otg, gnmi.OTG().Flow("Flow").Counters().InPkts().State()) + gnmi.Await(t, otg, gnmi.OTG().Flow(flowID).Counters().OutPkts().State(), 5*time.Second, txPkts) + rxPkts := gnmi.Get(t, otg, gnmi.OTG().Flow(flowID).Counters().InPkts().State()) return txPkts, rxPkts } From f438667b5ab9ee8902230c6d30c5667ec5914c9a Mon Sep 17 00:00:00 2001 From: Guo-shiuan Wang Date: Fri, 29 Mar 2024 00:20:12 +0000 Subject: [PATCH 3/4] Revert changes in integration_tests/dataplane/basictraffic/basic_traffic_test.go --- .../dataplane/basictraffic/basic_traffic_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/integration_tests/dataplane/basictraffic/basic_traffic_test.go b/integration_tests/dataplane/basictraffic/basic_traffic_test.go index 7535e88a..91779fd9 100644 --- a/integration_tests/dataplane/basictraffic/basic_traffic_test.go +++ b/integration_tests/dataplane/basictraffic/basic_traffic_test.go @@ -193,21 +193,6 @@ func TestTraffic(t *testing.T) { } } -func TestTrafficAgain(t *testing.T) { - ate := ondatra.ATE(t, "ate") - ateTop := configureATE(t, ate) - ate.OTG().PushConfig(t, ateTop) - ate.OTG().StartProtocols(t) - - dut := ondatra.DUT(t, "dut") - configureDUT(t, dut) - - loss := testTraffic(t, ate, ateTop, atePort1, dutPort1, atePort2) - if loss > 1 { - t.Errorf("loss %f, greater than 1", loss) - } -} - // configureATE configures port1 and port2 on the ATE. func configureATE(t *testing.T, ate *ondatra.ATEDevice) gosnappi.Config { top := gosnappi.NewConfig() From 15719e183bbc91dfc90c37a628433437a8c133d3 Mon Sep 17 00:00:00 2001 From: Guo-shiuan Wang Date: Fri, 29 Mar 2024 17:16:04 +0000 Subject: [PATCH 4/4] Revise the code based on feedback. --- .../dataplane/mymac/mymac_test.go | 111 +++++++++--------- 1 file changed, 54 insertions(+), 57 deletions(-) diff --git a/integration_tests/dataplane/mymac/mymac_test.go b/integration_tests/dataplane/mymac/mymac_test.go index 2bc86e54..467a7eb0 100644 --- a/integration_tests/dataplane/mymac/mymac_test.go +++ b/integration_tests/dataplane/mymac/mymac_test.go @@ -240,71 +240,78 @@ func printMyMacEntries(t *testing.T, sc saipb.SwitchClient, mmc saipb.MyMacClien return nil } -// entry contains the MyMac request information. -type entry struct { - priority uint32 - macAddr string - macAddrMask []byte -} - type myMacTest struct { - desc string - clearMyMac bool - dstMAC string - entries []entry - passed bool + desc string + clearMyMac bool + dstMAC string + reqs []*saipb.CreateMyMacRequest + wantTrafficDropped bool } func TestMyMac(t *testing.T) { specialMAC := "00:1A:11:17:5F:80" + toMACAddr := func(ma string) []byte { + maddr, err := net.ParseMAC(ma) + if err != nil { + t.Fatalf("failed to parse MAC address: %v", err) + } + return maddr + } + tests := []myMacTest{{ - desc: "Traffic passed by default", - clearMyMac: false, - passed: true, + desc: "Traffic passed by default", + clearMyMac: false, + wantTrafficDropped: false, }, { - desc: "Traffic dropped with clearing MyMac table", // Remove the default entry that allows all traffic to L3. - clearMyMac: true, - passed: false, + desc: "Traffic dropped with clearing MyMac table", // Remove the default entry that allows all traffic to L3. + clearMyMac: true, + wantTrafficDropped: true, }, { desc: "Traffic passed with specific allowed MAC address", clearMyMac: true, - entries: []entry{{ - priority: 2010, - macAddr: dutPort1.MAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort1.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { - priority: 2000, - macAddr: specialMAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }}, - passed: true, + wantTrafficDropped: false, }, { desc: "Traffic dropped with specific allowed MAC address", clearMyMac: true, - entries: []entry{{ - priority: 2010, - macAddr: dutPort2.MAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort2.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { - priority: 2000, - macAddr: specialMAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }}, - passed: false, + wantTrafficDropped: true, }, { - desc: "Traffic passed with special MAC address", + desc: "Traffic passed where dst MAC is not interface MAC", clearMyMac: true, dstMAC: specialMAC, // special MAC other than the port MAC. - entries: []entry{{ - priority: 2010, - macAddr: dutPort2.MAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort2.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }, { - priority: 2000, - macAddr: specialMAC, - macAddrMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, }}, - passed: true, + wantTrafficDropped: false, }} ate := ondatra.ATE(t, "ate") ateTop := configureATE(t, ate) @@ -323,9 +330,9 @@ func TestMyMac(t *testing.T) { t.Fatalf("no packet sent") } switch { - case tt.passed && rx != tx: + case !tt.wantTrafficDropped && rx != tx: t.Errorf("got %d, expect %d", rx, tx) - case !tt.passed && rx != 0: + case tt.wantTrafficDropped && rx != 0: t.Errorf("got %d, expect 0", rx) } } @@ -351,21 +358,11 @@ func testTraffic(t *testing.T, tt myMacTest, sc saipb.SwitchClient, mmc saipb.My clearMyMac(t, sc, mmc) defer restoreMyMac(t, mmc) } - for _, ent := range tt.entries { - maddr, err := net.ParseMAC(ent.macAddr) - if err != nil { - t.Fatalf("failed to parse MAC address: %v", err) - } - resp, err := mmc.CreateMyMac(context.Background(), &saipb.CreateMyMacRequest{ - Switch: 1, - Priority: &ent.priority, - MacAddress: maddr, - MacAddressMask: ent.macAddrMask, - }) + for _, req := range tt.reqs { + _, err := mmc.CreateMyMac(context.Background(), req) if err != nil { t.Fatalf("failed to create MyMac entry: %v", err) } - t.Logf("[%s] Added MyMac Entry %d: %s with mask %v", tt.desc, resp.GetOid(), ent.macAddr, ent.macAddrMask) } otg := ate.OTG()