| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <MWHPR21MB0190DDCDB0B5F8FED3265A60CE850@MWHPR21MB0190.namprd21.prod.outlook.com>
Date: Wed, 23 Aug 2017 19:10:38 +0000
From: Long Li <longli@...rosoft.com>
To: Leon Romanovsky <leon@...nel.org>
CC: Steve French <sfrench@...ba.org>,
"linux-cifs@...r.kernel.org" <linux-cifs@...r.kernel.org>,
"samba-technical@...ts.samba.org" <samba-technical@...ts.samba.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
Christoph Hellwig <hch@...radead.org>,
"Tom Talpey" <ttalpey@...rosoft.com>,
Matthew Wilcox <mawilcox@...rosoft.com>
Subject: RE: [Patch v2 13/19] CIFS: SMBD: Use registered memory RDMA read for
SMB write
> -----Original Message-----
> From: Leon Romanovsky [mailto:leon@...nel.org]
> Sent: Wednesday, August 23, 2017 12:02 PM
> To: Long Li <longli@...rosoft.com>
> Cc: Steve French <sfrench@...ba.org>; linux-cifs@...r.kernel.org; samba-
> technical@...ts.samba.org; linux-kernel@...r.kernel.org; linux-
> rdma@...r.kernel.org; Christoph Hellwig <hch@...radead.org>; Tom Talpey
> <ttalpey@...rosoft.com>; Matthew Wilcox <mawilcox@...rosoft.com>
> Subject: Re: [Patch v2 13/19] CIFS: SMBD: Use registered memory RDMA
> read for SMB write
>
> On Wed, Aug 23, 2017 at 06:09:11PM +0000, Long Li wrote:
> >
> >
> > > -----Original Message-----
> > > From: Leon Romanovsky [mailto:leon@...nel.org]
> > > Sent: Wednesday, August 23, 2017 6:52 AM
> > > To: Long Li <longli@...rosoft.com>
> > > Cc: Steve French <sfrench@...ba.org>; linux-cifs@...r.kernel.org;
> > > samba- technical@...ts.samba.org; linux-kernel@...r.kernel.org;
> > > linux- rdma@...r.kernel.org; Christoph Hellwig <hch@...radead.org>;
> > > Tom Talpey <ttalpey@...rosoft.com>; Matthew Wilcox
> > > <mawilcox@...rosoft.com>; Long Li <longli@...rosoft.com>
> > > Subject: Re: [Patch v2 13/19] CIFS: SMBD: Use registered memory RDMA
> > > read for SMB write
> > >
> > > On Sun, Aug 20, 2017 at 12:04:37PM -0700, Long Li wrote:
> > > > From: Long Li <longli@...rosoft.com>
> > > >
> > > > When sending I/O, if size is larger than rdma_readwrite_threshold
> > > > we
> > > prepare to send SMB WRITE packet for a RDMA read via memory
> registration.
> > > The actual I/O is done out-of-the-band, so modify the relevant
> > > fields in the packet accordingly.
> > > >
> > > > Signed-off-by: Long Li <longli@...rosoft.com>
> > > > ---
> > > > fs/cifs/smb2pdu.c | 45
> > > ++++++++++++++++++++++++++++++++++++++++++++-
> > > > 1 file changed, 44 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index
> > > > 5cc5f6c..5581afd 100644
> > > > --- a/fs/cifs/smb2pdu.c
> > > > +++ b/fs/cifs/smb2pdu.c
> > > > @@ -48,6 +48,7 @@
> > > > #include "smb2glob.h"
> > > > #include "cifspdu.h"
> > > > #include "cifs_spnego.h"
> > > > +#include "smbdirect.h"
> > > >
> > > > /*
> > > > * The following table defines the expected "StructureSize" of
> > > > SMB2 requests @@ -2716,6 +2717,41 @@ smb2_async_writev(struct
> > > cifs_writedata *wdata,
> > > > offsetof(struct smb2_write_req, Buffer) - 4);
> > > > req->RemainingBytes = 0;
> > > >
> > > > + /*
> > > > + * If we want to do a server RDMA read, fill in and append
> > > > + * smbd_buffer_descriptor_v1 to the end of write request
> > > > + */
> > > > + if (server->rdma && wdata->bytes >
> > > > + server->smbd_conn->rdma_readwrite_threshold) {
> > > > +
> > > > + struct smbd_buffer_descriptor_v1 *v1;
> > > > + bool need_invalidate = server->dialect == SMB30_PROT_ID;
> > > > +
> > > > + wdata->mr = smbd_register_mr(
> > > > + server->smbd_conn, wdata->pages,
> > > > + wdata->nr_pages, wdata->tailsz,
> > > > + false, need_invalidate);
> > > > + if (!wdata->mr) {
> > > > + rc = -ENOBUFS;
> > > > + goto async_writev_out;
> > > > + }
> > > > + req->Length = 0;
> > > > + req->DataOffset = 0;
> > > > + req->RemainingBytes =
> > >
> > > Wow, we have CamelCase variables in linux kernel. It will help if
> > > you start your patchset with small cleanup to convert those
> > > variables from CamelCase to normal names.
> >
> > They are used everywhere in the upper layer code for packet
> > definitions, written a long time ago. (most in fs/cifs/smb2pdu.h and
> > fs/cifs/cifspdu.h)
>
> "everywhere" is a little bit over estimated in this case.
> ➜ linux-rdma git:(master) git grep RemainingBytes
> fs/cifs/smb2pdu.c: req->RemainingBytes =
> cpu_to_le32(remaining_bytes);
> fs/cifs/smb2pdu.c: req->RemainingBytes = 0;
> fs/cifs/smb2pdu.c: req->RemainingBytes = 0;
> fs/cifs/smb2pdu.c: req->RemainingBytes = 0;
> fs/cifs/smb2pdu.h: __le32 RemainingBytes;
> fs/cifs/smb2pdu.h: __le32 RemainingBytes;
>
> One simple "sed -i" will replace all them in one shot and it doesn't look like
> undoable task.
I mean cifspdu.h and smb2pdu.h. use CamelCase for all packet definitions. For example another one in smb2pdu.h:
struct smb2_negotiate_rsp {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 65 */
__le16 SecurityMode;
__le16 DialectRevision;
__le16 NegotiateContextCount; /* Prior to SMB3.1.1 was Reserved & MBZ */
__u8 ServerGUID[16];
__le32 Capabilities;
__le32 MaxTransactSize;
__le32 MaxReadSize;
__le32 MaxWriteSize;
__le64 SystemTime; /* MBZ */
__le64 ServerStartTime;
__le16 SecurityBufferOffset;
__le16 SecurityBufferLength;
__le32 NegotiateContextOffset; /* Pre:SMB3.1.1 was reserved/ignored */
__u8 Buffer[1]; /* variable length GSS security buffer */
} __packed;
We may want to change them all together to keep naming consistent, that's a lot of changes.
>
> >
> > I suggest we do another cleanup patch to clean things up.
>
> Yes, another cleanup patch is needed before your patches. You are adding
> your code in 2017 and you are expected to follow present coding standards
> like everyone else in the kernel.
>
> Thanks
Powered by blists - more mailing lists