From c8cb20e75bb830f1a9fe1037356a4007faaba04d Mon Sep 17 00:00:00 2001 From: ISHIDA Wataru Date: Tue, 18 Apr 2017 06:29:42 +0000 Subject: [PATCH] policy: send a withdrawal if export policy blocks an existing route This patch fixes a bug of export policy and implicit withdrawal. When a path is filtered by export policy, we need to check whether the old path (implicit withdrawn path) was sent before the new path. If it has been sent, we need to send a withdrawal message. Signed-off-by: ISHIDA Wataru --- config/default.go | 2 ++ server/peer.go | 8 ++++++ server/server.go | 4 +-- server/server_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/config/default.go b/config/default.go index 094afb2b..405c17ad 100644 --- a/config/default.go +++ b/config/default.go @@ -101,8 +101,10 @@ func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, asn ui if n.Config.PeerAs != n.Config.LocalAs { n.Config.PeerType = PEER_TYPE_EXTERNAL + n.State.PeerType = PEER_TYPE_EXTERNAL } else { n.Config.PeerType = PEER_TYPE_INTERNAL + n.State.PeerType = PEER_TYPE_INTERNAL } n.State.PeerAs = n.Config.PeerAs diff --git a/server/peer.go b/server/peer.go index 15029bd6..ed4d1ae4 100644 --- a/server/peer.go +++ b/server/peer.go @@ -278,6 +278,14 @@ func (peer *Peer) filterpath(path, old *table.Path) *table.Path { Info: peer.fsm.peerInfo, } path = peer.policy.ApplyPolicy(peer.TableID(), table.POLICY_DIRECTION_EXPORT, path, options) + // When 'path' is filetered (path == nil), check 'old' has been sent to this peer. + // If it has, send withdrawal to the peer. + if path == nil && old != nil { + o := peer.policy.ApplyPolicy(peer.TableID(), table.POLICY_DIRECTION_EXPORT, old, options) + if o != nil { + path = old.Clone(true) + } + } // draft-uttaro-idr-bgp-persistence-02 // 4.3. Processing LLGR_STALE Routes diff --git a/server/server.go b/server/server.go index 5238c33b..da6feba2 100644 --- a/server/server.go +++ b/server/server.go @@ -367,7 +367,7 @@ func filterpath(peer *Peer, path, old *table.Path) *table.Path { } if ignore { - if path.IsWithdraw == false && old != nil { + if !path.IsWithdraw && old != nil && old.GetSource().Address.String() != peer.ID() { // we advertise a route from ebgp, // which is the old best. We got the // new best from ibgp. We don't @@ -400,7 +400,7 @@ func filterpath(peer *Peer, path, old *table.Path) *table.Path { // the withdrawal path. // Thing is same when peer A and we advertized prefix P (as local // route), then, we withdraws the prefix. - if path.IsWithdraw == false && old != nil { + if !path.IsWithdraw && old != nil && old.GetSource().Address.String() != peer.ID() { return old.Clone(true) } } diff --git a/server/server_test.go b/server/server_test.go index 8f5fcc4d..3aaa7374 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -187,11 +187,13 @@ func TestNumGoroutineWithAddDeleteNeighbor(t *testing.T) { func newPeerandInfo(myAs, as uint32, address string, rib *table.TableManager) (*Peer, *table.PeerInfo) { nConf := &config.Neighbor{Config: config.NeighborConfig{PeerAs: as, NeighborAddress: address}} config.SetDefaultNeighborConfigValues(nConf, myAs) + policy := table.NewRoutingPolicy() + policy.Reset(&config.RoutingPolicy{}, nil) p := NewPeer( &config.Global{Config: config.GlobalConfig{As: myAs}}, nConf, rib, - &table.RoutingPolicy{}) + policy) for _, f := range rib.GetRFlist() { p.fsm.rfMap[f] = true } @@ -283,3 +285,61 @@ func TestFilterpathWithiBGP(t *testing.T) { assert.Nil(t, path) } + +func TestFilterpathWithRejectPolicy(t *testing.T) { + rib1 := table.NewTableManager([]bgp.RouteFamily{bgp.RF_IPv4_UC}) + _, pi1 := newPeerandInfo(1, 2, "192.168.0.1", rib1) + rib2 := table.NewTableManager([]bgp.RouteFamily{bgp.RF_IPv4_UC}) + p2, _ := newPeerandInfo(1, 3, "192.168.0.2", rib2) + + comSet1 := config.CommunitySet{ + CommunitySetName: "comset1", + CommunityList: []string{"100:100"}, + } + s, _ := table.NewCommunitySet(comSet1) + p2.policy.AddDefinedSet(s) + + statement := config.Statement{ + Name: "stmt1", + Conditions: config.Conditions{ + BgpConditions: config.BgpConditions{ + MatchCommunitySet: config.MatchCommunitySet{ + CommunitySet: "comset1", + }, + }, + }, + Actions: config.Actions{ + RouteDisposition: config.ROUTE_DISPOSITION_REJECT_ROUTE, + }, + } + policy := config.PolicyDefinition{ + Name: "policy1", + Statements: []config.Statement{statement}, + } + p, _ := table.NewPolicy(policy) + p2.policy.AddPolicy(p, false) + policies := []*config.PolicyDefinition{ + &config.PolicyDefinition{ + Name: "policy1", + }, + } + p2.policy.AddPolicyAssignment(p2.TableID(), table.POLICY_DIRECTION_EXPORT, policies, table.ROUTE_TYPE_ACCEPT) + + for _, addCommunity := range []bool{false, true, false, true} { + nlri := bgp.NewIPAddrPrefix(24, "10.10.10.0") + pa1 := []bgp.PathAttributeInterface{bgp.NewPathAttributeAsPath([]bgp.AsPathParamInterface{bgp.NewAs4PathParam(2, []uint32{1})}), bgp.NewPathAttributeLocalPref(200)} + if addCommunity { + 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}) + assert.Equal(t, new, path1) + path2 := p2.filterpath(new, old) + if addCommunity { + assert.True(t, path2.IsWithdraw) + } else { + assert.False(t, path2.IsWithdraw) + } + } + +}