rx: Remove surplus call to FindPeer
authorSimon Wilkinson <sxw@your-file-system.com>
Fri, 13 Apr 2012 18:14:44 +0000 (19:14 +0100)
committerJeffrey Altman <jaltman@secure-endpoints.com>
Mon, 16 Apr 2012 15:58:19 +0000 (08:58 -0700)
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>

src/rx/rx.c
src/rx/rx_packet.c

index 1bf61e6..ca563f7 100644 (file)
@@ -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) {
index 3652330..94e7482 100644 (file)
@@ -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