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 B9308440B5;
	Fri, 24 May 2024 12:01:23 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 9C59140A67;
	Fri, 24 May 2024 12:01:23 +0200 (CEST)
Received: from mail-lj1-f172.google.com (mail-lj1-f172.google.com
 [209.85.208.172])
 by mails.dpdk.org (Postfix) with ESMTP id 5B2374026B
 for <dev@dpdk.org>; Fri, 24 May 2024 12:01:22 +0200 (CEST)
Received: by mail-lj1-f172.google.com with SMTP id
 38308e7fff4ca-2e95a74d51fso10711831fa.2
 for <dev@dpdk.org>; Fri, 24 May 2024 03:01:22 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=linaro.org; s=google; t=1716544882; x=1717149682; darn=dpdk.org;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:from:to:cc:subject:date:message-id:reply-to;
 bh=mXAY2lZ+eht83xJ5Rqke/Nt81RjHGdd4Y10zsCoajMA=;
 b=g6xYjKodHGXA8S9+1QcYyqnavzIm0WIYbqTjlRczk/pk+Hh6DnKQc8DRzm1/ZXdMKH
 TFga6UG5ydH3R1QWsr/YU1Ac8Z3diRp6EmfQvQUu+duhhXZQ11r2G3fVSZ2wz7p9rzs7
 UuAF3jwXCwAr9/16I4BNmpsIQmcPQMS2xKSJipQ/9fnO6hysrr8bXgKksfp6OMm1BzOg
 a7nMlsliFLJEDxT+v4/EeFRCrGhKN1gF7RlsoEENLcrVMKY1sX1giBuBXe2TP/YspnB0
 NHglL5Nr4zdrya+APTBM3xeA3Pvfzmz07fW5WafZmMFqtX0TKkQVRV97joln08+EojJr
 OGcA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1716544882; x=1717149682;
 h=cc:to:subject:message-id:date:from:in-reply-to:references
 :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id
 :reply-to;
 bh=mXAY2lZ+eht83xJ5Rqke/Nt81RjHGdd4Y10zsCoajMA=;
 b=HiOZeKOUsxSnIWiWomwcCPlewclj3AhG5fzX+XMkHQ6OWeGpUV4ImiYs2jQAlgOike
 rSCbioQXhtWnt2Ojj4CT379XvEaTXBGtrmEvG7VXzDQjrmpT8Ua23EXarJ7VRpgiBJD4
 eBuEa3eSvSqYXBnFK8hfrzSRxMAwu5PeA0qC1Ek/BJMk6egK4+RTFCUb/isQVFsZjmOJ
 eiqTxHvsE6pl3Wsakyyp5+zDHxmX7zfToHpdnAGKc0kRyClsb1ZsGUUv5wr9+5KVka+y
 MnUo2LS/74lBwPBzl4FBBR/sbcJpm8Yyq/Q2s2OzX2+0+LKvs39BSXzB3ANA9cmWr2oM
 od0A==
X-Forwarded-Encrypted: i=1;
 AJvYcCXW5pEg0ACHZdefkjvlXp6WSinyfshAoO9A5N2W+38Sa8wMF4iW365CTrTnrA1ENNo73OdaH5ikMYHJvJs=
X-Gm-Message-State: AOJu0YzU7rCGu7Jkf+f++PLMGXlyzD96vhENaaGy1UQuCl2gZJgY2WiK
 iwhrNGW6yp1ZkGkKSy7rgh5LZpYB5j+O/1gC+J8tKSRWipOBJTBkd7zpjEpeuQ8iQHTfygIRWRR
 2PzFSziCodN/tgtnCyksit3gjuzAGvVMx3aCwqw==
X-Google-Smtp-Source: AGHT+IG5FEI6lEJr+4mPdBaeozKMza0jG6u68KZ2dvTBt0pgmJshHNmOfzmX7U7inwAJVmGKxrNMO0Ody9Pr9qPpyMI=
X-Received: by 2002:a05:6512:750:b0:524:1fea:7626 with SMTP id
 2adb3069b0e04-52966005a90mr1196100e87.32.1716544881610; Fri, 24 May 2024
 03:01:21 -0700 (PDT)
MIME-Version: 1.0
References: <20240422143102.251-1-zhangfei.gao@linaro.org>
 <20240422143102.251-3-zhangfei.gao@linaro.org>
 <CO6PR18MB4484695527069B0AB6F54F25D8F42@CO6PR18MB4484.namprd18.prod.outlook.com>
In-Reply-To: <CO6PR18MB4484695527069B0AB6F54F25D8F42@CO6PR18MB4484.namprd18.prod.outlook.com>
From: Zhangfei Gao <zhangfei.gao@linaro.org>
Date: Fri, 24 May 2024 18:01:10 +0800
Message-ID: <CABQgh9Ep_FqJ=LEi40L6=BtN-B8rarr+gmJuQCQH57Y4Q7ukZw@mail.gmail.com>
Subject: Re: [EXTERNAL] [PATCH 2/3] compress/uadk: support basic operations
To: Akhil Goyal <gakhil@marvell.com>
Cc: Fan Zhang <fanzhang.oss@gmail.com>, Ashish Gupta <ashishg@marvell.com>, 
 "dev@dpdk.org" <dev@dpdk.org>
Content-Type: text/plain; charset="UTF-8"
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

Hi, Akhil

Thanks for your time.

On Thu, 23 May 2024 at 23:38, Akhil Goyal <gakhil@marvell.com> wrote:
>
> Hi Zhangfei,
>
> Overall, a well written driver.
> Please see below comment.
>
> > +static int
> > +uadk_compress_pmd_config(struct rte_compressdev *dev,
> > +                      struct rte_compressdev_config *config)
> > +{
> > +     char mp_name[RTE_MEMPOOL_NAMESIZE];
> > +     struct uadk_compress_priv *priv;
> > +     struct rte_mempool *mp;
> > +     int ret;
> > +
> > +     if (dev == NULL || config == NULL)
> > +             return -EINVAL;
> > +
> > +     snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
> > +              "stream_mp_%u", dev->data->dev_id);
> > +     priv = dev->data->dev_private;
> > +
> > +     /* alloc resources */
> > +     ret = wd_comp_env_init(NULL);
> > +     if (ret < 0)
> > +             return -EINVAL;
> > +
> > +     mp = priv->mp;
> > +     if (mp == NULL) {
> > +             mp = rte_mempool_create(mp_name,
> > +                                     config->max_nb_priv_xforms +
> > +                                     config->max_nb_streams,
> > +                                     sizeof(struct uadk_stream),
> > +                                     0, 0, NULL, NULL, NULL,
> > +                                     NULL, config->socket_id, 0);
> > +             if (mp == NULL) {
> > +                     UADK_LOG(ERR, "Cannot create private xform pool on
> > socket %d\n",
> > +                              config->socket_id);
> > +                     ret = -ENOMEM;
> > +                     goto err_mempool;
> > +             }
> > +             priv->mp = mp;
> > +     }
>
> Do you really need a mempool here? It is for uadk_stream which is just struct of pointer and an enum.
> It can simply be rte_malloc.
> And even you do not need uadk_compress_priv.
> This can be simplified. Right?

Yes, good idea, this can be simplified, and can remove
uadk_compress_priv as well.

But it looks like rte_compressdev_pmd_create requires the priv data,
otherwise it will return an error if private_data_size == 0.
Could rte_compressdev_pmd_create be changed only alloc
compressdev->data->dev_private only if data_size != 0.

Or I am checking whether to simply add one priv.

>
> Also remove the execution part of documentation from 1/3 and add it in 3/3
> Since the PMD is complete in 3/3, release notes and execution part of documentation should be in last patch.
OK.

One more question,
rte_compressdev_pmd_init_params does not have .max_nb_queue_pairs as
rte_cryptodev_pmd_init_params.
So dpdk-test-compress-perf  will use 128 queues by default, except
adding -l 1,2.
Is this expected?

Thanks

>