On Mon, Feb 7, 2011 at 3:14 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > 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. I am not convinced which way is best and it likely doesn't matter. > 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? aah - that makes it tougher to check. If we don't mark ses->Status == NEED_RECONNECT or something other than CifsGood then your approach is easier. > 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? -- Thanks, Steve -- 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
