Add GetRIB method for an individual RIBHolder.

.github/workflows/go.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches: [ main ]
   pull_request:
-    branches: [ main ]
 
 jobs:
   build:

rib/rib.go

@@ -18,6 +18,7 @@ package rib
 import (
 	"errors"
 	"fmt"
+	"sort"
 	"sync"
 	"time"
 
@@ -190,6 +191,20 @@ func (r *RIB) NetworkInstanceRIB(s string) (*RIBHolder, bool) {
 	return rh, ok
 }
 
+// KnownNetworkInstances returns the name of all known network instances
+// within the RIB.
+func (r *RIB) KnownNetworkInstances() []string {
+	r.nrMu.RLock()
+	defer r.nrMu.RUnlock()
+	names := []string{}
+	for n := range r.niRIB {
+		names = append(names, n)
+	}
+	// return the RIB names in a stable order.
+	sort.Strings(names)
+	return names
+}
+
 // OpResult contains the result of an operation (Add, Modify, Delete).
 type OpResult struct {
 	// ID is the ID of the operation as specified in the input request.
@@ -653,26 +668,21 @@ func (r *RIBHolder) DeleteIPv4(e *aftpb.Afts_Ipv4EntryKey) error {
 		return errors.New("invalid RIB structure, nil")
 	}
 
-	ribE := r.r.Afts.Ipv4Entry[e.GetPrefix()]
-	if ribE == nil {
-		return status.Newf(codes.NotFound, "cannot find IPv4Entry to delete, %s", e.Prefix).Err()
-	}
-
 	// This is an optional check, today some servers do not implement it and return true
 	// even if the load does not match. Compliance tests should note this.
 	if e.GetIpv4Entry() != nil {
-		existingEntryProto, err := concreteIPv4Proto(ribE)
+		// We do not mind if we don't find this entry - since this shouldn't be an
+		// error.
+		existingEntryProto, _, err := r.ipv4EntryProto(e.GetPrefix())
 		if err != nil {
-			return status.Newf(codes.Internal, "invalid existing entry in RIB %s", e).Err()
+			return err
 		}
-
 		if !proto.Equal(existingEntryProto, e) {
 			return status.Newf(codes.NotFound, "delete of an entry with non-matching, existing: %s, candidate: %s", existingEntryProto, e).Err()
 		}
 	}
 
 	de := r.r.Afts.Ipv4Entry[e.GetPrefix()]
-
 	r.doDeleteIPv4(e.GetPrefix())
 
 	if r.postChangeHook != nil {
@@ -682,6 +692,25 @@ func (r *RIBHolder) DeleteIPv4(e *aftpb.Afts_Ipv4EntryKey) error {
 	return nil
 }
 
+// ipv4EntryProto returns a protobuf message for the specified IPv4 prefix. It returns
+// the found prefix as a Ipv4EntryKey protobuf, along with a bool indicating whether the
+// prefix was found in the RIB.
+func (r *RIBHolder) ipv4EntryProto(pfx string) (*aftpb.Afts_Ipv4EntryKey, bool, error) {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+	ribE := r.r.Afts.Ipv4Entry[pfx]
+	if ribE == nil {
+		return nil, false, nil
+	}
+
+	existingEntryProto, err := concreteIPv4Proto(ribE)
+	if err != nil {
+		return nil, true, status.Newf(codes.Internal, "invalid existing entry in RIB %v", ribE).Err()
+	}
+
+	return existingEntryProto, true, nil
+}
+
 // doDeleteIPv4 deletes pfx from the IPv4Entry RIB holding the shortest possible lock.
 func (r *RIBHolder) doDeleteIPv4(pfx string) {
 	r.mu.Lock()
@@ -824,6 +853,48 @@ func concreteIPv4Proto(e *aft.Afts_Ipv4Entry) (*aftpb.Afts_Ipv4EntryKey, error)
 	}, nil
 }
 
+// concreteNextHopProto takes the input NextHop GoStruct and returns it as a gRIBI
+// NextHopEntryKey protobuf. It returns an error if the protobuf cannot be marshalled.
+func concreteNextHopProto(e *aft.Afts_NextHop) (*aftpb.Afts_NextHopKey, error) {
+	nhproto := &aftpb.Afts_NextHop{}
+	if err := protoFromGoStruct(e, &gpb.Path{
+		Elem: []*gpb.PathElem{{
+			Name: "afts",
+		}, {
+			Name: "next-hops",
+		}, {
+			Name: "next-hop",
+		}},
+	}, nhproto); err != nil {
+		return nil, fmt.Errorf("cannot marshal next-hop index %d, %v", e.GetIndex(), err)
+	}
+	return &aftpb.Afts_NextHopKey{
+		Index:   *e.Index,
+		NextHop: nhproto,
+	}, nil
+}
+
+// concreteNextHopGroupProto takes the input NextHopGroup GoStruct and returns it as a gRIBI
+// NextHopGroupEntryKey protobuf. It returns an error if the protobuf cannot be marshalled.
+func concreteNextHopGroupProto(e *aft.Afts_NextHopGroup) (*aftpb.Afts_NextHopGroupKey, error) {
+	nhgproto := &aftpb.Afts_NextHopGroup{}
+	if err := protoFromGoStruct(e, &gpb.Path{
+		Elem: []*gpb.PathElem{{
+			Name: "afts",
+		}, {
+			Name: "next-hop-groups",
+		}, {
+			Name: "next-hop-group",
+		}},
+	}, nhgproto); err != nil {
+		return nil, fmt.Errorf("cannot marshal next-hop index %d, %v", e.GetId(), err)
+	}
+	return &aftpb.Afts_NextHopGroupKey{
+		Id:           *e.Id,
+		NextHopGroup: nhgproto,
+	}, nil
+}
+
 // protoFromGoStruct takes the input GoStruct and marshals into the supplied pb
 // protobuf message, trimming the prefix specified from the annotated paths within
 // the protobuf.
@@ -851,3 +922,85 @@ func protoFromGoStruct(s ygot.GoStruct, prefix *gpb.Path, pb proto.Message) erro
 
 	return nil
 }
+
+func (r *RIBHolder) GetRIB(msgCh chan *spb.GetResponse, stopCh chan struct{}) error {
+	// TODO(robjs): since we are wanting to ensure that we tell the client
+	// exactly what is installed, this leads to a decision to make about locking
+	// of the RIB -- either we can go and lock the entire network instance RIB,
+	// or be more granular than that.
+	//
+	//  * we take the NI-level lock: in the incoming master case, the client can
+	//    ensure that they wait for the Get to complete before writing ==> there
+	//    is no convergence impact. In the multi-master case (or even a consistency)
+	//    check case, we impact convergence.
+	//  * we take a more granular lock, in this case we do not impact convergence
+	//    for any other entity than that individual entry.
+	//
+	// The latter is a better choice for a high-performance implementation, but
+	// its not clear that we need to worry about this for this implementation *yet*.
+	// In the future we should consider a fine-grained per-entry lock.
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+
+	for pfx, e := range r.r.Afts.Ipv4Entry {
+		select {
+		case <-stopCh:
+			return nil
+		default:
+			p, err := concreteIPv4Proto(e)
+			if err != nil {
+				return status.Errorf(codes.Internal, "cannot marshal IPv4Entry for %s into GetResponse, %v", pfx, err)
+			}
+			msgCh <- &spb.GetResponse{
+				Entry: []*spb.AFTEntry{{
+					NetworkInstance: r.name,
+					Entry: &spb.AFTEntry_Ipv4{
+						Ipv4: p,
+					},
+				}},
+			}
+		}
+	}
+
+	for index, e := range r.r.Afts.NextHopGroup {
+		select {
+		case <-stopCh:
+			return nil
+		default:
+			p, err := concreteNextHopGroupProto(e)
+			if err != nil {
+				return status.Errorf(codes.Internal, "cannot marshal NextHopGroupEntry for index %d into GetResponse, %v", index, err)
+			}
+			msgCh <- &spb.GetResponse{
+				Entry: []*spb.AFTEntry{{
+					NetworkInstance: r.name,
+					Entry: &spb.AFTEntry_NextHopGroup{
+						NextHopGroup: p,
+					},
+				}},
+			}
+		}
+	}
+
+	for id, e := range r.r.Afts.NextHop {
+		select {
+		case <-stopCh:
+			return nil
+		default:
+			p, err := concreteNextHopProto(e)
+			if err != nil {
+				return status.Errorf(codes.Internal, "cannot marshal NextHopEntry for ID %d into GetResponse, %v", id, err)
+			}
+			msgCh <- &spb.GetResponse{
+				Entry: []*spb.AFTEntry{{
+					NetworkInstance: r.name,
+					Entry: &spb.AFTEntry_NextHop{
+						NextHop: p,
+					},
+				}},
+			}
+		}
+	}
+
+	return nil
+}

rib/rib_test.go

@@ -1319,3 +1319,167 @@ func TestRIBAddEntry(t *testing.T) {
 		})
 	}
 }
+
+func TestKnownNetworkInstances(t *testing.T) {
+	tests := []struct {
+		desc      string
+		inRIB     *RIB
+		wantNames []string
+	}{{
+		desc:      "empty set of network instances",
+		inRIB:     &RIB{},
+		wantNames: []string{},
+	}, {
+		desc: "set of known names",
+		inRIB: &RIB{
+			niRIB: map[string]*RIBHolder{
+				"one": {},
+				"two": {},
+			},
+		},
+		wantNames: []string{"one", "two"},
+	}, {
+		desc: "ensure alphabetical",
+		inRIB: &RIB{
+			niRIB: map[string]*RIBHolder{
+				"zebra":    {},
+				"antelope": {},
+			},
+		},
+		wantNames: []string{"antelope", "zebra"},
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got := tt.inRIB.KnownNetworkInstances()
+			if diff := cmp.Diff(got, tt.wantNames); diff != "" {
+				t.Fatalf("did not get expected set of network instance names, diff(-got,+want):\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestGetRIB(t *testing.T) {
+	tests := []struct {
+		desc          string
+		inRIB         *RIBHolder
+		wantResponses []*spb.GetResponse
+		wantErr       bool
+	}{{
+		desc:          "empty RIB",
+		inRIB:         NewRIBHolder("VRF-1"),
+		wantResponses: []*spb.GetResponse{},
+	}, {
+		desc: "ipv4 entry",
+		inRIB: func() *RIBHolder {
+			r := NewRIBHolder("VRF-1")
+
+			cr := &aft.RIB{}
+			ipv4 := cr.GetOrCreateAfts().GetOrCreateIpv4Entry("1.1.1.1/32")
+			ipv4.NextHopGroup = ygot.Uint64(42)
+
+			if err := r.doAddIPv4("1.1.1.1/32", cr); err != nil {
+				panic(fmt.Sprintf("cannot build RIB, %v", err))
+			}
+			return r
+		}(),
+		wantResponses: []*spb.GetResponse{{
+			Entry: []*spb.AFTEntry{{
+				NetworkInstance: "VRF-1",
+				Entry: &spb.AFTEntry_Ipv4{
+					Ipv4: &aftpb.Afts_Ipv4EntryKey{
+						Prefix: "1.1.1.1/32",
+						Ipv4Entry: &aftpb.Afts_Ipv4Entry{
+							NextHopGroup: &wpb.UintValue{Value: 42},
+						},
+					},
+				},
+			}},
+		}},
+	}, {
+		desc: "next-hop-group entry",
+		inRIB: func() *RIBHolder {
+			r := NewRIBHolder("VRF-42")
+
+			cr := &aft.RIB{}
+			nhg := cr.GetOrCreateAfts().GetOrCreateNextHopGroup(42)
+			nhg.Color = ygot.Uint64(1)
+
+			if err := r.doAddNHG(42, cr); err != nil {
+				panic(fmt.Sprintf("cannot build RIB, %v", err))
+			}
+			return r
+		}(),
+		wantResponses: []*spb.GetResponse{{
+			Entry: []*spb.AFTEntry{{
+				NetworkInstance: "VRF-42",
+				Entry: &spb.AFTEntry_NextHopGroup{
+					NextHopGroup: &aftpb.Afts_NextHopGroupKey{
+						Id: 42,
+						NextHopGroup: &aftpb.Afts_NextHopGroup{
+							Color: &wpb.UintValue{Value: 1},
+						},
+					},
+				},
+			}},
+		}},
+	}, {
+		desc: "next-hop entry",
+		inRIB: func() *RIBHolder {
+			r := NewRIBHolder("VRF-42")
+
+			cr := &aft.RIB{}
+			nh := cr.GetOrCreateAfts().GetOrCreateNextHop(1)
+			nh.IpAddress = ygot.String("1.1.1.1/32")
+
+			if err := r.doAddNHG(42, cr); err != nil {
+				panic(fmt.Sprintf("cannot build RIB, %v", err))
+			}
+			return r
+		}(),
+		wantResponses: []*spb.GetResponse{{
+			Entry: []*spb.AFTEntry{{
+				NetworkInstance: "VRF-42",
+				Entry: &spb.AFTEntry_NextHop{
+					NextHop: &aftpb.Afts_NextHopKey{
+						Index: 1,
+						NextHop: &aftpb.Afts_NextHop{
+							IpAddress: &wpb.StringValue{Value: "1.1.1.1/32"},
+						},
+					},
+				},
+			}},
+		}},
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			msgCh := make(chan *spb.GetResponse)
+			stopCh := make(chan struct{})
+
+			doneCh := make(chan struct{})
+			defer close(doneCh)
+
+			got := []*spb.GetResponse{}
+			go func() {
+				for {
+					select {
+					case r := <-msgCh:
+						got = append(got, r)
+					case <-doneCh:
+						return
+					}
+				}
+			}()
+
+			if err := tt.inRIB.GetRIB(msgCh, stopCh); (err != nil) != tt.wantErr {
+				t.Fatalf("did not get expected error, got: %v, wantErr? %v", err, tt.wantErr)
+			}
+			doneCh <- struct{}{}
+
+			if diff := cmp.Diff(got, tt.wantResponses, protocmp.Transform()); diff != "" {
+				t.Fatalf("did not get expected responses, diff(-got,+want):\n%s", diff)
+			}
+		})
+	}
+}