diff --git a/dataplane/proto/sai/next_hop_group.proto b/dataplane/proto/sai/next_hop_group.proto index c049ba999..535629cbb 100644 --- a/dataplane/proto/sai/next_hop_group.proto +++ b/dataplane/proto/sai/next_hop_group.proto @@ -211,6 +211,14 @@ message RemoveNextHopGroupsResponse { repeated RemoveNextHopGroupResponse resps = 1; } +message SetNextHopGroupsAttributeRequest { + repeated SetNextHopGroupAttributeRequest reqs = 1; +} + +message SetNextHopGroupsAttributeResponse { + repeated SetNextHopGroupAttributeResponse resps = 1; +} + service NextHopGroup { rpc CreateNextHopGroup (CreateNextHopGroupRequest) returns (CreateNextHopGroupResponse) {} @@ -229,4 +237,5 @@ service NextHopGroup { rpc GetNextHopGroupMapAttribute (GetNextHopGroupMapAttributeRequest) returns (GetNextHopGroupMapAttributeResponse) {} rpc CreateNextHopGroups (CreateNextHopGroupsRequest) returns (CreateNextHopGroupsResponse) {} rpc RemoveNextHopGroups (RemoveNextHopGroupsRequest) returns (RemoveNextHopGroupsResponse) {} + rpc SetNextHopGroupsAttribute (SetNextHopGroupsAttributeRequest) returns (SetNextHopGroupsAttributeResponse) {} } diff --git a/dataplane/saiserver/routing.go b/dataplane/saiserver/routing.go index 4ffacb1ae..93e44a552 100644 --- a/dataplane/saiserver/routing.go +++ b/dataplane/saiserver/routing.go @@ -298,26 +298,98 @@ func (nhg *nextHopGroup) updateNextHopGroupMember(ctx context.Context, nhgid, mi // RemoveNextHopGroup removes the next hop group specified in the OID. func (nhg *nextHopGroup) RemoveNextHopGroup(_ context.Context, req *saipb.RemoveNextHopGroupRequest) (*saipb.RemoveNextHopGroupResponse, error) { - oid := req.GetOid() - if _, ok := nhg.groups[oid]; !ok { - return nil, status.Errorf(codes.NotFound, "group %d does not exist", oid) + if _, ok := nhg.groups[req.GetOid()]; !ok { + return nil, status.Errorf(codes.NotFound, "group %d does not exist", req.GetOid()) } - delete(nhg.groups, oid) - - entry := fwdconfig.EntryDesc(fwdconfig.ExactEntry( - fwdconfig.PacketFieldBytes(fwdpb.PacketFieldNum_PACKET_FIELD_NUM_NEXT_HOP_GROUP_ID).WithUint64(oid))).Build() - nhgReq := &fwdpb.TableEntryRemoveRequest{ + if _, err := nhg.dataplane.TableEntryRemove(context.Background(), &fwdpb.TableEntryRemoveRequest{ ContextId: &fwdpb.ContextId{Id: nhg.dataplane.ID()}, - TableId: &fwdpb.TableId{ObjectId: &fwdpb.ObjectId{Id: NHGTable}}, - EntryDesc: entry, - } - - if _, err := nhg.dataplane.TableEntryRemove(context.Background(), nhgReq); err != nil { + TableId: &fwdpb.TableId{ + ObjectId: &fwdpb.ObjectId{ + Id: NHGTable, + }, + }, + EntryDesc: &fwdpb.EntryDesc{ + Entry: &fwdpb.EntryDesc_Exact{ + Exact: &fwdpb.ExactEntryDesc{ + Fields: []*fwdpb.PacketFieldBytes{{ + Bytes: binary.BigEndian.AppendUint64(nil, req.GetOid()), + FieldId: &fwdpb.PacketFieldId{ + Field: &fwdpb.PacketField{ + FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_NEXT_HOP_GROUP_ID, + }, + }, + }}, + }, + }, + }, + }); err != nil { return nil, err } + // Copying keys to avoid modification while iterating + var membersToRemove []uint64 + if group, ok := nhg.groups[req.GetOid()]; ok { + for mid := range group { + membersToRemove = append(membersToRemove, mid) + } + } + for _, mid := range membersToRemove { + if _, err := attrmgr.InvokeAndSave(context.Background(), nhg.mgr, nhg.RemoveNextHopGroupMember, &saipb.RemoveNextHopGroupMemberRequest{Oid: mid}); err != nil { + return nil, err + } + } + delete(nhg.groups, req.GetOid()) return &saipb.RemoveNextHopGroupResponse{}, nil } +// SetNextHopGroupAttribute sets the attributes of a next hop group. +func (nhg *nextHopGroup) SetNextHopGroupAttribute(ctx context.Context, req *saipb.SetNextHopGroupAttributeRequest) (*saipb.SetNextHopGroupAttributeResponse, error) { + if len(req.GetNextHopList()) > 0 { + var membersToRemove []uint64 + if group, ok := nhg.groups[req.GetOid()]; ok { + for mid := range group { + membersToRemove = append(membersToRemove, mid) + } + } + for _, mid := range membersToRemove { + if _, err := attrmgr.InvokeAndSave(ctx, nhg.mgr, nhg.RemoveNextHopGroupMember, &saipb.RemoveNextHopGroupMemberRequest{Oid: mid}); err != nil { + return nil, err + } + } + + memReq := &saipb.CreateNextHopGroupMembersRequest{} + for i, nh := range req.GetNextHopList() { + weight := uint32(1) + if i < len(req.GetNextHopMemberWeightList()) { + weight = req.GetNextHopMemberWeightList()[i] + } + memReq.Reqs = append(memReq.Reqs, &saipb.CreateNextHopGroupMemberRequest{ + Switch: switchID, + NextHopGroupId: proto.Uint64(req.GetOid()), + NextHopId: proto.Uint64(nh), + Weight: proto.Uint32(weight), + }) + } + if _, err := attrmgr.InvokeAndSave(ctx, nhg.mgr, nhg.CreateNextHopGroupMembers, memReq); err != nil { + return nil, err + } + } + return &saipb.SetNextHopGroupAttributeResponse{}, nil +} + +// SetNextHopGroupsAttribute sets the attributes of multiple next hop groups. +func (nhg *nextHopGroup) SetNextHopGroupsAttribute(ctx context.Context, r *saipb.SetNextHopGroupsAttributeRequest) (*saipb.SetNextHopGroupsAttributeResponse, error) { + resp := &saipb.SetNextHopGroupsAttributeResponse{} + for _, req := range r.GetReqs() { + res, err := attrmgr.InvokeAndSave(ctx, nhg.mgr, nhg.SetNextHopGroupAttribute, req) + if err != nil { + return nil, err + } + resp.Resps = append(resp.Resps, res) + } + return resp, nil +} + + // RemoveNextHopGroups removes multiple next hop groups specified in the OID. func (nhg *nextHopGroup) RemoveNextHopGroups(ctx context.Context, req *saipb.RemoveNextHopGroupsRequest) (*saipb.RemoveNextHopGroupsResponse, error) { resp := &saipb.RemoveNextHopGroupsResponse{} diff --git a/dataplane/saiserver/routing_test.go b/dataplane/saiserver/routing_test.go index 34f809113..f44a155ae 100644 --- a/dataplane/saiserver/routing_test.go +++ b/dataplane/saiserver/routing_test.go @@ -181,6 +181,73 @@ func TestRemoveNextHopGroup(t *testing.T) { } } + +func TestSetNextHopGroupAttribute(t *testing.T) { + tests := []struct { + desc string + req *saipb.SetNextHopGroupAttributeRequest + wantAttr *saipb.NextHopGroupAttribute + wantErr string + }{{ + desc: "replace members", + req: &saipb.SetNextHopGroupAttributeRequest{ + NextHopList: []uint64{11, 12}, // New members + NextHopMemberWeightList: []uint32{2, 3}, + }, + wantAttr: &saipb.NextHopGroupAttribute{ + Type: saipb.NextHopGroupType_NEXT_HOP_GROUP_TYPE_ECMP_WITH_MEMBERS.Enum(), + NextHopList: []uint64{11, 12}, + NextHopMemberWeightList: []uint32{2, 3}, + }, + }} + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + dplane := &fakeSwitchDataplane{} + c, mgr, stopFn := newTestNextHopGroup(t, dplane) + defer stopFn() + + // Setup initial state: Group with 1 member (ID 10) + mgr.StoreAttributes(switchID, &saipb.SwitchAttribute{ + EcmpHashIpv4: proto.Uint64(5), + EcmpHashIpv6: proto.Uint64(5), + }) + mgr.StoreAttributes(5, &saipb.HashAttribute{ + NativeHashFieldList: []saipb.NativeHashField{saipb.NativeHashField_NATIVE_HASH_FIELD_DST_IP}, + }) + mgr.StoreAttributes(10, &saipb.NextHopAttribute{Ip: []byte{127, 0, 0, 1}}) + mgr.StoreAttributes(11, &saipb.NextHopAttribute{Ip: []byte{127, 0, 0, 2}}) + mgr.StoreAttributes(12, &saipb.NextHopAttribute{Ip: []byte{127, 0, 0, 3}}) + + resp, err := c.CreateNextHopGroup(context.Background(), &saipb.CreateNextHopGroupRequest{ + Type: saipb.NextHopGroupType_NEXT_HOP_GROUP_TYPE_ECMP_WITH_MEMBERS.Enum(), + NextHopList: []uint64{10}, + NextHopMemberWeightList: []uint32{1}, + }) + if err != nil { + t.Fatal(err) + } + + tt.req.Oid = resp.Oid + _, gotErr := c.SetNextHopGroupAttribute(context.TODO(), tt.req) + if diff := errdiff.Check(gotErr, tt.wantErr); diff != "" { + t.Fatalf("SetNextHopGroupAttribute() unexpected err: %s", diff) + } + if gotErr != nil { + return + } + + // Verify attributes + attr := &saipb.NextHopGroupAttribute{} + if err := mgr.PopulateAllAttributes("1", attr); err != nil { + t.Fatal(err) + } + if d := cmp.Diff(attr, tt.wantAttr, protocmp.Transform()); d != "" { + t.Errorf("SetNextHopGroupAttribute() failed: diff(-got,+want)\n:%s", d) + } + }) + } +} + func TestCreateNextHopGroupMember(t *testing.T) { tests := []struct { desc string diff --git a/dataplane/standalone/sai/next_hop_group.cc b/dataplane/standalone/sai/next_hop_group.cc index fb9a02a0a..02126f266 100644 --- a/dataplane/standalone/sai/next_hop_group.cc +++ b/dataplane/standalone/sai/next_hop_group.cc @@ -154,6 +154,35 @@ switch (attr_list[i].id) { return msg; } +lemming::dataplane::sai::SetNextHopGroupAttributeRequest convert_set_next_hop_group_attribute(sai_object_id_t next_hop_group_id, const sai_attribute_t *attr) { + lemming::dataplane::sai::SetNextHopGroupAttributeRequest req; + req.set_oid(next_hop_group_id); + switch (attr->id) { + case SAI_NEXT_HOP_GROUP_ATTR_SET_SWITCHOVER: + req.set_set_switchover(attr->value.booldata); + break; + case SAI_NEXT_HOP_GROUP_ATTR_COUNTER_ID: + req.set_counter_id(attr->value.oid); + break; + case SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP: + req.set_selection_map(attr->value.oid); + break; + case SAI_NEXT_HOP_GROUP_ATTR_ARS_OBJECT_ID: + req.set_ars_object_id(attr->value.oid); + break; + case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_LIST: + req.mutable_next_hop_list()->Add(attr->value.objlist.list, attr->value.objlist.list + attr->value.objlist.count); + break; + case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_MEMBER_WEIGHT_LIST: + req.mutable_next_hop_member_weight_list()->Add(attr->value.u32list.list, attr->value.u32list.list + attr->value.u32list.count); + break; + case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_MEMBER_COUNTER_LIST: + req.mutable_next_hop_member_counter_list()->Add(attr->value.objlist.list, attr->value.objlist.list + attr->value.objlist.count); + break; + } + return req; +} + sai_status_t l_create_next_hop_group(sai_object_id_t *next_hop_group_id, sai_object_id_t switch_id, uint32_t attr_count, const sai_attribute_t *attr_list) { LOG(INFO) << "Func: " << __PRETTY_FUNCTION__; @@ -205,38 +234,9 @@ sai_status_t l_remove_next_hop_group(sai_object_id_t next_hop_group_id) { sai_status_t l_set_next_hop_group_attribute(sai_object_id_t next_hop_group_id, const sai_attribute_t *attr) { LOG(INFO) << "Func: " << __PRETTY_FUNCTION__; - lemming::dataplane::sai::SetNextHopGroupAttributeRequest req; lemming::dataplane::sai::SetNextHopGroupAttributeResponse resp; grpc::ClientContext context; - req.set_oid(next_hop_group_id); - - - - -switch (attr->id) { - - case SAI_NEXT_HOP_GROUP_ATTR_SET_SWITCHOVER: - req.set_set_switchover(attr->value.booldata); - break; - case SAI_NEXT_HOP_GROUP_ATTR_COUNTER_ID: - req.set_counter_id(attr->value.oid); - break; - case SAI_NEXT_HOP_GROUP_ATTR_SELECTION_MAP: - req.set_selection_map(attr->value.oid); - break; - case SAI_NEXT_HOP_GROUP_ATTR_ARS_OBJECT_ID: - req.set_ars_object_id(attr->value.oid); - break; - case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_LIST: - req.mutable_next_hop_list()->Add(attr->value.objlist.list, attr->value.objlist.list + attr->value.objlist.count); - break; - case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_MEMBER_WEIGHT_LIST: - req.mutable_next_hop_member_weight_list()->Add(attr->value.u32list.list, attr->value.u32list.list + attr->value.u32list.count); - break; - case SAI_NEXT_HOP_GROUP_ATTR_NEXT_HOP_MEMBER_COUNTER_LIST: - req.mutable_next_hop_member_counter_list()->Add(attr->value.objlist.list, attr->value.objlist.list + attr->value.objlist.count); - break; -} + lemming::dataplane::sai::SetNextHopGroupAttributeRequest req = convert_set_next_hop_group_attribute(next_hop_group_id, attr); grpc::Status status = next_hop_group->SetNextHopGroupAttribute(&context, req, &resp); if (!status.ok()) { @@ -730,7 +730,34 @@ sai_status_t l_remove_next_hop_groups(uint32_t object_count, const sai_object_id } sai_status_t l_set_next_hop_groups_attribute(uint32_t object_count, const sai_object_id_t *object_id, const sai_attribute_t *attr_list, sai_bulk_op_error_mode_t mode, sai_status_t *object_statuses) { - LOG(INFO) << "Func: " << __PRETTY_FUNCTION__ << " is not implemented but by-passing check"; + LOG(INFO) << "Func: " << __PRETTY_FUNCTION__; + + lemming::dataplane::sai::SetNextHopGroupsAttributeRequest req; + lemming::dataplane::sai::SetNextHopGroupsAttributeResponse resp; + grpc::ClientContext context; + + for (uint32_t i = 0; i < object_count; i++) { + auto r = convert_set_next_hop_group_attribute(object_id[i], &attr_list[i]); + *req.add_reqs() = r; + } + + grpc::Status status = next_hop_group->SetNextHopGroupsAttribute(&context, req, &resp); + if (!status.ok()) { + auto it = context.GetServerTrailingMetadata().find("traceparent"); + if (it != context.GetServerTrailingMetadata().end()) { + LOG(ERROR) << "Lucius RPC error: Trace ID " << it->second << " msg: " << status.error_message(); + } else { + LOG(ERROR) << "Lucius RPC error: " << status.error_message(); + } + return SAI_STATUS_FAILURE; + } + if (object_count != resp.resps().size()) { + return SAI_STATUS_FAILURE; + } + for (uint32_t i = 0; i < object_count; i++) { + object_statuses[i] = SAI_STATUS_SUCCESS; + } + return SAI_STATUS_SUCCESS; }