> Subject: Re: [PATCH 00/14] cifs: add compounding support for smb2+
>
> Hi Ronnie, all,
>
> I've rebasing this on current for-next since it couldn't be applied anymore.
> Beside the simple conflicts there are a couple of non-trivial changes I've
> made.
Hi Aurelien,
I tried to apply your patch to for-next, but it couldn't apply.
I was using the for-next branch of
git://git.samba.org/sfrench/cifs-2.6.git
Is this the branch this patch is rebased for?
>
> Since some SMB DIRECT code made it in the meantime I had to handle some
> conflicts.
>
> in smb2pdu.c, smb2_async_writev():
>
> > rqst.rq_iov = iov;
> > rqst.rq_nvec = 2;
> > rqst.rq_pages = wdata->pages;
> > rqst.rq_npages = wdata->nr_pages;
> > rqst.rq_pagesz = wdata->pagesz;
> > rqst.rq_tailsz = wdata->tailsz;
> > #ifdef CONFIG_CIFS_SMB_DIRECT
> > if (wdata->mr) {
> > iov[0].iov_len += sizeof(struct smbd_buffer_descriptor_v1);
> > rqst.rq_npages = 0;
> > }
> > #endif
>
> since iov is an array of size 1, I've changed the SMB_DIRECT code to use iov[0]
> instead of iov[1]. But I have no idea if it's really correct.
Your change is correct. SMBD appends the Buffer at the end of the smb2_write for memory registration. It needs to pass those data to the transport.
>
> The thing that bothers me is iov[] has a size of 1 yet
> > rqst.rq_iov = iov;
> > rqst.rq_nvec = 2;
>
> mhh..
This is wrong. rq_nvec should be 1. I’m wondering why it didn't panic in your test. Have you tried testing with memory debugging on?
>
> =========
>
> The other thing I changed was moving the SMB311 preauth hash update to
> the new
> cifs_send_receive() wrapper like so:
>
> > int
> > cifs_send_recv(const unsigned int xid, struct cifs_ses *ses,
> > struct smb_rqst *rqst, int *resp_buf_type, const int flags,
> > struct kvec *resp_iov)
> > {
> > int rc;
> >
> > #ifdef CONFIG_CIFS_SMB311
> > if (ses->status == CifsNew)
> > smb311_update_preauth_hash(ses, rqst->rq_iov,
> > rqst->rq_nvec);
> > #endif
> >
> > rc = compound_send_recv(xid, ses, flags, 1, rqst, resp_buf_type,
> > resp_iov);
> >
> > #ifdef CONFIG_CIFS_SMB311
> > if (ses->status == CifsNew) {
> > smb311_update_preauth_hash(ses, resp_iov, 1);
> > }
> > #endif
> > return rc;
> >
>
> =========
>
> Small change for a sparse warning I was getting:
>
> > #define COMPOUND_FID 0xFFFFFFFFFFFFFFFFULL
>
> Added the ULL suffix as the constant is too big for an int.
>
> =========
>
> Now these I have left these as-is but I have some comments:
>
> =========
>
> in transport.c, compound_send_receive():
>
> > for (i = 0; i < num_rqst; i++) {
> > cifs_save_when_sent(midQ[i]);
> >
> > if (rc < 0)
> > ses->server->sequence_number -= 2;
> > mutex_unlock(&ses->server->srv_mutex);
>
> We potentially unlock the mutex at every loop. Is that on purpose?
>
> =========
>
> In cifs_demultiplex_thread():
>
> > pdu_length = get_rfc1002_length(buf);
> > - server->total_size = pdu_length;
> >
> > - cifs_dbg(FYI, "RFC1002 header 0x%x\n", pdu_length);
> > + cifs_dbg(FYI, "RFC1002 header 0x%x\n",
> > + server->total_size);
> > if (!is_smb_response(server, buf[0]))
> > continue;
> > +next_pdu:
> > + server->total_size = pdu_length;
>
> You set server->total_size *after* printing it.
>
> =========
>
>
> After the rebase I've tried testing mounting a WS2016 share and listing the
> root. If the correct files are listed its OK anything else is ERR:
>
> vers=1.0 ... OK
> vers=2.0 ... OK
> vers=2.0,sec=ntlmsspi ... ERR
> vers=2.1 ... OK
> vers=2.1,sec=ntlmsspi ... ERR
> vers=3.0 ... OK
> vers=3.0,sec=ntlmsspi ... ERR
> vers=3.0,seal ... OK
> vers=3.0,seal,sec=ntlmsspi ... OK
> vers=3.11 ... ERR
> vers=3.11,sec=ntlmsspi ... ERR
> vers=3.11,seal ... ERR
> vers=3.11,seal,sec=ntlmsspi ... ERR
>
> Most of the ntlmsspi errors (thats when we require signing) are due to bad
> signing on certain packets (ones triggered by ls):
>
> Find Request File: SMB2_FIND_ID_FULL_DIRECTORY_INFO Pattern: * Find
> Response, Error: STATUS_ACCESS_DENIED
>
> (if access is ok without signing -> error due to wrong sig)
>
> Weirdly enough, encryption+signingon 3.0 works.
>
> As for SMB311, mount fails and I see lengths issues
>
> > fs/cifs/transport.c: Sending smb: smb_len=170
> > fs/cifs/connect.c: RFC1002 header 0x0
> > fs/cifs/smb2misc.c: SMB2 data length 120 offset 128
> > fs/cifs/smb2misc.c: SMB2 len 248
> > fs/cifs/smb2misc.c: Calculated size 248 length 308 mismatch mid 0
> > SMB2 server sent bad RFC1001 len 308 not 248
>
> And it does weird things from there.
>
> I can provide full kernel debug log and network captures each of the run.
>
> I've attached my rebased patches (I've added my sign-off to patches I had to
> modify to apply or added changes to).
>
> Cheers,
>
> --
> Aurélien Aptel / SUSE Labs Samba Team
> GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 SUSE Linux
> GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG
> Nürnberg)
��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
