From 8907f5e426652e419ff08f8e6346e3448f876fd0 Mon Sep 17 00:00:00 2001 From: FUJITA Tomonori Date: Sun, 29 Jun 2014 20:55:42 +0900 Subject: bgp: clean up getpeername() and getsockname() usage 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 --- 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 d6989a34..28ae2864 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 b64f92ac..fa9ee184 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 c7703657..853473ec 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 994993bf..f276b833 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): -- cgit v1.2.3