RE: [PATCH 3/9] common/dpaax: support 12bit SN in pdcp uplane

[Gagandeep Singh <G.Singh@nxp.com>]

Hi,

> -----Original Message-----
> From: Hemant Agrawal (OSS) <hemant.agrawal@oss.nxp.com>
> Sent: Tuesday, May 20, 2025 11:49 AM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@nxp.com>
> Subject: Re: [PATCH 3/9] common/dpaax: support 12bit SN in pdcp uplane
> 
> 
> On 20-05-2025 11:21, Gagandeep Singh wrote:
> > update pdcp_insert_uplane_no_int_op() to support 12bit SN size
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > ---
> >   drivers/common/dpaax/caamflib/desc/pdcp.h | 13 ++++++-------
> >   1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/common/dpaax/caamflib/desc/pdcp.h
> b/drivers/common/dpaax/caamflib/desc/pdcp.h
> > index c90eff26a8..55159d53f0 100644
> > --- a/drivers/common/dpaax/caamflib/desc/pdcp.h
> > +++ b/drivers/common/dpaax/caamflib/desc/pdcp.h
> > @@ -1988,10 +1988,10 @@ pdcp_insert_uplane_no_int_op(struct program *p,
> >   		return 0;
> >   	}
> >
> > -	if (sn_size == PDCP_SN_SIZE_15) {
> > +	if (sn_size == PDCP_SN_SIZE_12) {
> >   		SEQLOAD(p, MATH0, 6, 2, 0);
> > -		sn_mask = (swap == false) ? PDCP_U_PLANE_15BIT_SN_MASK :
> > -					PDCP_U_PLANE_15BIT_SN_MASK_BE;
> > +		sn_mask = (swap == false) ? PDCP_12BIT_SN_MASK :
> > +			PDCP_12BIT_SN_MASK_BE;
> how the 15 bit will be handled now?
Just above this change, we are handling 15bit case via PROTOCOL instruction:
if ((sn_size == PDCP_SN_SIZE_15)) {
                PROTOCOL(p, dir, OP_PCLID_LTE_PDCP_USER,
                         (uint16_t)cipherdata->algtype);
                return 0;
        }

> >   	} else { /* SN Size == PDCP_SN_SIZE_18 */
> >   		SEQLOAD(p, MATH0, 5, 3, 0);
> >   		sn_mask = (swap == false) ? PDCP_U_PLANE_18BIT_SN_MASK :
> > @@ -2000,7 +2000,7 @@ pdcp_insert_uplane_no_int_op(struct program *p,
> >   	JUMP(p, 1, LOCAL_JUMP, ALL_TRUE, CALM);
> >   	MATHB(p, MATH0, AND, sn_mask, MATH1, 8, IFB | IMMED2);
> >
> > -	if (sn_size == PDCP_SN_SIZE_15)
> > +	if (sn_size == PDCP_SN_SIZE_12)
> >   		SEQSTORE(p, MATH0, 6, 2, 0);
> >   	else /* SN Size == PDCP_SN_SIZE_18 */
> >   		SEQSTORE(p, MATH0, 5, 3, 0);
> > @@ -2047,7 +2047,7 @@ pdcp_insert_uplane_no_int_op(struct program *p,
> >
> >   	default:
> >   		pr_err("%s: Invalid encrypt algorithm selected: %d\n",
> > -		       "pdcp_insert_uplane_15bit_op", cipherdata->algtype);
> > +		       "pdcp_insert_uplane_no_int_op", cipherdata->algtype);
> 
> Why not use DPDK LOG APIs?
> 
> also, why not use __func__ ?

pr_err is defined as RTE_LOG in drivers/common/dpaax/caamflib/compat.h
and __func__ is part of it.
> 
> >   		return -EINVAL;
> >   	}
> >
> > @@ -2734,8 +2734,7 @@ cnstr_shdsc_pdcp_u_plane_encap(uint32_t
> *descbuf,
> >   					cipherdata->algtype ==
> PDCP_CIPHER_TYPE_AES
> >   					&& !authdata
> >   					&& sn_size == PDCP_SN_SIZE_12) {
> > -				err = pdcp_insert_cplane_enc_only_op(p, swap,
> cipherdata,
> > -						authdata,
> > +				err = pdcp_insert_uplane_no_int_op(p, swap,
> cipherdata,
> >   						OP_TYPE_ENCAP_PROTOCOL,
> sn_size);
> >   				if (err)
> >   					return err;