Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move SelectionOptions and UseMultiplePath to server-local vars #2716

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Move SelectionOptions and UseMultiplePath to server-local vars #2716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 21 additions & 24 deletions internal/pkg/table/destination.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ import (
"github.com/osrg/gobgp/v3/pkg/packet/bgp"
)

var SelectionOptions oc.RouteSelectionOptionsConfig
var UseMultiplePaths oc.UseMultiplePathsConfig

type BestPathReason uint8

const (
Expand Down Expand Up @@ -228,7 +225,7 @@ func (dd *Destination) GetMultiBestPath(id string) []*Path {
//
// Modifies destination's state related to stored paths. Removes withdrawn
// paths from known paths. Also, adds new paths to known paths.
func (dest *Destination) Calculate(logger log.Logger, newPath *Path) *Update {
func (dest *Destination) Calculate(logger log.Logger, newPath *Path, selectionOptions *oc.RouteSelectionOptionsConfig) *Update {
oldKnownPathList := make([]*Path, len(dest.knownPathList))
copy(oldKnownPathList, dest.knownPathList)

Expand All @@ -255,7 +252,7 @@ func (dest *Destination) Calculate(logger log.Logger, newPath *Path) *Update {
}
}
// Compute new best path
dest.computeKnownBestPath()
dest.computeKnownBestPath(selectionOptions)

l := make([]*Path, len(dest.knownPathList))
copy(l, dest.knownPathList)
Expand Down Expand Up @@ -344,8 +341,8 @@ func (dest *Destination) implicitWithdraw(logger log.Logger, newPath *Path) {
}
}

func (dest *Destination) computeKnownBestPath() (*Path, BestPathReason, error) {
if SelectionOptions.DisableBestPathSelection {
func (dest *Destination) computeKnownBestPath(selectionOptions *oc.RouteSelectionOptionsConfig) (*Path, BestPathReason, error) {
if selectionOptions.DisableBestPathSelection {
return nil, BPR_DISABLED, nil
}

Expand All @@ -366,7 +363,7 @@ func (dest *Destination) computeKnownBestPath() (*Path, BestPathReason, error) {
}
return dest.knownPathList[0], BPR_ONLY_PATH, nil
}
reason := dest.sort()
reason := dest.sort(selectionOptions)
newBest := dest.knownPathList[0]
// If the first path has the invalidated next-hop, which evaluated by IGP,
// returns no path with the reason of the next-hop reachability.
Expand All @@ -376,7 +373,7 @@ func (dest *Destination) computeKnownBestPath() (*Path, BestPathReason, error) {
return newBest, reason, nil
}

func (dst *Destination) sort() BestPathReason {
func (dst *Destination) sort(selectionOptions *oc.RouteSelectionOptionsConfig) BestPathReason {
reason := BPR_UNKNOWN

sort.SliceStable(dst.knownPathList, func(i, j int) bool {
Expand Down Expand Up @@ -438,15 +435,15 @@ func (dst *Destination) sort() BestPathReason {
reason = BPR_LOCAL_ORIGIN
}
if better == nil {
better = compareByASPath(path1, path2)
better = compareByASPath(path1, path2, selectionOptions)
reason = BPR_ASPATH
}
if better == nil {
better = compareByOrigin(path1, path2)
reason = BPR_ORIGIN
}
if better == nil {
better = compareByMED(path1, path2)
better = compareByMED(path1, path2, selectionOptions)
reason = BPR_MED
}
if better == nil {
Expand All @@ -457,11 +454,11 @@ func (dst *Destination) sort() BestPathReason {
// compareByIGPCost was a no-op and was removed.

if better == nil {
better = compareByAge(path1, path2)
better = compareByAge(path1, path2, selectionOptions)
reason = BPR_OLDER
}
if better == nil {
better, _ = compareByRouterID(path1, path2)
better, _ = compareByRouterID(path1, path2, selectionOptions)
reason = BPR_ROUTER_ID
}
if better == nil {
Expand Down Expand Up @@ -521,7 +518,7 @@ func (u *Update) GetWithdrawnPath() []*Path {
return l
}

func (u *Update) GetChanges(id string, as uint32, peerDown bool) (*Path, *Path, []*Path) {
func (u *Update) GetChanges(id string, as uint32, peerDown bool, useMultiplePaths *oc.UseMultiplePathsConfig) (*Path, *Path, []*Path) {
best, old := func(id string) (*Path, *Path) {
old := getBestPath(id, as, u.OldKnownPathList)
best := getBestPath(id, as, u.KnownPathList)
Expand Down Expand Up @@ -564,7 +561,7 @@ func (u *Update) GetChanges(id string, as uint32, peerDown bool) (*Path, *Path,

var multi []*Path

if id == GLOBAL_RIB_NAME && UseMultiplePaths.Enabled {
if id == GLOBAL_RIB_NAME && useMultiplePaths.Enabled {
diff := func(lhs, rhs []*Path) bool {
if len(lhs) != len(rhs) {
return true
Expand Down Expand Up @@ -659,12 +656,12 @@ func compareByLocalOrigin(path1, path2 *Path) *Path {
return nil
}

func compareByASPath(path1, path2 *Path) *Path {
func compareByASPath(path1, path2 *Path, selectionOptions *oc.RouteSelectionOptionsConfig) *Path {
// Calculated the best-paths by comparing as-path lengths.
//
// Shortest as-path length is preferred. If both path have same lengths,
// we return None.
if SelectionOptions.IgnoreAsPathLength {
if selectionOptions.IgnoreAsPathLength {
return nil
}

Expand Down Expand Up @@ -706,7 +703,7 @@ func compareByOrigin(path1, path2 *Path) *Path {
}
}

func compareByMED(path1, path2 *Path) *Path {
func compareByMED(path1, path2 *Path, selectionOptions *oc.RouteSelectionOptionsConfig) *Path {
// Select the path based with lowest MED value.
//
// If both paths have same MED, return None.
Expand Down Expand Up @@ -739,7 +736,7 @@ func compareByMED(path1, path2 *Path) *Path {
return firstAS(path1) != 0 && firstAS(path1) == firstAS(path2)
}()

if SelectionOptions.AlwaysCompareMed || isInternal || isSameAS {
if selectionOptions.AlwaysCompareMed || isInternal || isSameAS {
getMed := func(path *Path) uint32 {
attribute := path.getPathAttr(bgp.BGP_ATTR_TYPE_MULTI_EXIT_DISC)
if attribute == nil {
Expand Down Expand Up @@ -784,7 +781,7 @@ func compareByASNumber(path1, path2 *Path) *Path {
return nil
}

func compareByRouterID(path1, path2 *Path) (*Path, error) {
func compareByRouterID(path1, path2 *Path, selectionOptions *oc.RouteSelectionOptionsConfig) (*Path, error) {
// Select the route received from the peer with the lowest BGP router ID.
//
// If both paths are eBGP paths, then we do not do any tie breaking, i.e we do
Expand All @@ -799,11 +796,11 @@ func compareByRouterID(path1, path2 *Path) (*Path, error) {

// If both paths are from eBGP peers, then according to RFC we need
// not tie break using router id.
if !SelectionOptions.ExternalCompareRouterId && !path1.IsIBGP() && !path2.IsIBGP() {
if !selectionOptions.ExternalCompareRouterId && !path1.IsIBGP() && !path2.IsIBGP() {
return nil, nil
}

if !SelectionOptions.ExternalCompareRouterId && path1.IsIBGP() != path2.IsIBGP() {
if !selectionOptions.ExternalCompareRouterId && path1.IsIBGP() != path2.IsIBGP() {
return nil, fmt.Errorf("this method does not support comparing ebgp with ibgp path")
}

Expand Down Expand Up @@ -844,8 +841,8 @@ func compareByNeighborAddress(path1, path2 *Path) *Path {
return nil
}

func compareByAge(path1, path2 *Path) *Path {
if !path1.IsIBGP() && !path2.IsIBGP() && !SelectionOptions.ExternalCompareRouterId {
func compareByAge(path1, path2 *Path, selectionOptions *oc.RouteSelectionOptionsConfig) *Path {
if !path1.IsIBGP() && !path2.IsIBGP() && !selectionOptions.ExternalCompareRouterId {
age1 := path1.GetTimestamp().UnixNano()
age2 := path2.GetTimestamp().UnixNano()
if age1 == age2 {
Expand Down
49 changes: 26 additions & 23 deletions internal/pkg/table/destination_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"
"time"

"github.com/osrg/gobgp/v3/pkg/config/oc"
"github.com/osrg/gobgp/v3/pkg/packet/bgp"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -83,7 +84,7 @@ func TestCalculate2(t *testing.T) {
path1 := ProcessMessage(update1, peer1, time.Now())[0]

d := NewDestination(nlri, 0)
d.Calculate(logger, path1)
d.Calculate(logger, path1, &oc.RouteSelectionOptionsConfig {})

// suppose peer2 sends grammaatically correct but semantically flawed update message
// which has a withdrawal nlri not advertised before
Expand All @@ -92,7 +93,7 @@ func TestCalculate2(t *testing.T) {
path2 := ProcessMessage(update2, peer2, time.Now())[0]
assert.Equal(t, path2.IsWithdraw, true)

d.Calculate(logger, path2)
d.Calculate(logger, path2, &oc.RouteSelectionOptionsConfig {})

// we have a path from peer1 here
assert.Equal(t, len(d.knownPathList), 1)
Expand All @@ -102,7 +103,7 @@ func TestCalculate2(t *testing.T) {
path3 := ProcessMessage(update3, peer2, time.Now())[0]
assert.Equal(t, path3.IsWithdraw, false)

d.Calculate(logger, path3)
d.Calculate(logger, path3, &oc.RouteSelectionOptionsConfig {})

// this time, we have paths from peer1 and peer2
assert.Equal(t, len(d.knownPathList), 2)
Expand All @@ -112,7 +113,7 @@ func TestCalculate2(t *testing.T) {
update4 := bgp.NewBGPUpdateMessage(nil, pathAttributes, []*bgp.IPAddrPrefix{nlri})
path4 := ProcessMessage(update4, peer3, time.Now())[0]

d.Calculate(logger, path4)
d.Calculate(logger, path4, &oc.RouteSelectionOptionsConfig {})

// we must have paths from peer1, peer2 and peer3
assert.Equal(t, len(d.knownPathList), 3)
Expand Down Expand Up @@ -156,7 +157,7 @@ func TestMedTieBreaker(t *testing.T) {
}()

// same AS
assert.Equal(t, compareByMED(p0, p1), p0)
assert.Equal(t, compareByMED(p0, p1, &oc.RouteSelectionOptionsConfig {}), p0)

p2 := func() *Path {
aspath := bgp.NewPathAttributeAsPath([]bgp.AsPathParamInterface{bgp.NewAs4PathParam(bgp.BGP_ASPATH_ATTR_TYPE_SEQ, []uint32{65003})})
Expand All @@ -165,7 +166,7 @@ func TestMedTieBreaker(t *testing.T) {
}()

// different AS
assert.Equal(t, compareByMED(p0, p2), (*Path)(nil))
assert.Equal(t, compareByMED(p0, p2, &oc.RouteSelectionOptionsConfig {}), (*Path)(nil))

p3 := func() *Path {
aspath := bgp.NewPathAttributeAsPath([]bgp.AsPathParamInterface{bgp.NewAs4PathParam(bgp.BGP_ASPATH_ATTR_TYPE_CONFED_SEQ, []uint32{65003, 65004}), bgp.NewAs4PathParam(bgp.BGP_ASPATH_ATTR_TYPE_SEQ, []uint32{65001, 65003})})
Expand All @@ -180,7 +181,7 @@ func TestMedTieBreaker(t *testing.T) {
}()

// ignore confed
assert.Equal(t, compareByMED(p3, p4), p3)
assert.Equal(t, compareByMED(p3, p4, &oc.RouteSelectionOptionsConfig {}), p3)

p5 := func() *Path {
attrs := []bgp.PathAttributeInterface{bgp.NewPathAttributeMultiExitDisc(0)}
Expand All @@ -193,7 +194,7 @@ func TestMedTieBreaker(t *testing.T) {
}()

// no aspath
assert.Equal(t, compareByMED(p5, p6), p5)
assert.Equal(t, compareByMED(p5, p6, &oc.RouteSelectionOptionsConfig {}), p5)
}

func TestTimeTieBreaker(t *testing.T) {
Expand All @@ -212,17 +213,19 @@ func TestTimeTieBreaker(t *testing.T) {
path2 := ProcessMessage(updateMsg, peer2, time.Now().Add(-1*time.Hour))[0] // older than path1

d := NewDestination(nlri, 0)
d.Calculate(logger, path1)
d.Calculate(logger, path2)
d.Calculate(logger, path1, &oc.RouteSelectionOptionsConfig {})
d.Calculate(logger, path2, &oc.RouteSelectionOptionsConfig {})

assert.Equal(t, len(d.knownPathList), 2)
assert.Equal(t, true, d.GetBestPath("", 0).GetSource().ID.Equal(net.IP{2, 2, 2, 2})) // path from peer2 win

// this option disables tie breaking by age
SelectionOptions.ExternalCompareRouterId = true
selectionOptions := oc.RouteSelectionOptionsConfig {
ExternalCompareRouterId: true,
}
d = NewDestination(nlri, 0)
d.Calculate(logger, path1)
d.Calculate(logger, path2)
d.Calculate(logger, path1, &selectionOptions)
d.Calculate(logger, path2, &selectionOptions)

assert.Equal(t, len(d.knownPathList), 2)
assert.Equal(t, true, d.GetBestPath("", 0).GetSource().ID.Equal(net.IP{1, 1, 1, 1})) // path from peer1 win
Expand Down Expand Up @@ -315,7 +318,9 @@ func updateMsgD3() *bgp.BGPMessage {
}

func TestMultipath(t *testing.T) {
UseMultiplePaths.Enabled = true
useMultiplePaths := oc.UseMultiplePathsConfig {
Enabled: true,
}
origin := bgp.NewPathAttributeOrigin(0)
aspathParam := []bgp.AsPathParamInterface{bgp.NewAs4PathParam(2, []uint32{65000})}
aspath := bgp.NewPathAttributeAsPath(aspathParam)
Expand Down Expand Up @@ -347,17 +352,17 @@ func TestMultipath(t *testing.T) {
path2 := ProcessMessage(updateMsg, peer2, time.Now())[0]

d := NewDestination(nlri[0], 0)
d.Calculate(logger, path2)
d.Calculate(logger, path2, &oc.RouteSelectionOptionsConfig{})

best, old, multi := d.Calculate(logger, path1).GetChanges(GLOBAL_RIB_NAME, 0, false)
best, old, multi := d.Calculate(logger, path1, &oc.RouteSelectionOptionsConfig{}).GetChanges(GLOBAL_RIB_NAME, 0, false, &useMultiplePaths)
assert.NotNil(t, best)
assert.Equal(t, old, path2)
assert.Equal(t, len(multi), 2)
assert.Equal(t, len(d.GetKnownPathList(GLOBAL_RIB_NAME, 0)), 2)

path3 := path2.Clone(true)
dd := d.Calculate(logger, path3)
best, old, multi = dd.GetChanges(GLOBAL_RIB_NAME, 0, false)
dd := d.Calculate(logger, path3, &oc.RouteSelectionOptionsConfig{})
best, old, multi = dd.GetChanges(GLOBAL_RIB_NAME, 0, false, &useMultiplePaths)
assert.Nil(t, best)
assert.Equal(t, old, path1)
assert.Equal(t, len(multi), 1)
Expand All @@ -374,8 +379,8 @@ func TestMultipath(t *testing.T) {
}
updateMsg = bgp.NewBGPUpdateMessage(nil, pathAttributes, nlri)
path4 := ProcessMessage(updateMsg, peer3, time.Now())[0]
dd = d.Calculate(logger, path4)
best, _, multi = dd.GetChanges(GLOBAL_RIB_NAME, 0, false)
dd = d.Calculate(logger, path4, &oc.RouteSelectionOptionsConfig{})
best, _, multi = dd.GetChanges(GLOBAL_RIB_NAME, 0, false, &useMultiplePaths)
assert.NotNil(t, best)
assert.Equal(t, len(multi), 1)
assert.Equal(t, len(d.GetKnownPathList(GLOBAL_RIB_NAME, 0)), 2)
Expand All @@ -389,12 +394,10 @@ func TestMultipath(t *testing.T) {
}
updateMsg = bgp.NewBGPUpdateMessage(nil, pathAttributes, nlri)
path5 := ProcessMessage(updateMsg, peer2, time.Now())[0]
best, _, multi = d.Calculate(logger, path5).GetChanges(GLOBAL_RIB_NAME, 0, false)
best, _, multi = d.Calculate(logger, path5, &oc.RouteSelectionOptionsConfig{}).GetChanges(GLOBAL_RIB_NAME, 0, false, &useMultiplePaths)
assert.NotNil(t, best)
assert.Equal(t, len(multi), 2)
assert.Equal(t, len(d.GetKnownPathList(GLOBAL_RIB_NAME, 0)), 3)

UseMultiplePaths.Enabled = false
}

func TestIdMap(t *testing.T) {
Expand Down
13 changes: 9 additions & 4 deletions internal/pkg/table/table_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (

farm "github.com/dgryski/go-farm"

"github.com/osrg/gobgp/v3/pkg/config/oc"
"github.com/osrg/gobgp/v3/pkg/log"
"github.com/osrg/gobgp/v3/pkg/packet/bgp"
)
Expand Down Expand Up @@ -114,14 +115,18 @@ func ProcessMessage(m *bgp.BGPMessage, peerInfo *PeerInfo, timestamp time.Time)
type TableManager struct {
Tables map[bgp.RouteFamily]*Table
Vrfs map[string]*Vrf
SelectionOptions *oc.RouteSelectionOptionsConfig
UseMultiplePaths *oc.UseMultiplePathsConfig
rfList []bgp.RouteFamily
logger log.Logger
}

func NewTableManager(logger log.Logger, rfList []bgp.RouteFamily) *TableManager {
func NewTableManager(logger log.Logger, rfList []bgp.RouteFamily, selectionOptions *oc.RouteSelectionOptionsConfig, useMultiplePaths *oc.UseMultiplePathsConfig) *TableManager {
t := &TableManager{
Tables: make(map[bgp.RouteFamily]*Table),
Vrfs: make(map[string]*Vrf),
SelectionOptions: selectionOptions,
UseMultiplePaths: useMultiplePaths,
rfList: rfList,
logger: logger,
}
Expand Down Expand Up @@ -192,7 +197,7 @@ func (manager *TableManager) update(newPath *Path) *Update {
t := manager.Tables[newPath.GetRouteFamily()]
t.validatePath(newPath)
dst := t.getOrCreateDest(newPath.GetNlri(), 64)
u := dst.Calculate(manager.logger, newPath)
u := dst.Calculate(manager.logger, newPath, manager.SelectionOptions)
if len(dst.knownPathList) == 0 {
t.deleteDest(dst)
}
Expand Down Expand Up @@ -292,7 +297,7 @@ func (manager *TableManager) getDestinationCount(rfList []bgp.RouteFamily) int {
}

func (manager *TableManager) GetBestPathList(id string, as uint32, rfList []bgp.RouteFamily) []*Path {
if SelectionOptions.DisableBestPathSelection {
if manager.SelectionOptions.DisableBestPathSelection {
// Note: If best path selection disabled, there is no best path.
return nil
}
Expand All @@ -304,7 +309,7 @@ func (manager *TableManager) GetBestPathList(id string, as uint32, rfList []bgp.
}

func (manager *TableManager) GetBestMultiPathList(id string, rfList []bgp.RouteFamily) [][]*Path {
if !UseMultiplePaths.Enabled || SelectionOptions.DisableBestPathSelection {
if !manager.UseMultiplePaths.Enabled || manager.SelectionOptions.DisableBestPathSelection {
// Note: If multi path not enabled or best path selection disabled,
// there is no best multi path.
return nil
Expand Down
Loading