On Mon, 7 Feb 2011 12:36:22 -0600 Steve French <smfrench@xxxxxxxxx> wrote: > On Mon, Feb 7, 2011 at 12:20 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Mon, 7 Feb 2011 11:42:05 -0600 > > Steve French <smfrench@xxxxxxxxx> wrote: > > > >> On Mon, Feb 7, 2011 at 7:54 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > >> > Commit 247ec9b4 makes the SMB echo workqueue job skip doing echoes > >> > unless the tcpStatus == CifsGood. Problem: tcpStatus == CifsGood is not > >> > a reliable indicator of whether the socket has had a NEGOTIATE done on > >> > it. This fixes that by adding a new tcpStatus of CifsNeedNegotiate that > >> > indicates this. It also cleans up cifs_reconnect_tcon a bit. > >> > > >> > This is not the only way to fix this issue. As Steve pointed out to me, > >> > we could also check to see whether there's a valid SMB session on the > >> > list as an indicator of whether a NEGOTIATE has been done. I think the > >> > approach I'm proposing clarifies the code better, but I'm willing to go > >> > with his approach if that's the general concensus. > >> > >> Jeff may be right that the code is clearer with this approach (I may try > >> to code the alternative to check) but ... we really shouldn't be sending > >> an SMBecho between negprot and sessionsetup anyway (so checking for > >> sessionsetup may make sense). > >> If a negprot times out or > >> sessionsetup times out we don't need to be sending SMBecho > >> (on the other hand perhaps a really slow krb5 sessionsetup?) > >> Once a sessionsetup is sent, SMBechos make more sense. > >> > > > > Why shouldn't we send one between the negotiate and session setup? The > > MS-CIFS spec makes no mention that a session setup is required prior to > > doing an echo. You may be correct, but that's not really what the spec > > says. > > I don't remember ever having seen one between negprot and sessionsetup > from other clients - but the broader point is that an SMBecho doesn't > serve any purpose in between negprot/sessionsetup (once at least one > session is setup, checking if the server stays up is more interesting) > I'm not sure I agree there. There's nothing that says that the NEGOTIATE and SESSION_SETUP have to be done within a certain amount of time of each other. Checking the validity of the connection there seems reasonable. ...but at this point, if you think checking for a valid session is the best approach then so be it. Practically, how do you suggest that we check for a valid SMB session? I assume the code will need to walk the smb_ses_list. The ses->status doesn't appear to actually change on reconnect however. It only sets the "need_reconnect" boolean. It appears that the ses->status is set to CifsGood and never changes after the initial session setup. Perhaps that field should be removed? We could replace the "need_reconnect" flag with an "established" flag, that's a clear indicator of whether the session setup conversation is complete. ...or is there a better way to do this? -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
