[dpdk-dev] [PATCH] cryptodev: fix ABI breakage

[dpdk-dev] [PATCH] cryptodev: fix ABI breakage

[Pablo de Lara]

In 17.08, the crypto operation was restructured,
and some reserved bytes (5) were added  to have the mempool
pointer aligned to 64 bits, since the structure is expected
to be aligned to 64 bits, allowing future additions with no
ABI breakage needed.

In 18.05, a new 2-byte field was added, so the reserved bytes
were reduced to 3. However, this field was added after the first 3 bytes
of the structure, causing it to be placed in an offset of 4 bytes,
and therefore, forcing the mempool pointer to be placed after 16 bytes,
instead of a 8 bytes, causing unintentionally the ABI breakage.

This commit fixes the breakage, by swapping the reserved bytes
and the private_data_offset field, so the latter is aligned to 2 bytes
and the offset of the mempool pointer returns to its original offset,
8 bytes.

Fixes: 54c836846603 ("cryptodev: set private data for session-less mode")
Cc: stable@dpdk.org

Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_cryptodev/rte_crypto.h | 51 +++++++++++++++++++------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
index 25404264b..a16be656d 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -73,26 +73,37 @@ enum rte_crypto_op_sess_type {
  * rte_cryptodev_enqueue_burst() / rte_cryptodev_dequeue_burst() .
  */
 struct rte_crypto_op {
-	uint8_t type;
-	/**< operation type */
-	uint8_t status;
-	/**<
-	 * operation status - this is reset to
-	 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation from mempool and
-	 * will be set to RTE_CRYPTO_OP_STATUS_SUCCESS after crypto operation
-	 * is successfully processed by a crypto PMD
-	 */
-	uint8_t sess_type;
-	/**< operation session type */
-	uint16_t private_data_offset;
-	/**< Offset to indicate start of private data (if any). The offset
-	 * is counted from the start of the rte_crypto_op including IV.
-	 * The private data may be used by the application to store
-	 * information which should remain untouched in the library/driver
-	 */
-
-	uint8_t reserved[3];
-	/**< Reserved bytes to fill 64 bits for future additions */
+	__extension__
+	union {
+		uint64_t raw;
+		__extension__
+		struct {
+			uint8_t type;
+			/**< operation type */
+			uint8_t status;
+			/**<
+			 * operation status - this is reset to
+			 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation
+			 * from mempool and will be set to
+			 * RTE_CRYPTO_OP_STATUS_SUCCESS after crypto operation
+			 * is successfully processed by a crypto PMD
+			 */
+			uint8_t sess_type;
+			/**< operation session type */
+			uint8_t reserved[3];
+			/**< Reserved bytes to fill 64 bits for
+			 * future additions
+			 */
+			uint16_t private_data_offset;
+			/**< Offset to indicate start of private data (if any).
+			 * The offset is counted from the start of the
+			 * rte_crypto_op including IV.
+			 * The private data may be used by the application
+			 * to store information which should remain untouched
+			 * in the library/driver
+			 */
+		};
+	};
 	struct rte_mempool *mempool;
 	/**< crypto operation mempool which operation is allocated from */
 
-- 
2.17.0

Re: [dpdk-dev] [PATCH] cryptodev: fix ABI breakage

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Wednesday, June 13, 2018 10:37 AM
> To: Doherty, Declan <declan.doherty@intel.com>; Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] cryptodev: fix ABI breakage
> 
> In 17.08, the crypto operation was restructured,
> and some reserved bytes (5) were added  to have the mempool
> pointer aligned to 64 bits, since the structure is expected
> to be aligned to 64 bits, allowing future additions with no
> ABI breakage needed.
> 
> In 18.05, a new 2-byte field was added, so the reserved bytes
> were reduced to 3. However, this field was added after the first 3 bytes
> of the structure, causing it to be placed in an offset of 4 bytes,
> and therefore, forcing the mempool pointer to be placed after 16 bytes,
> instead of a 8 bytes, causing unintentionally the ABI breakage.
> 
> This commit fixes the breakage, by swapping the reserved bytes
> and the private_data_offset field, so the latter is aligned to 2 bytes
> and the offset of the mempool pointer returns to its original offset,
> 8 bytes.
> 
> Fixes: 54c836846603 ("cryptodev: set private data for session-less mode")
> Cc: stable@dpdk.org
> 
> Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto.h | 51 +++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
> index 25404264b..a16be656d 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -73,26 +73,37 @@ enum rte_crypto_op_sess_type {
>   * rte_cryptodev_enqueue_burst() / rte_cryptodev_dequeue_burst() .
>   */
>  struct rte_crypto_op {
> -	uint8_t type;
> -	/**< operation type */
> -	uint8_t status;
> -	/**<
> -	 * operation status - this is reset to
> -	 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation from mempool and
> -	 * will be set to RTE_CRYPTO_OP_STATUS_SUCCESS after crypto operation
> -	 * is successfully processed by a crypto PMD
> -	 */
> -	uint8_t sess_type;
> -	/**< operation session type */
> -	uint16_t private_data_offset;
> -	/**< Offset to indicate start of private data (if any). The offset
> -	 * is counted from the start of the rte_crypto_op including IV.
> -	 * The private data may be used by the application to store
> -	 * information which should remain untouched in the library/driver
> -	 */
> -
> -	uint8_t reserved[3];
> -	/**< Reserved bytes to fill 64 bits for future additions */
> +	__extension__
> +	union {
> +		uint64_t raw;
> +		__extension__
> +		struct {
> +			uint8_t type;
> +			/**< operation type */
> +			uint8_t status;
> +			/**<
> +			 * operation status - this is reset to
> +			 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation
> +			 * from mempool and will be set to
> +			 * RTE_CRYPTO_OP_STATUS_SUCCESS after crypto operation
> +			 * is successfully processed by a crypto PMD
> +			 */
> +			uint8_t sess_type;
> +			/**< operation session type */
> +			uint8_t reserved[3];
> +			/**< Reserved bytes to fill 64 bits for
> +			 * future additions
> +			 */
> +			uint16_t private_data_offset;
> +			/**< Offset to indicate start of private data (if any).
> +			 * The offset is counted from the start of the
> +			 * rte_crypto_op including IV.
> +			 * The private data may be used by the application
> +			 * to store information which should remain untouched
> +			 * in the library/driver
> +			 */
> +		};
> +	};
>  	struct rte_mempool *mempool;
>  	/**< crypto operation mempool which operation is allocated from */
> 
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.17.0

Re: [dpdk-dev] [PATCH] cryptodev: fix ABI breakage

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Wednesday, June 13, 2018 3:07 PM
> To: Doherty, Declan <declan.doherty@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] cryptodev: fix ABI breakage
> 
> In 17.08, the crypto operation was restructured, and some reserved bytes (5)
> were added  to have the mempool pointer aligned to 64 bits, since the structure
> is expected to be aligned to 64 bits, allowing future additions with no ABI
> breakage needed.
> 
> In 18.05, a new 2-byte field was added, so the reserved bytes were reduced to 3.
> However, this field was added after the first 3 bytes of the structure, causing it
> to be placed in an offset of 4 bytes, and therefore, forcing the mempool pointer
> to be placed after 16 bytes, instead of a 8 bytes, causing unintentionally the ABI
> breakage.
> 
> This commit fixes the breakage, by swapping the reserved bytes and the
> private_data_offset field, so the latter is aligned to 2 bytes and the offset of the
> mempool pointer returns to its original offset,
> 8 bytes.
> 
> Fixes: 54c836846603 ("cryptodev: set private data for session-less mode")
> Cc: stable@dpdk.org
> 
> Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto.h | 51 +++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
> index 25404264b..a16be656d 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -73,26 +73,37 @@ enum rte_crypto_op_sess_type {
>   * rte_cryptodev_enqueue_burst() / rte_cryptodev_dequeue_burst() .
>   */
>  struct rte_crypto_op {
> -	uint8_t type;
> -	/**< operation type */
> -	uint8_t status;
> -	/**<
> -	 * operation status - this is reset to
> -	 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation from
> mempool and
> -	 * will be set to RTE_CRYPTO_OP_STATUS_SUCCESS after crypto
> operation
> -	 * is successfully processed by a crypto PMD
> -	 */
> -	uint8_t sess_type;
> -	/**< operation session type */
> -	uint16_t private_data_offset;
> -	/**< Offset to indicate start of private data (if any). The offset
> -	 * is counted from the start of the rte_crypto_op including IV.
> -	 * The private data may be used by the application to store
> -	 * information which should remain untouched in the library/driver
> -	 */
> -
> -	uint8_t reserved[3];
> -	/**< Reserved bytes to fill 64 bits for future additions */
> +	__extension__
> +	union {
> +		uint64_t raw;
> +		__extension__
> +		struct {
> +			uint8_t type;
> +			/**< operation type */
> +			uint8_t status;
> +			/**<
> +			 * operation status - this is reset to
> +			 * RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on
> allocation
> +			 * from mempool and will be set to
> +			 * RTE_CRYPTO_OP_STATUS_SUCCESS after crypto
> operation
> +			 * is successfully processed by a crypto PMD
> +			 */
> +			uint8_t sess_type;
> +			/**< operation session type */
> +			uint8_t reserved[3];
> +			/**< Reserved bytes to fill 64 bits for
> +			 * future additions
> +			 */
> +			uint16_t private_data_offset;
> +			/**< Offset to indicate start of private data (if any).
> +			 * The offset is counted from the start of the
> +			 * rte_crypto_op including IV.
> +			 * The private data may be used by the application
> +			 * to store information which should remain untouched
> +			 * in the library/driver
> +			 */
> +		};
> +	};
>  	struct rte_mempool *mempool;
>  	/**< crypto operation mempool which operation is allocated from */
> 
> --

Acked-by: Abhinandan Gujjar <Abhinandan.gujjar@intel.com>

> 2.17.0

Re: [dpdk-dev] [PATCH] cryptodev: fix ABI breakage

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Wednesday, June 13, 2018 11:01 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] cryptodev: fix ABI breakage
> 
> 
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Wednesday, June 13, 2018 3:07 PM
> > To: Doherty, Declan <declan.doherty@intel.com>; Gujjar, Abhinandan S
> > <abhinandan.gujjar@intel.com>
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>; stable@dpdk.org
> > Subject: [PATCH] cryptodev: fix ABI breakage
> >
> > In 17.08, the crypto operation was restructured, and some reserved
> > bytes (5) were added  to have the mempool pointer aligned to 64 bits,
> > since the structure is expected to be aligned to 64 bits, allowing
> > future additions with no ABI breakage needed.
> >
> > In 18.05, a new 2-byte field was added, so the reserved bytes were reduced to
> 3.
> > However, this field was added after the first 3 bytes of the
> > structure, causing it to be placed in an offset of 4 bytes, and
> > therefore, forcing the mempool pointer to be placed after 16 bytes,
> > instead of a 8 bytes, causing unintentionally the ABI breakage.
> >
> > This commit fixes the breakage, by swapping the reserved bytes and the
> > private_data_offset field, so the latter is aligned to 2 bytes and the
> > offset of the mempool pointer returns to its original offset,
> > 8 bytes.
> >
> > Fixes: 54c836846603 ("cryptodev: set private data for session-less
> > mode")
> > Cc: stable@dpdk.org
> >
> > Reported-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

...

> Acked-by: Abhinandan Gujjar <Abhinandan.gujjar@intel.com>
> 
> > 2.17.0

Applied to dpdk-next-crypto.

Pablo