[dpdk-dev] [EXT] [PATCH v3 15/15] crypto/mlx5: set feature flags and capabilities

[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.

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.

Hence I just recommend to have 3-4 lines to enable the compilation
In the PMD doc.

> > >
> > > > 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.
Test app is supposed to have test vectors which will work on any platform.
Hence data validation is done properly.

> 
> 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.

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.

> 
> 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.

I hope the doubts are clear now and we are OK to defer to next release.

Regards,
Akhil