rxstats: correctly distinguish client and server stats 74/14374/3
authorMark Vitale <mvitale@sinenomine.net>
Mon, 28 Sep 2020 19:40:34 +0000 (15:40 -0400)
committerBenjamin Kaduk <kaduk@mit.edu>
Fri, 23 Oct 2020 16:24:37 +0000 (12:24 -0400)
Commit d3eaa39da3693bba708fa2fa951568009e929550 'rx: Make the rx_call
structure private' inadvertently caused all rxstats (aka rpcstats) to be
recorded as client stats by hardcoding the value for isServer to 1.

Therefore, when peer or process rxstats are enabled for a OpenAFS
component, the rxstat_get_process and rxstat_get_peer utilities will
erroneously report both client and server stats as "accessed as a client".

This is particularly problematic for ubik VOTE_* and DISK_* RPC stats,
for which a given ubik server may be both client and server over time.
In this case, both client and server stats are conflated into the same
"accessed as a client" counters.

Instead, properly pass the value of isServer from
rx_RecordCallStatistics through to rxi_IncrementTimeAndCount.

Note to maintainers:
This bug is only in master and all 1.8.x releases; no 1.6.x releases are
affected.

Note:
Confusingly, isServer=1 indicates client stats and isServer=0 indicates
server stats.  However, this is a quirk of the original implementation
and wire format of the RXSTATS_* RPCs and cannot be changed.  isServer
is actually shorthand for "remote is server"; thus all RPC client stubs
record their rxstats with isServer == 1, and all RPC server stubs record
their rxstats with isServer == 0.

Change-Id: I2420f807e2c18ddfb9de7093a487825fa2d0a68e
Reviewed-on: https://gerrit.openafs.org/14374
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

src/rx/rx_call.c

index 6f22ce0..af75b85 100644 (file)
@@ -70,7 +70,7 @@ rx_RecordCallStatistics(struct rx_call *call, unsigned int rxInterface,
 
     rxi_IncrementTimeAndCount(call->conn->peer, rxInterface, currentFunc,
                             totalFunc, &queue, &exec, call->app.bytesSent,
-                            call->app.bytesRcvd, 1);
+                            call->app.bytesRcvd, isServer);
 }
 
 /*