From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledrvdehuddguddufecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej ueeiiedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrh fuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgr lhhonhdrnhgvth X-ME-Proxy: 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 To: Akhil Goyal , Matan Azrad Cc: "dev@dpdk.org" , Suanming Mou , Shiri Kuzin , asafp@nvidia.com, Shy Shyman Date: Wed, 12 May 2021 09:01:08 +0200 Message-ID: <1721058.9ygUTGhKN4@thomas> In-Reply-To: References: <20210429154712.2820159-1-matan@nvidia.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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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.