> From: Akhil Goyal > > Hi Matan, > > > > > > > +Prerequisites > > > > > > > +------------- > > > > > > > + > > > > > > > +- Mellanox OFED version: **5.3** > > > > > > > + see :doc:`../../nics/mlx5` guide for more Mellanox OFED > > > > > > > details. > > > > > > > > > > > > Since the driver is by default compiled off due to the > > > > > > dependency on external Libraries, I would recommend to add few > > > > > > lines here as well for compilation. > > > > > > Like to compile rdma-core and set PKG_CONFIG_LIBDIR. > > > > > > > > > > Why? all Mellanox drivers has the same external dependencies. > > > > > I added here link for the doc explains it well. > > > > > > > > This is a crypto PMD, not a NIC PMD. Somebody working on crypto > > > > PMDs, > > > do > > > > not really care about the NIC PMDs. > > > > Hence it would be convenient to have compilation information here as > > > well. > > > > You can refer to other document for details, but basic info should > > > > be added here as well. > > > > > > The link explains how to install OFED, this is only what the user need > > > to take from the link. > > > The basic is to install OFED. > > > I don't see a reason to duplicate doc section which are exactly the same. > > > > As I compiled the PMD, it was not convenient to read the whole document. > > And it is not needed to compile linux and everything. > > I just needed rdma-core and set it in PKG_CONFIG_LIBDIR. > > But compilation is not enough to run, you still cannot test if you break thigs > by compilation. > You need to install also the kernel modules. > That's what we explain in all our drivers. >
For a person doing minor changes in the Lib, he cannot test each hardware. He can only do compilation and that is what he is expected to do. > > > The reason I am insisting here is, when somebody do small changes in > Crypto > > library, he may need to do subsequent changes in all PMDs. > > For which compilation steps should be easily accessible in the PMD doc So > > that the patch can be compiled properly. > > Not enough. > > > Hence I just recommend to have 3-4 lines to enable the compilation In the > > PMD doc. > > We are not doing it in others mlx5 drivers. > If you insist, we will do. > > > > > > > And I do not see any updates to the test application for testing > > > > > > this > > > driver. > > > > > > > > > > You can see update to l2fwd_crypto, we tested with this example > > > > > for the first stage. > > > > > Everything looks ok there. > > > > > > > > L2fwd-crypto is an app which only test data path with no packet > > validation. > > > > It does not tell if your encryption is correctly done as per standards > > > > or > > not. > > > > Did you test interoperability with l2fwd-crypto? > > > > All basic configuration tests are also not done, like cleanup etc of the > > PMD. > > > > I haven't seen a driver getting merge without the unit test application > > run. > > > > Test app helps you comply with the way dpdk drivers are meant to be > > > > written. > > > > > > We adjusted the l2fwd-crypto to the dataunit feature and wrapped keys. > > > We validated data integrity from the packet returns back from the > > > crypto net port. > > > As I said, encryption\decryption with AES-XTS is working well. > > > > Do you test interoperability here? Encryption by MLX5 and decryption By > > another PMD/stack and vice-versa. > > Compared to open-ssl results. > > > Test app is supposed to have test vectors which will work on any platform. > > Hence data validation is done properly. > > I think open-ssl is good standard to check too. Yes it is good standard. OK but what about the other basic cases that I listed below. Did you mention any limitation that all these cases are not supported as of now. > > > > > > > Now, is too late to update the test application to the above features, > > > the driver code is here for a long time, no one ask about the test > > > adjustment until now. > > I acked as maintainer of this PMD. Is it really enough for the new PMD? > > > Can we defer to next release? I apologize for not asking it earlier. But > > this is > > kind of obvious for somebody working in DPDK. > > Please check that none of the PMD is merged without test app in the past 3- > > 4yrs. > > This is not true. > > > > We can add the adjustment to increase validity for the next release to > > > all the remaining crypto apps (test\test-crypto-perf). > > > > > > For now, we have one validation with l2fwd-crypto And any user can run > > > it and see how to use mlx5 driver. > > > > The user cannot be sure of the basic things of a crypto PMD are in order or > > not. > > As l2fwd-crypto does not test every basic thing. > > For eg. Session deletion, PMD stop, PMD close, PMD restart, setting up > > multiple sessions(l2fwd support single session). > > > > Running datapath of a single use case is not sufficient for a PMD. > > This is a POC and it need to comply with the environment. > > There are only one\ two cases we need in this version and we tested them. > > > I hope the doubts are clear now and we are OK to defer to next release. > > As I said, we don't agree, it is yours. > As I said, fix the issues, I can still merge today but not after that. Regards, Akhil
