diff mbox series

[01/17] RDMA/core: Introduce new header file for signature operations

Message ID 1549302646-18446-2-git-send-email-maxg@mellanox.com (mailing list archive)
State Superseded
Headers show
Series Introduce new API for T10-PI offload | expand

Commit Message

Max Gurtovoy Feb. 4, 2019, 5:50 p.m. UTC 
This commit ought to ease the exhausted ib_verbs.h file and make the
code more readable.

Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
Signed-off-by: Israel Rukshin <israelr@mellanox.com>
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
---
 include/rdma/ib_verbs.h  | 112 +------------------------------------------
 include/rdma/signature.h | 120 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 121 insertions(+), 111 deletions(-)
 create mode 100644 include/rdma/signature.h

Comments

Haakon Bugge Feb. 4, 2019, 6:48 p.m. UTC | #1 
> On 4 Feb 2019, at 18:50, Max Gurtovoy <maxg@mellanox.com> wrote:
> 
> This commit ought to ease the exhausted ib_verbs.h file and make the
> code more readable.
> 
> Signed-off-by: Max Gurtovoy <maxg@mellanox.com>
> Signed-off-by: Israel Rukshin <israelr@mellanox.com>
> Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
> ---
> include/rdma/ib_verbs.h  | 112 +------------------------------------------
> include/rdma/signature.h | 120 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 121 insertions(+), 111 deletions(-)
> create mode 100644 include/rdma/signature.h
> 
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index a3ceed3a040a..9ab46886adc3 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -61,6 +61,7 @@
> #include <linux/cgroup_rdma.h>
> #include <uapi/rdma/ib_user_verbs.h>
> #include <rdma/restrack.h>
> +#include <rdma/signature.h>
> #include <uapi/rdma/rdma_user_ioctl.h>
> #include <uapi/rdma/ib_user_ioctl_verbs.h>
> 
> @@ -240,17 +241,6 @@ enum ib_device_cap_flags {
> 	IB_DEVICE_PCI_WRITE_END_PADDING		= (1ULL << 36),
> };
> 
> -enum ib_signature_prot_cap {
> -	IB_PROT_T10DIF_TYPE_1 = 1,
> -	IB_PROT_T10DIF_TYPE_2 = 1 << 1,
> -	IB_PROT_T10DIF_TYPE_3 = 1 << 2,
> -};
> -
> -enum ib_signature_guard_cap {
> -	IB_GUARD_T10DIF_CRC	= 1,
> -	IB_GUARD_T10DIF_CSUM	= 1 << 1,
> -};
> -
> enum ib_atomic_cap {
> 	IB_ATOMIC_NONE,
> 	IB_ATOMIC_HCA,
> @@ -773,106 +763,6 @@ enum ib_mr_type {
> 	IB_MR_TYPE_SG_GAPS,
> };
> 
> -/**
> - * Signature types
> - * IB_SIG_TYPE_NONE: Unprotected.
> - * IB_SIG_TYPE_T10_DIF: Type T10-DIF
> - */
> -enum ib_signature_type {
> -	IB_SIG_TYPE_NONE,
> -	IB_SIG_TYPE_T10_DIF,
> -};
> -
> -/**
> - * Signature T10-DIF block-guard types
> - * IB_T10DIF_CRC: Corresponds to T10-PI mandated CRC checksum rules.
> - * IB_T10DIF_CSUM: Corresponds to IP checksum rules.
> - */
> -enum ib_t10_dif_bg_type {
> -	IB_T10DIF_CRC,
> -	IB_T10DIF_CSUM
> -};
> -
> -/**
> - * struct ib_t10_dif_domain - Parameters specific for T10-DIF
> - *     domain.
> - * @bg_type: T10-DIF block guard type (CRC|CSUM)
> - * @pi_interval: protection information interval.
> - * @bg: seed of guard computation.
> - * @app_tag: application tag of guard block
> - * @ref_tag: initial guard block reference tag.
> - * @ref_remap: Indicate wethear the reftag increments each block
> - * @app_escape: Indicate to skip block check if apptag=0xffff
> - * @ref_escape: Indicate to skip block check if reftag=0xffffffff
> - * @apptag_check_mask: check bitmask of application tag.
> - */
> -struct ib_t10_dif_domain {
> -	enum ib_t10_dif_bg_type bg_type;
> -	u16			pi_interval;
> -	u16			bg;
> -	u16			app_tag;
> -	u32			ref_tag;
> -	bool			ref_remap;
> -	bool			app_escape;
> -	bool			ref_escape;
> -	u16			apptag_check_mask;
> -};
> -
> -/**
> - * struct ib_sig_domain - Parameters for signature domain
> - * @sig_type: specific signauture type
> - * @sig: union of all signature domain attributes that may
> - *     be used to set domain layout.
> - */
> -struct ib_sig_domain {
> -	enum ib_signature_type sig_type;
> -	union {
> -		struct ib_t10_dif_domain dif;
> -	} sig;
> -};
> -
> -/**
> - * struct ib_sig_attrs - Parameters for signature handover operation
> - * @check_mask: bitmask for signature byte check (8 bytes)
> - * @mem: memory domain layout desciptor.
> - * @wire: wire domain layout desciptor.
> - */
> -struct ib_sig_attrs {
> -	u8			check_mask;
> -	struct ib_sig_domain	mem;
> -	struct ib_sig_domain	wire;
> -};
> -
> -enum ib_sig_err_type {
> -	IB_SIG_BAD_GUARD,
> -	IB_SIG_BAD_REFTAG,
> -	IB_SIG_BAD_APPTAG,
> -};
> -
> -/**
> - * Signature check masks (8 bytes in total) according to the T10-PI standard:
> - *  -------- -------- ------------
> - * | GUARD  | APPTAG |   REFTAG   |
> - * |  2B    |  2B    |    4B      |
> - *  -------- -------- ------------
> - */
> -enum {
> -	IB_SIG_CHECK_GUARD	= 0xc0,
> -	IB_SIG_CHECK_APPTAG	= 0x30,
> -	IB_SIG_CHECK_REFTAG	= 0x0f,
> -};
> -
> -/**
> - * struct ib_sig_err - signature error descriptor
> - */
> -struct ib_sig_err {
> -	enum ib_sig_err_type	err_type;
> -	u32			expected;
> -	u32			actual;
> -	u64			sig_err_offset;
> -	u32			key;
> -};
> -
> enum ib_mr_status_check {
> 	IB_MR_CHECK_SIG_STATUS = 1,
> };
> diff --git a/include/rdma/signature.h b/include/rdma/signature.h
> new file mode 100644
> index 000000000000..7b75696c78e1
> --- /dev/null
> +++ b/include/rdma/signature.h
> @@ -0,0 +1,120 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB) */
> +/*
> + * Copyright (c) 2017-2018 Mellanox Technologies. All rights reserved.
> + */
> +
> +#ifndef _RDMA_SIGNATURE_H_
> +#define _RDMA_SIGNATURE_H_
> +
> +enum ib_signature_prot_cap {
> +	IB_PROT_T10DIF_TYPE_1 = 1,
> +	IB_PROT_T10DIF_TYPE_2 = 1 << 1,
> +	IB_PROT_T10DIF_TYPE_3 = 1 << 2,
> +};
> +
> +enum ib_signature_guard_cap {
> +	IB_GUARD_T10DIF_CRC	= 1,
> +	IB_GUARD_T10DIF_CSUM	= 1 << 1,
> +};
> +
> +/**
> + * Signature types
> + * IB_SIG_TYPE_NONE: Unprotected.
> + * IB_SIG_TYPE_T10_DIF: Type T10-DIF
> + */
> +enum ib_signature_type {
> +	IB_SIG_TYPE_NONE,
> +	IB_SIG_TYPE_T10_DIF,
> +};
> +
> +/**
> + * Signature T10-DIF block-guard types
> + * IB_T10DIF_CRC: Corresponds to T10-PI mandated CRC checksum rules.
> + * IB_T10DIF_CSUM: Corresponds to IP checksum rules.
> + */
> +enum ib_t10_dif_bg_type {
> +	IB_T10DIF_CRC,
> +	IB_T10DIF_CSUM

add a comma so this is as good as your other enums?

Thxs, Håkon

> +};
> +
> +/**
> + * struct ib_t10_dif_domain - Parameters specific for T10-DIF
> + *     domain.
> + * @bg_type: T10-DIF block guard type (CRC|CSUM)
> + * @pi_interval: protection information interval.
> + * @bg: seed of guard computation.
> + * @app_tag: application tag of guard block
> + * @ref_tag: initial guard block reference tag.
> + * @ref_remap: Indicate wethear the reftag increments each block
> + * @app_escape: Indicate to skip block check if apptag=0xffff
> + * @ref_escape: Indicate to skip block check if reftag=0xffffffff
> + * @apptag_check_mask: check bitmask of application tag.
> + */
> +struct ib_t10_dif_domain {
> +	enum ib_t10_dif_bg_type bg_type;
> +	u16			pi_interval;
> +	u16			bg;
> +	u16			app_tag;
> +	u32			ref_tag;
> +	bool			ref_remap;
> +	bool			app_escape;
> +	bool			ref_escape;
> +	u16			apptag_check_mask;
> +};
> +
> +/**
> + * struct ib_sig_domain - Parameters for signature domain
> + * @sig_type: specific signauture type
> + * @sig: union of all signature domain attributes that may
> + *     be used to set domain layout.
> + */
> +struct ib_sig_domain {
> +	enum ib_signature_type sig_type;
> +	union {
> +		struct ib_t10_dif_domain dif;
> +	} sig;
> +};
> +
> +/**
> + * struct ib_sig_attrs - Parameters for signature handover operation
> + * @check_mask: bitmask for signature byte check (8 bytes)
> + * @mem: memory domain layout descriptor.
> + * @wire: wire domain layout descriptor.
> + */
> +struct ib_sig_attrs {
> +	u8			check_mask;
> +	struct ib_sig_domain	mem;
> +	struct ib_sig_domain	wire;
> +};
> +
> +enum ib_sig_err_type {
> +	IB_SIG_BAD_GUARD,
> +	IB_SIG_BAD_REFTAG,
> +	IB_SIG_BAD_APPTAG,
> +};
> +
> +/**
> + * Signature check masks (8 bytes in total) according to the T10-PI standard:
> + *  -------- -------- ------------
> + * | GUARD  | APPTAG |   REFTAG   |
> + * |  2B    |  2B    |    4B      |
> + *  -------- -------- ------------
> + */
> +enum {
> +	IB_SIG_CHECK_GUARD = 0xc0,
> +	IB_SIG_CHECK_APPTAG = 0x30,
> +	IB_SIG_CHECK_REFTAG = 0x0f,
> +};
> +
> +/**
> + * struct ib_sig_err - signature error descriptor
> + */
> +struct ib_sig_err {
> +	enum ib_sig_err_type	err_type;
> +	u32			expected;
> +	u32			actual;
> +	u64			sig_err_offset;
> +	u32			key;
> +};
> +
> +#endif /* _RDMA_SIGNATURE_H_ */
> -- 
> 2.16.3
>
Bart Van Assche Feb. 5, 2019, 4:25 a.m. UTC | #2 
On 2/4/19 9:50 AM, Max Gurtovoy wrote:
> +#include <rdma/signature.h>

Do we really need this #include directive in <rdma/ib_verbs.h>? Can this 
#include directive be moved into the source files that need it?

Thanks,

Bart.
Israel Rukshin Feb. 5, 2019, 2:56 p.m. UTC | #3 
On 2/5/2019 6:25 AM, Bart Van Assche wrote:
> On 2/4/19 9:50 AM, Max Gurtovoy wrote:
>> +#include <rdma/signature.h>
>
> Do we really need this #include directive in <rdma/ib_verbs.h>? Can 
> this #include directive be moved into the source files that need it?
>
> Thanks,
>
> Bart.

Yes, we need this include at <rdma/ib_verbs.h>.

<rdma/ib_verbs.h> uses struct ib_sig_err which is defined at 
<rdma/signature.h>.

Thanks,

Israel
Bart Van Assche Feb. 5, 2019, 4:08 p.m. UTC | #4 
On Tue, 2019-02-05 at 14:56 +0000, Israel Rukshin wrote:
> On 2/5/2019 6:25 AM, Bart Van Assche wrote:
> > On 2/4/19 9:50 AM, Max Gurtovoy wrote:
> > > +#include <rdma/signature.h>
> > 
> > Do we really need this #include directive in <rdma/ib_verbs.h>? Can 
> > this #include directive be moved into the source files that need it?
> 
> Yes, we need this include at <rdma/ib_verbs.h>.
> 
> <rdma/ib_verbs.h> uses struct ib_sig_err which is defined at 
> <rdma/signature.h>.

Hi Israel,

Are you perhaps referring to struct ib_mr_status? It seems to me that that
structure is only used in code that is related to protection information.
Have you considered to move the definition of that structure into
<rdma/signature.h> and add a forward declaration of that structure in
<rdma/ib_verbs.h> instead?

Thanks,

Bart.
Max Gurtovoy Feb. 6, 2019, 10:57 a.m. UTC | #5 
On 2/5/2019 6:08 PM, Bart Van Assche wrote:
> On Tue, 2019-02-05 at 14:56 +0000, Israel Rukshin wrote:
>> On 2/5/2019 6:25 AM, Bart Van Assche wrote:
>>> On 2/4/19 9:50 AM, Max Gurtovoy wrote:
>>>> +#include <rdma/signature.h>
>>> Do we really need this #include directive in <rdma/ib_verbs.h>? Can
>>> this #include directive be moved into the source files that need it?
>> Yes, we need this include at <rdma/ib_verbs.h>.
>>
>> <rdma/ib_verbs.h> uses struct ib_sig_err which is defined at
>> <rdma/signature.h>.
> Hi Israel,
>
> Are you perhaps referring to struct ib_mr_status? It seems to me that that
> structure is only used in code that is related to protection information.
> Have you considered to move the definition of that structure into
> <rdma/signature.h> and add a forward declaration of that structure in
> <rdma/ib_verbs.h> instead?
>
> Thanks,
>
> Bart.


Hi Bart,

it's kind of hard to change all ib_verbs.h in one-shot. We're trying to 
make things easier and this is why we added the signature.h file.

It need more work and renaming to move ib_mr_status to be signature 
specific and I'm not sure it the right thing to do now. I prefer to stay 
with the include since we don't really harm anything comparing the 
current state.

We just ease the endless ib_verbs.h file and make the code more readable 
as we mentioned in the commit message.
diff mbox series

Patch

diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index a3ceed3a040a..9ab46886adc3 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -61,6 +61,7 @@ 
 #include <linux/cgroup_rdma.h>
 #include <uapi/rdma/ib_user_verbs.h>
 #include <rdma/restrack.h>
+#include <rdma/signature.h>
 #include <uapi/rdma/rdma_user_ioctl.h>
 #include <uapi/rdma/ib_user_ioctl_verbs.h>
 
@@ -240,17 +241,6 @@  enum ib_device_cap_flags {
 	IB_DEVICE_PCI_WRITE_END_PADDING		= (1ULL << 36),
 };
 
-enum ib_signature_prot_cap {
-	IB_PROT_T10DIF_TYPE_1 = 1,
-	IB_PROT_T10DIF_TYPE_2 = 1 << 1,
-	IB_PROT_T10DIF_TYPE_3 = 1 << 2,
-};
-
-enum ib_signature_guard_cap {
-	IB_GUARD_T10DIF_CRC	= 1,
-	IB_GUARD_T10DIF_CSUM	= 1 << 1,
-};
-
 enum ib_atomic_cap {
 	IB_ATOMIC_NONE,
 	IB_ATOMIC_HCA,
@@ -773,106 +763,6 @@  enum ib_mr_type {
 	IB_MR_TYPE_SG_GAPS,
 };
 
-/**
- * Signature types
- * IB_SIG_TYPE_NONE: Unprotected.
- * IB_SIG_TYPE_T10_DIF: Type T10-DIF
- */
-enum ib_signature_type {
-	IB_SIG_TYPE_NONE,
-	IB_SIG_TYPE_T10_DIF,
-};
-
-/**
- * Signature T10-DIF block-guard types
- * IB_T10DIF_CRC: Corresponds to T10-PI mandated CRC checksum rules.
- * IB_T10DIF_CSUM: Corresponds to IP checksum rules.
- */
-enum ib_t10_dif_bg_type {
-	IB_T10DIF_CRC,
-	IB_T10DIF_CSUM
-};
-
-/**
- * struct ib_t10_dif_domain - Parameters specific for T10-DIF
- *     domain.
- * @bg_type: T10-DIF block guard type (CRC|CSUM)
- * @pi_interval: protection information interval.
- * @bg: seed of guard computation.
- * @app_tag: application tag of guard block
- * @ref_tag: initial guard block reference tag.
- * @ref_remap: Indicate wethear the reftag increments each block
- * @app_escape: Indicate to skip block check if apptag=0xffff
- * @ref_escape: Indicate to skip block check if reftag=0xffffffff
- * @apptag_check_mask: check bitmask of application tag.
- */
-struct ib_t10_dif_domain {
-	enum ib_t10_dif_bg_type bg_type;
-	u16			pi_interval;
-	u16			bg;
-	u16			app_tag;
-	u32			ref_tag;
-	bool			ref_remap;
-	bool			app_escape;
-	bool			ref_escape;
-	u16			apptag_check_mask;
-};
-
-/**
- * struct ib_sig_domain - Parameters for signature domain
- * @sig_type: specific signauture type
- * @sig: union of all signature domain attributes that may
- *     be used to set domain layout.
- */
-struct ib_sig_domain {
-	enum ib_signature_type sig_type;
-	union {
-		struct ib_t10_dif_domain dif;
-	} sig;
-};
-
-/**
- * struct ib_sig_attrs - Parameters for signature handover operation
- * @check_mask: bitmask for signature byte check (8 bytes)
- * @mem: memory domain layout desciptor.
- * @wire: wire domain layout desciptor.
- */
-struct ib_sig_attrs {
-	u8			check_mask;
-	struct ib_sig_domain	mem;
-	struct ib_sig_domain	wire;
-};
-
-enum ib_sig_err_type {
-	IB_SIG_BAD_GUARD,
-	IB_SIG_BAD_REFTAG,
-	IB_SIG_BAD_APPTAG,
-};
-
-/**
- * Signature check masks (8 bytes in total) according to the T10-PI standard:
- *  -------- -------- ------------
- * | GUARD  | APPTAG |   REFTAG   |
- * |  2B    |  2B    |    4B      |
- *  -------- -------- ------------
- */
-enum {
-	IB_SIG_CHECK_GUARD	= 0xc0,
-	IB_SIG_CHECK_APPTAG	= 0x30,
-	IB_SIG_CHECK_REFTAG	= 0x0f,
-};
-
-/**
- * struct ib_sig_err - signature error descriptor
- */
-struct ib_sig_err {
-	enum ib_sig_err_type	err_type;
-	u32			expected;
-	u32			actual;
-	u64			sig_err_offset;
-	u32			key;
-};
-
 enum ib_mr_status_check {
 	IB_MR_CHECK_SIG_STATUS = 1,
 };
diff --git a/include/rdma/signature.h b/include/rdma/signature.h
new file mode 100644
index 000000000000..7b75696c78e1
--- /dev/null
+++ b/include/rdma/signature.h
@@ -0,0 +1,120 @@ 
+/* SPDX-License-Identifier: (GPL-2.0 OR Linux-OpenIB) */
+/*
+ * Copyright (c) 2017-2018 Mellanox Technologies. All rights reserved.
+ */
+
+#ifndef _RDMA_SIGNATURE_H_
+#define _RDMA_SIGNATURE_H_
+
+enum ib_signature_prot_cap {
+	IB_PROT_T10DIF_TYPE_1 = 1,
+	IB_PROT_T10DIF_TYPE_2 = 1 << 1,
+	IB_PROT_T10DIF_TYPE_3 = 1 << 2,
+};
+
+enum ib_signature_guard_cap {
+	IB_GUARD_T10DIF_CRC	= 1,
+	IB_GUARD_T10DIF_CSUM	= 1 << 1,
+};
+
+/**
+ * Signature types
+ * IB_SIG_TYPE_NONE: Unprotected.
+ * IB_SIG_TYPE_T10_DIF: Type T10-DIF
+ */
+enum ib_signature_type {
+	IB_SIG_TYPE_NONE,
+	IB_SIG_TYPE_T10_DIF,
+};
+
+/**
+ * Signature T10-DIF block-guard types
+ * IB_T10DIF_CRC: Corresponds to T10-PI mandated CRC checksum rules.
+ * IB_T10DIF_CSUM: Corresponds to IP checksum rules.
+ */
+enum ib_t10_dif_bg_type {
+	IB_T10DIF_CRC,
+	IB_T10DIF_CSUM
+};
+
+/**
+ * struct ib_t10_dif_domain - Parameters specific for T10-DIF
+ *     domain.
+ * @bg_type: T10-DIF block guard type (CRC|CSUM)
+ * @pi_interval: protection information interval.
+ * @bg: seed of guard computation.
+ * @app_tag: application tag of guard block
+ * @ref_tag: initial guard block reference tag.
+ * @ref_remap: Indicate wethear the reftag increments each block
+ * @app_escape: Indicate to skip block check if apptag=0xffff
+ * @ref_escape: Indicate to skip block check if reftag=0xffffffff
+ * @apptag_check_mask: check bitmask of application tag.
+ */
+struct ib_t10_dif_domain {
+	enum ib_t10_dif_bg_type bg_type;
+	u16			pi_interval;
+	u16			bg;
+	u16			app_tag;
+	u32			ref_tag;
+	bool			ref_remap;
+	bool			app_escape;
+	bool			ref_escape;
+	u16			apptag_check_mask;
+};
+
+/**
+ * struct ib_sig_domain - Parameters for signature domain
+ * @sig_type: specific signauture type
+ * @sig: union of all signature domain attributes that may
+ *     be used to set domain layout.
+ */
+struct ib_sig_domain {
+	enum ib_signature_type sig_type;
+	union {
+		struct ib_t10_dif_domain dif;
+	} sig;
+};
+
+/**
+ * struct ib_sig_attrs - Parameters for signature handover operation
+ * @check_mask: bitmask for signature byte check (8 bytes)
+ * @mem: memory domain layout descriptor.
+ * @wire: wire domain layout descriptor.
+ */
+struct ib_sig_attrs {
+	u8			check_mask;
+	struct ib_sig_domain	mem;
+	struct ib_sig_domain	wire;
+};
+
+enum ib_sig_err_type {
+	IB_SIG_BAD_GUARD,
+	IB_SIG_BAD_REFTAG,
+	IB_SIG_BAD_APPTAG,
+};
+
+/**
+ * Signature check masks (8 bytes in total) according to the T10-PI standard:
+ *  -------- -------- ------------
+ * | GUARD  | APPTAG |   REFTAG   |
+ * |  2B    |  2B    |    4B      |
+ *  -------- -------- ------------
+ */
+enum {
+	IB_SIG_CHECK_GUARD = 0xc0,
+	IB_SIG_CHECK_APPTAG = 0x30,
+	IB_SIG_CHECK_REFTAG = 0x0f,
+};
+
+/**
+ * struct ib_sig_err - signature error descriptor
+ */
+struct ib_sig_err {
+	enum ib_sig_err_type	err_type;
+	u32			expected;
+	u32			actual;
+	u64			sig_err_offset;
+	u32			key;
+};
+
+#endif /* _RDMA_SIGNATURE_H_ */