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 A222EA00C2; Mon, 26 Sep 2022 11:10:24 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 456824069B; Mon, 26 Sep 2022 11:10:24 +0200 (CEST) Received: from mail-pg1-f172.google.com (mail-pg1-f172.google.com [209.85.215.172]) by mails.dpdk.org (Postfix) with ESMTP id D0F43400D7 for ; Mon, 26 Sep 2022 11:10:22 +0200 (CEST) Received: by mail-pg1-f172.google.com with SMTP id q9so5984913pgq.8 for ; Mon, 26 Sep 2022 02:10:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-language:content-transfer-encoding:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc :subject:date; bh=KCUcUkd0cikenfFH0r8Ahp3Q/zmxB3EIdWG/xab1tRE=; b=Zz09rjBa/ykmuXTZyl4CSybdiYxn/qeAGXZ16k88AuTU3KWGT24UBPN5oQ8c/ScBTG iUvHfGprxubNnl0ho36znuoAtPrwkUS0HC+JqAN0o/GF/vgWj3iof/fLzNRxKCv8irmz 3cPZL7CKvHoLRMHmEmeELIuACCmk4IBhMP9+YxpY9ZJxaYQMoRmuB3SCyw83D3LV/GBl OBabUOsGVdNgK6ULBV2b8Rh2BoSBTshAz8Rl/XWWCljuWekeaIDsBmMuTUkGxS03zYwx GDZLQN12FyCb0ILMW6fEYSoWhEzYjwkg1j79qkffCANB9otwBAJXW0vXiJBIIZbtjVH2 95Cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-language:content-transfer-encoding:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc:subject:date; bh=KCUcUkd0cikenfFH0r8Ahp3Q/zmxB3EIdWG/xab1tRE=; b=rTwzb21D+zFU/VD9Pl3aCaogayF82s2SVgV8LVWIkbB6nA9hjcj1if7L7itsDWff0S AexxclY98ZhQZy0a+8TZTSp0k+qytcGzh5QzCN70IxcOVA5CasFVtwaoFg/6V52muiqC /lTwmn4R4yAO3NPWMfnxk/7iQ5aEMhqe8hta2dIo53n0F6a18Or5V/8VeyiK7pb6BAne yjE4IqhoyqGIWefhim3HAwOizHLFk5juvOE+7J4KL9Vr4NHVP3sqvhvMNdO9zPCnNOSn 9WURPLUeWtiAl6L0yovGUeAT+pW323JCkWcEGdBheP0AvjQJpa+52vmumm6E997W7Mu+ Ks8A== X-Gm-Message-State: ACrzQf0InkuP5urqbwF56YDZ0m3V+2eiY991FrbQ77rpq5V+Y/jzlcPM 3YbxV3c+9nYaYQmJENub8AbkWQ== X-Google-Smtp-Source: AMsMyM64E8czqY1LVUztc6bd6JDXPL7v6OfadfChFxFCy7++jIIWAAuP7rL+UPwKt3P+Ke2vnS1BSw== X-Received: by 2002:a05:6a00:1990:b0:545:aa9e:be3d with SMTP id d16-20020a056a00199000b00545aa9ebe3dmr22802093pfl.59.1664183421956; Mon, 26 Sep 2022 02:10:21 -0700 (PDT) Received: from [10.173.0.54] ([199.101.192.21]) by smtp.gmail.com with ESMTPSA id y2-20020a623202000000b0052c849d0886sm11518227pfy.86.2022.09.26.02.10.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 26 Sep 2022 02:10:21 -0700 (PDT) Subject: Re: [EXT] [PATCH v2 2/5] crypto/uadk: introduce uadk crypto driver To: Akhil Goyal , Zhangfei Gao , Declan Doherty , Fan Zhang , Ashish Gupta , Ray Kinsella Cc: "dev@dpdk.org" , "acc@openeuler.org" References: <20220923024023.15849-1-zhangfei.gao@linaro.org> <20220923024023.15849-3-zhangfei.gao@linaro.org> From: Zhangfei Gao Message-ID: Date: Mon, 26 Sep 2022 17:10:12 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US 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 Hi, Akhil On 2022/9/26 下午4:36, Akhil Goyal wrote: >> Introduce a new crypto PMD for hardware accelerators based on UADK [1]. >> >> UADK is a framework for user applications to access hardware accelerators. >> UADK relies on IOMMU SVA (Shared Virtual Address) feature, which share >> the same page table between IOMMU and MMU. >> Thereby user application can directly use virtual address for device dma, >> which enhances the performance as well as easy usability. >> >> This patch adds the basic framework. >> >> [1] https://github.com/Linaro/uadk >> >> Test: >> sudo dpdk-test --vdev=crypto_uadk (--log-level=6) >> RTE>>cryptodev_uadk_autotest >> RTE>>quit > Remove this test info. It can be in your last patch where test app changes are introduced. > > >> Signed-off-by: Zhangfei Gao >> --- >> drivers/crypto/meson.build | 1 + >> drivers/crypto/uadk/meson.build | 36 +++ >> drivers/crypto/uadk/uadk_crypto_pmd.c | 450 ++++++++++++++++++++++++++ >> drivers/crypto/uadk/version.map | 3 + >> 4 files changed, 490 insertions(+) >> create mode 100644 drivers/crypto/uadk/meson.build >> create mode 100644 drivers/crypto/uadk/uadk_crypto_pmd.c >> create mode 100644 drivers/crypto/uadk/version.map >> >> diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build >> index 147b8cf633..ee5377deff 100644 >> --- a/drivers/crypto/meson.build >> +++ b/drivers/crypto/meson.build >> @@ -18,6 +18,7 @@ drivers = [ >> 'octeontx', >> 'openssl', >> 'scheduler', >> + 'uadk', >> 'virtio', >> ] >> >> diff --git a/drivers/crypto/uadk/meson.build b/drivers/crypto/uadk/meson.build >> new file mode 100644 >> index 0000000000..a67c6c7ca5 >> --- /dev/null >> +++ b/drivers/crypto/uadk/meson.build >> @@ -0,0 +1,36 @@ >> +# SPDX-License-Identifier: BSD-3-Clause >> +# Copyright 2022-2023 Huawei Technologies Co.,Ltd. All rights reserved. >> +# Copyright 2022-2023 Linaro ltd. >> + >> +if not is_linux >> + build = false >> + reason = 'only supported on Linux' >> + subdir_done() >> +endif >> + >> +if arch_subdir != 'arm' or not dpdk_conf.get('RTE_ARCH_64') >> + build = false >> + reason = 'only supported on aarch64' >> + subdir_done() >> +endif >> + >> +sources = files( >> + 'uadk_crypto_pmd.c', >> +) >> + >> +deps += 'bus_vdev' >> +dep = cc.find_library('libwd_crypto', dirs: ['/usr/local/lib'], required: false) > I believe dirs is not required. You cannot assume that the lib is installed in /usr/local/lib/. > Check other PMDs which have such dependencies. Eg. Ipsec-mb > >> +if not dep.found() >> + build = false >> + reason = 'missing dependency, "libwd_crypto"' >> +else >> + ext_deps += dep >> +endif >> + >> +dep = cc.find_library('libwd', dirs: ['/usr/local/lib'], required: false) >> +if not dep.found() >> + build = false >> + reason = 'missing dependency, "libwd"' >> +else >> + ext_deps += dep >> +endif >> diff --git a/drivers/crypto/uadk/uadk_crypto_pmd.c >> b/drivers/crypto/uadk/uadk_crypto_pmd.c >> new file mode 100644 >> index 0000000000..aad42524d6 >> --- /dev/null >> +++ b/drivers/crypto/uadk/uadk_crypto_pmd.c >> @@ -0,0 +1,450 @@ >> +/* SPDX-License-Identifier: BSD-3-Clause >> + * Copyright 2022-2023 Huawei Technologies Co.,Ltd. All rights reserved. >> + * Copyright 2022-2023 Linaro ltd. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/* Maximum length for digest (SHA-512 needs 64 bytes) */ >> +#define DIGEST_LENGTH_MAX 64 >> + >> +struct uadk_qp { >> + struct rte_ring *processed_pkts; >> + /* Ring for placing process packets */ >> + struct rte_cryptodev_stats qp_stats; >> + /* Queue pair statistics */ >> + uint16_t id; >> + /* Queue Pair Identifier */ >> + char name[RTE_CRYPTODEV_NAME_MAX_LEN]; >> + /* Unique Queue Pair Name */ >> + uint8_t temp_digest[DIGEST_LENGTH_MAX]; >> + /* Buffer used to store the digest generated >> + * by the driver when verifying a digest provided >> + * by the user (using authentication verify operation) >> + */ >> +} __rte_cache_aligned; > These comments should be added before the variable name or else use '<' Yes,  I see comments /**< Queue Pair Identifier */ in other driver, but not understand '<', so remove it. Thanks for the info. > > The patches are not properly organized. > I am skipping the review for now. OK, sorry about that. > > Please split the patches as below. > 1. introduce driver - create files with meson.build and with probe/remove > and device ops defined but not implemented. You do not need to write empty functions. > Add basic documentation also which defines what the driver is. > You can explain the build dependency here. > 2. define queue structs and setup/remove APIs > 3. Add data path > 4. implement cipher op. Add capabilities and documentation of what is supported in each of the patch. Add feature flags etc. > 5. implement auth, add capabilities and documentation > 6. test app changes. Will update as soon as possible. Thanks for your time. Thanks