[PATCH] app/test-crypto-perf: fix segment size for IPsec operation

[PATCH] app/test-crypto-perf: fix segment size for IPsec operation

[Gagandeep Singh]

Application calculates segment size based on buffer size plus
digest size only, But if the operation mode is IPsec then
packet length can be increased upto 73 bytes due to IPsec
overhead.

In this patch, adding the IPsec overhead length in segment size
when there is no user given segment size.

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
---
 app/test-crypto-perf/cperf_options.h         | 1 +
 app/test-crypto-perf/cperf_options_parsing.c | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h
index 031b238b20..cdbc027b89 100644
--- a/app/test-crypto-perf/cperf_options.h
+++ b/app/test-crypto-perf/cperf_options.h
@@ -61,6 +61,7 @@
 #define CPERF_PMDCC_DELAY_MS	("pmd-cyclecount-delay-ms")
 
 #define MAX_LIST 32
+#define CPERF_IPSEC_OVERHEAD 73
 
 enum cperf_perf_test_type {
 	CPERF_TEST_TYPE_THROUGHPUT,
diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-perf/cperf_options_parsing.c
index c244f81bbf..268f544936 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -1132,9 +1132,12 @@ cperf_options_check(struct cperf_options *options)
 	 * If segment size is not set, assume only one segment,
 	 * big enough to contain the largest buffer and the digest
 	 */
-	if (options->segment_sz == 0)
+	if (options->segment_sz == 0) {
 		options->segment_sz = options->max_buffer_size +
 				options->digest_sz;
+		if (options->op_type == CPERF_IPSEC)
+			options->segment_sz += CPERF_IPSEC_OVERHEAD;
+	}
 
 	if (options->segment_sz < options->digest_sz) {
 		RTE_LOG(ERR, USER1,
-- 
2.25.1

[PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation

[Gagandeep Singh]

Application calculates segment size based on buffer size plus
digest size only, But if the operation mode is IPsec then
packet length can be increased up to 73 bytes due to IPsec
overhead.

In this patch, adding the IPsec overhead length in segment size
when there is no user given segment size.

Fixes: 28dde5da503e ("app/crypto-perf: support lookaside IPsec")

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
---
v2-change-log:
Update commit message with fixline.

 app/test-crypto-perf/cperf_options.h         | 1 +
 app/test-crypto-perf/cperf_options_parsing.c | 5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-perf/cperf_options.h
index 031b238b20..cdbc027b89 100644
--- a/app/test-crypto-perf/cperf_options.h
+++ b/app/test-crypto-perf/cperf_options.h
@@ -61,6 +61,7 @@
 #define CPERF_PMDCC_DELAY_MS	("pmd-cyclecount-delay-ms")
 
 #define MAX_LIST 32
+#define CPERF_IPSEC_OVERHEAD 73
 
 enum cperf_perf_test_type {
 	CPERF_TEST_TYPE_THROUGHPUT,
diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-perf/cperf_options_parsing.c
index c244f81bbf..268f544936 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -1132,9 +1132,12 @@ cperf_options_check(struct cperf_options *options)
 	 * If segment size is not set, assume only one segment,
 	 * big enough to contain the largest buffer and the digest
 	 */
-	if (options->segment_sz == 0)
+	if (options->segment_sz == 0) {
 		options->segment_sz = options->max_buffer_size +
 				options->digest_sz;
+		if (options->op_type == CPERF_IPSEC)
+			options->segment_sz += CPERF_IPSEC_OVERHEAD;
+	}
 
 	if (options->segment_sz < options->digest_sz) {
 		RTE_LOG(ERR, USER1,
-- 
2.25.1

RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation

[Akhil Goyal]

> Application calculates segment size based on buffer size plus
> digest size only, But if the operation mode is IPsec then
> packet length can be increased up to 73 bytes due to IPsec
> overhead.
> 

Can you explain the calculation for 73 bytes in the code?
Will it be sufficient for IPv6?

> In this patch, adding the IPsec overhead length in segment size
> when there is no user given segment size.
> 
> Fixes: 28dde5da503e ("app/crypto-perf: support lookaside IPsec")
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> ---
> v2-change-log:
> Update commit message with fixline.
> 
>  app/test-crypto-perf/cperf_options.h         | 1 +
>  app/test-crypto-perf/cperf_options_parsing.c | 5 ++++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-
> perf/cperf_options.h
> index 031b238b20..cdbc027b89 100644
> --- a/app/test-crypto-perf/cperf_options.h
> +++ b/app/test-crypto-perf/cperf_options.h
> @@ -61,6 +61,7 @@
>  #define CPERF_PMDCC_DELAY_MS	("pmd-cyclecount-delay-ms")
> 
>  #define MAX_LIST 32
> +#define CPERF_IPSEC_OVERHEAD 73
> 
>  enum cperf_perf_test_type {
>  	CPERF_TEST_TYPE_THROUGHPUT,
> diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-
> perf/cperf_options_parsing.c
> index c244f81bbf..268f544936 100644
> --- a/app/test-crypto-perf/cperf_options_parsing.c
> +++ b/app/test-crypto-perf/cperf_options_parsing.c
> @@ -1132,9 +1132,12 @@ cperf_options_check(struct cperf_options
> *options)
>  	 * If segment size is not set, assume only one segment,
>  	 * big enough to contain the largest buffer and the digest
>  	 */
> -	if (options->segment_sz == 0)
> +	if (options->segment_sz == 0) {
>  		options->segment_sz = options->max_buffer_size +
>  				options->digest_sz;
> +		if (options->op_type == CPERF_IPSEC)
> +			options->segment_sz += CPERF_IPSEC_OVERHEAD;
> +	}
> 
>  	if (options->segment_sz < options->digest_sz) {
>  		RTE_LOG(ERR, USER1,
> --
> 2.25.1

RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation

[Gagandeep Singh]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Tuesday, November 16, 2021 4:12 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Subject: RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec
> operation
> 
> > Application calculates segment size based on buffer size plus
> > digest size only, But if the operation mode is IPsec then
> > packet length can be increased up to 73 bytes due to IPsec
> > overhead.
> >
> 
> Can you explain the calculation for 73 bytes in the code?
> Will it be sufficient for IPv6?
No, it will not cover IPv6, As currently only IPv4 test cases are there in the app.
But I guess it covers all the scenario which are supported by the app.
73 are the maximum bytes which can be added in 
AES - SHA  algo mode (41 +12 + 20 (including any padding)) 

But it will also not cover other complex scenario like NAT-T, AH.
I have verified this change with aes-cbc-hmac-sha1 and aes-gcm algos for 64,128,256, 512, 1280 bytes.

 What's your opinion on this? 

> 
> > In this patch, adding the IPsec overhead length in segment size
> > when there is no user given segment size.
> >
> > Fixes: 28dde5da503e ("app/crypto-perf: support lookaside IPsec")
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > ---
> > v2-change-log:
> > Update commit message with fixline.
> >
> >  app/test-crypto-perf/cperf_options.h         | 1 +
> >  app/test-crypto-perf/cperf_options_parsing.c | 5 ++++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-crypto-perf/cperf_options.h b/app/test-crypto-
> > perf/cperf_options.h
> > index 031b238b20..cdbc027b89 100644
> > --- a/app/test-crypto-perf/cperf_options.h
> > +++ b/app/test-crypto-perf/cperf_options.h
> > @@ -61,6 +61,7 @@
> >  #define CPERF_PMDCC_DELAY_MS	("pmd-cyclecount-delay-ms")
> >
> >  #define MAX_LIST 32
> > +#define CPERF_IPSEC_OVERHEAD 73
> >
> >  enum cperf_perf_test_type {
> >  	CPERF_TEST_TYPE_THROUGHPUT,
> > diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-
> > perf/cperf_options_parsing.c
> > index c244f81bbf..268f544936 100644
> > --- a/app/test-crypto-perf/cperf_options_parsing.c
> > +++ b/app/test-crypto-perf/cperf_options_parsing.c
> > @@ -1132,9 +1132,12 @@ cperf_options_check(struct cperf_options
> > *options)
> >  	 * If segment size is not set, assume only one segment,
> >  	 * big enough to contain the largest buffer and the digest
> >  	 */
> > -	if (options->segment_sz == 0)
> > +	if (options->segment_sz == 0) {
> >  		options->segment_sz = options->max_buffer_size +
> >  				options->digest_sz;
> > +		if (options->op_type == CPERF_IPSEC)
> > +			options->segment_sz += CPERF_IPSEC_OVERHEAD;
> > +	}
> >
> >  	if (options->segment_sz < options->digest_sz) {
> >  		RTE_LOG(ERR, USER1,
> > --
> > 2.25.1

RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation

[Akhil Goyal]

> > Can you explain the calculation for 73 bytes in the code?
> > Will it be sufficient for IPv6?
> No, it will not cover IPv6, As currently only IPv4 test cases are there in the
> app.
> But I guess it covers all the scenario which are supported by the app.
> 73 are the maximum bytes which can be added in
> AES - SHA  algo mode (41 +12 + 20 (including any padding))

Please explain and define macros appropriately so that we know what all is
Covered in 73Bytes. Or why not increase it by RTE_PKTMBUF_HEADROOM.

> 
> But it will also not cover other complex scenario like NAT-T, AH.
> I have verified this change with aes-cbc-hmac-sha1 and aes-gcm algos for
> 64,128,256, 512, 1280 bytes.
> 
>  What's your opinion on this?
> 
I believe using RTE_PKTMBUF_HEADROOM will resolve most of the scenarios.

RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation

[Gagandeep Singh]

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Wednesday, November 17, 2021 12:06 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Subject: RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec
> operation
> 
> > > Can you explain the calculation for 73 bytes in the code?
> > > Will it be sufficient for IPv6?
> > No, it will not cover IPv6, As currently only IPv4 test cases are there in the
> > app.
> > But I guess it covers all the scenario which are supported by the app.
> > 73 are the maximum bytes which can be added in
> > AES - SHA  algo mode (41 +12 + 20 (including any padding))
> 
> Please explain and define macros appropriately so that we know what all is
> Covered in 73Bytes. Or why not increase it by RTE_PKTMBUF_HEADROOM.
> 
> >
> > But it will also not cover other complex scenario like NAT-T, AH.
> > I have verified this change with aes-cbc-hmac-sha1 and aes-gcm algos for
> > 64,128,256, 512, 1280 bytes.
> >
> >  What's your opinion on this?
> >
> I believe using RTE_PKTMBUF_HEADROOM will resolve most of the scenarios.

Agree with you, I will verify it and send the next version soon.

[PATCH v3] app/test-crypto-perf: fix segment size for IPsec operation

[Gagandeep Singh]

Application calculates segment size based on buffer size plus
digest size only, But if the operation mode is IPsec then
packet length can be increased by some more bytes depends upon
the algorithm.

In this patch, increasing segment size with RTE_PKTMBUF_HEADROOM
when there is no user given segment size.

Fixes: 28dde5da503e ("app/crypto-perf: support lookaside IPsec")

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
---
v2-change-log:
    Update commit message with fixline.
v3-change-log:
    increasing the segment sizeby headroom size instead of
    an algo specific max overhead value.

 app/test-crypto-perf/cperf_options_parsing.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/test-crypto-perf/cperf_options_parsing.c b/app/test-crypto-perf/cperf_options_parsing.c
index c244f81bbf..771e116ebb 100644
--- a/app/test-crypto-perf/cperf_options_parsing.c
+++ b/app/test-crypto-perf/cperf_options_parsing.c
@@ -1132,9 +1132,17 @@ cperf_options_check(struct cperf_options *options)
 	 * If segment size is not set, assume only one segment,
 	 * big enough to contain the largest buffer and the digest
 	 */
-	if (options->segment_sz == 0)
+	if (options->segment_sz == 0) {
 		options->segment_sz = options->max_buffer_size +
 				options->digest_sz;
+		/* In IPsec operation, packet length will be increased
+		 * by some bytes depend upon the algorithm, so increasing
+		 * the segment size by headroom to cover the most of
+		 * the scenarios.
+		 */
+		if (options->op_type == CPERF_IPSEC)
+			options->segment_sz += RTE_PKTMBUF_HEADROOM;
+	}
 
 	if (options->segment_sz < options->digest_sz) {
 		RTE_LOG(ERR, USER1,
-- 
2.25.1

RE: [EXT] [PATCH v3] app/test-crypto-perf: fix segment size for IPsec operation

[Akhil Goyal]

> Application calculates segment size based on buffer size plus
> digest size only, But if the operation mode is IPsec then
> packet length can be increased by some more bytes depends upon
> the algorithm.
> 
> In this patch, increasing segment size with RTE_PKTMBUF_HEADROOM
> when there is no user given segment size.
> 
> Fixes: 28dde5da503e ("app/crypto-perf: support lookaside IPsec")
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
Applied to dpdk-next-crypto

Thanks.