Re: [dpdk-dev] [PATCH v2 00/15] add OCTEONTX2 inline IPsec support

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Jan 28, 2020 at 2:08 PM Akhil Goyal <akhil.goyal@nxp.com> wrote:
>
> Hi Jerin,

Hi Akhil,

> >
> > On Mon, Jan 27, 2020 at 8:24 PM Anoob Joseph <anoobj@marvell.com> wrote:
> > >
> > > Hi Jerin, Akhil,
> > >
> > > Let me summarize the design changes from the discussions below.
> > >
> > > Currently, drivers/crypto/octeontx2/otx2_security.c defines all security ctx ops
> > for the ethdev (idea was to add all crypto security ctx for lookaside also there).
> > That will be moved to drivers/net/octeontx2 as is. The routines which are doing
> > qp_add & qp_remove would be moved to common (discussed below). Otherwise,
> > the rest should remain as is. If Jerin/Akhil wants further isolation, please do
> > share specifics. Almost all functions in otx2_security.c is dereferencing
> > 'rte_eth_dev'. So having (void *) will not help.
> > >
> > > The functions in otx2_security.c is calling inline functions in otx2_ipsec_fp.h
> > (which has lower level implementations of session create etc). This will remain
> > as is in drivers/crypto/octeontx2 but would be called from
> > drivers/net/octeontx2/otx2_security.c.
> > >
> > > We will need to include otx2_cryptodev_qp.h (internal header in
> > drivers/crypto/octeontx2) since the crypto queue pair is required for outbound
> > processing. Since otx2_cryptodev_qp.h has dependency on rte_cryptodev.h, the
> > ethdev file will have dependency on rte_cryptodev.h.
> > >
> > > I want all the maintainers (Akhil, Jerin & Ferruh) to ack the above behavior so
> > that I can proceed with the restructuring. (Currently issue is rte_ethdev.h getting
> > included in a cryptodev PMD file. The case we are proposing is the exact mirror
> > of that)
> >
> > I think, Following rework would be required.
> >
> > 1) Don't access rte_eth_dev symbols in driver/crypto/octeontx2
> Yes
>
> > 2) Don't access rte_crypto_dev symbols in drier/net/octeontx2
> I am not sure how you can work without rte_cryptodev.h in net driver.

We would need to include the only rte_security.h. Right?
Meaning access should be limited to rte_securty_* symbols.

> As I mentioned, security_ctx for ethernet device along with it's ops should
> Be defined in ethernet driver. And call crypto specific inline functions

Yes. Ops should should be defined in an ethernet driver.

It can be the hook to the real implementation in driver/crypto/octeontx2
have some code for ethdev specific locally and other in crypto-specific.

> Placed in a header file in drivers/crypto/octeontx2/
>
> I believe you would need cryptodev.h included in ethernet driver like it is
> Getting used in ixgbe driver. The difference would be, all crypto
> Base functionality would be inside the crypto driver(inline functions in .h).
>
> > 3) Communication between both drivers should both through "custom
> > structure"(say struct otx2_eth_sec or so for inline, otx2_crypto_sec
> > for look side)
> > defined in driver/common/octeonxt2 which holds data.
> > Processing function through "function pointer" registration provided
> > through in driver/common/octeonx2 as idev framework to avoid build
> > dependency.
> >
> > I am not sure anything else can be done beyond the above.