From: Akhil Goyal
> > > > > > +static void
> > > > > > +mlx5_crypto_sym_session_clear(struct rte_cryptodev *dev,
> > > > > > + struct rte_cryptodev_sym_session *sess)
> > > > > > {
> > > > > > + struct mlx5_crypto_priv *priv = dev->data->dev_private;
> > > > > > + struct mlx5_crypto_session *sess_private_data =
> > > > > > + get_sym_session_private_data(sess,
> > > > > > +dev->driver_id);
> > > > > > +
> > > > > > + if (unlikely(sess_private_data == NULL)) {
> > > > > > + DRV_LOG(ERR, "Failed to get session %p private data.",
> > > > > > + sess_private_data);
> > > > > > + return;
> > > > > > + }
> > > > > > + mlx5_crypto_dek_destroy(priv, sess_private_data->dek);
> > > > > > + DRV_LOG(DEBUG, "Session %p was cleared.",
> > > > > > + sess_private_data);
> > }
> > > > >
> > > > > Memory leakage, mempool is not freed.
> > > >
> > > > Yes, good catch, this part was missed.
> > > >
> > > > > IMO, this driver is not properly tested with the unit test app.
> > > >
> > > > The only app we tested until now is l2fwd_crypto and it works fine!
> > > > We can add it to doc.
> > > >
> > > > > Please add a note in the documentation that it is tested with
> > > > > autotest.
> > > >
> > > >
> > > > The next app we want to test with, is test-crypto-perf.
> > > >
> > > I would recommend to run the test app first.
> > > It will catch most of your basic bugs like the one above.
> >
> > It is too late for this, will add the test adjustment later.
>
> Can we postpone the PMD to next release.
We can, but this is not our plan.
We met all the DPDK rules to push it on time.
> I believe test application makes
> The PMD look robust as per the DPDK crypto PMD API usage.
Every test will add robustness to the PMD.
> I haven't seen a PMD getting merged without test app.
compress/mlx5, vdpa/mlx5, regex/mlx5, net/mlx5, vdev_netvsc....
> And I apologize I did not mentioned it earlier, but it is kind of obvious
> thing to
> run test app before sending it to upstream.
In fact, no, I added more than one PMD, no one require specific test.
> L2fwd-crypto is not doing data validation hence you cannot be sure that it is
> working fine as per other standard stacks like Linux.
It is not do data validation, but we check that the packet payload return back
has the expected encrypted\decrypted data using open-ssl.
Also for us it was mandatory requirement to check it.
For us, the current validation is enough, we don't support a lot of things in
the crypto PMD currently, only one algorithm in the most basic modes.
For sure we will continue to add tests and to increase robustness.
Also adding more features is in plan for future.
If you postpone it, it yours, we don't agree with it.
> Regards,
> Akhil
