> > > 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.
>
In that case, it can be a separate patch. But not in this patch.
> >
> >
> > > + 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.
In auth_xform, digest_length is the expected length of digest coming out of
crypto device.
Now if the device is expected to support PDCP, and we are reusing the crypto
APIs,
We can specify the digest length required for NULL auth.
The PMDs capability can be updated accordingly.
You can add a comment in the rte_crypto_auth_xform specifically for NULL auth
for PDCP case.
The reason, I am insisting on this is, for the user, while configuring
auth_xform,
it is setting digest length as 0 and when the packet is received the packet
length
is increased by digest. This will create confusion.
And it will also help in identifying the case of no integrity and null
integrity.
>
> > 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?
I am ok either way. Having defined in library would be better, to be used in
lib and PMD as well.
>
> > > +
> > > + 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.
This count value is being used to establish entity. I am saying, instead of
taking
Count, take sn as input and in the libpdcp combine pdcp_xfrm.hfn and sn as
needed
to create count and store in entity_priv.
Just wanted to reduce the application headache to make bitshifting and ORing to
SN.
