From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [RFC v2 5/9] ipsec: add SA data-path API Date: Wed, 31 Oct 2018 06:37:06 +0000 Message-ID: <20181031063653.GA22275@jerin> References: <1535129598-27301-1-git-send-email-konstantin.ananyev@intel.com> <1539109420-13412-6-git-send-email-konstantin.ananyev@intel.com> <20181018173745.GA14157@jerin> <2601191342CEEE43887BDE71AB9772580102FEA53E@IRSMSX106.ger.corp.intel.com> <20181024120346.GA15208@jerin> <2601191342CEEE43887BDE71AB9772580103064BF1@irsmsx105.ger.corp.intel.com> <20181029101905.GB4738@jerin> <2601191342CEEE43887BDE71AB97725801030661D4@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "Awal, Mohammad Abdul" , "Joseph, Anoob" , "Athreya, Narayana Prasad" To: "Ananyev, Konstantin" Return-path: Received: from NAM04-SN1-obe.outbound.protection.outlook.com (mail-eopbgr700069.outbound.protection.outlook.com [40.107.70.69]) by dpdk.org (Postfix) with ESMTP id 7137ADED for ; Wed, 31 Oct 2018 07:37:08 +0100 (CET) In-Reply-To: <2601191342CEEE43887BDE71AB97725801030661D4@irsmsx105.ger.corp.intel.com> Content-Language: en-US Content-ID: <20E8DA66848F71438A91CFF4E0FDA8A9@namprd07.prod.outlook.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" -----Original Message----- > Date: Tue, 30 Oct 2018 13:53:30 +0000 > From: "Ananyev, Konstantin" > To: Jerin Jacob > CC: "dev@dpdk.org" , "Awal, Mohammad Abdul" > , "Joseph, Anoob" > , "Athreya, Narayana Prasad" > > Subject: RE: [dpdk-dev] [RFC v2 5/9] ipsec: add SA data-path API >=20 >=20 > Hi Jerin, >=20 >=20 > > > > > > > + > > > > > > > +/** > > > > > > > + * Checks that inside given rte_ipsec_session crypto/securit= y fields > > > > > > > + * are filled correctly and setups function pointers based o= n 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 op= s 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 b= elong 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 co= de 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[], u= int16_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, str= uct 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, stru= ct rte_mbuf *mb[],uint16_t num); > > > > > uint16_t (*event_process)( const struct rte_ipsec_session *ss, st= ruct 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 ch= eck > > > > by anonymous union of both functions > > > > > > > > RTE_STD_C11 > > > > union { > > > > uint16_t (*pkt_process)( const struct rte_ipsec_session *ss,s= truct 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 function= s, > > > Depending on input type, i.e: > > > > > > static inline uint16_t __rte_experimental > > > rte_ipsec_event_process(const struct rte_ipsec_session *ss, struct rt= e_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[], uin= t16_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. > > >=20 > 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 >=20