On Fri, Sep 25, 2020 at 09:59:51AM -0400, Chuck Lever wrote: > Thanks Bruce, for your time, attention, and comments! > > > On Sep 24, 2020, at 5:36 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > > > On Mon, Sep 21, 2020 at 02:10:49PM -0400, Chuck Lever wrote: > >> As I've been working on various server bugs, I've been adding > >> tracepoints that record NFS operation arguments. Here's an updated > >> snapshot of this work for your review and comment. > >> > >> The idea here is to provide a degree of NFS traffic observability > >> without needing network capture. Tracepoints are generally lighter- > >> weight than full network capture, allowing effective capture-time > >> data reduction: > > > > I do wonder when tracepoints seem to duplicate information you could get > > from network traces, so thanks for taking the time to explain this. It > > makes sense to me. > > > > The patches look fine. The only one I'm I'm on the fence about is the > > last with the split up of the dispatch functions. I'll ask some > > questions there.... > > To be clear to everyone, this series is still "preview". I expect > more churn in these patches, thus I don't consider the series ready > to be merged by any stretch. OK! One thing I was wondering about: how would you limit tracing to a single client, say if you wanted to see all DELEGRETURNs from a single client? I guess you'd probably turn on a tracepoint in the receive code, look for your client's IP address, then mask the task id to match later nfs-level tracepoints. Is there enough information in those tracepoints (including network namespace) to uniquely identify a client? --b. > >> - One or a handful of these can be enabled at a time > >> - Each tracepoint records much less data per operation than capture > >> - Extra capture-time filtering can reduce data amount even further > >> - Some of these operations are infrequent enough that their > >> tracepoint could be enabled persistently without a significant > >> performance impact (for example, for security auditing) > >> > >> The topic branch has been updated as well: > >> > >> git://git.linux-nfs.org/projects/cel/cel-2.6.git nfsd-more-tracepoints > >> > >> > >> Changes since RFC: > >> * s/SPDK/SPDX and corrected the spelling of Christoph's surname > >> * Fixed a build error noticed by <lkp@xxxxxxxxx> > >> * Introduced generic headers for VFS and NFS protocol display macros > >> * nfsd4_compoundstatus now displays NFS4ERR codes symbolically > >> * The svc_process tracepoint now displays the RPC procedure symbolically > >> * NFSD dispatcher now displays procedure names and status codes symbolically > >> * fh_verify tracepoint tentatively included; it adds a lot of noise, but perhaps not much value > >> * Cleaned up the remaining PROC() macros in the server code > >> * Removed trace_printk's that were introduced during the RFC series > >> * Removed redundant nfsd4_close tracepoint > >> > >> --- > >> > >> Chuck Lever (27): > >> NFS: Move generic FS show macros to global header > >> NFS: Move NFS protocol display macros to global header > >> NFSD: Add SPDX header for fs/nfsd/trace.c > >> SUNRPC: Move the svc_xdr_recvfrom() tracepoint > >> SUNRPC: Add svc_xdr_authenticate tracepoint > >> lockd: Replace PROC() macro with open code > >> NFSACL: Replace PROC() macro with open code > >> SUNRPC: Make trace_svc_process() display the RPC procedure symbolically > >> NFSD: Clean up the show_nf_may macro > >> NFSD: Remove extra "0x" in tracepoint format specifier > >> NFSD: Constify @fh argument of knfsd_fh_hash() > >> NFSD: Add tracepoint in nfsd_setattr() > >> NFSD: Add tracepoint for nfsd_access() > >> NFSD: nfsd_compound_status tracepoint should record XID > >> NFSD: Add client ID lifetime tracepoints > >> NFSD: Add tracepoints to report NFSv4 session state > >> NFSD: Add a tracepoint to report the current filehandle > >> NFSD: Add GETATTR tracepoint > >> NFSD: Add tracepoint in nfsd4_stateid_preprocess() > >> NFSD: Add tracepoint to report arguments to NFSv4 OPEN > >> NFSD: Add a tracepoint for DELEGRETURN > >> NFSD: Add a lookup tracepoint > >> NFSD: Add lock and locku tracepoints > >> NFSD: Add tracepoints to record the result of TEST_STATEID and FREE_STATEID > >> NFSD: Rename nfsd_ tracepoints to nfsd4_ > >> NFSD: Add tracepoints in the NFS dispatcher > >> NFSD: Replace dprintk callsites in fs/nfsd/nfsfh.c > >> > >> > >> fs/lockd/svc4proc.c | 263 +++++++++-- > >> fs/lockd/svcproc.c | 265 +++++++++-- > >> fs/nfs/callback_xdr.c | 2 + > >> fs/nfs/nfs4trace.h | 387 ++-------------- > >> fs/nfs/nfstrace.h | 113 +---- > >> fs/nfs/pnfs.h | 4 - > >> fs/nfsd/nfs2acl.c | 79 +++- > >> fs/nfsd/nfs3acl.c | 54 ++- > >> fs/nfsd/nfs3proc.c | 25 + > >> fs/nfsd/nfs4callback.c | 28 +- > >> fs/nfsd/nfs4layouts.c | 16 +- > >> fs/nfsd/nfs4proc.c | 43 +- > >> fs/nfsd/nfs4state.c | 100 ++-- > >> fs/nfsd/nfsd.h | 1 + > >> fs/nfsd/nfsfh.c | 36 +- > >> fs/nfsd/nfsfh.h | 7 +- > >> fs/nfsd/nfsproc.c | 21 + > >> fs/nfsd/nfssvc.c | 198 +++++--- > >> fs/nfsd/trace.c | 1 + > >> fs/nfsd/trace.h | 844 ++++++++++++++++++++++++++++++---- > >> fs/nfsd/vfs.c | 18 +- > >> fs/nfsd/xdr4.h | 3 +- > >> include/linux/nfs4.h | 4 + > >> include/linux/sunrpc/svc.h | 1 + > >> include/trace/events/fs.h | 30 ++ > >> include/trace/events/nfs.h | 511 ++++++++++++++++++++ > >> include/trace/events/sunrpc.h | 33 +- > >> include/uapi/linux/nfsacl.h | 2 + > >> net/sunrpc/svc_xprt.c | 4 +- > >> net/sunrpc/svcauth.c | 5 +- > >> 30 files changed, 2187 insertions(+), 911 deletions(-) > >> create mode 100644 include/trace/events/nfs.h > >> > >> -- > >> Chuck Lever > > -- > Chuck Lever > >
