[PATCH v2 09/22] app/test: add lib pdcp tests

[Anoob Joseph]

Hi Akhil,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: Akhil Goyal <gakhil at marvell.com>
> Sent: Thursday, May 18, 2023 1:34 PM
> To: Anoob Joseph <anoobj at marvell.com>; Thomas Monjalon
> <thomas at monjalon.net>; Jerin Jacob Kollanukkaran <jerinj at marvell.com>;
> Konstantin Ananyev <konstantin.v.ananyev at yandex.ru>; Bernard
> Iremonger <bernard.iremonger at intel.com>
> Cc: Hemant Agrawal <hemant.agrawal at nxp.com>; Mattias Rönnblom
> <mattias.ronnblom at ericsson.com>; Kiran Kumar Kokkilagadda
> <kirankumark at marvell.com>; Volodymyr Fialko <vfialko at marvell.com>;
> dev at dpdk.org; Olivier Matz <olivier.matz at 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.