Conversation
robshakir
commented
Jun 9, 2021
robshakir
commented
* (M) aft/oc.go
* (M) aft/update.sh
- Update generated code.
* (M) go.mod
* (M) go.sum
- Go mod tidying.
* (M) rib/rib.go
* (M) rib/rib_test.go
- Add IPv4 entry add and delete.
* (M) go.mod
* (M) go.sum
- Tidy up modules.
* (M) rib/rib.go
* (M) rib/rib_test.go
- Add support for network instances in the RIB rather than
a single RIB.
sthesayi
left a comment
sthesayi
left a comment
There was a problem hiding this comment.
Just minor comments, so approving.
| } | ||
|
|
||
| // ribHolder is a container for a set of RIBs. | ||
| type ribHolder struct { |
There was a problem hiding this comment.
Just for my understanding... will this eventually become an array of RIBs? I would have thought a network instance will have exactly one RIB table. If so then is ribHolder struct necessary?
There was a problem hiding this comment.
I think there are two reasons to have ribHolder exist:
-
There are likely to be some things that the ygot structs don't have in them that we need, for example, mutexes around the different maps that make up the RIB. We probably need to put these somewhere - either by changing the generated code to include them (not that easy to do today), or by putting them outside of the generated datastructure. I'd propose that they go here.
-
I'd like the option to be able to have per-protocol RIBs at some point in the future - today this RIB is really the gRIBI RIB - but one thing that I have in mind is maybe we can create an "on-box protocol" RIB alongside this gRIBI one. The reason we would do is that if we're doing replay of routes from another source (e.g., by capturing updates from our BGP daemon in the controller), then we could replay them into the device using gRIBI as the API to do so - in this case, there might be >1 set of RIBs per NI.
| // AddIPv4 adds the IPv4 entry described by e to the RIB. It returns an error | ||
| // if the entry cannot be added. | ||
| func (r *ribHolder) AddIPv4(e *aftpb.Afts_Ipv4EntryKey) error { | ||
| // This is a hack, since ygot does not know that the field that we |
There was a problem hiding this comment.
Should we check e != nil ?
2 participants
