Implement the removal of NextHopGroup and NextHopGroupMember.

dataplane/dplanerc/routes.go

@@ -120,15 +120,15 @@ func (ni *Reconciler) StartRoute(ctx context.Context, client *ygnmi.Client) erro
 				return ygnmi.Continue
 			}
 			for i, nh := range route.GetNextHops().GetHops() {
-				hopID, err = ni.createNextHop(ctx, nh)
+				hID, err := ni.createNextHop(ctx, nh)
 				if err != nil {
 					log.Warningf("failed to create next hop: %v", err)
 					return ygnmi.Continue
 				}
 				_, err = ni.nextHopGroupClient.CreateNextHopGroupMember(ctx, &saipb.CreateNextHopGroupMemberRequest{
 					Switch:         ni.switchID,
 					NextHopGroupId: &group.Oid,
-					NextHopId:      &hopID,
+					NextHopId:      &hID,
 					Weight:         proto.Uint32(uint32(route.GetNextHops().Weights[i])),
 				})
 				if err != nil {

dataplane/saiserver/routing.go

@@ -116,7 +116,7 @@ func newNextHopGroup(mgr *attrmgr.AttrMgr, dataplane switchDataplaneAPI, s *grpc
 	return n
 }
 
-// CreateNextHopGroupMember creates a next hop group.
+// CreateNextHopGroup creates a next hop group.
 func (nhg *nextHopGroup) CreateNextHopGroup(_ context.Context, req *saipb.CreateNextHopGroupRequest) (*saipb.CreateNextHopGroupResponse, error) {
 	id := nhg.mgr.NextID()
 
@@ -130,16 +130,17 @@ func (nhg *nextHopGroup) CreateNextHopGroup(_ context.Context, req *saipb.Create
 	}, nil
 }
 
-// CreateNextHopGroupMember adds a next hop to a next hop group.
-func (nhg *nextHopGroup) CreateNextHopGroupMember(ctx context.Context, req *saipb.CreateNextHopGroupMemberRequest) (*saipb.CreateNextHopGroupMemberResponse, error) {
-	memberID := nhg.mgr.NextID()
-	group := nhg.groups[req.GetNextHopGroupId()]
+// updateNextHopGroupMember updates the next hop group.
+// If m is nil, remove mid from the group(key: nhgid), otherwise add m to group with mid as the key.
+func updateNextHopGroupMember(ctx context.Context, nhg *nextHopGroup, nhgid, mid uint64, m *groupMember) (*fwdpb.TableEntryAddReply, error) {
+	group := nhg.groups[nhgid]
 	if group == nil {
-		return nil, status.Errorf(codes.FailedPrecondition, "group %v does not exist", req.GetNextHopGroupId())
+		return nil, status.Errorf(codes.FailedPrecondition, "group %d does not exist", nhgid)
 	}
-	group[memberID] = &groupMember{
-		nextHop: req.GetNextHopId(),
-		weight:  req.GetWeight(),
+	if m != nil {
+		group[mid] = m
+	} else {
+		delete(group, mid)
 	}
 	var actLists []*fwdpb.ActionList
 	for _, member := range group {
@@ -191,7 +192,7 @@ func (nhg *nextHopGroup) CreateNextHopGroupMember(ctx context.Context, req *saip
 				Entry: &fwdpb.EntryDesc_Exact{
 					Exact: &fwdpb.ExactEntryDesc{
 						Fields: []*fwdpb.PacketFieldBytes{{
-							Bytes: binary.BigEndian.AppendUint64(nil, req.GetNextHopGroupId()),
+							Bytes: binary.BigEndian.AppendUint64(nil, nhgid),
 							FieldId: &fwdpb.PacketFieldId{
 								Field: &fwdpb.PacketField{
 									FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_NEXT_HOP_GROUP_ID,
@@ -204,11 +205,59 @@ func (nhg *nextHopGroup) CreateNextHopGroupMember(ctx context.Context, req *saip
 			Actions: actions,
 		}},
 	}
-	if _, err := nhg.dataplane.TableEntryAdd(ctx, entries); err != nil {
+	return nhg.dataplane.TableEntryAdd(ctx, entries)
+}
+
+// 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.FailedPrecondition, "group %d does not exist", oid)
+	}
+	// Emit an error if the NHG still has members.
+	if len(nhg.groups[oid]) != 0 {
+		return nil, status.Errorf(codes.FailedPrecondition, "cannot remove non-empty group %d", oid)
+	}
+	delete(nhg.groups, oid)
+	return &saipb.RemoveNextHopGroupResponse{}, nil
+}
+
+// CreateNextHopGroupMember adds a next hop to a next hop group.
+func (nhg *nextHopGroup) CreateNextHopGroupMember(ctx context.Context, req *saipb.CreateNextHopGroupMemberRequest) (*saipb.CreateNextHopGroupMemberResponse, error) {
+	nhgid := req.GetNextHopGroupId()
+	mid := nhg.mgr.NextID()
+	m := &groupMember{
+		nextHop: req.GetNextHopId(),
+		weight:  req.GetWeight(),
+	}
+	if _, err := updateNextHopGroupMember(ctx, nhg, nhgid, mid, m); err != nil {
 		return nil, err
 	}
+	return &saipb.CreateNextHopGroupMemberResponse{Oid: mid}, nil
+}
 
-	return &saipb.CreateNextHopGroupMemberResponse{Oid: memberID}, nil
+// RemoveNextHopGroupMember remove the next hop group member specified in the OID.
+// Only need to remove with the desc.
+func (nhg *nextHopGroup) RemoveNextHopGroupMember(ctx context.Context, req *saipb.RemoveNextHopGroupMemberRequest) (*saipb.RemoveNextHopGroupMemberResponse, error) {
+	locateMember := func(oid uint64) (uint64, uint64, error) {
+		for nhgid, nhg := range nhg.groups {
+			for mid := range nhg {
+				if mid == oid {
+					return nhgid, mid, nil
+				}
+			}
+		}
+		return 0, 0, fmt.Errorf("cannot find member with id=%d", oid)
+	}
+	nhgid, mid, err := locateMember(req.GetOid())
+	if err != nil {
+		return nil, err
+	}
+
+	if _, err := updateNextHopGroupMember(ctx, nhg, nhgid, mid, nil); err != nil {
+		return nil, err
+	}
+	return &saipb.RemoveNextHopGroupMemberResponse{}, nil
 }
 
 type nextHop struct {

dataplane/saiserver/routing_test.go

@@ -113,6 +113,57 @@ func TestCreateNextHopGroup(t *testing.T) {
 	}
 }
 
+func TestRemoveNextHopGroup(t *testing.T) {
+	tests := []struct {
+		desc      string
+		memberReq *saipb.CreateNextHopGroupMemberRequest
+		oid       uint64 // specify this if you want an arbitrary OID to remove.
+		wantErr   string
+	}{{
+		desc: "success",
+	}, {
+		desc:    "fail: group not found",
+		oid:     15, // a non-existing OID.
+		wantErr: "group 15 does not exist",
+	}, {
+		desc: "fail: group not empty",
+		memberReq: &saipb.CreateNextHopGroupMemberRequest{
+			NextHopId: proto.Uint64(1),
+			Weight:    proto.Uint32(100),
+		},
+		wantErr: "cannot remove non-empty group",
+	}}
+	nhgReq := &saipb.CreateNextHopGroupRequest{
+		Type: saipb.NextHopGroupType_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP.Enum(),
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			dplane := &fakeSwitchDataplane{}
+			c, _, stopFn := newTestNextHopGroup(t, dplane)
+			defer stopFn()
+			ctx := context.Background()
+			resp, err := c.CreateNextHopGroup(ctx, nhgReq)
+			if err != nil {
+				t.Fatalf("unexpected err: %s", err)
+			}
+			oid := tt.oid
+			if oid == 0 {
+				oid = resp.Oid
+			}
+			if tt.memberReq != nil {
+				tt.memberReq.NextHopGroupId = proto.Uint64(oid)
+				if _, err := c.CreateNextHopGroupMember(ctx, tt.memberReq); err != nil {
+					t.Fatalf("unexpected err: %s", err)
+				}
+			}
+			_, gotErr := c.RemoveNextHopGroup(context.TODO(), &saipb.RemoveNextHopGroupRequest{Oid: oid})
+			if diff := errdiff.Check(gotErr, tt.wantErr); diff != "" {
+				t.Fatalf("RemoveNextHopGroup() unexpected err: %s", diff)
+			}
+		})
+	}
+}
+
 func TestCreateNextHopGroupMember(t *testing.T) {
 	tests := []struct {
 		desc     string
@@ -217,6 +268,142 @@ func TestCreateNextHopGroupMember(t *testing.T) {
 	}
 }
 
+func TestRemoveNextHopGroupMember(t *testing.T) {
+	tests := []struct {
+		desc     string
+		mid      uint64
+		reqIndex int
+		attrID   string
+		wantAttr *saipb.NextHopGroupMemberAttribute
+		wantReq  *fwdpb.TableEntryAddRequest
+		wantErr  string
+	}{{
+		desc:     "success",
+		mid:      0,
+		reqIndex: 2,
+		attrID:   "3",
+		wantReq: &fwdpb.TableEntryAddRequest{
+			ContextId: &fwdpb.ContextId{Id: "foo"},
+			TableId:   &fwdpb.TableId{ObjectId: &fwdpb.ObjectId{Id: NHGTable}},
+			Entries: []*fwdpb.TableEntryAddRequest_Entry{{
+				EntryDesc: &fwdpb.EntryDesc{
+					Entry: &fwdpb.EntryDesc_Exact{
+						Exact: &fwdpb.ExactEntryDesc{
+							Fields: []*fwdpb.PacketFieldBytes{{
+								FieldId: &fwdpb.PacketFieldId{Field: &fwdpb.PacketField{
+									FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_NEXT_HOP_GROUP_ID,
+								}},
+								Bytes: []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01},
+							}},
+						},
+					},
+				},
+				Actions: []*fwdpb.ActionDesc{{
+					ActionType: fwdpb.ActionType_ACTION_TYPE_SELECT_ACTION_LIST,
+					Action: &fwdpb.ActionDesc_Select{
+						Select: &fwdpb.SelectActionListActionDesc{
+							SelectAlgorithm: fwdpb.SelectActionListActionDesc_SELECT_ALGORITHM_CRC32,
+							FieldIds: []*fwdpb.PacketFieldId{
+								{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_IP_PROTO}},
+								{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_IP_ADDR_SRC}},
+								{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_IP_ADDR_DST}},
+								{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_L4_PORT_SRC}},
+								{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_L4_PORT_DST}},
+							},
+							ActionLists: []*fwdpb.ActionList{{
+								Weight: 66,
+								Actions: []*fwdpb.ActionDesc{{
+									ActionType: fwdpb.ActionType_ACTION_TYPE_UPDATE,
+									Action: &fwdpb.ActionDesc_Update{
+										Update: &fwdpb.UpdateActionDesc{
+											FieldId: &fwdpb.PacketFieldId{Field: &fwdpb.PacketField{FieldNum: fwdpb.PacketFieldNum_PACKET_FIELD_NUM_NEXT_HOP_ID}},
+											Type:    fwdpb.UpdateType_UPDATE_TYPE_SET,
+											Field:   &fwdpb.PacketFieldId{Field: &fwdpb.PacketField{}},
+											Value:   []byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0c},
+										},
+									},
+								}},
+							}},
+						},
+					},
+				}, {
+					ActionType: fwdpb.ActionType_ACTION_TYPE_LOOKUP,
+					Action: &fwdpb.ActionDesc_Lookup{
+						Lookup: &fwdpb.LookupActionDesc{
+							TableId: &fwdpb.TableId{ObjectId: &fwdpb.ObjectId{Id: NHTable}},
+						},
+					},
+				}},
+			}},
+		},
+		wantAttr: &saipb.NextHopGroupMemberAttribute{
+			NextHopGroupId: proto.Uint64(1),
+			NextHopId:      proto.Uint64(12),
+			Weight:         proto.Uint32(66),
+		},
+	}, {
+		desc:    "fail: member not found",
+		mid:     100,
+		wantErr: "cannot find member with id",
+	}}
+
+	// Creates two members.
+	createReqs := []*saipb.CreateNextHopGroupMemberRequest{
+		{
+			NextHopId: proto.Uint64(11),
+			Weight:    proto.Uint32(33),
+		}, {
+			NextHopId: proto.Uint64(12),
+			Weight:    proto.Uint32(66),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			dplane := &fakeSwitchDataplane{}
+			c, mgr, stopFn := newTestNextHopGroup(t, dplane)
+			ctx := context.Background()
+			r, err := c.CreateNextHopGroup(ctx, &saipb.CreateNextHopGroupRequest{Type: saipb.NextHopGroupType_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP.Enum()})
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer stopFn()
+			mid := tt.mid // mid is either the mid specified in test case, or the mid of the first added member.
+			if mid == 0 {
+				for _, req := range createReqs {
+					req.NextHopGroupId = &r.Oid
+					resp, err := c.CreateNextHopGroupMember(ctx, req)
+					if err != nil {
+						t.Fatal("unexpected error: %v", err)
+					}
+					// Stores the first member ID.
+					if mid == 0 {
+						mid = resp.GetOid()
+					}
+				}
+			}
+
+			_, err = c.RemoveNextHopGroupMember(ctx, &saipb.RemoveNextHopGroupMemberRequest{Oid: mid})
+			if diff := errdiff.Check(err, tt.wantErr); diff != "" {
+				t.Fatalf("RemoveNextHopGroupMember() unexpected err: %s", diff)
+			}
+			if tt.wantErr != "" {
+				return
+			}
+
+			if d := cmp.Diff(dplane.gotEntryAddReqs[tt.reqIndex], tt.wantReq, protocmp.Transform()); d != "" {
+				t.Errorf("RemoveNextHopGroupMember() failed: diff(-got,+want)\n:%s", d)
+			}
+			attr := &saipb.NextHopGroupMemberAttribute{}
+			if err := mgr.PopulateAllAttributes(tt.attrID, attr); err != nil {
+				t.Fatal(err)
+			}
+			if d := cmp.Diff(attr, tt.wantAttr, protocmp.Transform()); d != "" {
+				t.Errorf("RemoveNextHopGroupMember() failed: diff(-got,+want)\n:%s", d)
+			}
+		})
+	}
+}
+
 func TestCreateNextHop(t *testing.T) {
 	tests := []struct {
 		desc     string