DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] app/test-crypto-perf: fix segment size for IPsec operation
@ 2021-11-16 10:23 Gagandeep Singh
  2021-11-16 10:37 ` [PATCH v2] " Gagandeep Singh
  0 siblings, 1 reply; 8+ messages in thread
From: Gagandeep Singh @ 2021-11-16 10:23 UTC (permalink / raw)
  To: gakhil, dev; +Cc: 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-16 10:23 [PATCH] app/test-crypto-perf: fix segment size for IPsec operation Gagandeep Singh
@ 2021-11-16 10:37 ` Gagandeep Singh
  2021-11-16 10:42   ` [EXT] " Akhil Goyal
  2021-11-17  7:40   ` [PATCH v3] " Gagandeep Singh
  0 siblings, 2 replies; 8+ messages in thread
From: Gagandeep Singh @ 2021-11-16 10:37 UTC (permalink / raw)
  To: gakhil, dev; +Cc: 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-16 10:37 ` [PATCH v2] " Gagandeep Singh
@ 2021-11-16 10:42   ` Akhil Goyal
  2021-11-16 11:47     ` Gagandeep Singh
  2021-11-17  7:40   ` [PATCH v3] " Gagandeep Singh
  1 sibling, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2021-11-16 10:42 UTC (permalink / raw)
  To: Gagandeep Singh, dev

> 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-16 10:42   ` [EXT] " Akhil Goyal
@ 2021-11-16 11:47     ` Gagandeep Singh
  2021-11-17  6:36       ` Akhil Goyal
  0 siblings, 1 reply; 8+ messages in thread
From: Gagandeep Singh @ 2021-11-16 11:47 UTC (permalink / raw)
  To: Akhil Goyal, dev



> -----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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-16 11:47     ` Gagandeep Singh
@ 2021-11-17  6:36       ` Akhil Goyal
  2021-11-17  7:00         ` Gagandeep Singh
  0 siblings, 1 reply; 8+ messages in thread
From: Akhil Goyal @ 2021-11-17  6:36 UTC (permalink / raw)
  To: Gagandeep Singh, dev

> > 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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXT] [PATCH v2] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-17  6:36       ` Akhil Goyal
@ 2021-11-17  7:00         ` Gagandeep Singh
  0 siblings, 0 replies; 8+ messages in thread
From: Gagandeep Singh @ 2021-11-17  7:00 UTC (permalink / raw)
  To: Akhil Goyal, dev



> -----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.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-16 10:37 ` [PATCH v2] " Gagandeep Singh
  2021-11-16 10:42   ` [EXT] " Akhil Goyal
@ 2021-11-17  7:40   ` Gagandeep Singh
  2021-11-23 19:49     ` [EXT] " Akhil Goyal
  1 sibling, 1 reply; 8+ messages in thread
From: Gagandeep Singh @ 2021-11-17  7:40 UTC (permalink / raw)
  To: gakhil, dev; +Cc: 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [EXT] [PATCH v3] app/test-crypto-perf: fix segment size for IPsec operation
  2021-11-17  7:40   ` [PATCH v3] " Gagandeep Singh
@ 2021-11-23 19:49     ` Akhil Goyal
  0 siblings, 0 replies; 8+ messages in thread
From: Akhil Goyal @ 2021-11-23 19:49 UTC (permalink / raw)
  To: Gagandeep Singh, dev

> 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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-11-23 19:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 10:23 [PATCH] app/test-crypto-perf: fix segment size for IPsec operation Gagandeep Singh
2021-11-16 10:37 ` [PATCH v2] " Gagandeep Singh
2021-11-16 10:42   ` [EXT] " Akhil Goyal
2021-11-16 11:47     ` Gagandeep Singh
2021-11-17  6:36       ` Akhil Goyal
2021-11-17  7:00         ` Gagandeep Singh
2021-11-17  7:40   ` [PATCH v3] " Gagandeep Singh
2021-11-23 19:49     ` [EXT] " Akhil Goyal

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git