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.
> 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.
> >
> > 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.
> 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.
> Regards,
> Akhil
>
