[dpdk-dev] [PATCH v3 3/3] app/test: add Kasumi tests in QAT test suite

[De Lara Guarch, Pablo]

Hi Deepak,

> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Deepak Kumar Jain
> Sent: Thursday, September 15, 2016 3:06 AM
> To: dev at dpdk.org
> Cc: Jain, Deepak K
> Subject: [dpdk-dev] [PATCH v3 3/3] app/test: add Kasumi tests in QAT test
> suite
> 
> This patch adds Kausmi tests in the QAT tesuite.

This patch implements tests for authenticated encryption and encrypted authentication,
and  adds them in the testsuite for KASUMI SW PMD (thanks for that!).
Could you include this info in the commit message.

> 
> Signed-off-by: Deepak Kumar Jain <deepak.k.jain at intel.com>
> ---
>  app/test/test_cryptodev.c                          | 244 +++++++++++++++++++--
>  app/test/test_cryptodev_kasumi_hash_test_vectors.h |  76 +++++++
>  app/test/test_cryptodev_kasumi_test_vectors.h      | 103 ++++++++-
>  3 files changed, 401 insertions(+), 22 deletions(-)
> 
> diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
> index 89d627f..4751467 100644
> --- a/app/test/test_cryptodev.c
> +++ b/app/test/test_cryptodev.c

...

>  static int
> +test_kasumi_auth_cipher(const struct kasumi_test_data *tdata)
> +{

...

> +
> +	/* Validate obuf */
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL_BIT(
> +			ciphertext,
> +			tdata->ciphertext.data,
> +			tdata->validCipherLenInBits.len,
> +			"Snow3G/Kasumi Ciphertext data not as expected");

This is in a kasumi test, so you can change the message to "Kasumi Ciphertext..."

> +	ut_params->digest = rte_pktmbuf_mtod(ut_params->obuf, uint8_t
> *)
> +	    + plaintext_pad_len + tdata->aad.len + tdata->iv.len;
> +
> +	/* Validate obuf */
> +	TEST_ASSERT_BUFFERS_ARE_EQUAL(
> +			ut_params->digest,
> +			tdata->digest.data,
> +			DIGEST_BYTE_LENGTH_KASUMI_F9,
> +			"Snow3G/KASUMI Generated auth tag not as
> expected");

Same here

> +	return 0;
> +}
> +

...

>  /* ***** AES-GCM Tests ***** */
> 
>  static int
> @@ -4139,6 +4323,21 @@ static struct unit_test_suite
> cryptodev_qat_testsuite  = {
>  		TEST_CASE_ST(ut_setup, ut_teardown,
>  			test_null_auth_cipher_operation),
> 

If you include a comment down here for KASUMI encryption only,
you should include one for authentication only here, and maybe comments
for the other two cases (auth_cipher/cipher_auth). You can also group all of them into "KASUMI tests", easier :)


> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +			test_kasumi_hash_generate_test_case_6),
> +
> +		/** KASUMI encrypt only (F8) */
> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +			test_kasumi_encryption_test_case_1),
> +
> +		/** KASUMI encrypt only (F8) */
> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +			test_kasumi_encryption_test_case_3),

You can combine both tests above, and have just one comment (or go for a single comment overall).

> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +			test_kasumi_auth_cipher_test_case_1),
> +		TEST_CASE_ST(ut_setup, ut_teardown,
> +			test_kasumi_cipher_auth_test_case_1),
> +
>  		TEST_CASES_END() /**< NULL terminate unit test array */
>  	}
>  };

...

> diff --git a/app/test/test_cryptodev_kasumi_hash_test_vectors.h
> b/app/test/test_cryptodev_kasumi_hash_test_vectors.h
> index c080b9f..a4b0205 100644
> --- a/app/test/test_cryptodev_kasumi_hash_test_vectors.h
> +++ b/app/test/test_cryptodev_kasumi_hash_test_vectors.h
> @@ -257,4 +257,80 @@ struct kasumi_hash_test_data
> kasumi_hash_test_case_5 = {
>  		.len  = 4
>  	}
>  };

Add blank line here.

> +struct kasumi_hash_test_data kasumi_hash_test_case_6 = {
> +	.key = {
> +		.data = {
> +			0x83, 0xFD, 0x23, 0xA2, 0x44, 0xA7, 0x4C, 0xF3,
> +			0x58, 0xDA, 0x30, 0x19, 0xF1, 0x72, 0x26, 0x35
> +		},
> +		.len = 16
> +	},
> +	.aad = {
> +		.data = {
> +			0x36, 0xAF, 0x61, 0x44, 0x4F, 0x30, 0x2A, 0xD2
> +		},
> +		.len = 8
> +	},
> +	.plaintext = {
> +		.data = {
> +			0x35, 0xC6, 0x87, 0x16, 0x63, 0x3C, 0x66, 0xFB,
> +			0x75, 0x0C, 0x26, 0x68, 0x65, 0xD5, 0x3C, 0x11,
> +			0xEA, 0x05, 0xB1, 0xE9, 0xFA, 0x49, 0xC8, 0x39,
> +			0x8D, 0x48, 0xE1, 0xEF, 0xA5, 0x90, 0x9D, 0x39,
> +			0x47, 0x90, 0x28, 0x37, 0xF5, 0xAE, 0x96, 0xD5,
> +			0xA0, 0x5B, 0xC8, 0xD6, 0x1C, 0xA8, 0xDB, 0xEF,
> +			0x1B, 0x13, 0xA4, 0xB4, 0xAB, 0xFE, 0x4F, 0xB1,
> +			0x00, 0x60, 0x45, 0xB6, 0x74, 0xBB, 0x54, 0x72,
> +			0x93, 0x04, 0xC3, 0x82, 0xBE, 0x53, 0xA5, 0xAF,
> +			0x05, 0x55, 0x61, 0x76, 0xF6, 0xEA, 0xA2, 0xEF,
> +			0x1D, 0x05, 0xE4, 0xB0, 0x83, 0x18, 0x1E, 0xE6,
> +			0x74, 0xCD, 0xA5, 0xA4, 0x85, 0xF7, 0x4D, 0x7A,
> +			0xC0
> +			},

Remove one extra tab before the brace.

> +		.len = 776
> +	},
> +	.validAuthLenInBits = {
> +		.len = 768
> +	},
> +	.validAuthOffsetLenInBits = {
> +		.len = 64
> +	},
> +	.digest = {
> +		.data = {0x95, 0xAE, 0x41, 0xBA},
> +		.len  = 4
> +	}
> +};
> +
> +struct kasumi_hash_test_data kasumi_hash_test_case_7 = {
> +	.key = {
> +		.data = {
> +			0x5A, 0xCB, 0x1D, 0x64, 0x4C, 0x0D, 0x51, 0x20,
> +			0x4E, 0xA5, 0xF1, 0x45, 0x10, 0x10, 0xD8, 0x52
> +		},
> +		.len = 16
> +	},
> +	.aad = {
> +		.data = {
> +			0x38, 0xA6, 0xF0, 0x56, 0x05, 0xD2, 0xEC, 0x49,
> +		},
> +		.len = 8
> +	},
> +	.plaintext = {
> +		.data = {
> +				0xAD, 0x9C, 0x44, 0x1F, 0x89, 0x0B, 0x38,
> 0xC4,
> +				0x57, 0xA4, 0x9D, 0x42, 0x14, 0x07, 0xE8,
> 0xC0

Remove extra tab here.

> +		},
> +		.len = 128
> +	},
> +	.validAuthLenInBits = {
> +		.len = 120
> +	},
> +	.validAuthOffsetLenInBits = {
> +		.len = 64
> +	},
> +	.digest = {
> +		.data = {0x87, 0x5F, 0xE4, 0x89},
> +		.len  = 4
> +	}
> +};
>  #endif /* TEST_CRYPTODEV_KASUMI_HASH_TEST_VECTORS_H_ */
> diff --git a/app/test/test_cryptodev_kasumi_test_vectors.h
> b/app/test/test_cryptodev_kasumi_test_vectors.h
> index 9163d7c..016af54 100644
> --- a/app/test/test_cryptodev_kasumi_test_vectors.h
> +++ b/app/test/test_cryptodev_kasumi_test_vectors.h

...

>  struct kasumi_test_data kasumi_test_case_1 = {
> @@ -183,12 +208,18 @@ struct kasumi_test_data kasumi_test_case_3 = {
>  		},
>  		.len = 8
>  	},
> +	.aad = {
> +		.data = {
> +			0x38, 0xA6, 0xF0, 0x56, 0x05, 0xD2, 0xEC, 0x49
> +		},
> +		.len = 8
> +	},
>  	.plaintext = {
>  		.data = {
>  			0xAD, 0x9C, 0x44, 0x1F, 0x89, 0x0B, 0x38, 0xC4,
> -			0x57, 0xA4, 0x9D, 0x42, 0x14, 0x07, 0xE8
> +			0x57, 0xA4, 0x9D, 0x42, 0x14, 0x07, 0xE8, 0xC0

Might be a good idea to include in the declaration of the structure "kasumi_test_data"
that plaintext may include the direction bit, so it explains the extra byte here (like in the hash vectors),
as you are using this for authentication too.

>  		},
> -		.len = 120
> +		.len = 128
>  	},
>  	.ciphertext = {
>  		.data = {
> @@ -197,11 +228,24 @@ struct kasumi_test_data kasumi_test_case_3 = {
>  		},
>  		.len = 120
>  	},
> +	.validDataLenInBits = {
> +			.len = 128
> +	},
>  	.validCipherLenInBits = {
>  		.len = 120
>  	},
>  	.validCipherOffsetLenInBits = {
>  		.len = 64
> +	},
> +	.validAuthLenInBits = {
> +		.len = 120
> +	},
> +	.validAuthOffsetLenInBits = {
> +		.len = 64
> +	},
> +	.digest = {
> +		.data = {0x87, 0x5F, 0xE4, 0x89},
> +		.len  = 4
>  	}
>  };