From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0080.outbound.protection.outlook.com [104.47.38.80]) by dpdk.org (Postfix) with ESMTP id 40CC01B1A0 for ; Wed, 24 Oct 2018 14:04:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+n1tx2Gl2we4cxefHaAXoqMg2d/el0QIdpTg5ZycndI=; b=EbJDGFTcmwM/4d7BQ/gQbqp7aJ4J4fOBFIfx1aLhhhw/lPT89m6nzrZwCLMv+GHiRqDnCWoJOKL4NogsxxpHLXyyWSmWBeAb46QjIuL9YRQnlyVN7vFPOC4k0tAWrMK4tKJb89+90zZEj+0rqT/9M7GxkuXMKhwGzl+Y4q2598Y= Received: from BYAPR07MB4997.namprd07.prod.outlook.com (52.135.238.214) by BYAPR07MB5655.namprd07.prod.outlook.com (20.177.231.221) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1250.30; Wed, 24 Oct 2018 12:03:58 +0000 Received: from BYAPR07MB4997.namprd07.prod.outlook.com ([fe80::c5c:4d86:b353:175a]) by BYAPR07MB4997.namprd07.prod.outlook.com ([fe80::c5c:4d86:b353:175a%4]) with mapi id 15.20.1250.028; Wed, 24 Oct 2018 12:03:58 +0000 From: Jerin Jacob To: "Ananyev, Konstantin" CC: "dev@dpdk.org" , "Awal, Mohammad Abdul" , "Joseph, Anoob" , "Athreya, Narayana Prasad" Thread-Topic: [dpdk-dev] [RFC v2 5/9] ipsec: add SA data-path API Thread-Index: AQHUX/1c7MgCwavRd0ecHBGPDZ77+aUlrsgAgASkkACABA/qAA== Date: Wed, 24 Oct 2018 12:03:58 +0000 Message-ID: <20181024120346.GA15208@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> In-Reply-To: <2601191342CEEE43887BDE71AB9772580102FEA53E@IRSMSX106.ger.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [106.200.252.218] x-clientproxiedby: SG2PR06CA0126.apcprd06.prod.outlook.com (2603:1096:1:1d::28) To BYAPR07MB4997.namprd07.prod.outlook.com (2603:10b6:a03:5b::22) authentication-results: spf=none (sender IP is ) smtp.mailfrom=Jerin.JacobKollanukkaran@cavium.com; x-ms-exchange-messagesentrepresentingtype: 1 x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; BYAPR07MB5655; 6:sFzCtxARr1GtV9XNvMjIsL15W8fNETgBA7LdpVvcRalFIH59Nqhzcux6dZ5yG4vf986k/JuJt8tMw5Afc9fIyy85PaEsCQFPSJe0Y3U4eqO5zDXkitkc+CmiRht16uSRaemt44OmHcCB+VTyX2gOulBpqHYjSWdzkc2Q+zSp734TUJ/+DraYftZSuaqHo1dRotqEtDt1VEQNMnG3LQJ5X+uXK6kFlAks6ArCVc6hcXgGnjk9LyJh8Vw2sDf8zPsTEcmGfTH+3uAn5nXYSWxZn83bGzY4DztUdVP/CevCCY7MIHx9s4X+KFO2Wo2tWbFSzTH4qvQ+64jwsPgcMwDdJZSnNn9blYKu0JVfQy+ElrgNKjYGfgPPJxXPuQZmir6V8CYNZlxs0NEREfYfXjxu68mYpG+qnRxrLm/awLJsulD46Gks3UAUfgexant+74k6hoeb43ZKlRBlJHBmMoSipw==; 5:Sgid8X6AIjMYkN5IDpv4fG1Fp/i5PA1wF3rZLBsKDc8BVY1P3/VPT/bkZZhwV49HWuTGvuUs1myjSqoLf4YwJZ3LaY2RYmq+uZ5KnuX1pbDxVXE0xUZPiL0m5klRwEpwebmNiWLmOTjH5Tm/SmIp5stBVmFGP4Sz3lbCjCVpb0g=; 7:WlytKiyC8vIbXegC40m2v/zXz30JkGlc6ZpRp33RnyfEJYsD7H2FfeNRpBZCna85ZQXAZUP4yy9nc+mFOaAeW9C224wC6VN/jmvPb+bN6kmhMOQmheuabyQn5nct87wF0KjkFSkgGzB9TRN2PLWyljFl0jMXxeqI9INR2ROnupnzg95y9mu1RRbqUMwKD0o3SRHE2sFM8RWk1yUfb5Q1vSC8Ctskv2IRr1TTHJdbI+uQPX19EAmPxPH6Cnvk7/wd x-ms-office365-filtering-correlation-id: 3e783e15-b125-455b-0f70-08d639a8c6f2 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(2017052603328)(7153060)(7193020); SRVR:BYAPR07MB5655; x-ms-traffictypediagnostic: BYAPR07MB5655: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(17755550239193)(192374486261705)(163750095850)(228905959029699); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3002001)(3231355)(944501410)(52105095)(10201501046)(93006095)(148016)(149066)(150057)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123562045)(20161123564045)(20161123560045)(201708071742011)(7699051)(76991095); SRVR:BYAPR07MB5655; BCL:0; PCL:0; RULEID:; SRVR:BYAPR07MB5655; x-forefront-prvs: 083526BF8A x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(7916004)(136003)(376002)(396003)(346002)(366004)(39860400002)(13464003)(189003)(199004)(33716001)(71190400001)(81166006)(5250100002)(105586002)(11346002)(316002)(7736002)(229853002)(486006)(102836004)(305945005)(5660300001)(6116002)(446003)(25786009)(106356001)(42882007)(93886005)(6916009)(186003)(476003)(4326008)(2900100001)(54906003)(33656002)(3846002)(2906002)(26005)(1076002)(66066001)(6512007)(14444005)(76176011)(9686003)(97736004)(99286004)(72206003)(386003)(52116002)(6486002)(71200400001)(68736007)(478600001)(6506007)(6246003)(33896004)(81156014)(14454004)(107886003)(6436002)(53936002)(8676002)(256004)(8936002); DIR:OUT; SFP:1101; SCL:1; SRVR:BYAPR07MB5655; H:BYAPR07MB4997.namprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: GruMYVx5L4SLVKPb6URLbknPQuScu+sZhoIoeRnNWa3aZ9QE7ib8Rw0Sfc/TiVU3leuSIgbu1zxytztzlRBHEiNgP8eOg8Ekec0aqrwzCJYm+/1Y8zscXS43R26DqUoO7i1OuqGv6UsAFkTzJMmAec33heZvXjfW3gg8EeA13H++uKLTNGYXvxN6w2JOPnEAYZZHe0DIzheoJpaeRgYC+DxsKg24po0gPjElKe6p2wz0OHXHyzOnTsS5gYuRLYyaYHNOeMcZXMOf6eMNdyqJIFHIyl0RdBfKFAhSdKH8CaAQxhsdqXeootTb5BcTRo2lyu6OXcpg7Hdptupbn3QQG6NNIkqZPGXYCkfubVAk7wI= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <55B2E0106887B54597708BDCF1AF13B6@namprd07.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3e783e15-b125-455b-0f70-08d639a8c6f2 X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Oct 2018 12:03:58.0889 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR07MB5655 Subject: Re: [dpdk-dev] [RFC v2 5/9] ipsec: add SA data-path API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Oct 2018 12:04:00 -0000 -----Original Message----- > Date: Sun, 21 Oct 2018 22:01:48 +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, Hi Konstantin, >=20 > > > > > + > > > +/** > > > + * IPsec session specific functions that will be used to: > > > + * - prepare - for input mbufs and given IPsec session prepare crypt= o ops > > > + * that can be enqueued into the cryptodev associated with given s= ession > > > + * (see *rte_ipsec_crypto_prepare* below for more details). > > > + * - process - finalize processing of packets after crypto-dev finis= hed > > > + * with them or process packets that are subjects to inline IPsec = offload > > > + * (see rte_ipsec_process for more details). > > > + */ > > > +struct rte_ipsec_sa_func { > > > + uint16_t (*prepare)(const struct rte_ipsec_session *ss, > > > + struct rte_mbuf *mb[], > > > + struct rte_crypto_op *cop[], > > > + uint16_t num); > > > + uint16_t (*process)(const struct rte_ipsec_session *ss, > > > + struct rte_mbuf *mb[], > > > + uint16_t num); > > > > IMO, It makes sense to have separate function pointers for inbound and > > outbound so that, implementation would be clean and we can avoid a > > "if" check. >=20 > SA object by itself is unidirectional (either inbound or outbound), so > I don't think we need 2 function pointers here. > Yes, right now, inside ipsec lib we select functions by action_type only, > but it doesn't have to stay that way. > It could be action_type, direction, mode (tunnel/transport), event/poll, = etc. > Let say inline_proto_process() could be split into: > inline_proto_outb_process() and inline_proto_inb_process() and > rte_ipsec_sa_func.process will point to appropriate one. > I probably will change things that way for next version. OK >=20 > > > > > +}; > > > + > > > +/** > > > + * rte_ipsec_session is an aggregate structure that defines particul= ar > > > + * IPsec Security Association IPsec (SA) on given security/crypto de= vice: > > > + * - pointer to the SA object > > > + * - security session action type > > > + * - pointer to security/crypto session, plus other related data > > > + * - session/device specific functions to prepare/process IPsec pack= ets. > > > + */ > > > +struct rte_ipsec_session { > > > + > > > + /** > > > + * SA that session belongs to. > > > + * Note that multiple sessions can belong to the same SA. > > > + */ > > > + struct rte_ipsec_sa *sa; > > > + /** session action type */ > > > + enum rte_security_session_action_type type; > > > + /** session and related data */ > > > + union { > > > + struct { > > > + struct rte_cryptodev_sym_session *ses; > > > + } crypto; > > > + struct { > > > + struct rte_security_session *ses; > > > + struct rte_security_ctx *ctx; > > > + uint32_t ol_flags; > > > + } security; > > > + }; > > > + /** functions to prepare/process IPsec packets */ > > > + struct rte_ipsec_sa_func func; > > > +}; > > > > IMO, it can be cache aligned as it is used in fast path. >=20 > Good point, will add. OK >=20 > > > > > + > > > +/** > > > + * 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 c= an 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* structu= res > > > + * which contain the input packets. > > > + * @param cop > > > + * The address of an array of *num* pointers to the output *rte_cr= ypto_op* > > > + * structures. > > > + * @param num > > > + * The maximum number of packets to process. > > > + * @return > > > + * Number of successfully processed packets, with error code set i= n 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); > > } >=20 > To fulfill that, we can either have 2 separate function pointers: > uint16_t (*pkt_process)( const struct rte_ipsec_session *ss, struct rte_m= buf *mb[],uint16_t num); > uint16_t (*event_process)( const struct rte_ipsec_session *ss, struct rte= _event *ev[],uint16_t num); >=20 > Or we can keep one function pointer, but change it to accept just array o= f pointers: > uint16_t (*process)( const struct rte_ipsec_session *ss, void *in[],uint1= 6_t num); > and then make session_prepare() to choose a proper function based on inpu= t. >=20 > 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_mbu= f *mb[],uint16_t num); uint16_t (*event_process)( const struct rte_ipsec_session *ss, struct rte_= event *ev[],uint16_t num); }; >=20 > > > > This is to, > > 1) Avoid Event mode application code duplication > > 2) Better I$ utilization rather moving event specific and mbuff > > specific at different code locations > > 3) Better performance as inside one function pointer we can do things > > in one shot rather splitting the work to application and library. > > 4) Event mode has different modes like ATQ, non ATQ etc, These things > > we can abstract through exiting function pointer scheme. > > 5) atomicity & ordering problems can be sorted out internally with the = events, > > having one function pointer for event would be enough. > > > > We will need some event related info (event dev, port, atomic queue to > > use etc) which need to be added in rte_ipsec_session *ss as UNION so it > > wont impact the normal mode. This way, all the required functionality o= f this library > > can be used with event-based model. >=20 > Yes, to support event model, I imagine ipsec_session might need to > contain some event specific data. > I am absolutely ok with that idea in general. > Related fields can be added to the ipsec_session structure as needed, > together with actual event mode implementation. OK >=20 > > > > See below some implementation thoughts on this. > > > > > + > > > +#ifdef __cplusplus > > > +} > > > +#endif > > > + > > > +#endif /* _RTE_IPSEC_H_ */ > > > diff --git a/lib/librte_ipsec/rte_ipsec_version.map b/lib/librte_ipse= c/rte_ipsec_version.map > > > +const struct rte_ipsec_sa_func * > > > +ipsec_sa_func_select(const struct rte_ipsec_session *ss) > > > +{ > > > + static const struct rte_ipsec_sa_func tfunc[] =3D { > > > + [RTE_SECURITY_ACTION_TYPE_NONE] =3D { > > > + .prepare =3D lksd_none_prepare, > > > + .process =3D lksd_none_process, > > > + }, > > > + [RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO] =3D { > > > + .prepare =3D NULL, > > > + .process =3D inline_crypto_process, > > > + }, > > > + [RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL] =3D { > > > + .prepare =3D NULL, > > > + .process =3D inline_proto_process, > > > + }, > > > + [RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL] =3D { > > > + .prepare =3D lksd_proto_prepare, > > > + .process =3D lksd_proto_process, > > > + }, > > > > [RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL][EVENT] =3D { > > .prepare =3D NULL, > > .process =3D NULL, > > .process_evt =3D lksd_event_process, > > }, > > [RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL][EVENT] =3D { > > .prepare =3D NULL, > > .process =3D NULL, > > .process_evt =3D inline_event_process, > > }, > > > > Probably add one more dimension in array to choose event/poll? >=20 > That's a static function/array, surely we can have here as many dimension= s as we need to. > As I said below, will probably need to select a function based on directi= on, mode, etc. anyway. > NP to have extra logic to select event/mbuf based functions. OK >=20 > > > > > > > + }; > > > + > > > + if (ss->type >=3D RTE_DIM(tfunc)) > > > + return NULL; > > > +int __rte_experimental > > > +rte_ipsec_session_prepare(struct rte_ipsec_session *ss) > > > +{ > > > > Probably add one more argument to choose event vs poll so that > > above function pointers can be selected. > > > > or have different API like rte_ipsec_use_mode(EVENT) or API > > other slow path scheme to select the mode. >=20 > Yes, we would need something here. > I think we can have some field inside ipsec_session that defines > which input types (mbuf/event) session expects. > I suppose we would need such field anyway - as you said above, > ipsec_session most likely will contain a union for event/non-event relate= d data. OK >=20 > Konstantin >=20