I actually like the idea proposed in the patch: this way we will be able to track logical operations better. In a case we need a deeper logging we already have mid traces which contains MID, TID, SID and CMD - everything you need to trace individual SMB packet regardless of whether it is a part of compounding chain or not. Now in order to match both logical operation and SMB packets traces we need something common - like XID. Currently we do not propagate XID down to the transport layer, let's do that - this way we can trace individual system call from the very beginning to the completeness on every layer inside cifs.ko. вт, 12 мар. 2019 г. в 00:19, Steve French <smfrench@xxxxxxxxx>: > > The general feeling I am getting from tracing the compounding is that > we have to have the more granular tracing available > (similar to this patch), and also (with followon patch) add noisier > optional trace points lower down (that are generic) that we turn on > less frequently (in particular when the filename is not good enough > and we must have the xid - even if it makes the trace very noisy) > > > On Tue, Mar 12, 2019 at 1:06 AM ronnie sahlberg > <ronniesahlberg@xxxxxxxxx> wrote: > > > > Hmm. > > > > I don't think this is the wrong layer to add the tracing. > > Instead of adding the tracing in say smb2_compound_op(), where we > > create the compound, > > I think it will be better to add the tracing down in the leaf > > functions, i.e. down in SMB2_open_init(), SMB2_query_info_init(), ... > > > > That way we know that we will always have tracing for all opens/closes/... > > > > But then what to do about the trace_smb3_..._err()/trace_smb3_..._done() calls ? > > These I think should be moved down and handled inside compound_send_recv(). > > That way we can have one single place where we call the appropriate > > _err()/_done() functions > > and we know that we will catch every single completed function. > > > > I.e. just put it in a single place instead of in every single call-site. > > In compound_send_recv we can just loop over all the responses, cast it > > to a smb2_hdr structure, looka at both the command code as well as the > > status code and call the appropriate xys_err() or xyz_done() functions > > as needed. > > > > Let me show an example of what I mean for a open/query/close compound : > > > > trace_smb3_open_enter(xid=5, filename="foo", ...) from SMB2_open_init() > > trace_smb3_query_info_enter(xid=5,... ) from SMB2_query_info_init() > > trace_smb3_close_enter(xid=5, ...) from SMB2_close_init() > > (maybe have additional tracing in compound_send_recv() that we are > > about to send xid=5 to the wire) > > trace_smb3_open_done(xid=5, ...) from compound_send_recv() > > trace_smb3_query_info_done(xid=5, ...) from compound_send_recv() > > trace_smb3_close_done(xid=5, ...) from compound_send_recv() > > As pointed above we already have a similar tracing inside compound_send_recv and other transport layer functions - command level tracing. The only problem is that we do not log failure cases when we didn't send the packet - we only log server responses in map_smb2_to_linux_error. This is another area of the tracing where a fix is needed. -- Best regards, Pavel Shilovsky
