Re: [dpdk-dev] [PATCH 03/20] security: add hfn override option in PDCP

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Anoob,

Thanks for review.
> > > @@ -270,6 +270,8 @@ struct rte_security_pdcp_xform {
> > >  	uint32_t hfn;
> > >  	/** HFN Threshold for key renegotiation */
> > >  	uint32_t hfn_threshold;
> > > +	/** Enable per packet HFN override */
> > > +	uint32_t hfn_ovrd;
> 
> [Anoob] I think you should document the fact that IV field will be used for HFN.
> Your patch description accurately describes the procedure but the above
> comment fails to capture it. Also I would suggest renaming "hfn_ovrd" to
> something else to make it obvious that IV field is being used. Something like,
> use_iv_for_hfn or something.

Will add comments here.
        /** HFN can be given as a per packet value also.
         * As we do not have IV in case of PDCP, and HFN is
         * used to generate IV. IV field can be used to get the
         * per packet HFN while enq/deq.
         * If hfn_ovrd field is set, user is expected to set the
         * per packet HFN in place of IV. PMDs will extract the HFN
         * and perform operations accordingly.
         */
But using a different name may not be useful. Here we want to specify that
HFN can be overridden from the per packet value.
Now the usage is explained in the comments. I believe hfn_ovrd is enough to
explain the intent. Though not a very strong opinion on this.

Will send the v2 shortly.

> 
> Otherwise, I don't see any issues with the approach.
> 
> > >  };
> > >
> > >  /**
> > > --
> > > 2.17.1