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 <fujita.tomonori@lab.ntt.co.jp>

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):