Hi Akhil,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Akhil Goyal <gak...@marvell.com>
> Sent: Thursday, May 18, 2023 1:34 PM
> To: Anoob Joseph <ano...@marvell.com>; Thomas Monjalon
> <tho...@monjalon.net>; Jerin Jacob Kollanukkaran <jer...@marvell.com>;
> Konstantin Ananyev <konstantin.v.anan...@yandex.ru>; Bernard
> Iremonger <bernard.iremon...@intel.com>
> Cc: Hemant Agrawal <hemant.agra...@nxp.com>; Mattias Rönnblom
> <mattias.ronnb...@ericsson.com>; Kiran Kumar Kokkilagadda
> <kirankum...@marvell.com>; Volodymyr Fialko <vfia...@marvell.com>;
> dev@dpdk.org; Olivier Matz <olivier.m...@6wind.com>
> Subject: RE: [PATCH v2 09/22] app/test: add lib pdcp tests
>
> > diff --git a/app/test/meson.build b/app/test/meson.build index
> > 52d9088578..0f658aa2ab 100644
> > --- a/app/test/meson.build
> > +++ b/app/test/meson.build
> > @@ -96,6 +96,7 @@ test_sources = files(
> > 'test_meter.c',
> > 'test_mcslock.c',
> > 'test_mp_secondary.c',
> > + 'test_pdcp.c',
> > 'test_per_lcore.c',
> > 'test_pflock.c',
> > 'test_pmd_perf.c',
> > diff --git a/app/test/test_cryptodev.h b/app/test/test_cryptodev.h
> > index abd795f54a..89057dba22 100644
> > --- a/app/test/test_cryptodev.h
> > +++ b/app/test/test_cryptodev.h
> > @@ -4,6 +4,9 @@
> > #ifndef TEST_CRYPTODEV_H_
> > #define TEST_CRYPTODEV_H_
> >
> > +#include <rte_crypto.h>
> > +#include <rte_cryptodev.h>
> > +
>
> Can we remove these includes from here and add in test_pdcp.c directly?
[Anoob] Why? 'test_cryptodev.h' already has many references to rte_cryptodev
symbols. Not including the dependencies is not correct.
>
>
> > + if (conf->is_integrity_protected) {
> > + if (conf->entity.pdcp_xfrm.pkt_dir ==
> > RTE_SECURITY_PDCP_UPLINK) {
> > + conf->entity.crypto_xfrm = &conf->a_xfrm;
> > +
> > + a_xfrm.auth.op =
> RTE_CRYPTO_AUTH_OP_GENERATE;
> > + a_xfrm.next = &conf->c_xfrm;
> > +
> > + c_xfrm.cipher.op =
> > RTE_CRYPTO_CIPHER_OP_ENCRYPT;
> > + c_xfrm.next = NULL;
> > + } else {
> > + conf->entity.crypto_xfrm = &conf->c_xfrm;
> > +
> > + c_xfrm.cipher.op =
> > RTE_CRYPTO_CIPHER_OP_DECRYPT;
> > + c_xfrm.next = &conf->a_xfrm;
> > +
> > + a_xfrm.auth.op = RTE_CRYPTO_AUTH_OP_VERIFY;
> > + a_xfrm.next = NULL;
> > + }
> > + } else {
> > + conf->entity.crypto_xfrm = &conf->c_xfrm;
> > + c_xfrm.next = NULL;
> > +
> > + if (conf->entity.pdcp_xfrm.pkt_dir ==
> > RTE_SECURITY_PDCP_UPLINK)
> > + c_xfrm.cipher.op =
> > RTE_CRYPTO_CIPHER_OP_ENCRYPT;
> > + else
> > + c_xfrm.cipher.op =
> > RTE_CRYPTO_CIPHER_OP_DECRYPT;
> > + }
> > + /* Update xforms to match PDCP requirements */
> > +
> > + if ((c_xfrm.cipher.algo == RTE_CRYPTO_CIPHER_AES_CTR) ||
> > + (c_xfrm.cipher.algo == RTE_CRYPTO_CIPHER_ZUC_EEA3 ||
> > + (c_xfrm.cipher.algo == RTE_CRYPTO_CIPHER_SNOW3G_UEA2)))
> > + c_xfrm.cipher.iv.length = 16;
> > + else
> > + c_xfrm.cipher.iv.length = 0;
> > +
> > + if (conf->is_integrity_protected) {
> > + if (a_xfrm.auth.algo == RTE_CRYPTO_AUTH_NULL)
> > + a_xfrm.auth.digest_length = 0;
> > + else
> > + a_xfrm.auth.digest_length = 4;
>
> This if-else is not needed. If is_integrity_protected, digest_length should
> always be 4.
[Anoob] I had explained this in v1 patch set. Will try again.
In PDCP, with AUTH_NULL it is expected to have 4 bytes of zeroized digest.
With AUTH_NULL, it is lib PDCP which would add zeroized digest. No PMD
currently supported in DPDK supports non-zero digest with AUTH-NULL. Also, it
is not specified what is the digest added in case of AUTH-NULL.
> Also define a macro for MAC-I len. It is being used at multiple places.
> Similarly for IV length macro can be defined.
>
[Anoob] Agreed. You want me to introduce RTE_PDCP_MAC_I_LEN or something local
would do?
> > +
> > + if ((a_xfrm.auth.algo == RTE_CRYPTO_AUTH_ZUC_EIA3) ||
> > + (a_xfrm.auth.algo ==
> RTE_CRYPTO_AUTH_SNOW3G_UIA2))
> > + a_xfrm.auth.iv.length = 16;
> > + else
> > + a_xfrm.auth.iv.length = 0;
> > + }
> > +
> > + conf->c_xfrm = c_xfrm;
> > + conf->a_xfrm = a_xfrm;
> > +
> > + conf->entity.dev_id = (uint8_t)cryptodev_id_get(conf-
> > >is_integrity_protected,
> > + &conf->c_xfrm, &conf->a_xfrm);
> > +
> > + if (pdcp_test_params[index].domain ==
> > RTE_SECURITY_PDCP_MODE_CONTROL ||
> > + pdcp_test_params[index].domain ==
> > RTE_SECURITY_PDCP_MODE_DATA) {
> > + data = pdcp_test_data_in[index];
> > + hfn = pdcp_test_hfn[index] <<
> pdcp_test_data_sn_size[index];
> > + sn = pdcp_sn_from_raw_get(data,
> > pdcp_test_data_sn_size[index]);
> > + count = hfn | sn;
> > + }
>
> The above logic can go inside lib PDCP as well.
> This is specific to PDCP and not user dependent.
> You can reuse the pdcp_xfrm.hfn as well.
>
[Anoob] Sorry, what exactly can go into lib PDCP? This snippet is reading SN
used in a test vector and constructs the count based on SN & HFN value from
vector.
