| Message ID | 524f69650901301750xb6d8e3cy246d92929d364384@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 30 Jan 2009 19:50:08 -0600 Steve French <smfrench@gmail.com> wrote: > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index 94c1ca0..e004f6d 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -164,9 +164,12 @@ struct TCP_Server_Info { > /* multiplexed reads or writes */ > unsigned int maxBuf; /* maxBuf specifies the maximum */ > /* message size the server can send or receive for non-raw SMBs */ > - unsigned int maxRw; /* maxRw specifies the maximum */ > + unsigned int max_rw; /* maxRw specifies the maximum */ > /* message size the server can send or receive for */ > /* SMB_COM_WRITE_RAW or SMB_COM_READ_RAW. */ > + unsigned int max_vcs; /* maximum number of smb sessions, at least > + those that can be specified uniquely with > + vcnumbers */ > char sessid[4]; /* unique token id for this session */ > /* (returned on Negotiate */ > int capabilities; /* allow selective disabling of caps by smb sess */ > @@ -210,6 +213,7 @@ struct cifsSesInfo { > unsigned overrideSecFlg; /* if non-zero override global sec flags */ > __u16 ipc_tid; /* special tid for connection to IPC share */ > __u16 flags; > + __u16 vcnum; > char *serverOS; /* name of operating system underlying server */ > char *serverNOS; /* name of network operating system of server */ > char *serverDomain; /* security realm of server */ > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 552642a..b0f4e56 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -528,14 +528,15 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) > server->maxReq = le16_to_cpu(rsp->MaxMpxCount); > server->maxBuf = min((__u32)le16_to_cpu(rsp->MaxBufSize), > (__u32)CIFSMaxBufSize + MAX_CIFS_HDR_SIZE); > + server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs); > GETU32(server->sessid) = le32_to_cpu(rsp->SessionKey); > /* even though we do not use raw we might as well set this > accurately, in case we ever find a need for it */ > if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) { > - server->maxRw = 0xFF00; > + server->max_rw = 0xFF00; > server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE; > } else { > - server->maxRw = 0;/* we do not need to use raw anyway */ > + server->max_rw = 0;/* we do not need to use raw anyway */ > server->capabilities = CAP_MPX_MODE; > } > tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone); > @@ -638,7 +639,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) > /* probably no need to store and check maxvcs */ > server->maxBuf = min(le32_to_cpu(pSMBr->MaxBufferSize), > (__u32) CIFSMaxBufSize + MAX_CIFS_HDR_SIZE); > - server->maxRw = le32_to_cpu(pSMBr->MaxRawSize); > + server->max_rw = le32_to_cpu(pSMBr->MaxRawSize); > cFYI(DBG2, ("Max buf = %d", ses->server->maxBuf)); > GETU32(ses->server->sessid) = le32_to_cpu(pSMBr->SessionKey); > server->capabilities = le32_to_cpu(pSMBr->Capabilities); > diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c > index 5f22de7..fdbfdca 100644 > --- a/fs/cifs/sess.c > +++ b/fs/cifs/sess.c > @@ -34,15 +34,62 @@ > extern void SMBNTencrypt(unsigned char *passwd, unsigned char *c8, > unsigned char *p24); > > +static __le16 get_next_vcnum(struct cifsSesInfo *ses) > +{ > + __u16 vcnum = 0; > + struct list_head *tmp; > + struct cifsSesInfo *tmp_ses; > + __u16 max_vcs = ses->server->max_vcs; > + __u16 i; > + int free_vc_found = 0; > + > + if (max_vcs < 2) > + max_vcs = 0xFFFF; > + A comment about the above situation seems warranted since it's basically a workaround for buggy servers. > + write_lock(&cifs_tcp_ses_lock); > + for(i = ses->server->srv_count - 1; i < max_vcs; i++) { > + if (i == 0) /* this is the only connection, use vc 0 */ > + break; > + > + free_vc_found = 1; > + > + list_for_each(tmp, &ses->server->smb_ses_list) { > + tmp_ses = list_entry(tmp, struct cifsSesInfo, > + smb_ses_list); > + if (tmp_ses->vcnum == i) { > + free_vc_found = 0; > + break; /* found duplicate, try next vcnum */ > + } > + } > + if (free_vc_found) > + break; /* we found a vcnumber that will work - use it */ > + } > + > + if (free_vc_found == 0) > + vcnum = 0; /* for most common case, ie if one smb session, use > + vc zero. Also for case when no free vcnum, zero > + is safest to send (some clients only send zero) */ > + else > + vcnum = i; > + ses->vcnum = vcnum; > + write_unlock(&cifs_tcp_ses_lock); > + > + return le16_to_cpu(vcnum); > +} > + > static __u32 cifs_ssetup_hdr(struct cifsSesInfo *ses, SESSION_SETUP_ANDX *pSMB) > { > __u32 capabilities = 0; > > /* init fields common to all four types of SessSetup */ > - /* note that header is initialized to zero in header_assemble */ > + /* Note that offsets for first seven fields in req struct are same */ > + /* in CIFS Specs so does not matter which of 3 forms of struct */ > + /* that we use in next few lines */ > + /* Note that header is initialized to zero in header_assemble */ > pSMB->req.AndXCommand = 0xFF; > pSMB->req.MaxBufferSize = cpu_to_le16(ses->server->maxBuf); > pSMB->req.MaxMpxCount = cpu_to_le16(ses->server->maxReq); > + pSMB->req.VcNumber = get_next_vcnum(ses); > > /* Now no need to set SMBFLG_CASELESS or obsolete CANONICAL PATH */ > > @@ -71,7 +118,6 @@ static __u32 cifs_ssetup_hdr(struct cifsSesInfo *ses, SESSION_SETUP_ANDX *pSMB) > if (ses->capabilities & CAP_UNIX) > capabilities |= CAP_UNIX; > > - /* BB check whether to init vcnum BB */ > return capabilities; > } > So what happens at reconnect? Shouldn't we "reset" all the vcnums? Shouldn't the first session setup on the reconnected socket use vcnum 0? I seem to recall reading that vcnum of 0 has some significance for the server... Other than that, the patch looks reasonable to me.
Jeff Layton wrote: : : > So what happens at reconnect? Shouldn't we "reset" all the vcnums? > Shouldn't the first session setup on the reconnected socket use vcnum > 0? I seem to recall reading that vcnum of 0 has some significance for > the server... The server sees VC number 0 from a given client to be the base VC. Older servers would drop all connections with a client if a new VC=0 from that client was established. TCP wasn't the only transport back then, so there were situations in which the server might not notice that the client had reset. I ran into a problem when I was testing jCIFS several years ago. jCIFS would send VC=0 whenever it connected, and all other CIFS connections (eg., smbclient and--back then--smbfs) from the same Linux box would drop. jCIFS now sends a minumum VC number of 1. Microsoft ran into this when customers started putting clients behind NATs. The Windows server would assume (based on the IP address of the client) that all of the connections were from the same client. Each new client that connected (Windows clients send VC=0) would cause any other connected client to be dropped. Only one client at a time could connect through the NAT and the attempts at reconnecting would cause a complete failure. Microsoft fixed this on the server side. There's a KB article on it somewhere. My general recommendation is that clients never send VC=0. :) Chris -)-----
On Tue, Feb 3, 2009 at 7:49 PM, Jeff Layton <jlayton@redhat.com> wrote: > On Fri, 30 Jan 2009 19:50:08 -0600 > Steve French <smfrench@gmail.com> wrote: >> + if (max_vcs < 2) >> + max_vcs = 0xFFFF; >> + > > A comment about the above situation seems warranted since it's basically > a workaround for buggy servers. Yes - Windows set(s) max vcs to 1 when it means unlimited so I agree, this should be commented. > So what happens at reconnect? Shouldn't we "reset" all the vcnums? > Shouldn't the first session setup on the reconnected socket use vcnum > 0? I seem to recall reading that vcnum of 0 has some significance for > the server... > > Other than that, the patch looks reasonable to me. I forgot to add apart that resets vcnumbers, I had originally intended to do that, but I am not certain that it is necessary, although without it, we increase the possiblility of vc number collisions. I thought Windows begins with vcnumber 0, but don't remember (that is probably safest, to follow Windows behavior here, although it is not always easy to get certain Windows clients to send more than one vc from the same client - as you could e.g. with terminal server/ I also remember the old situation that Chris H. mentions about vc numbers and NATs and the nasty behavior change it required.
Steve French wrote: : > I also remember the old situation that Chris H. mentions about vc > numbers and NATs and the nasty behavior change it required. NATs, and multiple connections from a single machine. Say someone has (for example) an smbclient session open and then mounts a share using CIFS. If CIFS sends VC=0 the smbclient session may be dropped. As I recall (haven't tested lately) newer Windows servers no longer drop existing connections if a new VC=0 connection comes in. If that is the case, then there's no point in sending VC=0 anyway. Again, I'm going from memory so I could be wrong here, but I think jCIFS sends VC=1 for all connections and doesn't have a problem. Two TCP connections from the same client with the same VC number would (assuming the old OS/2 idea of a VC is supported) be seen as a single virtual circuit over two physical circuits. Chris -)-----
In the situation you describe - I thought that the server could recognize that they are distinct sockets, and source ports, but perhaps with Win9x didn't On Tue, Feb 3, 2009 at 9:19 PM, Christopher R. Hertel <crh@ubiqx.mn.org> wrote: > Steve French wrote: > : >> I also remember the old situation that Chris H. mentions about vc >> numbers and NATs and the nasty behavior change it required. > > NATs, and multiple connections from a single machine. Say someone has (for > example) an smbclient session open and then mounts a share using CIFS. If > CIFS sends VC=0 the smbclient session may be dropped. > > As I recall (haven't tested lately) newer Windows servers no longer drop > existing connections if a new VC=0 connection comes in. If that is the > case, then there's no point in sending VC=0 anyway. > > Again, I'm going from memory so I could be wrong here, but I think jCIFS > sends VC=1 for all connections and doesn't have a problem. Two TCP > connections from the same client with the same VC number would (assuming the > old OS/2 idea of a VC is supported) be seen as a single virtual circuit over > two physical circuits. > > Chris -)----- > > -- > "Implementing CIFS - the Common Internet FileSystem" ISBN: 013047116X > Samba Team -- http://www.samba.org/ -)----- Christopher R. Hertel > jCIFS Team -- http://jcifs.samba.org/ -)----- ubiqx development, uninq. > ubiqx Team -- http://www.ubiqx.org/ -)----- crh@ubiqx.mn.org > OnLineBook -- http://ubiqx.org/cifs/ -)----- crh@ubiqx.org >
Steve French wrote: > In the situation you describe - I thought that the server could > recognize that they are distinct sockets, and source ports, but > perhaps with Win9x didn't I'll have to test this behavior again, just to see. I *think* I was testing smbclient and jCIFS simultaneously against W2K when I ran into it years ago. Here's that KB article I mentioned: http://support.microsoft.com/default.aspx?scid=KB;en-us;301673 Curious... It says NT4 servers don't have this problem. I'll have to check that too. :) Here's what I wrote about VCs 7 or 8 years ago: http://www.ubiqx.org/cifs/SMB.html#SMB.7.1.1 The problem is that there's theory and there's practice. I think of the whole VC thing as a vestigial organ, like an appendix. It isn't used much any more, so how it actually works in any give implementation depends... Chris -)----- > On Tue, Feb 3, 2009 at 9:19 PM, Christopher R. Hertel <crh@ubiqx.mn.org> wrote: >> Steve French wrote: >> : >>> I also remember the old situation that Chris H. mentions about vc >>> numbers and NATs and the nasty behavior change it required. >> NATs, and multiple connections from a single machine. Say someone has (for >> example) an smbclient session open and then mounts a share using CIFS. If >> CIFS sends VC=0 the smbclient session may be dropped. >> >> As I recall (haven't tested lately) newer Windows servers no longer drop >> existing connections if a new VC=0 connection comes in. If that is the >> case, then there's no point in sending VC=0 anyway. >> >> Again, I'm going from memory so I could be wrong here, but I think jCIFS >> sends VC=1 for all connections and doesn't have a problem. Two TCP >> connections from the same client with the same VC number would (assuming the >> old OS/2 idea of a VC is supported) be seen as a single virtual circuit over >> two physical circuits. >> >> Chris -)----- >> >> -- >> "Implementing CIFS - the Common Internet FileSystem" ISBN: 013047116X >> Samba Team -- http://www.samba.org/ -)----- Christopher R. Hertel >> jCIFS Team -- http://jcifs.samba.org/ -)----- ubiqx development, uninq. >> ubiqx Team -- http://www.ubiqx.org/ -)----- crh@ubiqx.mn.org >> OnLineBook -- http://ubiqx.org/cifs/ -)----- crh@ubiqx.org >> > > >
On Tue, Feb 03, 2009 at 10:13:58PM -0600, Christopher R. Hertel wrote: > The problem is that there's theory and there's practice. I think of the > whole VC thing as a vestigial organ, like an appendix. It isn't used much > any more, so how it actually works in any give implementation depends... I've added the "reset on zero vc" parameter to smbd a few years ago. At that point (w2k3 was already released I think) Windows servers still disconnected all others when one with vc=0 came in. Volker
Volker Lendecke wrote: > On Tue, Feb 03, 2009 at 10:13:58PM -0600, Christopher R. Hertel wrote: >> The problem is that there's theory and there's practice. I think of the >> whole VC thing as a vestigial organ, like an appendix. It isn't used much >> any more, so how it actually works in any give implementation depends... > > I've added the "reset on zero vc" parameter to smbd a few > years ago. At that point (w2k3 was already released I think) > Windows servers still disconnected all others when one with > vc=0 came in. > > Volker Cool. That makes Samba's behavior (or misbehavior, depending upon your perspective) tunable. Nifty. :)
>>> + if (max_vcs < 2) >>> + max_vcs = 0xFFFF; >>> + >> >> A comment about the above situation seems warranted since it's basically >> a workaround for buggy servers. > > Yes - Windows set(s) max vcs to 1 when it means unlimited so I agree, > this should be commented. > >> So what happens at reconnect? Shouldn't we "reset" all the vcnums? >> Shouldn't the first session setup on the reconnected socket use vcnum >> 0? I seem to recall reading that vcnum of 0 has some significance for >> the server... >> >> Other than that, the patch looks reasonable to me. > I forgot to add apart that resets vcnumbers, I had originally intended > to do that, but I am not certain that it is necessary, although > without it, we increase the possiblility of vc number collisions. I > thought Windows begins with vcnumber 0, but don't remember (that is > probably safest, to follow Windows behavior here, although it is not > always easy to get certain Windows clients to send more than one vc > from the same client - as you could e.g. with terminal server/ > > I also remember the old situation that Chris H. mentions about vc > numbers and NATs and the nasty behavior change it required. Hi Steve, Do you have plans to push this patch upstream in the near future? Thanks, Leonardo
On Wed, Feb 11, 2009 at 6:06 AM, Leonardo Chiquitto < leonardo.lists@gmail.com> wrote: > >>> + if (max_vcs < 2) > >>> + max_vcs = 0xFFFF; > >>> + > >> > >> A comment about the above situation seems warranted since it's basically > >> a workaround for buggy servers. > > > > Yes - Windows set(s) max vcs to 1 when it means unlimited so I agree, > > this should be commented. > > > >> So what happens at reconnect? Shouldn't we "reset" all the vcnums? > >> Shouldn't the first session setup on the reconnected socket use vcnum > >> 0? I seem to recall reading that vcnum of 0 has some significance for > >> the server... > >> > >> Other than that, the patch looks reasonable to me. > > I forgot to add apart that resets vcnumbers, I had originally intended > > to do that, but I am not certain that it is necessary, although > > without it, we increase the possiblility of vc number collisions. I > > thought Windows begins with vcnumber 0, but don't remember (that is > > probably safest, to follow Windows behavior here, although it is not > > always easy to get certain Windows clients to send more than one vc > > from the same client - as you could e.g. with terminal server/ > > > > I also remember the old situation that Chris H. mentions about vc > > numbers and NATs and the nasty behavior change it required. > > Hi Steve, > > Do you have plans to push this patch upstream in the near future? > > Yes - I plan to push upstream, but wanted to fix the reconnection case (so on reconnect we would send vc number 0, on the first smb session, and non-zero unique vc numbers on the subsequent smb sessions).
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 94c1ca0..e004f6d 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -164,9 +164,12 @@ struct TCP_Server_Info { /* multiplexed reads or writes */ unsigned int maxBuf; /* maxBuf specifies the maximum */ /* message size the server can send or receive for non-raw SMBs */ - unsigned int maxRw; /* maxRw specifies the maximum */ + unsigned int max_rw; /* maxRw specifies the maximum */ /* message size the server can send or receive for */ /* SMB_COM_WRITE_RAW or SMB_COM_READ_RAW. */ + unsigned int max_vcs; /* maximum number of smb sessions, at least + those that can be specified uniquely with + vcnumbers */ char sessid[4]; /* unique token id for this session */ /* (returned on Negotiate */ int capabilities; /* allow selective disabling of caps by smb sess */ @@ -210,6 +213,7 @@ struct cifsSesInfo { unsigned overrideSecFlg; /* if non-zero override global sec flags */ __u16 ipc_tid; /* special tid for connection to IPC share */ __u16 flags; + __u16 vcnum; char *serverOS; /* name of operating system underlying server */ char *serverNOS; /* name of network operating system of server */ char *serverDomain; /* security realm of server */ diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 552642a..b0f4e56 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -528,14 +528,15 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) server->maxReq = le16_to_cpu(rsp->MaxMpxCount); server->maxBuf = min((__u32)le16_to_cpu(rsp->MaxBufSize), (__u32)CIFSMaxBufSize + MAX_CIFS_HDR_SIZE); + server->max_vcs = le16_to_cpu(rsp->MaxNumberVcs); GETU32(server->sessid) = le32_to_cpu(rsp->SessionKey); /* even though we do not use raw we might as well set this accurately, in case we ever find a need for it */ if ((le16_to_cpu(rsp->RawMode) & RAW_ENABLE) == RAW_ENABLE) { - server->maxRw = 0xFF00; + server->max_rw = 0xFF00; server->capabilities = CAP_MPX_MODE | CAP_RAW_MODE; } else { - server->maxRw = 0;/* we do not need to use raw anyway */ + server->max_rw = 0;/* we do not need to use raw anyway */ server->capabilities = CAP_MPX_MODE; } tmp = (__s16)le16_to_cpu(rsp->ServerTimeZone); @@ -638,7 +639,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses) /* probably no need to store and check maxvcs */ server->maxBuf = min(le32_to_cpu(pSMBr->MaxBufferSize), (__u32) CIFSMaxBufSize + MAX_CIFS_HDR_SIZE); - server->maxRw = le32_to_cpu(pSMBr->MaxRawSize); + server->max_rw = le32_to_cpu(pSMBr->MaxRawSize); cFYI(DBG2, ("Max buf = %d", ses->server->maxBuf)); GETU32(ses->server->sessid) = le32_to_cpu(pSMBr->SessionKey); server->capabilities = le32_to_cpu(pSMBr->Capabilities); diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c index 5f22de7..fdbfdca 100644 --- a/fs/cifs/sess.c +++ b/fs/cifs/sess.c @@ -34,15 +34,62 @@ extern void SMBNTencrypt(unsigned char *passwd, unsigned char *c8, unsigned char *p24); +static __le16 get_next_vcnum(struct cifsSesInfo *ses) +{ + __u16 vcnum = 0; + struct list_head *tmp; + struct cifsSesInfo *tmp_ses; + __u16 max_vcs = ses->server->max_vcs; + __u16 i; + int free_vc_found = 0; + + if (max_vcs < 2) + max_vcs = 0xFFFF; + + write_lock(&cifs_tcp_ses_lock); + for(i = ses->server->srv_count - 1; i < max_vcs; i++) { + if (i == 0) /* this is the only connection, use vc 0 */ + break; + + free_vc_found = 1; + + list_for_each(tmp, &ses->server->smb_ses_list) { + tmp_ses = list_entry(tmp, struct cifsSesInfo, + smb_ses_list); + if (tmp_ses->vcnum == i) { + free_vc_found = 0; + break; /* found duplicate, try next vcnum */ + } + } + if (free_vc_found) + break; /* we found a vcnumber that will work - use it */ + } + + if (free_vc_found == 0) + vcnum = 0; /* for most common case, ie if one smb session, use + vc zero. Also for case when no free vcnum, zero + is safest to send (some clients only send zero) */ + else + vcnum = i; + ses->vcnum = vcnum; + write_unlock(&cifs_tcp_ses_lock); + + return le16_to_cpu(vcnum); +} + static __u32 cifs_ssetup_hdr(struct cifsSesInfo *ses, SESSION_SETUP_ANDX *pSMB) { __u32 capabilities = 0; /* init fields common to all four types of SessSetup */ - /* note that header is initialized to zero in header_assemble */ + /* Note that offsets for first seven fields in req struct are same */ + /* in CIFS Specs so does not matter which of 3 forms of struct */ + /* that we use in next few lines */ + /* Note that header is initialized to zero in header_assemble */ pSMB->req.AndXCommand = 0xFF; pSMB->req.MaxBufferSize = cpu_to_le16(ses->server->maxBuf); pSMB->req.MaxMpxCount = cpu_to_le16(ses->server->maxReq); + pSMB->req.VcNumber = get_next_vcnum(ses); /* Now no need to set SMBFLG_CASELESS or obsolete CANONICAL PATH */ @@ -71,7 +118,6 @@ static __u32 cifs_ssetup_hdr(struct cifsSesInfo *ses, SESSION_SETUP_ANDX *pSMB) if (ses->capabilities & CAP_UNIX) capabilities |= CAP_UNIX; - /* BB check whether to init vcnum BB */ return capabilities; }