[[PATCH,v1] 08/37] [CIFS] SMBD: Define packet format for SMBD data transfer message
diff mbox

Message ID 1501704648-20159-9-git-send-email-longli@exchange.microsoft.com
State New
Headers show

Commit Message

Long Li Aug. 2, 2017, 8:10 p.m. UTC
From: Long Li <longli@microsoft.com>

Define the packet format for a SMBD data packet with payload

Signed-off-by: Long Li <longli@microsoft.com>
---
 fs/cifs/cifsrdma.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Christoph Hellwig Aug. 13, 2017, 10:15 a.m. UTC | #1
> +// SMBD data transfer packet with payload [MS-SMBD] 2.2.3
> +struct smbd_data_transfer {
> +	__le16 credits_requested;
> +	__le16 credits_granted;
> +	__le16 flags;
> +	__le16 reserved;
> +	__le32 remaining_data_length;
> +	__le32 data_offset;
> +	__le32 data_length;
> +	__le32 padding;
> +	char buffer[0];

Please use the actually standardized [] syntax for variable sized
arrays.  Also normally this would be a __u8 to fit with the other
types, but I haven't seen the usage yet.

> +} __packed;

The structure is natually packed already, no need to add the
attribute.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Layton Aug. 14, 2017, 10:24 a.m. UTC | #2
On Sun, 2017-08-13 at 03:15 -0700, Christoph Hellwig wrote:
> > +// SMBD data transfer packet with payload [MS-SMBD] 2.2.3
> > +struct smbd_data_transfer {
> > +	__le16 credits_requested;
> > +	__le16 credits_granted;
> > +	__le16 flags;
> > +	__le16 reserved;
> > +	__le32 remaining_data_length;
> > +	__le32 data_offset;
> > +	__le32 data_length;
> > +	__le32 padding;
> > +	char buffer[0];
> 
> Please use the actually standardized [] syntax for variable sized
> arrays.  Also normally this would be a __u8 to fit with the other
> types, but I haven't seen the usage yet.
> 

Yes, having a single-element array makes it harder to handle the
indexes, etc. Flexible arrays are better.
 
> > +} __packed;
> 
> The structure is natually packed already, no need to add the
> attribute.

I think this should remain on structs that are intended to go across the
wire. Could we ever end up with some exotic arch that stuffs some
padding in there? Maybe I'm just paranoid, but I don't see any harm in
leaving that here.

Patch
diff mbox

diff --git a/fs/cifs/cifsrdma.h b/fs/cifs/cifsrdma.h
index 78ce2bf..ed0ff54 100644
--- a/fs/cifs/cifsrdma.h
+++ b/fs/cifs/cifsrdma.h
@@ -78,6 +78,21 @@  enum smbd_message_type {
 	SMBD_TRANSFER_DATA,
 };
 
+#define SMB_DIRECT_RESPONSE_REQUESTED 0x0001
+
+// SMBD data transfer packet with payload [MS-SMBD] 2.2.3
+struct smbd_data_transfer {
+	__le16 credits_requested;
+	__le16 credits_granted;
+	__le16 flags;
+	__le16 reserved;
+	__le32 remaining_data_length;
+	__le32 data_offset;
+	__le32 data_length;
+	__le32 padding;
+	char buffer[0];
+} __packed;
+
 // The context for a SMBD response
 struct cifs_rdma_response {
 	struct cifs_rdma_info *info;