-----Original Message-----
> Date: Tue, 30 Oct 2018 13:53:30 +0000
> From: "Ananyev, Konstantin" <konstantin.anan...@intel.com>
> To: Jerin Jacob <jerin.ja...@caviumnetworks.com>
> CC: "dev@dpdk.org" <dev@dpdk.org>, "Awal, Mohammad Abdul"
> <mohammad.abdul.a...@intel.com>, "Joseph, Anoob"
> <anoob.jos...@cavium.com>, "Athreya, Narayana Prasad"
> <narayanaprasad.athr...@cavium.com>
> Subject: RE: [dpdk-dev] [RFC v2 5/9] ipsec: add SA data-path API
>
>
> Hi Jerin,
>
>
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * Checks that inside given rte_ipsec_session crypto/security
> > > > > > > fields
> > > > > > > + * are filled correctly and setups function pointers based on
> > > > > > > these values.
> > > > > > > + * @param ss
> > > > > > > + * Pointer to the *rte_ipsec_session* object
> > > > > > > + * @return
> > > > > > > + * - Zero if operation completed successfully.
> > > > > > > + * - -EINVAL if the parameters are invalid.
> > > > > > > + */
> > > > > > > +int __rte_experimental
> > > > > > > +rte_ipsec_session_prepare(struct rte_ipsec_session *ss);
> > > > > > > +
> > > > > > > +/**
> > > > > > > + * For input mbufs and given IPsec session prepare crypto ops
> > > > > > > that can be
> > > > > > > + * enqueued into the cryptodev associated with given session.
> > > > > > > + * expects that for each input packet:
> > > > > > > + * - l2_len, l3_len are setup correctly
> > > > > > > + * Note that erroneous mbufs are not freed by the function,
> > > > > > > + * but are placed beyond last valid mbuf in the *mb* array.
> > > > > > > + * It is a user responsibility to handle them further.
> > > > > > > + * @param ss
> > > > > > > + * Pointer to the *rte_ipsec_session* object the packets
> > > > > > > belong to.
> > > > > > > + * @param mb
> > > > > > > + * The address of an array of *num* pointers to *rte_mbuf*
> > > > > > > structures
> > > > > > > + * which contain the input packets.
> > > > > > > + * @param cop
> > > > > > > + * The address of an array of *num* pointers to the output
> > > > > > > *rte_crypto_op*
> > > > > > > + * structures.
> > > > > > > + * @param num
> > > > > > > + * The maximum number of packets to process.
> > > > > > > + * @return
> > > > > > > + * Number of successfully processed packets, with error code
> > > > > > > set in rte_errno.
> > > > > > > + */
> > > > > > > +static inline uint16_t __rte_experimental
> > > > > > > +rte_ipsec_crypto_prepare(const struct rte_ipsec_session *ss,
> > > > > > > + struct rte_mbuf *mb[], struct rte_crypto_op *cop[],
> > > > > > > uint16_t num)
> > > > > > > +{
> > > > > > > + return ss->func.prepare(ss, mb, cop, num);
> > > > > > > +}
> > > > > > > +
> > > > > > static inline uint16_t __rte_experimental
> > > > > > rte_ipsec_event_process(const struct rte_ipsec_session *ss, struct
> > > > > > rte_event *ev[], uint16_t num)
> > > > > > {
> > > > > > return ss->func.event_process(ss, ev, num);
> > > > > > }
> > > > >
> > > > > To fulfill that, we can either have 2 separate function pointers:
> > > > > uint16_t (*pkt_process)( const struct rte_ipsec_session *ss, struct
> > > > > rte_mbuf *mb[],uint16_t num);
> > > > > uint16_t (*event_process)( const struct rte_ipsec_session *ss, struct
> > > > > rte_event *ev[],uint16_t num);
> > > > >
> > > > > Or we can keep one function pointer, but change it to accept just
> > > > > array of pointers:
> > > > > uint16_t (*process)( const struct rte_ipsec_session *ss, void
> > > > > *in[],uint16_t num);
> > > > > and then make session_prepare() to choose a proper function based on
> > > > > input.
> > > > >
> > > > > I am ok with both schemes, but second one seems a bit nicer to me.
> > > >
> > > > How about best of both worlds, i.e save space and enable compile check
> > > > by anonymous union of both functions
> > > >
> > > > RTE_STD_C11
> > > > union {
> > > > uint16_t (*pkt_process)( const struct rte_ipsec_session
> > > > *ss,struct rte_mbuf *mb[],uint16_t num);
> > > > uint16_t (*event_process)( const struct rte_ipsec_session *ss,
> > > > struct rte_event *ev[],uint16_t num);
> > > > };
> > > >
> > >
> > > Yes, it is definitely possible, but then we still need 2 API functions,
> > > Depending on input type, i.e:
> > >
> > > static inline uint16_t __rte_experimental
> > > rte_ipsec_event_process(const struct rte_ipsec_session *ss, struct
> > > rte_event *ev[], uint16_t num)
> > > {
> > > return ss->func.event_process(ss, ev, num);
> > > }
> > >
> > > static inline uint16_t __rte_experimental
> > > rte_ipsec_pkt_process(const struct rte_ipsec_session *ss, struct rte_mbuf
> > > *mb[], uint16_t num)
> > > {
> > > return ss->func.pkt_process(ss, mb, num);
> > > }
> > >
> > > While if we'll have void *[], we can have just one function for both
> > > cases:
> > >
> > > static inline uint16_t __rte_experimental
> > > rte_ipsec_process(const struct rte_ipsec_session *ss, void *in[],
> > > uint16_t num)
> > > {
> > > return ss->func.process(ss, in, num);
> > > }
> >
> > Since it will be called from different application code path. I would
> > prefer to have separate functions to allow strict compiler check.
> >
>
> Ok, let's keep them separate, NP with that.
> I'll rename ipsec_(prepare|process) to ipsec_pkt_(prepare_process),
> so you guys can add '_event_' functions later.
OK
> Konstantin
>
