Skip to content

Commit

Permalink
Add support for next-hop-group into reconciler.
Browse files Browse the repository at this point in the history 
  * (M) rib/reconciler/reconcile.go
  * (M) rib/reconciler/reconcile_test.go
    - support add/delete/replace for NextHopGroup entries within a RIB.
  • Loading branch information
@robshakir
robshakir committed Oct 23, 2023
1 parent b0017da commit 2c0e1f7
Show file tree
Hide file tree
Showing 2 changed files with 304 additions and 8 deletions.
71 changes: 64 additions & 7 deletions rib/reconciler/reconcile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp
return nil, fmt.Errorf("cannot copy destination RIB contents, err: %v", err)
}

ops := []*spb.AFTOperation{}
// Store the "top-level" operations (i.e., IPv4, IPv6, MPLS) and then the NHG and NHs
// separately. This allows us to return the operations separately so that they can be
// ordered in terms of programming. NHs need to be installed/replaced before NHGs, and
// then subsequently top-level entries.
topLevelOps, nhgOps, nhOps := []*spb.AFTOperation{}, []*spb.AFTOperation{}, []*spb.AFTOperation{}
var id uint64
for srcNI, srcNIEntries := range srcContents {
dstNIEntries, ok := dstContents[srcNI]
Expand All @@ -152,8 +156,18 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp
if err != nil {
return nil, err
}
ops = append(ops, op)
topLevelOps = append(topLevelOps, op)
}

for nhgID, e := range srcNIEntries.GetAfts().NextHopGroup {
id++
op, err := nhgOperation(spb.AFTOperation_ADD, srcNI, nhgID, id, e)
if err != nil {
return nil, err
}
nhgOps = append(nhgOps, op)
}

continue
}
// For each AFT:
Expand All @@ -171,7 +185,22 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp
if err != nil {
return nil, err
}
ops = append(ops, op)
topLevelOps = append(topLevelOps, op)
}
}

for nhgID, srcE := range srcNIEntries.GetAfts().NextHopGroup {
if dstE, ok := dstNIEntries.GetAfts().NextHopGroup[nhgID]; !ok || !reflect.DeepEqual(srcE, dstE) {
opType := spb.AFTOperation_ADD
if ok && explicitReplace[spb.AFTType_NEXTHOP_GROUP] {
opType = spb.AFTOperation_REPLACE
}
id++
op, err := nhgOperation(opType, srcNI, nhgID, id, srcE)
if err != nil {
return nil, err
}
nhgOps = append(nhgOps, op)
}
}

Expand All @@ -182,13 +211,19 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp
if err != nil {
return nil, err
}
ops = append(ops, op)
topLevelOps = append(topLevelOps, op)
}
}

if srcN, dstN := len(srcNIEntries.GetAfts().NextHopGroup), len(dstNIEntries.GetAfts().NextHopGroup); srcN != 0 || dstN != 0 {
// TODO(robjs): Implement diffing of NHG entries.
klog.Warningf("next-hop-group reconcilation unimplemented, NHG entries, src: %d, dst: %d", srcN, dstN)
for nhgID, dstE := range dstNIEntries.GetAfts().NextHopGroup {
if _, ok := srcNIEntries.GetAfts().NextHopGroup[nhgID]; !ok {
id++
op, err := nhgOperation(spb.AFTOperation_DELETE, srcNI, nhgID, id, dstE)
if err != nil {
return nil, err
}
nhgOps = append(nhgOps, op)
}
}

if srcN, dstN := len(srcNIEntries.GetAfts().NextHop), len(dstNIEntries.GetAfts().NextHop); srcN != 0 || dstN != 0 {
Expand All @@ -197,6 +232,10 @@ func diff(src, dst *rib.RIB, explicitReplace map[spb.AFTType]bool) ([]*spb.AFTOp
}
}

ops := append([]*spb.AFTOperation{}, nhOps...)
ops = append(ops, nhgOps...)
ops = append(ops, topLevelOps...)

return ops, nil
}

Expand All @@ -217,3 +256,21 @@ func v4Operation(method spb.AFTOperation_Operation, ni, pfx string, id uint64, e
},
}, nil
}

// nhgOperation builds a gRIBI NHG operation with the specified method, corresponding to the
// NHG ID nhgID, in network instance ni, using the specified ID for the operation. The
// contents of the operation are the entry e.
func nhgOperation(method spb.AFTOperation_Operation, ni string, nhgID, id uint64, e *aft.Afts_NextHopGroup) (*spb.AFTOperation, error) {
p, err := rib.ConcreteNextHopGroupProto(e)
if err != nil {
return nil, fmt.Errorf("cannot create operation for NHG %d, %v", nhgID, err)
}
return &spb.AFTOperation{
Id: id,
NetworkInstance: ni,
Op: method,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: p,
},
}, nil
}
241 changes: 240 additions & 1 deletion rib/reconciler/reconcile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,23 @@ func TestDiff(t *testing.T) {
}(),
inDst: rib.NewFake(dn).RIB(),
wantOps: []*spb.AFTOperation{{
Id: 2,
NetworkInstance: "FOO",
Op: spb.AFTOperation_ADD,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 1,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 1},
},
}},
},
},
},
}, {
Id: 1,
NetworkInstance: "FOO",
Op: spb.AFTOperation_ADD,
Expand Down Expand Up @@ -164,6 +181,23 @@ func TestDiff(t *testing.T) {
return r.RIB()
}(),
wantOps: []*spb.AFTOperation{{
Id: 2,
NetworkInstance: dn,
Op: spb.AFTOperation_ADD,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 2,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 2},
},
}},
},
},
},
}, {
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_ADD,
Expand Down Expand Up @@ -211,6 +245,23 @@ func TestDiff(t *testing.T) {
spb.AFTType_IPV4: true,
},
wantOps: []*spb.AFTOperation{{
Id: 2,
NetworkInstance: dn,
Op: spb.AFTOperation_ADD,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 2,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 2},
},
}},
},
},
},
}, {
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_REPLACE,
Expand All @@ -223,6 +274,191 @@ func TestDiff(t *testing.T) {
},
},
}},
}, {
desc: "NHG installed",
inSrc: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
inDst: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
return r.RIB()
}(),
wantOps: []*spb.AFTOperation{{
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_ADD,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 1,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 1},
},
}},
},
},
},
}},
}, {
desc: "NHG deleted",
inSrc: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
return r.RIB()
}(),
inDst: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
wantOps: []*spb.AFTOperation{{
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_DELETE,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 1,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 1},
},
}},
},
},
},
}},
}, {
desc: "NHG implicitly replaced",
inSrc: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNH(dn, 2); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{
1: 2,
2: 4,
}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
inDst: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNH(dn, 2); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
wantOps: []*spb.AFTOperation{{
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_ADD,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 1,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 2},
},
}, {
Index: 2,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 4},
},
}},
},
},
},
}},
}, {
desc: "NHG explicitly replaced",
inSrc: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNH(dn, 2); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{
1: 2,
2: 4,
}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
inDst: func() *rib.RIB {
r := rib.NewFake(dn)
if err := r.InjectNH(dn, 1); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNH(dn, 2); err != nil {
t.Fatalf("cannot add NH, %v", err)
}
if err := r.InjectNHG(dn, 1, map[uint64]uint64{1: 1}); err != nil {
t.Fatalf("cannot add NHG 1, %v", err)
}
return r.RIB()
}(),
inExplicitReplace: map[spb.AFTType]bool{
spb.AFTType_NEXTHOP_GROUP: true,
},
wantOps: []*spb.AFTOperation{{
Id: 1,
NetworkInstance: dn,
Op: spb.AFTOperation_REPLACE,
Entry: &spb.AFTOperation_NextHopGroup{
NextHopGroup: &aftpb.Afts_NextHopGroupKey{
Id: 1,
NextHopGroup: &aftpb.Afts_NextHopGroup{
NextHop: []*aftpb.Afts_NextHopGroup_NextHopKey{{
Index: 1,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 2},
},
}, {
Index: 2,
NextHop: &aftpb.Afts_NextHopGroup_NextHop{
Weight: &wpb.UintValue{Value: 4},
},
}},
},
},
},
}},
}}

for _, tt := range tests {
Expand All @@ -231,7 +467,10 @@ func TestDiff(t *testing.T) {
if (err != nil) != tt.wantErr {
t.Fatalf("did not get expected error, got: %v, wantErr? %v", err, tt.wantErr)
}
if diff := cmp.Diff(got, tt.wantOps, protocmp.Transform()); diff != "" {
if diff := cmp.Diff(got, tt.wantOps,
protocmp.Transform(),
protocmp.SortRepeatedFields(&aftpb.Afts_NextHopGroup{}, "next_hop"),
); diff != "" {
t.Fatalf("diff(%s, %s): did not get expected operations, diff(-got,+want):\n%s", tt.inSrc, tt.inDst, diff)
}
})
Expand Down

0 comments on commit 2c0e1f7

Please sign in to comment.