[linux-cifs-client] use non-zero vcnumbers patch
diff mbox

Message ID 524f69650901301750xb6d8e3cy246d92929d364384@mail.gmail.com
State New, archived
Headers show

Commit Message

Steve French Jan. 31, 2009, 1:50 a.m. UTC
Looking at (Samba bugzilla) bug 6004, something like the following
should fix it so that we don't overload vcnum zero with multiple guest
sessions.

Still has to be tested.

Comments

Jeff Layton Feb. 4, 2009, 1:49 a.m. UTC | #1
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.
Christopher R. Hertel Feb. 4, 2009, 2:43 a.m. UTC | #2
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 -)-----
Steve French Feb. 4, 2009, 3:03 a.m. UTC | #3
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.
Christopher R. Hertel Feb. 4, 2009, 3:19 a.m. UTC | #4
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 -)-----
Steve French Feb. 4, 2009, 3:58 a.m. UTC | #5
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
>
Christopher R. Hertel Feb. 4, 2009, 4:13 a.m. UTC | #6
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
>>
> 
> 
>
Volker Lendecke Feb. 4, 2009, 7:12 a.m. UTC | #7
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
Christopher R. Hertel Feb. 4, 2009, 7:20 a.m. UTC | #8
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.  :)
Leonardo Chiquitto Feb. 11, 2009, 12:06 p.m. UTC | #9
>>> +     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
Steve French Feb. 11, 2009, 2:13 p.m. UTC | #10
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).

Patch
diff mbox

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;
 }