r38997 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r38996‎ | r38997 | r38998 >
Date:13:39, 9 August 2008
Author:mark
Status:old
Tags:
Comment:
- Fix equality checking of IPPrefix with another type
- Support flat AS paths as input to ASPathAttribute
- Add NextHopAttribute.ANY as input to NextHopAttribute, which can be replaced by e.g. the local IP address on sending
- Fix missing argument for keepAliveTimer.reset in updateSent
- Do not require manually setting the local BGP id, calculate it from the local IP address on connection establishment instead
- Add optional myASN and peerAddr parameters to Factory class constructors
- Replace NextHopAttribute.ANY attributes by the local IP address in NaiveBGPPeering on sending
Modified paths:
  • /trunk/routing/twistedbgp/src/bgp.py (modified) (history)
  • /trunk/routing/twistedbgp/src/test.py (modified) (history)

Diff [purge]

Index: trunk/routing/twistedbgp/src/bgp.py
——@@ -184,7 +184,7 @@
185185
186186 def __eq__(self, other):
187187 # FIXME: masked ips
188 - return self.prefixlen == other.prefixlen and self.prefix == other.prefix
 188+ return isinstance(other, IPPrefix) and self.prefixlen == other.prefixlen and self.prefix == other.prefix
189189
190190 def __ne__(self, other):
191191 return not self.__eq__(other)
——@@ -363,6 +363,10 @@
364364 super(ASPathAttribute, self).__init__(None)
365365 self.optional = False
366366 self.transitive = True
 367+
 368+ if value and type(value) is list and reduce(lambda r,n: r and type(n) is int, value, True):
 369+ # Flat sequential path of ASNs
 370+ value = [(2, value)]
367371 self.value = value or [(2, [])] # One segment with one AS path sequence
368372
369373 def fromTuple(self, attrTuple):
——@@ -393,8 +397,12 @@
394398 class NextHopAttribute(Attribute):
395399 name = 'Next Hop'
396400 typeCode = ATTR_TYPE_NEXT_HOP
 401+
 402+ ANY = None
397403
398404 def __init__(self, value=None):
 405+ self.any = False
 406+
399407 if type(value) is tuple:
400408 super(NextHopAttribute, self).__init__(value)
401409 self.fromTuple(value)
——@@ -402,10 +410,7 @@
403411 super(NextHopAttribute, self).__init__(None)
404412 self.optional = False
405413 self.transitive = True
406 - if value:
407 - self.value = IPv4IP(value)
408 - else:
409 - self.value = IPv4IP('0.0.0.0')
 414+ self.set(value)
410415
411416 def fromTuple(self, attrTuple):
412417 value = attrTuple[2]
——@@ -413,14 +418,21 @@
414419 if self.optional or not self.transitive:
415420 raise AttributeException(ERR_MSG_UPDATE_ATTR_FLAGS, attrTuple)
416421 if len(value) != 4:
417 - raise AttributeException(ERR_MSG_UPDATE_ATTR_LEN, attrTuple)
418 - if value in (0, 2**32-1):
419 - raise AttributeException(ERR_MSG_UPDATE_INVALID_NEXTHOP, attrTuple)
 422+ raise AttributeException(ERR_MSG_UPDATE_ATTR_LEN, attrTuple)
420423
421 - self.value = IPv4IP(value)
 424+ self.set(value)
422425
423426 def encode(self):
424427 return struct.pack('!BBB', self.flags(), self.typeCode, len(self.value.packed())) + self.value.packed()
 428+
 429+ def set(self, value):
 430+ if value:
 431+ if value in (0, 2**32-1):
 432+ raise AttributeException(ERR_MSG_UPDATE_INVALID_NEXTHOP, attrTuple)
 433+ self.value = IPv4IP(value)
 434+ else:
 435+ self.value = IPv4IP('0.0.0.0')
 436+ self.any = True
425437
426438 class MEDAttribute(Attribute):
427439 name = 'MED'
——@@ -1042,7 +1054,7 @@
10431055 """Called by the protocol instance when it just sent an Update message."""
10441056
10451057 if self.holdTime > 0:
1046 - self.keepAliveTimer.reset()
 1058+ self.keepAliveTimer.reset(self.keepAliveTime)
10471059
10481060 def _errorClose(self):
10491061 """Internal method that closes a connection and returns the state
——@@ -1094,6 +1106,10 @@
10951107 # DEBUG
10961108 print "Connection established"
10971109
 1110+ # Set the local BGP id from the local IP address if it's not set
 1111+ if self.factory.bgpId is None:
 1112+ self.factory.bgpId = IPv4IP(self.transport.getHost().host).ipToInt() # FIXME: IPv6
 1113+
10981114 try:
10991115 self.fsm.connectionMade()
11001116 except NotificationSent, e:
——@@ -1520,7 +1536,7 @@
15211537 def buildProtocol(self, addr):
15221538 """Builds a BGPProtocol instance"""
15231539
1524 - assert self.myASN is not None and self.bgpId is not None
 1540+ assert self.myASN is not None
15251541
15261542 return protocol.Factory.buildProtocol(self, addr)
15271543
——@@ -1538,8 +1554,9 @@
15391555 (factory) instance.
15401556 """
15411557
1542 - def __init__(self, peers):
 1558+ def __init__(self, peers, myASN=None):
15431559 self.peers = peers
 1560+ self.myASN = myASN
15441561
15451562 def buildProtocol(self, addr):
15461563 """Builds a BGPProtocol instance by finding an appropriate
——@@ -1563,8 +1580,9 @@
15641581
15651582 implements(IBGPPeering, interfaces.IPushProducer)
15661583
1567 - def __init__(self):
1568 - self.peerAddr = None
 1584+ def __init__(self, myASN=None, peerAddr=None):
 1585+ self.myASN = myASN
 1586+ self.peerAddr = peerAddr
15691587 self.peerId = None
15701588 self.fsm = BGPFactory.FSM(self)
15711589 self.inConnections = []
——@@ -1847,8 +1865,8 @@
18481866 Currently even assumes that all prefixes fit in a single BGP UPDATE message.
18491867 """
18501868
1851 - def __init__(self):
1852 - BGPPeering.__init__(self)
 1869+ def __init__(self, myASN=None, peerAddr=None):
 1870+ BGPPeering.__init__(self, myASN, peerAddr)
18531871
18541872 # (advertisements, attributes)
18551873 self.advertisedSet = (set(), set())
——@@ -1885,6 +1903,10 @@
18861904 self._sendUpdate(withdrawals, attributes, newPrefixes)
18871905
18881906 def _sendUpdate(self, withdrawals, attributes, advertisements):
1889 - if self.estabProtocol and self.fsm.state == ST_ESTABLISHED:
 1907+ if self.estabProtocol and self.fsm.state == ST_ESTABLISHED and len(withdrawals) + len(advertisements) > 0:
 1908+ # Check if the NextHop attribute needs to be replaced
 1909+ if attributes.nextHop.any:
 1910+ attributes.nextHop.set(self.estabProtocol.transport.getHost().host) # FIXME: IPv6
 1911+
18901912 self.estabProtocol.sendUpdate(withdrawals, attributes, advertisements)
18911913 self.advertisedSet = (advertisements, attributes)
\ No newline at end of file
Index: trunk/routing/twistedbgp/src/test.py
——@@ -62,37 +62,22 @@
6363 except: pass
6464
6565
66 -#bgpprot = bgp.BGP(myASN=14907, bgpId=1)
67 -
68 -#print [ord(i) for i in bgpprot.constructOpen()]
69 -
7066 peers = {}
7167
72 -peering = bgp.NaiveBGPPeering()
73 -peering.peerAddr = '91.198.174.247'
74 -
 68+peering = bgp.NaiveBGPPeering(myASN=64600, peerAddr='91.198.174.247')
7569 advertisements = [bgp.IPPrefix('127.127.127.127/32')]
7670 attrSet = bgp.AttributeSet([bgp.OriginAttribute(), bgp.ASPathAttribute([(2, [64600])]), bgp.NextHopAttribute('192.168.255.254')])
7771 peering.setAdvertisements(set(advertisements), attrSet)
78 -
7972 peers[peering.peerAddr] = peering
8073
81 -peering2 = bgp.BGPPeering()
82 -peering2.peerAddr = '127.0.0.1'
83 -
 74+#peering2 = bgp.BGPPeering(myASN=64600, peerAddr='127.0.0.1')
8475 #peers[peering2.peerAddr] = peering2
8576
86 -peering3 = bgp.BGPPeering()
87 -peering3.peerAddr = '192.168.37.1'
88 -
89 -
 77+#peering3 = bgp.BGPPeering(myASN=64600, peerAddr='192.168.37.1')
9078 #peers[peering3.peerAddr] = peering3
9179
9280 for peer in peers.values():
93 - peer.myASN = 64600
94 - peer.bgpId = 111
9581 peer.registerConsumer(BGPDebug())
96 -
9782
9883 bgpServer = bgp.BGPServerFactory(peers)
9984 reactor.listenTCP(1000+bgp.PORT, bgpServer)

Status & tagging log