> > On Wed, Jan 22, 2020 at 6:26 PM Akhil Goyal <akhil.go...@nxp.com> wrote: > > > > > > > > > > > > Hi Jerin, > > > > > > > > > > Will do the suggested change (RX/rx-> Rx & TX/tx->Tx). Do you want me > > > > > to trim the headline as well? > > > > > > > > > > > > > Hi Anoob, > > > > > > > > > @Akhil, did you get a chance to review the series? Do you have any > > > > > comments on the patches? > > > > > > > > > > > > > You are adding inline ipsec support to ethernet device and not a crypto > > > > device. > > > > These patches should not be part of crypto PMD. There will be cyclic > > > > dependency Between ethernet device and crypto device which can be > easily > > > > avoided. > > > > > > [Anoob] We have plans to use lookaside protocol to handle the "fallback" > > > session. And that involves session sharing between inline and lookaside > protocol > > > offloads. Also, though the feature is exposed as a feature of ethdev, on > > > our > > > platform, it's the crypto block which primarily implements the feature. > > > And > so, if > > > the code is moved to ethdev dir, there would be lot of code duplication. > > > The > > > idea is to have all security related code in one place. > > > > > > Also, the PMDs don't have any calls to each other. The communication > between > > > the two happens via common. The crypto dev PMD will register the required > > > security ops to a common structure and ethdev would get it from there. So > there > > > won't be an issue of build dependency. > > > > > > > - The code that need to be duplicated can be moved to drivers/common/ > > I would like to keep the common code that is common to all the coprocessors. > > Moreover, there are logistic issues in that case where > a) drivers/common/octeontx2/ going through master repo. So we will be > creating unnecessary dependency with that 'master' tree. > b) crypto and ethdev work is done by different teams so we would like > to make responsibly clear wrt the review and ownership.
I can agree upon that the code shall not be placed in the common/octeontx2, But you can have a header file in crypto/octeontx2/ which has some inline functions Which can be called from the ethernet as well as crypto device for inline proto And lookaside cases respectively. I think with that the maintainability would not be an issue. IMO, following approach can be looked upon and may be cleaner - define security ctx and its ops in net/octeontx2.c or net/octeontx2_security.c These are all control path and should not be any issue. - define crypto specific code (inline functions) in crypto/octeontx2_security.h Which can be called for both inline protocol as well as lookaside proto case. - for data path you can have the processing as is. I hope all dependencies can Be dealt with as the code will be there in crypto driver for all the mapping of Event and crypto queues. - all APIs which are common for ethernet device and crypto device in octeontx2_security.h should not prefix eth as is the case in current APIs. I hope this will clear the ugliness of the code. > > I would like to keep the security/crypto-related code to driver/crypto and > hook to driver/net/octeontx2 with required functionalities over the > driver/common > using a few function pointers to remove the cyclic build dependency. > > Considering there is no cyclic build and shared library dependency > now, Can we make forward progress > with the existing scheme? > > > - You may not need to include rte_ethdev.h inside drivers/crypto > > - otx2_sec_eth_ctx_create should be part of ethdev and you would need > similar API > > for crypto device as well when you would support the fallback session > > support. > > So that would go in crypto. Session creation code may be common and can go > > in drivers/common. > > - You would have separate security_ctx for both eth device and crypto device > and that should > > In net device and crypto device separately. Similarly security->ops should > > be > different in both of them. > > However if they may have same session creation code and that can go in > common. > >
