The current code calls getpeername() and getsockname() at lots of places. The formats of the return values of getpeername() and getsockname() are different between ipv4 and v6. So the code became messy.
Calling getpeername() and getsockname() at one place and caching the results is ideal. But that needs some refactoring. This patch is kinda halfway. Signed-off-by: FUJITA Tomonori <[email protected]> --- ryu/services/protocols/bgp/base.py | 24 ++++++++++++++++++------ ryu/services/protocols/bgp/core.py | 10 +++------- ryu/services/protocols/bgp/peer.py | 6 ++---- ryu/services/protocols/bgp/speaker.py | 34 ++++++++++++---------------------- 4 files changed, 35 insertions(+), 39 deletions(-) diff --git a/ryu/services/protocols/bgp/base.py b/ryu/services/protocols/bgp/base.py index d6989a3..28ae286 100644 --- a/ryu/services/protocols/bgp/base.py +++ b/ryu/services/protocols/bgp/base.py @@ -314,6 +314,20 @@ class Activity(object): self._timers = weakref.WeakValueDictionary() LOG.debug('Stopping activity %s finished.' % self.name) + def _canonicalize_ip(self, ip): + addr = netaddr.IPAddress(ip) + if addr.is_ipv4_mapped(): + ip = str(addr.ipv4()) + return ip + + def get_remotename(self, sock): + addr, port = sock.getpeername()[:2] + return (self._canonicalize_ip(addr), str(port)) + + def get_localname(self, sock): + addr, port = sock.getsockname()[:2] + return (self._canonicalize_ip(addr), str(port)) + def _listen_tcp(self, loc_addr, conn_handle): """Creates a TCP server socket which listens on `port` number. @@ -330,12 +344,9 @@ class Activity(object): # We now wait for connection requests from client. while True: sock, client_address = server.accept() - client_address, port = client_address[:2] + client_address, port = self.get_remotename(sock) LOG.debug('Connect request received from client for port' ' %s:%s' % (client_address, port)) - if 'ffff:' in client_address: - client_address = str(netaddr.IPAddress(client_address).ipv4()) - client_name = self.name + '_client@' + client_address self._asso_socket_map[client_name] = sock self._spawn(client_name, conn_handle, sock) @@ -359,8 +370,9 @@ class Activity(object): if sock: # Connection name for pro-active connection is made up # of local end address + remote end address - conn_name = ('L: ' + str(sock.getsockname()) + ', R: ' + - str(sock.getpeername())) + local = self.get_localname(sock)[0] + remote = self.get_remotename(sock)[0] + conn_name = ('L: ' + local + ', R: ' + remote) self._asso_socket_map[conn_name] = sock # If connection is established, we call connection handler # in a new thread. diff --git a/ryu/services/protocols/bgp/core.py b/ryu/services/protocols/bgp/core.py index b64f92a..fa9ee18 100644 --- a/ryu/services/protocols/bgp/core.py +++ b/ryu/services/protocols/bgp/core.py @@ -381,7 +381,7 @@ class CoreService(Factory, Activity): def build_protocol(self, socket): assert socket # Check if its a reactive connection or pro-active connection - _, remote_port = socket.getpeername()[:2] + _, remote_port = self.get_remotename(socket) is_reactive_conn = True if remote_port == STD_BGP_SERVER_PORT_NUM: is_reactive_conn = False @@ -400,9 +400,7 @@ class CoreService(Factory, Activity): protocol. """ assert socket - peer_addr, peer_port = socket.getpeername()[:2] - if 'ffff:' in peer_addr: - peer_addr = str(netaddr.IPAddress(peer_addr).ipv4()) + peer_addr, peer_port = self.get_remotename(socket) peer = self._peer_manager.get_by_addr(peer_addr) bgp_proto = self.build_protocol(socket) @@ -427,9 +425,7 @@ class CoreService(Factory, Activity): subcode = BGP_ERROR_SUB_CONNECTION_COLLISION_RESOLUTION bgp_proto.send_notification(code, subcode) else: - bind_ip, bind_port = socket.getsockname()[:2] - if 'ffff:'in bind_ip: - bind_ip = str(netaddr.IPAddress(bind_ip).ipv4()) + bind_ip, bind_port = self.get_localname(socket) peer._host_bind_ip = bind_ip peer._host_bind_port = bind_port self._spawn_activity(bgp_proto, peer) diff --git a/ryu/services/protocols/bgp/peer.py b/ryu/services/protocols/bgp/peer.py index c770365..853473e 100644 --- a/ryu/services/protocols/bgp/peer.py +++ b/ryu/services/protocols/bgp/peer.py @@ -839,10 +839,8 @@ class Peer(Source, Sink, NeighborConfListener, Activity): self._protocol = proto # Update state attributes - self.state.peer_ip, self.state.peer_port = \ - self._protocol.get_peername()[:2] - self.state.local_ip, self.state.local_port = \ - self._protocol.get_sockname()[:2] + self.state.peer_ip, self.state.peer_port = self._protocol._remotename + self.state.local_ip, self.state.local_port = self._protocol._localname # self.state.bgp_state = self._protocol.state # Stop connect_loop retry timer as we are now connected if self._protocol and self._connect_retry_event.is_set(): diff --git a/ryu/services/protocols/bgp/speaker.py b/ryu/services/protocols/bgp/speaker.py index 994993b..f276b83 100644 --- a/ryu/services/protocols/bgp/speaker.py +++ b/ryu/services/protocols/bgp/speaker.py @@ -96,9 +96,11 @@ class BgpProtocol(Protocol, Activity): # Validate input. if socket is None: raise ValueError('Invalid arguments passed.') - activity_name = ('BgpProtocol %s, %s, %s' % ( - is_reactive_conn, socket.getpeername(), socket.getsockname()) - ) + self._remotename = self.get_remotename(socket) + self._localname = self.get_localname(socket) + activity_name = ('BgpProtocol %s, %s, %s' % (is_reactive_conn, + self._remotename, + self._localname)) Activity.__init__(self, name=activity_name) # Intialize instance variables. self._peer = None @@ -122,12 +124,6 @@ class BgpProtocol(Protocol, Activity): self.recv_open_msg = None self._is_bound = False - def get_peername(self): - return self._socket.getpeername() - - def get_sockname(self): - return self._socket.getsockname() - @property def is_reactive(self): return self._is_reactive @@ -146,8 +142,8 @@ class BgpProtocol(Protocol, Activity): '`BgpProtocol`') # Compare protocol connection end point's addresses - if (self.get_peername()[0] == other_protocol.get_peername()[0] and - self.get_sockname()[0] == other_protocol.get_sockname()[0]): + if (self._remotename[0] == other_protoco._remotename[0] and + self._localname[0] == other_protocol._localname[0]): return True return False @@ -366,10 +362,8 @@ class BgpProtocol(Protocol, Activity): reason = notification.reason self._send_with_lock(notification) self._signal_bus.bgp_error(self._peer, code, subcode, reason) - LOG.error( - 'Sent notification to %r >> %s' % - (self._socket.getpeername(), notification) - ) + LOG.error('Sent notification to %r >> %s' % (self._localname(), + notification)) self._socket.close() def _send_with_lock(self, msg): @@ -386,18 +380,15 @@ class BgpProtocol(Protocol, Activity): raise BgpProtocolException('Tried to send message to peer when ' 'this protocol instance is not started' ' or is no longer is started state.') - # get peername before senging msg because sending msg can occur - # conncetion lost - peername = self.get_peername() self._send_with_lock(msg) if msg.type == BGP_MSG_NOTIFICATION: LOG.error('Sent notification to %s >> %s' % - (peername, msg)) + (self._remotename, msg)) self._signal_bus.bgp_notification_sent(self._peer, msg) else: - LOG.debug('Sent msg to %s >> %s' % (peername, msg)) + LOG.debug('Sent msg to %s >> %s' % (self._remotename, msg)) def stop(self): Activity.stop(self) @@ -443,8 +434,7 @@ class BgpProtocol(Protocol, Activity): message except for *Open* and *Notification* message. On receiving *Notification* message we close connection with peer. """ - LOG.debug('Received msg from %s << %s' % (str(self.get_peername()[0]), - msg)) + LOG.debug('Received msg from %s << %s' % (self._remotename, msg)) # If we receive open message we try to bind to protocol if (msg.type == BGP_MSG_OPEN): -- 1.8.5.2 (Apple Git-48) ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft _______________________________________________ Ryu-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ryu-devel
