rx: Remove surplus call to FindPeer

When stats are enabled, rxi_ReadPacket calls FindPeer immediately
the packet is received from the wire. The peer structure that it
gets is used solely to increment a counter, and then thrown away.
Given that FindPeer requires a lock, and a hash lookup, this is
really inefficent.

Instead, delay the compilation of statistics until rxi_ReceivePacket.
Call FindPeer for version and debug packets which have no associated
connection otherwise wait until we have found the packet's connection,
and use the peer which is linked from there.

Change-Id: Ic2eb08e52b97d6b033b9d3de59da9346e012d70d
Reviewed-on: http://gerrit.openafs.org/7206
Reviewed-by: Simon Wilkinson <simonxwilkinson@gmail.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/rx/rx.c b/src/rx/rx.c
index 1bf61e63faca6867170e24ef14061fabaa985e8b..ca563f747748ade0d0789c0068521a018a2bb7dc 100644 (file)
--- a/src/rx/rx.c
+++ b/src/rx/rx.c
@@ -3185,6 +3185,26 @@ rxi_ReceivePacket(struct rx_packet *np, osi_socket socket,
         np->header.seq, np->header.flags, np));
 #endif
 
+    /* Account for connectionless packets */
+    if (rx_stats_active &&
+       ((np->header.type == RX_PACKET_TYPE_VERSION) ||
+         (np->header.type == RX_PACKET_TYPE_DEBUG))) {
+       struct rx_peer *peer;
+
+       /* Try to look up the peer structure, but don't create one */
+       peer = rxi_FindPeer(host, port, 0, 0);
+
+       /* Since this may not be associated with a connection, it may have
+        * no refCount, meaning we could race with ReapConnections
+        */
+
+       if (peer && (peer->refCount > 0)) {
+           MUTEX_ENTER(&peer->peer_lock);
+           hadd32(peer->bytesReceived, np->length);
+           MUTEX_EXIT(&peer->peer_lock);
+       }
+    }
+
     if (np->header.type == RX_PACKET_TYPE_VERSION) {
        return rxi_ReceiveVersionPacket(np, socket, host, port, 1);
     }
@@ -3231,6 +3251,13 @@ rxi_ReceivePacket(struct rx_packet *np, osi_socket socket,
        return np;
     }
 
+    /* If we're doing statistics, then account for the incoming packet */
+    if (rx_stats_active) {
+       MUTEX_ENTER(&conn->peer->peer_lock);
+       hadd32(conn->peer->bytesReceived, np->length);
+       MUTEX_EXIT(&conn->peer->peer_lock);
+    }
+
     /* If the connection is in an error state, send an abort packet and ignore
      * the incoming packet */
     if (conn->error) {

diff --git a/src/rx/rx_packet.c b/src/rx/rx_packet.c
index 36523303ad1d580d6316a1c7aba614eba9a2aa8b..94e7482203b60b0d1ed74ccf956e23a39cd8679c 100644 (file)
--- a/src/rx/rx_packet.c
+++ b/src/rx/rx_packet.c
@@ -1480,32 +1480,10 @@ rxi_ReadPacket(osi_socket socket, struct rx_packet *p, afs_uint32 * host,
 
        *host = from.sin_addr.s_addr;
        *port = from.sin_port;
-       if (p->header.type > 0 && p->header.type < RX_N_PACKET_TYPES) {
-            if (rx_stats_active) {
-                struct rx_peer *peer;
-                rx_atomic_inc(&rx_stats.packetsRead[p->header.type - 1]);
-                /*
-                 * Try to look up this peer structure.  If it doesn't exist,
-                 * don't create a new one -
-                 * we don't keep count of the bytes sent/received if a peer
-                 * structure doesn't already exist.
-                 *
-                 * The peer/connection cleanup code assumes that there is 1 peer
-                 * per connection.  If we actually created a peer structure here
-                 * and this packet was an rxdebug packet, the peer structure would
-                 * never be cleaned up.
-                 */
-                peer = rxi_FindPeer(*host, *port, 0, 0);
-                /* Since this may not be associated with a connection,
-                 * it may have no refCount, meaning we could race with
-                 * ReapConnections
-                 */
-                if (peer && (peer->refCount > 0)) {
-                    MUTEX_ENTER(&peer->peer_lock);
-                    hadd32(peer->bytesReceived, p->length);
-                    MUTEX_EXIT(&peer->peer_lock);
-                }
-            }
+       if (rx_stats_active
+           && p->header.type > 0 && p->header.type < RX_N_PACKET_TYPES) {
+
+               rx_atomic_inc(&rx_stats.packetsRead[p->header.type - 1]);
        }
 
 #ifdef RX_TRIMDATABUFS