diff --git a/internal/pkg/table/destination.go b/internal/pkg/table/destination.go index c75332848..1fb981b67 100644 --- a/internal/pkg/table/destination.go +++ b/internal/pkg/table/destination.go @@ -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 ( @@ -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) @@ -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) @@ -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 } @@ -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. @@ -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 { @@ -438,7 +435,7 @@ 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 { @@ -446,7 +443,7 @@ func (dst *Destination) sort() BestPathReason { reason = BPR_ORIGIN } if better == nil { - better = compareByMED(path1, path2) + better = compareByMED(path1, path2, selectionOptions) reason = BPR_MED } if better == nil { @@ -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 { @@ -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) @@ -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 @@ -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 } @@ -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. @@ -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 { @@ -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 @@ -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") } @@ -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 { diff --git a/internal/pkg/table/destination_test.go b/internal/pkg/table/destination_test.go index b09ae203b..851907b6a 100644 --- a/internal/pkg/table/destination_test.go +++ b/internal/pkg/table/destination_test.go @@ -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" @@ -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 @@ -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) @@ -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) @@ -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) @@ -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})}) @@ -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})}) @@ -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)} @@ -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) { @@ -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 @@ -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) @@ -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) @@ -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) @@ -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) { diff --git a/internal/pkg/table/table_manager.go b/internal/pkg/table/table_manager.go index f7a2a0128..8c2b7f243 100644 --- a/internal/pkg/table/table_manager.go +++ b/internal/pkg/table/table_manager.go @@ -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" ) @@ -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, } @@ -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) } @@ -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 } @@ -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 diff --git a/internal/pkg/table/table_manager_test.go b/internal/pkg/table/table_manager_test.go index 174b8f0a3..cc458489a 100644 --- a/internal/pkg/table/table_manager_test.go +++ b/internal/pkg/table/table_manager_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/osrg/gobgp/v3/pkg/config/oc" "github.com/osrg/gobgp/v3/pkg/log" "github.com/osrg/gobgp/v3/pkg/packet/bgp" @@ -38,7 +39,7 @@ func (manager *TableManager) ProcessUpdate(fromPeer *PeerInfo, message *bgp.BGPM dsts = append(dsts, manager.Update(path)...) } for _, d := range dsts { - b, _, _ := d.GetChanges(GLOBAL_RIB_NAME, 0, false) + b, _, _ := d.GetChanges(GLOBAL_RIB_NAME, 0, false, manager.UseMultiplePaths) pathList = append(pathList, b) } return pathList, nil @@ -78,7 +79,7 @@ func peerR3() *PeerInfo { // test best path calculation and check the result path is from R1 func TestProcessBGPUpdate_0_select_onlypath_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) bgpMessage := update_fromR1() peer := peerR1() @@ -128,7 +129,7 @@ func TestProcessBGPUpdate_0_select_onlypath_ipv4(t *testing.T) { // test best path calculation and check the result path is from R1 func TestProcessBGPUpdate_0_select_onlypath_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) bgpMessage := update_fromR1_ipv6() peer := peerR1() @@ -179,7 +180,7 @@ func TestProcessBGPUpdate_0_select_onlypath_ipv6(t *testing.T) { // test: compare localpref func TestProcessBGPUpdate_1_select_high_localpref_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // low localpref message origin1 := bgp.NewPathAttributeOrigin(0) @@ -259,7 +260,7 @@ func TestProcessBGPUpdate_1_select_high_localpref_ipv4(t *testing.T) { func TestProcessBGPUpdate_1_select_high_localpref_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000}) @@ -341,7 +342,7 @@ func TestProcessBGPUpdate_1_select_high_localpref_ipv6(t *testing.T) { // test: compare localOrigin func TestProcessBGPUpdate_2_select_local_origin_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // low localpref message origin1 := bgp.NewPathAttributeOrigin(0) @@ -423,7 +424,7 @@ func TestProcessBGPUpdate_2_select_local_origin_ipv4(t *testing.T) { func TestProcessBGPUpdate_2_select_local_origin_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000}) @@ -508,7 +509,7 @@ func TestProcessBGPUpdate_2_select_local_origin_ipv6(t *testing.T) { // test: compare AS_PATH func TestProcessBGPUpdate_3_select_aspath_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) bgpMessage1 := update_fromR2viaR1() peer1 := peerR1() @@ -563,7 +564,7 @@ func TestProcessBGPUpdate_3_select_aspath_ipv4(t *testing.T) { func TestProcessBGPUpdate_3_select_aspath_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) bgpMessage1 := update_fromR2viaR1_ipv6() peer1 := peerR1() @@ -620,7 +621,7 @@ func TestProcessBGPUpdate_3_select_aspath_ipv6(t *testing.T) { // test: compare Origin func TestProcessBGPUpdate_4_select_low_origin_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // low origin message origin1 := bgp.NewPathAttributeOrigin(1) @@ -700,7 +701,7 @@ func TestProcessBGPUpdate_4_select_low_origin_ipv4(t *testing.T) { func TestProcessBGPUpdate_4_select_low_origin_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(1) aspath1 := createAsPathAttribute([]uint32{65200, 65000}) @@ -782,7 +783,7 @@ func TestProcessBGPUpdate_4_select_low_origin_ipv6(t *testing.T) { // test: compare MED func TestProcessBGPUpdate_5_select_low_med_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // low origin message origin1 := bgp.NewPathAttributeOrigin(0) @@ -862,7 +863,7 @@ func TestProcessBGPUpdate_5_select_low_med_ipv4(t *testing.T) { func TestProcessBGPUpdate_5_select_low_med_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65200, 65000}) @@ -944,7 +945,7 @@ func TestProcessBGPUpdate_5_select_low_med_ipv6(t *testing.T) { // test: compare AS_NUMBER(prefer eBGP path) func TestProcessBGPUpdate_6_select_ebgp_path_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // low origin message origin1 := bgp.NewPathAttributeOrigin(0) @@ -1024,7 +1025,7 @@ func TestProcessBGPUpdate_6_select_ebgp_path_ipv4(t *testing.T) { func TestProcessBGPUpdate_6_select_ebgp_path_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000, 65200}) @@ -1107,9 +1108,7 @@ func TestProcessBGPUpdate_6_select_ebgp_path_ipv6(t *testing.T) { // test: compare Router ID func TestProcessBGPUpdate_7_select_low_routerid_path_ipv4(t *testing.T) { - - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) - SelectionOptions.ExternalCompareRouterId = true + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{ ExternalCompareRouterId: true}, &oc.UseMultiplePathsConfig{}) // low origin message origin1 := bgp.NewPathAttributeOrigin(0) @@ -1189,7 +1188,7 @@ func TestProcessBGPUpdate_7_select_low_routerid_path_ipv4(t *testing.T) { func TestProcessBGPUpdate_7_select_low_routerid_path_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000, 65200}) @@ -1271,7 +1270,7 @@ func TestProcessBGPUpdate_7_select_low_routerid_path_ipv6(t *testing.T) { // test: withdraw and mpunreach path func TestProcessBGPUpdate_8_withdraw_path_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // path1 origin1 := bgp.NewPathAttributeOrigin(0) @@ -1374,7 +1373,7 @@ func TestProcessBGPUpdate_8_withdraw_path_ipv4(t *testing.T) { // TODO MP_UNREACH func TestProcessBGPUpdate_8_mpunreach_path_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000}) @@ -1502,7 +1501,7 @@ func TestProcessBGPUpdate_8_mpunreach_path_ipv6(t *testing.T) { // handle bestpath lost func TestProcessBGPUpdate_bestpath_lost_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // path1 origin1 := bgp.NewPathAttributeOrigin(0) @@ -1572,7 +1571,7 @@ func TestProcessBGPUpdate_bestpath_lost_ipv4(t *testing.T) { func TestProcessBGPUpdate_bestpath_lost_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000}) @@ -1643,7 +1642,7 @@ func TestProcessBGPUpdate_bestpath_lost_ipv6(t *testing.T) { // test: implicit withdrawal case func TestProcessBGPUpdate_implicit_withdrwal_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) // path1 origin1 := bgp.NewPathAttributeOrigin(0) @@ -1724,7 +1723,7 @@ func TestProcessBGPUpdate_implicit_withdrwal_ipv4(t *testing.T) { func TestProcessBGPUpdate_implicit_withdrwal_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) origin1 := bgp.NewPathAttributeOrigin(0) aspath1 := createAsPathAttribute([]uint32{65000, 65100, 65200}) @@ -1831,7 +1830,7 @@ func TestProcessBGPUpdate_implicit_withdrwal_ipv6(t *testing.T) { // check multiple paths func TestProcessBGPUpdate_multiple_nlri_ipv4(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) createPathAttr := func(aspaths []uint32, nh string) []bgp.PathAttributeInterface { origin := bgp.NewPathAttributeOrigin(0) @@ -1964,7 +1963,7 @@ func TestProcessBGPUpdate_multiple_nlri_ipv4(t *testing.T) { // check multiple paths func TestProcessBGPUpdate_multiple_nlri_ipv6(t *testing.T) { - tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}) + tm := NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv6_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) createPathAttr := func(aspaths []uint32) []bgp.PathAttributeInterface { origin := bgp.NewPathAttributeOrigin(0) diff --git a/pkg/server/server.go b/pkg/server/server.go index 2b3cbcb02..566a3c4ca 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -778,7 +778,7 @@ func (s *BgpServer) setPathVrfIdMap(paths []*table.Path, m map[uint32]bool) { // Note: the destination would be the same for all the paths passed here // The wather (only zapi) needs a unique list of vrf IDs func (s *BgpServer) notifyBestWatcher(best []*table.Path, multipath [][]*table.Path) { - if table.SelectionOptions.DisableBestPathSelection { + if s.bgpConfig.Global.RouteSelectionOptions.Config.DisableBestPathSelection { // Note: If best path selection disabled, no best path to notify. return } @@ -786,12 +786,12 @@ func (s *BgpServer) notifyBestWatcher(best []*table.Path, multipath [][]*table.P clonedM := make([][]*table.Path, len(multipath)) for i, pathList := range multipath { clonedM[i] = clonePathList(pathList) - if table.UseMultiplePaths.Enabled { + if s.bgpConfig.Global.UseMultiplePaths.Config.Enabled { s.setPathVrfIdMap(clonedM[i], m) } } clonedB := clonePathList(best) - if !table.UseMultiplePaths.Enabled { + if !s.bgpConfig.Global.UseMultiplePaths.Config.Enabled { s.setPathVrfIdMap(clonedB, m) } w := &watchEventBestPath{PathList: clonedB, MultiPathList: clonedM} @@ -1257,13 +1257,13 @@ func (s *BgpServer) propagateUpdate(peer *peer, pathList []*table.Path) { } } -func dstsToPaths(id string, as uint32, dsts []*table.Update) ([]*table.Path, []*table.Path, [][]*table.Path) { +func dstsToPaths(id string, as uint32, dsts []*table.Update, useMultiplePaths *oc.UseMultiplePathsConfig) ([]*table.Path, []*table.Path, [][]*table.Path) { bestList := make([]*table.Path, 0, len(dsts)) oldList := make([]*table.Path, 0, len(dsts)) mpathList := make([][]*table.Path, 0, len(dsts)) for _, dst := range dsts { - best, old, mpath := dst.GetChanges(id, as, false) + best, old, mpath := dst.GetChanges(id, as, false, useMultiplePaths) bestList = append(bestList, best) oldList = append(oldList, old) if mpath != nil { @@ -1274,13 +1274,13 @@ func dstsToPaths(id string, as uint32, dsts []*table.Update) ([]*table.Path, []* } func (s *BgpServer) propagateUpdateToNeighbors(source *peer, newPath *table.Path, dsts []*table.Update, needOld bool) { - if table.SelectionOptions.DisableBestPathSelection { + if s.bgpConfig.Global.RouteSelectionOptions.Config.DisableBestPathSelection { return } var gBestList, gOldList, bestList, oldList []*table.Path var mpathList [][]*table.Path if source == nil || !source.isRouteServerClient() { - gBestList, gOldList, mpathList = dstsToPaths(table.GLOBAL_RIB_NAME, 0, dsts) + gBestList, gOldList, mpathList = dstsToPaths(table.GLOBAL_RIB_NAME, 0, dsts, &s.bgpConfig.Global.UseMultiplePaths.Config) s.notifyBestWatcher(gBestList, mpathList) } family := newPath.GetRouteFamily() @@ -1336,7 +1336,7 @@ func (s *BgpServer) propagateUpdateToNeighbors(source *peer, newPath *table.Path } continue } - bestList, oldList, _ = dstsToPaths(targetPeer.TableID(), targetPeer.AS(), dsts) + bestList, oldList, _ = dstsToPaths(targetPeer.TableID(), targetPeer.AS(), dsts, &s.bgpConfig.Global.UseMultiplePaths.Config) } else { bestList = gBestList oldList = gOldList @@ -2255,16 +2255,14 @@ func (s *BgpServer) StartBgp(ctx context.Context, r *api.StartBgpRequest) error } rfs, _ := oc.AfiSafis(c.AfiSafis).ToRfList() - s.globalRib = table.NewTableManager(s.logger, rfs) - s.rsRib = table.NewTableManager(s.logger, rfs) + s.globalRib = table.NewTableManager(s.logger, rfs, &c.RouteSelectionOptions.Config, &c.UseMultiplePaths.Config) + s.rsRib = table.NewTableManager(s.logger, rfs, &c.RouteSelectionOptions.Config, &c.UseMultiplePaths.Config) if err := s.policy.Initialize(); err != nil { return err } s.bgpConfig.Global = *c - // update route selection options - table.SelectionOptions = c.RouteSelectionOptions.Config - table.UseMultiplePaths = c.UseMultiplePaths.Config + return nil }, false) } @@ -2679,7 +2677,7 @@ func (s *BgpServer) ListPath(ctx context.Context, r *api.ListPathRequest, fn fun knownPathList := dst.GetAllKnownPathList() for i, path := range knownPathList { p := toPathApi(path, getValidation(v, path), r.EnableOnlyBinary, r.EnableNlriBinary, r.EnableAttributeBinary) - if !table.SelectionOptions.DisableBestPathSelection { + if !s.bgpConfig.Global.RouteSelectionOptions.Config.DisableBestPathSelection { if i == 0 { switch r.TableType { case api.TableType_LOCAL, api.TableType_GLOBAL: diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 9fe43405b..18162821b 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -734,12 +734,12 @@ func newPeerandInfo(myAs, as uint32, address string, rib *table.TableManager) (* return p, &table.PeerInfo{AS: as, Address: net.ParseIP(address), ID: net.ParseIP(address)} } -func process(rib *table.TableManager, l []*table.Path) (*table.Path, *table.Path) { +func process(rib *table.TableManager, l []*table.Path, useMultiplePaths *oc.UseMultiplePathsConfig) (*table.Path, *table.Path) { dsts := make([]*table.Update, 0) for _, path := range l { dsts = append(dsts, rib.Update(path)...) } - news, olds, _ := dstsToPaths(table.GLOBAL_RIB_NAME, 0, dsts) + news, olds, _ := dstsToPaths(table.GLOBAL_RIB_NAME, 0, dsts, useMultiplePaths) if len(news) != 1 { panic("can't handle multiple paths") } @@ -751,7 +751,7 @@ func TestFilterpathWitheBGP(t *testing.T) { as := uint32(65000) p1As := uint32(65001) p2As := uint32(65002) - rib := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + rib := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) p1, pi1 := newPeerandInfo(as, p1As, "192.168.0.1", rib) p2, pi2 := newPeerandInfo(as, p2As, "192.168.0.2", rib) @@ -763,12 +763,12 @@ func TestFilterpathWitheBGP(t *testing.T) { path2 := table.NewPath(pi2, nlri, false, pa2, time.Now(), false) rib.Update(path2) d := rib.Update(path1) - new, old, _ := d[0].GetChanges(table.GLOBAL_RIB_NAME, 0, false) + new, old, _ := d[0].GetChanges(table.GLOBAL_RIB_NAME, 0, false, &oc.UseMultiplePathsConfig{}) assert.Equal(t, new, path1) filterpath(p1, new, old) filterpath(p2, new, old) - new, old = process(rib, []*table.Path{path1.Clone(true)}) + new, old = process(rib, []*table.Path{path1.Clone(true)}, &oc.UseMultiplePathsConfig{}) assert.Equal(t, new, path2) // p1 and p2 advertized the same prefix and p1's was best. Then p1 withdraw it, so p2 must get withdawal. path := filterpath(p2, new, old) @@ -778,7 +778,7 @@ func TestFilterpathWitheBGP(t *testing.T) { // p1 should get the new best (from p2) assert.Equal(t, filterpath(p1, new, old), path2) - new, old = process(rib, []*table.Path{path2.Clone(true)}) + new, old = process(rib, []*table.Path{path2.Clone(true)}, &oc.UseMultiplePathsConfig{}) assert.True(t, new.IsWithdraw) // p2 withdraw so p1 should get withdrawal. path = filterpath(p1, new, old) @@ -792,7 +792,7 @@ func TestFilterpathWitheBGP(t *testing.T) { func TestFilterpathWithiBGP(t *testing.T) { as := uint32(65000) - rib := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + rib := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) p1, pi1 := newPeerandInfo(as, as, "192.168.0.1", rib) //p2, pi2 := newPeerandInfo(as, as, "192.168.0.2", rib) p2, _ := newPeerandInfo(as, as, "192.168.0.2", rib) @@ -804,14 +804,14 @@ func TestFilterpathWithiBGP(t *testing.T) { path1 := table.NewPath(pi1, nlri, false, pa1, time.Now(), false) //path2 := table.NewPath(pi2, nlri, false, pa2, time.Now(), false) - new, old := process(rib, []*table.Path{path1}) + new, old := process(rib, []*table.Path{path1}, &oc.UseMultiplePathsConfig{}) assert.Equal(t, new, path1) path := filterpath(p1, new, old) assert.Nil(t, path) path = filterpath(p2, new, old) assert.Nil(t, path) - new, old = process(rib, []*table.Path{path1.Clone(true)}) + new, old = process(rib, []*table.Path{path1.Clone(true)}, &oc.UseMultiplePathsConfig{}) path = filterpath(p1, new, old) assert.Nil(t, path) path = filterpath(p2, new, old) @@ -820,9 +820,9 @@ func TestFilterpathWithiBGP(t *testing.T) { } func TestFilterpathWithRejectPolicy(t *testing.T) { - rib1 := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + rib1 := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) _, pi1 := newPeerandInfo(1, 2, "192.168.0.1", rib1) - rib2 := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}) + rib2 := table.NewTableManager(logger, []bgp.RouteFamily{bgp.RF_IPv4_UC}, &oc.RouteSelectionOptionsConfig{}, &oc.UseMultiplePathsConfig{}) p2, _ := newPeerandInfo(1, 3, "192.168.0.2", rib2) comSet1 := oc.CommunitySet{ @@ -865,7 +865,7 @@ func TestFilterpathWithRejectPolicy(t *testing.T) { pa1 = append(pa1, bgp.NewPathAttributeCommunities([]uint32{100<<16 | 100})) } path1 := table.NewPath(pi1, nlri, false, pa1, time.Now(), false) - new, old := process(rib2, []*table.Path{path1}) + new, old := process(rib2, []*table.Path{path1}, &oc.UseMultiplePathsConfig{}) assert.Equal(t, new, path1) s := NewBgpServer() path2 := s.filterpath(p2, new, old) @@ -913,18 +913,18 @@ func TestPeerGroup(test *testing.T) { }, }, } - configured := map[string]interface{}{ - "config": map[string]interface{}{ + ocured := map[string]interface{}{ + "oc": map[string]interface{}{ "neigbor-address": "127.0.0.1", "peer-group": "g", }, "transport": map[string]interface{}{ - "config": map[string]interface{}{ + "oc": map[string]interface{}{ "passive-mode": true, }, }, } - oc.RegisterConfiguredFields("127.0.0.1", configured) + oc.RegisterConfiguredFields("127.0.0.1", ocured) err = s.AddPeer(context.Background(), &api.AddPeerRequest{Peer: oc.NewPeerFromConfigStruct(n)}) assert.Nil(err) @@ -1211,7 +1211,7 @@ func peerServers(t *testing.T, ctx context.Context, servers []*BgpServer, famili }, } - // first server to get neighbor config is passive to hopefully make handshake faster + // first server to get neighbor oc is passive to hopefully make handshake faster if j > i { neighborConfig.Transport.Config.PassiveMode = true } diff --git a/pkg/server/zclient.go b/pkg/server/zclient.go index 92527ae2e..8d0f963e7 100644 --- a/pkg/server/zclient.go +++ b/pkg/server/zclient.go @@ -420,7 +420,7 @@ func (z *zebraClient) loop() { case ev := <-w.Event(): switch msg := ev.(type) { case *watchEventBestPath: - if table.UseMultiplePaths.Enabled { + if z.server.bgpConfig.Global.UseMultiplePaths.Config.Enabled { for _, paths := range msg.MultiPathList { z.updatePathByNexthopCache(paths) for i := range msg.Vrf {