[v2] app/test-crypto-perf: fix segment size for IPsec operation
Checks
Commit Message
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(-)
Comments
> 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
> -----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
> > 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.
> -----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.
@@ -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,
@@ -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,