summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--server/peer.go22
-rw-r--r--server/server.go21
-rw-r--r--test/lib/base.py16
-rw-r--r--test/pip-requires.txt4
-rw-r--r--test/scenario_test/bgp_router_test.py34
5 files changed, 72 insertions, 25 deletions
diff --git a/server/peer.go b/server/peer.go
index 030044d0..d27f96e6 100644
--- a/server/peer.go
+++ b/server/peer.go
@@ -128,7 +128,7 @@ func (peer *Peer) getAccepted(rfList []bgp.RouteFamily) []*table.Path {
return peer.adjRibIn.PathList(rfList, true)
}
-func (peer *Peer) filterpath(path *table.Path) *table.Path {
+func (peer *Peer) filterpath(path *table.Path, withdrawals []*table.Path) *table.Path {
// special handling for RTC nlri
// see comments in (*Destination).Calculate()
if path != nil && path.GetRouteFamily() == bgp.RF_RTC_UC && !path.IsWithdraw {
@@ -147,7 +147,7 @@ func (peer *Peer) filterpath(path *table.Path) *table.Path {
}
}
}
- if filterpath(peer, path) == nil {
+ if path = filterpath(peer, path, withdrawals); path == nil {
return nil
}
@@ -172,7 +172,7 @@ func (peer *Peer) getBestFromLocal(rfList []bgp.RouteFamily) ([]*table.Path, []*
pathList := []*table.Path{}
filtered := []*table.Path{}
for _, path := range peer.localRib.GetBestPathList(peer.TableID(), rfList) {
- if p := peer.filterpath(path); p != nil {
+ if p := peer.filterpath(path, nil); p != nil {
pathList = append(pathList, p)
} else {
filtered = append(filtered, path)
@@ -200,23 +200,9 @@ func (peer *Peer) processOutgoingPaths(paths, withdrawals []*table.Path) []*tabl
}
outgoing := make([]*table.Path, 0, len(paths))
- // Note: multiple paths having the same prefix could exist the
- // withdrawals list in the case of Route Server setup with
- // import policies modifying paths. In such case, gobgp sends
- // duplicated update messages; withdraw messages for the same
- // prefix.
- // However, currently we don't support local path for Route
- // Server setup so this is NOT the case.
- for _, path := range withdrawals {
- if path.IsLocal() {
- if _, ok := peer.fsm.rfMap[path.GetRouteFamily()]; ok {
- outgoing = append(outgoing, path)
- }
- }
- }
for _, path := range paths {
- if p := peer.filterpath(path); p != nil {
+ if p := peer.filterpath(path, withdrawals); p != nil {
outgoing = append(outgoing, p)
}
}
diff --git a/server/server.go b/server/server.go
index 1954d124..c2386f94 100644
--- a/server/server.go
+++ b/server/server.go
@@ -267,7 +267,7 @@ func isASLoop(peer *Peer, path *table.Path) bool {
return false
}
-func filterpath(peer *Peer, path *table.Path) *table.Path {
+func filterpath(peer *Peer, path *table.Path, withdrawals []*table.Path) *table.Path {
if path == nil {
return nil
}
@@ -349,6 +349,25 @@ func filterpath(peer *Peer, path *table.Path) *table.Path {
}
if peer.ID() == path.GetSource().Address.String() {
+ // Say, gobgp was advertising prefix A and peer P also.
+ // When gobgp withdraws prefix A, best path calculation chooses
+ // the path from P as the best path for prefix A.
+ // For peers other than P, this path should be advertised
+ // (as implicit withdrawal). However for P, we should advertise
+ // the local withdraw path.
+
+ // Note: multiple paths having the same prefix could exist the
+ // withdrawals list in the case of Route Server setup with
+ // import policies modifying paths. In such case, gobgp sends
+ // duplicated update messages; withdraw messages for the same
+ // prefix.
+ // However, currently we don't support local path for Route
+ // Server setup so this is NOT the case.
+ for _, w := range withdrawals {
+ if w.IsLocal() && path.GetNlri().String() == w.GetNlri().String() {
+ return w
+ }
+ }
log.WithFields(log.Fields{
"Topic": "Peer",
"Key": peer.ID(),
diff --git a/test/lib/base.py b/test/lib/base.py
index 4f2e8a69..38cac97a 100644
--- a/test/lib/base.py
+++ b/test/lib/base.py
@@ -158,6 +158,7 @@ class Container(object):
self.ip6_addrs = []
self.is_running = False
self.eths = []
+ self.tcpdump_running = False
if self.docker_name() in get_containers():
self.remove()
@@ -230,12 +231,21 @@ class Container(object):
return int(local(cmd, capture=True))
return -1
- def start_tcpdump(self, interface=None, filename=None):
+ def start_tcpdump(self, interface=None, filename=None, expr='tcp port 179'):
+ if self.tcpdump_running:
+ raise Exception('tcpdump already running')
+ self.tcpdump_running = True
if not interface:
interface = "eth0"
if not filename:
- filename = "{0}/{1}.dump".format(self.shared_volumes[0][1], interface)
- self.local("tcpdump -i {0} -w {1}".format(interface, filename), detach=True)
+ filename = '{0}.dump'.format(interface)
+ self.local("tcpdump -i {0} -w {1}/{2} {3}".format(interface, self.shared_volumes[0][1], filename, expr), detach=True)
+ return '{0}/{1}'.format(self.shared_volumes[0][0], filename)
+
+ def stop_tcpdump(self):
+ self.local("pkill tcpdump")
+ self.tcpdump_running = False
+
class BGPContainer(Container):
diff --git a/test/pip-requires.txt b/test/pip-requires.txt
index 708597d8..e7e580ab 100644
--- a/test/pip-requires.txt
+++ b/test/pip-requires.txt
@@ -1,10 +1,8 @@
nose
toml
pyyaml
-ciscoconfparse
-ecdsa
-pycrypto>=2.1
fabric
netaddr
nsenter
docker-py
+ryu
diff --git a/test/scenario_test/bgp_router_test.py b/test/scenario_test/bgp_router_test.py
index a528c36d..4f7e981d 100644
--- a/test/scenario_test/bgp_router_test.py
+++ b/test/scenario_test/bgp_router_test.py
@@ -25,6 +25,9 @@ import time
import nose
from noseplugin import OptionParser, parser_option
from itertools import chain
+import ryu.lib.pcaplib as pcap
+from ryu.lib.packet.packet import Packet
+from ryu.lib.packet.bgp import BGPMessage
class GoBGPTestBase(unittest.TestCase):
@@ -354,6 +357,37 @@ class GoBGPTestBase(unittest.TestCase):
g1.wait_for(expected_state=BGP_FSM_ESTABLISHED, peer=e1)
+ def test_20_check_withdrawal_2(self):
+ g1 = self.gobgp
+ g2 = self.quaggas['g2']
+
+ dumpfile = g2.start_tcpdump()
+
+ g1.add_route('10.40.0.0/24')
+
+ time.sleep(1)
+
+ paths = g2.get_global_rib('10.40.0.0/24')
+ self.assertTrue(len(paths) == 1)
+
+ g1.local('gobgp global rib del 10.40.0.0/24')
+
+ time.sleep(1)
+
+ paths = g2.get_global_rib('10.40.0.0/24')
+ self.assertTrue(len(paths) == 0)
+
+ g2.stop_tcpdump()
+ time.sleep(1)
+
+ cnt = 0
+ for pkt in pcap.Reader(open(dumpfile)):
+ last = Packet(pkt[1]).protocols[-1]
+ if type(last) == str:
+ pkt = BGPMessage.parser(last)[0]
+ cnt += len(pkt.withdrawn_routes)
+
+ self.assertTrue(cnt == 1)
if __name__ == '__main__':
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-15 22:52:08 +0000