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/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/mymac/mymac_test.go b/integration_tests/dataplane/mymac/mymac_test.go index f5bd85c5..467a7eb0 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,47 +186,132 @@ 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 } +type myMacTest struct { + desc string + clearMyMac bool + dstMAC string + reqs []*saipb.CreateMyMacRequest + wantTrafficDropped 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 dropped", // Remove the default entry that allows all traffic to L3. + 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, + wantTrafficDropped: 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, + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort1.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + wantTrafficDropped: false, + }, { + desc: "Traffic dropped with specific allowed MAC address", + clearMyMac: true, + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort2.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + wantTrafficDropped: true, + }, { + desc: "Traffic passed where dst MAC is not interface MAC", clearMyMac: true, - passed: false, + dstMAC: specialMAC, // special MAC other than the port MAC. + reqs: []*saipb.CreateMyMacRequest{{ + Switch: 1, + Priority: proto.Uint32(2010), + MacAddress: toMACAddr(dutPort2.MAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }, { + Switch: 1, + Priority: proto.Uint32(2000), + MacAddress: toMACAddr(specialMAC), + MacAddressMask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, + }}, + wantTrafficDropped: false, }} ate := ondatra.ATE(t, "ate") ateTop := configureATE(t, ate) @@ -237,17 +319,21 @@ func TestMyMac(t *testing.T) { ate.OTG().StartProtocols(t) dut := ondatra.DUT(t, "dut") - configureDUT(t, dut) - - for _, tt := range tests { - tx, rx := testTraffic(t, ate, ateTop, atePort1, dutPort1, atePort2, 10*time.Second, dut, tt.clearMyMac) + 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, !tt.passed && rx != 0: + case !tt.wantTrafficDropped && rx != tx: t.Errorf("got %d, expect %d", rx, tx) + case tt.wantTrafficDropped && rx != 0: + t.Errorf("got %d, expect 0", rx) } } } @@ -267,44 +353,48 @@ 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 _, req := range tt.reqs { + _, err := mmc.CreateMyMac(context.Background(), req) + if err != nil { + t.Fatalf("failed to create MyMac entry: %v", err) + } } + 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(flowID) + 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(100) + ipFlow.Rate().SetPps(100) + 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) + // Change the traffic's dst MAC if specified. + if tt.dstMAC != "" { + eth.Dst().SetValue(tt.dstMAC) + } - 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) - 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()) - 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 } 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(