Skip to content

Commit

Permalink
fix send-max limit in case of route update
Browse files Browse the repository at this point in the history
  • Loading branch information
Maxime Peim committed May 31, 2024
1 parent 076e9fc commit 4bc5a97
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 155 deletions.
31 changes: 28 additions & 3 deletions internal/pkg/table/path.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ type Path struct {
IsNexthopInvalid bool
IsWithdraw bool
}

type FilteredType uint8

const (
Expand All @@ -150,7 +151,21 @@ const (
SendMaxFiltered
)

type PathLocalKey string
type PathDestLocalKey struct {
Family bgp.RouteFamily
Prefix string
}
type PathLocalKey struct {
PathDestLocalKey
Id uint32
}

func NewPathDestLocalKey(f bgp.RouteFamily, destPrefix string) *PathDestLocalKey {
return &PathDestLocalKey{
Family: f,
Prefix: destPrefix,
}
}

var localSource = &PeerInfo{}

Expand Down Expand Up @@ -590,8 +605,18 @@ func (path *Path) String() string {

// GetLocalKey identifies the path in the local BGP server.
func (path *Path) GetLocalKey() PathLocalKey {
// return PathLocalKey(path.GetPrefix())
return PathLocalKey(fmt.Sprintf("%s:%s:%d", path.GetRouteFamily(), path.GetNlri(), path.GetNlri().PathLocalIdentifier()))
return PathLocalKey{
PathDestLocalKey: path.GetDestLocalKey(),
Id: path.GetNlri().PathLocalIdentifier(),
}
}

// GetDestLocalKey identifies the path destination in the local BGP server.
func (path *Path) GetDestLocalKey() PathDestLocalKey {
return PathDestLocalKey{
Family: path.GetRouteFamily(),
Prefix: path.GetNlri().String(),
}
}

func (path *Path) GetPrefix() string {
Expand Down
124 changes: 40 additions & 84 deletions pkg/server/peer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,14 @@ func newDynamicPeer(g *oc.Global, neighborAddress string, pg *oc.PeerGroup, loc
}

type peer struct {
tableId string
fsm *fsm
adjRibIn *table.AdjRib
policy *table.RoutingPolicy
localRib *table.TableManager
prefixLimitWarned map[bgp.RouteFamily]bool
dstRoutesCount map[bgp.RouteFamily]map[string]uint8
tableId string
fsm *fsm
adjRibIn *table.AdjRib
policy *table.RoutingPolicy
localRib *table.TableManager
prefixLimitWarned map[bgp.RouteFamily]bool
// map of path local identifiers sent for that prefix
sentPaths map[table.PathDestLocalKey]map[uint32]struct{}
sendMaxPathFiltered map[table.PathLocalKey]struct{}
llgrEndChs []chan struct{}
}
Expand All @@ -115,7 +116,7 @@ func newPeer(g *oc.Global, conf *oc.Neighbor, loc *table.TableManager, policy *t
policy: policy,
fsm: newFSM(g, conf, logger),
prefixLimitWarned: make(map[bgp.RouteFamily]bool),
dstRoutesCount: make(map[bgp.RouteFamily]map[string]uint8),
sentPaths: make(map[table.PathDestLocalKey]map[uint32]struct{}),
sendMaxPathFiltered: make(map[table.PathLocalKey]struct{}),
}
if peer.isRouteServerClient() {
Expand Down Expand Up @@ -215,56 +216,39 @@ func (peer *peer) getAddPathSendMax(family bgp.RouteFamily) uint8 {
return 0
}

func (peer *peer) getRoutesCount(dstPrefix string, family bgp.RouteFamily) uint8 {
peer.fsm.lock.RLock()
defer peer.fsm.lock.RUnlock()
if _, ok := peer.dstRoutesCount[family]; ok {
return peer.dstRoutesCount[family][dstPrefix]
func (peer *peer) getRoutesCount(family bgp.RouteFamily, dstPrefix string) uint8 {
destLocalKey := table.NewPathDestLocalKey(family, dstPrefix)
if identifiers, ok := peer.sentPaths[*destLocalKey]; ok {
count := len(identifiers)
// the send-max config is uint8, so we need to check for overflow
if count > int(^uint8(0)) {
return ^uint8(0)
}
return uint8(count)
}
return 0
}

func (peer *peer) setRoutesCount(dstPrefix string, family bgp.RouteFamily, count uint8) {
peer.fsm.lock.Lock()
defer peer.fsm.lock.Unlock()
if _, ok := peer.dstRoutesCount[family]; !ok {
peer.dstRoutesCount[family] = make(map[string]uint8)
}
peer.dstRoutesCount[family][dstPrefix] = count
}

func (peer *peer) incrementRoutesCount(dstPrefix string, family bgp.RouteFamily, inc uint8) {
if inc == 0 {
return
}

peer.fsm.lock.Lock()
defer peer.fsm.lock.Unlock()
if _, ok := peer.dstRoutesCount[family]; !ok {
peer.dstRoutesCount[family] = make(map[string]uint8)
}
newCount := peer.dstRoutesCount[family][dstPrefix] + inc
if newCount < peer.dstRoutesCount[family][dstPrefix] {
newCount = 0xFF
}
peer.dstRoutesCount[family][dstPrefix] = newCount
}

func (peer *peer) decrementRoutesCount(dstPrefix string, family bgp.RouteFamily, dec uint8) {
if dec == 0 {
func (peer *peer) updateRoutes(paths ...*table.Path) {
if len(paths) == 0 {
return
}

peer.fsm.lock.Lock()
defer peer.fsm.lock.Unlock()
if _, ok := peer.dstRoutesCount[family]; !ok {
peer.dstRoutesCount[family] = make(map[string]uint8)
}
newCount := peer.dstRoutesCount[family][dstPrefix] - dec
if newCount > peer.dstRoutesCount[family][dstPrefix] {
newCount = 0
for _, path := range paths {
localKey := path.GetLocalKey()
destLocalKey := localKey.PathDestLocalKey
identifiers, destExists := peer.sentPaths[destLocalKey]
if path.IsWithdraw && destExists {
delete(identifiers, path.GetNlri().PathLocalIdentifier())
} else if !path.IsWithdraw {
if !destExists {
peer.sentPaths[destLocalKey] = make(map[uint32]struct{})
}
identifiers := peer.sentPaths[destLocalKey]
if len(identifiers) < int(peer.getAddPathSendMax(destLocalKey.Family)) {
identifiers[localKey.Id] = struct{}{}
}
}
}
peer.dstRoutesCount[family][dstPrefix] = newCount
}

func (peer *peer) isPathSendMaxFiltered(path *table.Path) bool {
Expand All @@ -286,44 +270,16 @@ func (peer *peer) unsetPathSendMaxFiltered(path *table.Path) bool {
return true
}

func (peer *peer) getSendMaxFilteredPathList(dest *table.Destination, limit int) []*table.Path {
knownPathList := dest.GetKnownPathList(peer.TableID(), peer.AS())
list := make([]*table.Path, 0, len(knownPathList))
for _, p := range knownPathList {
if !peer.isPathSendMaxFiltered(p) {
continue
}
list = append(list, p)
if limit > 0 && len(list) == limit {
break
}
}
return list
}

func (peer *peer) canSendPathWithinLimit(path *table.Path) bool {
func (peer *peer) hasPathAlreadyBeenSent(path *table.Path) bool {
if path == nil {
return false
}

family := path.GetRouteFamily()
dstPrefix := path.GetPrefix()
sendMax := peer.getAddPathSendMax(family)
dstRouteCount := peer.getRoutesCount(dstPrefix, family)

if dstRouteCount >= sendMax {
peer.sendMaxPathFiltered[path.GetLocalKey()] = struct{}{}
destLocalKey := path.GetDestLocalKey()
if _, dstExist := peer.sentPaths[destLocalKey]; !dstExist {
return false
}

if dstRouteCount > 0 && path.IsWithdraw {
peer.decrementRoutesCount(dstPrefix, family, 1)
} else if dstRouteCount < sendMax && !path.IsWithdraw {
peer.incrementRoutesCount(dstPrefix, family, 1)
} else {
return false
}
return true
_, pathExist := peer.sentPaths[destLocalKey][path.GetNlri().PathLocalIdentifier()]
return pathExist
}

func (peer *peer) isDynamicNeighbor() bool {
Expand Down
108 changes: 67 additions & 41 deletions pkg/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1346,15 +1346,19 @@ func (s *BgpServer) propagateUpdateToNeighbors(rib *table.TableManager, source *
return family
}()
if targetPeer.isAddPathSendEnabled(f) {
// in case of multiple paths to the same destination, we need to
// filter the paths before counting the number of paths to be sent.
if newPath.IsWithdraw {
bestList = func() []*table.Path {
l := make([]*table.Path, 0, len(dsts))
l := []*table.Path{}
for _, d := range dsts {
toDelete := d.GetWithdrawnPath()
toActuallyDelete := make([]*table.Path, 0, len(toDelete))
for _, p := range toDelete {
// if the path is filtered, there is no need to send the withdrawal
p := s.filterpath(targetPeer, p, nil)
// the path was never advertized to the peer
if targetPeer.unsetPathSendMaxFiltered(p) {
if p == nil || targetPeer.unsetPathSendMaxFiltered(p) {
continue
}
toActuallyDelete = append(toActuallyDelete, p)
Expand All @@ -1365,8 +1369,6 @@ func (s *BgpServer) propagateUpdateToNeighbors(rib *table.TableManager, source *
}

destination := rib.GetDestination(toActuallyDelete[0])
dstPrefix := toActuallyDelete[0].GetPrefix()
targetPeer.decrementRoutesCount(dstPrefix, f, uint8(len(toActuallyDelete)))
l = append(l, toActuallyDelete...)

// the destination has been removed from the table
Expand All @@ -1375,54 +1377,78 @@ func (s *BgpServer) propagateUpdateToNeighbors(rib *table.TableManager, source *
continue
}

toAdd := targetPeer.getSendMaxFilteredPathList(destination, len(toActuallyDelete))
targetPeer.incrementRoutesCount(dstPrefix, f, uint8(len(toAdd)))
for _, p := range toAdd {
targetPeer.unsetPathSendMaxFiltered(p)
knownPathList := destination.GetKnownPathList(targetPeer.TableID(), targetPeer.AS())
toAdd := make([]*table.Path, 0, len(knownPathList))
for _, p := range knownPathList {
// if the path is filtered, there is no need to send the path
p := s.filterpath(targetPeer, p, nil)
if p == nil || !targetPeer.isPathSendMaxFiltered(p) {
continue
}
toAdd = append(toAdd, p)
if len(toAdd) == len(toActuallyDelete) {
break
}
}
l = append(l, toAdd...)
}
targetPeer.updateRoutes(l...)
return l
}()
} else if targetPeer.canSendPathWithinLimit(newPath) {
bestList = []*table.Path{newPath}
if newPath.GetRouteFamily() == bgp.RF_RTC_UC {
// we assumes that new "path" nlri was already sent before. This assumption avoids the
// infinite UPDATE loop between Route Reflector and its clients.
for _, old := range dsts[0].OldKnownPathList {
if old.IsLocal() {
bestList = []*table.Path{}
break
} else {
alreadySent := targetPeer.hasPathAlreadyBeenSent(newPath)
newPath := s.filterpath(targetPeer, newPath, nil)
// if the path is not filtered and the path has already been sent or land in the limit, we can send it
if newPath == nil {
bestList = []*table.Path{}
} else if alreadySent || targetPeer.getRoutesCount(f, newPath.GetPrefix()) < targetPeer.getAddPathSendMax(f) {
bestList = []*table.Path{newPath}
if !alreadySent {
targetPeer.updateRoutes(newPath)
}
if newPath.GetRouteFamily() == bgp.RF_RTC_UC {
// we assumes that new "path" nlri was already sent before. This assumption avoids the
// infinite UPDATE loop between Route Reflector and its clients.
for _, old := range dsts[0].OldKnownPathList {
if old.IsLocal() {
bestList = []*table.Path{}
break
}
}
}
} else {
bestList = []*table.Path{}
targetPeer.sendMaxPathFiltered[newPath.GetLocalKey()] = struct{}{}
s.logger.Warn("exceeding max routes for prefix",
log.Fields{
"Topic": "Peer",
"Key": targetPeer.ID(),
"Prefix": newPath.GetPrefix(),
})
}
} else {
bestList = []*table.Path{}
s.logger.Warn("exceeding max routes for prefix",
log.Fields{
"Topic": "Peer",
"Key": targetPeer.ID(),
"Prefix": newPath.GetPrefix(),
})
}
oldList = nil
} else if targetPeer.isRouteServerClient() {
if targetPeer.isSecondaryRouteEnabled() {
if paths := s.sendSecondaryRoutes(targetPeer, newPath, dsts); len(paths) > 0 {
sendfsmOutgoingMsg(targetPeer, paths, nil, false)
}
continue
if needToAdvertise(targetPeer) && len(bestList) > 0 {
sendfsmOutgoingMsg(targetPeer, bestList, nil, false)
}
bestList, oldList, _ = dstsToPaths(targetPeer.TableID(), targetPeer.AS(), dsts)
} else {
bestList = gBestList
oldList = gOldList
}
if !needOld {
oldList = nil
}
if paths := s.processOutgoingPaths(targetPeer, bestList, oldList); len(paths) > 0 {
sendfsmOutgoingMsg(targetPeer, paths, nil, false)
if targetPeer.isRouteServerClient() {
if targetPeer.isSecondaryRouteEnabled() {
if paths := s.sendSecondaryRoutes(targetPeer, newPath, dsts); len(paths) > 0 {
sendfsmOutgoingMsg(targetPeer, paths, nil, false)
}
continue
}
bestList, oldList, _ = dstsToPaths(targetPeer.TableID(), targetPeer.AS(), dsts)
} else {
bestList = gBestList
oldList = gOldList
}
if !needOld {
oldList = nil
}
if paths := s.processOutgoingPaths(targetPeer, bestList, oldList); len(paths) > 0 {
sendfsmOutgoingMsg(targetPeer, paths, nil, false)
}
}
}
}
Expand Down
Loading

0 comments on commit 4bc5a97

Please sign in to comment.