From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 1EBC8A0C42;
Wed, 12 May 2021 09:01:16 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id 018E2410DE;
Wed, 12 May 2021 09:01:16 +0200 (CEST)
Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com
[66.111.4.28]) by mails.dpdk.org (Postfix) with ESMTP id 1348D4003E
for <dev@dpdk.org>; Wed, 12 May 2021 09:01:14 +0200 (CEST)
Received: from compute4.internal (compute4.nyi.internal [10.202.2.44])
by mailout.nyi.internal (Postfix) with ESMTP id 9CD815C012C;
Wed, 12 May 2021 03:01:13 -0400 (EDT)
Received: from mailfrontend2 ([10.202.2.163])
by compute4.internal (MEProxy); Wed, 12 May 2021 03:01:13 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
from:to:cc:subject:date:message-id:in-reply-to:references
:mime-version:content-transfer-encoding:content-type; s=fm1; bh=
mZtp5rxfIB2YTg/A0fnGAtdpNvAUTmFhbD1I4fF8p6o=; b=a/mmsftl1wAI3AYR
yGt8WdIoEzzJnLxNYRBcUrSnbkGfnuQN1K+XDhkwLsybYdb62CSo6W/85lrZhWqL
PZC8HH78NAQdKvV1sDTXMm/8jpCyHsmdERnEbe4+kjNM2obM2MZ67Cwp/O7XAcAU
cRWwBaRwDLV7AfHJ4+Y7vPzPldr3gb43XfAorsBBaWcPAYG4FfBO1dERRgl/yjDa
NImyeYtkSvylpuLTHKgL9RXVQrB+G+veR2pneweOMBZ7qHmy+zGiPmFWrK8KKRNJ
vwVKI9yLTZquVo81T5VHC+1TXDpHvn3VK13cXcA9zvD4rULb3mxsZToziuAv0F2G
y6pIJg==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
messagingengine.com; h=cc:content-transfer-encoding:content-type
:date:from:in-reply-to:message-id:mime-version:references
:subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender
:x-sasl-enc; s=fm2; bh=mZtp5rxfIB2YTg/A0fnGAtdpNvAUTmFhbD1I4fF8p
6o=; b=UcUjcBVLkqPtheKEYMwd9N/gVP4yYGhP4YrxswmfJl+m4xFEtKnRSUda0
hxLoOMt1kZTRnBeNQeSEGp8aQHLsWqRqA1q6xUm6VndO5HqlOsPfFFPylnsWYw0u
nYonHYgKZlBqU47/MGLyDrGQ680/P8GFy7j1yLt3Xjiz7qlKxLDRgQfhFN1XkJ6n
Hwi6knLqtpMJNx6V/SMuPpQSKwZ2k2KVLPAYAusPIYKwfxi2DIDbzjPodMw27Lln
2vBS8aNT8DNGStKlWhXiynUw6Ywv8qkb+sWBBimfjyoIUaHhbnrRBkVBLMTJIEia
0FFm9+ZEPKfcOqnEkDNyVdS7SEwIQ==
X-ME-Sender: <xms:OX2bYASF0xL89og3jfFmfA2Ugq7649pnn4xEYll_2HwrgddnCwgIAA>
<xme:OX2bYNxWN22GiralQSnPrXtXJUNb9nPHrws9PV2YTltCGNbow2ka1ZErEthRxqqbo
dLgoKg_s444nz-eNQ>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdehuddguddufecutefuodetggdotefrod
ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh
necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd
enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm
rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc
ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej
ueeiiedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrh
fuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgr
lhhonhdrnhgvth
X-ME-Proxy: <xmx:OX2bYN37yQoy5eXcu-dsu3tX8rWNVlaw86Um2nGRu0KqewmFyCdtyA>
<xmx:OX2bYEA36CoqOq2-33-MJ1piP-Al-epOn0nmO5T51InQwHMFmWnnpA>
<xmx:OX2bYJjKxpFDHPGa30ZtmszFlJ946jiKArj3bpeNovECVgmeTWOYNg>
<xmx:OX2bYJc0p3UvtbaPIXAPv2v5GgiVh5xLhNpRsHr3DriYZtxdAIdbfg>
Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184])
by mail.messagingengine.com (Postfix) with ESMTPA;
Wed, 12 May 2021 03:01:12 -0400 (EDT)
From: Thomas Monjalon <thomas@monjalon.net>
To: Akhil Goyal <gakhil@marvell.com>, Matan Azrad <matan@nvidia.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Suanming Mou <suanmingm@nvidia.com>,
Shiri Kuzin <shirik@nvidia.com>, asafp@nvidia.com,
Shy Shyman <shys@nvidia.com>
Date: Wed, 12 May 2021 09:01:08 +0200
Message-ID: <1721058.9ygUTGhKN4@thomas>
In-Reply-To: <DM4PR12MB53891BB77E00C44C2173E4F9DF529@DM4PR12MB5389.namprd12.prod.outlook.com>
References: <20210429154712.2820159-1-matan@nvidia.com>
<MW2PR18MB2284E56315847F54CC1AC160D8539@MW2PR18MB2284.namprd18.prod.outlook.com>
<DM4PR12MB53891BB77E00C44C2173E4F9DF529@DM4PR12MB5389.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Subject: Re: [dpdk-dev] [EXT] [PATCH v3 03/15] crypto/mlx5: support session
operations
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
12/05/2021 07:51, Matan Azrad:
> 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.
There is no guarantee a patch will be pushed. This is not due.
> > 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....
There are functional test app for regex, and testpmd for net driver.
Running these tests are basic requirements.
> > 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.
Each driver class may have its own expectations.
> > 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.
Not clear who is "we".
I am OK with postponing this PMD.
Thank you Akhil for your time.