| Message ID | 160071167664.1468.1365570508917640511.stgit@klimt.1015granger.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | NFSD operation monitoring tracepoints | expand |
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.... --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@intel.com> > * 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
Thanks Bruce, for your time, attention, and comments! > On Sep 24, 2020, at 5:36 PM, J. Bruce Fields <bfields@fieldses.org> 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. > --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@intel.com> >> * 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
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@fieldses.org> 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@intel.com> > >> * 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 > >
> On Sep 25, 2020, at 10:32 AM, Bruce Fields <bfields@fieldses.org> wrote: > > 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@fieldses.org> 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? Client IP address information is in the RPC layer trace data. The DELEGRETURN trace record includes client ID. So maybe not as straightforward as it could be. > --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@intel.com> >>>> * 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 -- Chuck Lever
On Fri, Sep 25, 2020 at 10:36:42AM -0400, Chuck Lever wrote: > > > > On Sep 25, 2020, at 10:32 AM, Bruce Fields <bfields@fieldses.org> wrote: > > > > 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@fieldses.org> 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? > > Client IP address information is in the RPC layer trace data. The > DELEGRETURN trace record includes client ID. So maybe not as > straightforward as it could be. I guess what I meant was "limit tracing to a single network endpoint", not exactly limt to a single NFSv4 client.... So, we can do that as long as all the relevant information is in rpc-layer tracepoints, and as long as task id is a reliable way to match up trace points. Is the network namespace in there anywhere? It looks like there'd be no way to distinguish clients in different namespaces if they had the same address. --b. > > > > --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@intel.com> > >>>> * 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 > > -- > Chuck Lever > >
> On Sep 25, 2020, at 11:00 AM, Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Sep 25, 2020 at 10:36:42AM -0400, Chuck Lever wrote: >> >> >>> On Sep 25, 2020, at 10:32 AM, Bruce Fields <bfields@fieldses.org> wrote: >>> >>> 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@fieldses.org> 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? >> >> Client IP address information is in the RPC layer trace data. The >> DELEGRETURN trace record includes client ID. So maybe not as >> straightforward as it could be. > > I guess what I meant was "limit tracing to a single network endpoint", > not exactly limt to a single NFSv4 client.... So, we can do that as > long as all the relevant information is in rpc-layer tracepoints, and as > long as task id is a reliable way to match up trace points. > > Is the network namespace in there anywhere? It looks like there'd be no > way to distinguish clients in different namespaces if they had the same > address. The client ID has the boot verifier for the net namespace. None of this helps NFSv3, though. -- Chuck Lever
> On Sep 25, 2020, at 11:05 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > > > >> On Sep 25, 2020, at 11:00 AM, Bruce Fields <bfields@fieldses.org> wrote: >> >> On Fri, Sep 25, 2020 at 10:36:42AM -0400, Chuck Lever wrote: >>> >>> >>>> On Sep 25, 2020, at 10:32 AM, Bruce Fields <bfields@fieldses.org> wrote: >>>> >>>> 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@fieldses.org> 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? >>> >>> Client IP address information is in the RPC layer trace data. The >>> DELEGRETURN trace record includes client ID. So maybe not as >>> straightforward as it could be. >> >> I guess what I meant was "limit tracing to a single network endpoint", >> not exactly limt to a single NFSv4 client.... So, we can do that as >> long as all the relevant information is in rpc-layer tracepoints, and as >> long as task id is a reliable way to match up trace points. >> >> Is the network namespace in there anywhere? It looks like there'd be no >> way to distinguish clients in different namespaces if they had the same >> address. > > The client ID has the boot verifier for the net namespace. > > None of this helps NFSv3, though. It probably wouldn't be difficult to stuff the client IP address and the boot verifier in the trace record for each procedure. Do you think that would be sufficient? -- Chuck Lever
On Fri, Sep 25, 2020 at 01:04:55PM -0400, Chuck Lever wrote: > > On Sep 25, 2020, at 11:05 AM, Chuck Lever <chuck.lever@oracle.com> wrote: > >> On Sep 25, 2020, at 11:00 AM, Bruce Fields <bfields@fieldses.org> wrote: > >> On Fri, Sep 25, 2020 at 10:36:42AM -0400, Chuck Lever wrote: > >>>> 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? > >>> > >>> Client IP address information is in the RPC layer trace data. The > >>> DELEGRETURN trace record includes client ID. So maybe not as > >>> straightforward as it could be. > >> > >> I guess what I meant was "limit tracing to a single network endpoint", > >> not exactly limt to a single NFSv4 client.... So, we can do that as > >> long as all the relevant information is in rpc-layer tracepoints, and as > >> long as task id is a reliable way to match up trace points. > >> > >> Is the network namespace in there anywhere? It looks like there'd be no > >> way to distinguish clients in different namespaces if they had the same > >> address. > > > > The client ID has the boot verifier for the net namespace. > > > > None of this helps NFSv3, though. > > It probably wouldn't be difficult to stuff the client IP address > and the boot verifier in the trace record for each procedure. > > Do you think that would be sufficient? Despite using 64 bits the boot verifier isn't even guaranteed to uniquely identify a network namespace. There should be something better. Digging around.... ns->inum, I think? I don't know if it (or ports or addresses) needs to be in every trace record. Maybe just once somewhere in one of the rpc layer tracepoints, and then we can use the nfsd task id to connect it with other tracepoints if necessary. Does that make sense? --b.
> On Sep 25, 2020, at 2:37 PM, Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Sep 25, 2020 at 01:04:55PM -0400, Chuck Lever wrote: >>> On Sep 25, 2020, at 11:05 AM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>> On Sep 25, 2020, at 11:00 AM, Bruce Fields <bfields@fieldses.org> wrote: >>>> On Fri, Sep 25, 2020 at 10:36:42AM -0400, Chuck Lever wrote: >>>>>> 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? >>>>> >>>>> Client IP address information is in the RPC layer trace data. The >>>>> DELEGRETURN trace record includes client ID. So maybe not as >>>>> straightforward as it could be. >>>> >>>> I guess what I meant was "limit tracing to a single network endpoint", >>>> not exactly limt to a single NFSv4 client.... So, we can do that as >>>> long as all the relevant information is in rpc-layer tracepoints, and as >>>> long as task id is a reliable way to match up trace points. >>>> >>>> Is the network namespace in there anywhere? It looks like there'd be no >>>> way to distinguish clients in different namespaces if they had the same >>>> address. >>> >>> The client ID has the boot verifier for the net namespace. >>> >>> None of this helps NFSv3, though. >> >> It probably wouldn't be difficult to stuff the client IP address >> and the boot verifier in the trace record for each procedure. >> >> Do you think that would be sufficient? > > Despite using 64 bits the boot verifier isn't even guaranteed to > uniquely identify a network namespace. There should be something > better. Digging around.... ns->inum, I think? Could use that, but NFSD already uses the boot verifier everywhere. I'll have a look. > I don't know if it (or ports or addresses) needs to be in every trace > record. Maybe just once somewhere in one of the rpc layer tracepoints, > and then we can use the nfsd task id to connect it with other > tracepoints if necessary. Does that make sense? If you want to do filtering during capture, you want to have this kind of information available in each record so the generic trace utilities can find it. We don't have to display it in every record that is output by trace-cmd report, though. -- Chuck Lever
Hi- 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: - 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@intel.com> * 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