From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0A2A6A0528;
	Thu,  9 Jul 2020 13:50:09 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id CD9911E537;
	Thu,  9 Jul 2020 13:50:08 +0200 (CEST)
Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com
 [207.211.31.120]) by dpdk.org (Postfix) with ESMTP id D6CDA1E535
 for <dev@dpdk.org>; Thu,  9 Jul 2020 13:50:07 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1594295407;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 in-reply-to:in-reply-to:references:references;
 bh=LCK+Np25HjGvGrPT/pxvJ2hPTtLwsR3KubdKLBeTZgM=;
 b=LUUyJ48Z7/Aj2F0QYULPlNwBj7nQBAS6ul63DH93mNemUHIr2Y3DROTiTjEO3l9IBFDuYe
 n9mXzJjgmmeoVPAf7g1l/wWSQV1nUD2nrsTDwS7ymQYOW4ZipwOEyFZeL93ur9TtZP6+Nu
 Kgq+kDICZlebiarprzW49p9taGeBV70=
Received: from mail-vk1-f199.google.com (mail-vk1-f199.google.com
 [209.85.221.199]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-429-7BRT-SjVPKqoD8Eptu3Dqg-1; Thu, 09 Jul 2020 07:50:05 -0400
X-MC-Unique: 7BRT-SjVPKqoD8Eptu3Dqg-1
Received: by mail-vk1-f199.google.com with SMTP id 131so371224vkw.6
 for <dev@dpdk.org>; Thu, 09 Jul 2020 04:50:05 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc;
 bh=LCK+Np25HjGvGrPT/pxvJ2hPTtLwsR3KubdKLBeTZgM=;
 b=XRgfx6OJh45q70A/xayD267J0xbvufa3+h1E5vdGjqKfVV1XJCHdAlZ0gJFxDR4bRK
 gXlSV4eqJ/oo5Vy3IX18oBA0Fhr4w7abqZ4fAAW91ahfbC+wA2LPuUhAorIe/f/mZUmb
 GKiJ5V94qONJ/6Zl26y5OSx+P/uNyU/NEKZ3AHz++IO+FcBlbyVeN7bAiArrlwrnRYc+
 Dpk76GzNE9m6j4t/E3ab+QV0AqBYNz2cNKIqV/1tjfB25LtUFJ3S5y14o/2qi0GsXbzb
 obhOG/R8B6fxUX2xrS0f1iqfOu036wO6cCJEmGbnwxqint1Zazln0aeNFhSXxTL0Dr4b
 0Nzw==
X-Gm-Message-State: AOAM533w2cY8rzcXanGB1ZHEmaAUrE8gYTbfMWQOpKafCxFFfFUkUQZu
 pEYPCFZfywfsxupH5SabnuSAjYE1HS7i9k90M+eepTE982Z4xm5kFh49aKML0z9iF5n0Buwst0H
 oB8rinzPusDRnrS0lY3E=
X-Received: by 2002:a67:c990:: with SMTP id y16mr48625076vsk.105.1594295405150; 
 Thu, 09 Jul 2020 04:50:05 -0700 (PDT)
X-Google-Smtp-Source: ABdhPJxQ+1R56lCmKkHpzkaN1JwBAK6+rzJVR1gE/BcLtL/i+d/lqmAtuovLRt9Mk1f0OeH0Bfe+22g0RDoy6SbVBhU=
X-Received: by 2002:a67:c990:: with SMTP id y16mr48625059vsk.105.1594295404884; 
 Thu, 09 Jul 2020 04:50:04 -0700 (PDT)
MIME-Version: 1.0
References: <20190906094534.36060-1-ruifeng.wang@arm.com>
 <20200709080250.200166-1-ruifeng.wang@arm.com>
 <20200709080250.200166-2-ruifeng.wang@arm.com>
In-Reply-To: <20200709080250.200166-2-ruifeng.wang@arm.com>
From: David Marchand <david.marchand@redhat.com>
Date: Thu, 9 Jul 2020 13:49:53 +0200
Message-ID: <CAJFAV8zKAC3u7MKV3N7bswRK7Qo+sNNTv=jCXSAM=Ns1DB1EGw@mail.gmail.com>
To: Ruifeng Wang <ruifeng.wang@arm.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>, 
 Vladimir Medvedkin <vladimir.medvedkin@intel.com>,
 John McNamara <john.mcnamara@intel.com>, 
 Marko Kovacevic <marko.kovacevic@intel.com>, Ray Kinsella <mdr@ashroe.eu>, 
 Neil Horman <nhorman@tuxdriver.com>, dev <dev@dpdk.org>, 
 "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, 
 Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>, nd <nd@arm.com>
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Subject: Re: [dpdk-dev] [PATCH v8 1/3] lib/lpm: integrate RCU QSBR
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
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>

Hello Ruifeng,

On Thu, Jul 9, 2020 at 10:03 AM Ruifeng Wang <ruifeng.wang@arm.com> wrote:
> diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c
> index 38ab512a4..4fbf5b6df 100644
> --- a/lib/librte_lpm/rte_lpm.c
> +++ b/lib/librte_lpm/rte_lpm.c
> @@ -1,5 +1,6 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
>   * Copyright(c) 2010-2014 Intel Corporation
> + * Copyright(c) 2020 Arm Limited
>   */
>
>  #include <string.h>
> @@ -39,6 +40,17 @@ enum valid_flag {
>         VALID
>  };
>
> +/** @internal LPM structure. */
> +struct __rte_lpm {
> +       /* LPM metadata. */
> +       struct rte_lpm lpm;
> +
> +       /* RCU config. */
> +       struct rte_rcu_qsbr *v;         /* RCU QSBR variable. */
> +       enum rte_lpm_qsbr_mode rcu_mode;/* Blocking, defer queue. */
> +       struct rte_rcu_qsbr_dq *dq;     /* RCU QSBR defer queue. */
> +};
> +
>  /* Macro to enable/disable run-time checks. */
>  #if defined(RTE_LIBRTE_LPM_DEBUG)
>  #include <rte_debug.h>
> @@ -122,6 +134,7 @@ rte_lpm_create(const char *name, int socket_id,
>                 const struct rte_lpm_config *config)
>  {
>         char mem_name[RTE_LPM_NAMESIZE];
> +       struct __rte_lpm *internal_lpm = NULL;

Nit: internal_lpm does not need to be initialised to NULL.


>         struct rte_lpm *lpm = NULL;
>         struct rte_tailq_entry *te;
>         uint32_t mem_size, rules_size, tbl8s_size;
> @@ -140,12 +153,6 @@ rte_lpm_create(const char *name, int socket_id,
>
>         snprintf(mem_name, sizeof(mem_name), "LPM_%s", name);
>
> -       /* Determine the amount of memory to allocate. */
> -       mem_size = sizeof(*lpm);
> -       rules_size = sizeof(struct rte_lpm_rule) * config->max_rules;
> -       tbl8s_size = (sizeof(struct rte_lpm_tbl_entry) *
> -                       RTE_LPM_TBL8_GROUP_NUM_ENTRIES * config->number_tbl8s);
> -
>         rte_mcfg_tailq_write_lock();
>
>         /* guarantee there's no existing */
> @@ -161,6 +168,12 @@ rte_lpm_create(const char *name, int socket_id,
>                 goto exit;
>         }
>
> +       /* Determine the amount of memory to allocate. */
> +       mem_size = sizeof(*internal_lpm);
> +       rules_size = sizeof(struct rte_lpm_rule) * config->max_rules;
> +       tbl8s_size = (sizeof(struct rte_lpm_tbl_entry) *
> +                       RTE_LPM_TBL8_GROUP_NUM_ENTRIES * config->number_tbl8s);
> +
>         /* allocate tailq entry */
>         te = rte_zmalloc("LPM_TAILQ_ENTRY", sizeof(*te), 0);
>         if (te == NULL) {
> @@ -170,22 +183,23 @@ rte_lpm_create(const char *name, int socket_id,
>         }
>
>         /* Allocate memory to store the LPM data structures. */
> -       lpm = rte_zmalloc_socket(mem_name, mem_size,
> +       internal_lpm = rte_zmalloc_socket(mem_name, mem_size,
>                         RTE_CACHE_LINE_SIZE, socket_id);
> -       if (lpm == NULL) {
> +       if (internal_lpm == NULL) {
>                 RTE_LOG(ERR, LPM, "LPM memory allocation failed\n");
>                 rte_free(te);
>                 rte_errno = ENOMEM;
>                 goto exit;
>         }
>
> +       lpm = &internal_lpm->lpm;

>From this point...

>         lpm->rules_tbl = rte_zmalloc_socket(NULL,
>                         (size_t)rules_size, RTE_CACHE_LINE_SIZE, socket_id);
>
>         if (lpm->rules_tbl == NULL) {
>                 RTE_LOG(ERR, LPM, "LPM rules_tbl memory allocation failed\n");
> -               rte_free(lpm);
> -               lpm = NULL;
> +               rte_free(internal_lpm);
> +               internal_lpm = NULL;

... lpm is set to &internal_lpm->lpm and will be returned by jumping
to the exit label.
So freeing internal_lpm is necessary, but the lpm variable must be set
to NULL too.


>                 rte_free(te);
>                 rte_errno = ENOMEM;
>                 goto exit;
> @@ -197,8 +211,8 @@ rte_lpm_create(const char *name, int socket_id,
>         if (lpm->tbl8 == NULL) {
>                 RTE_LOG(ERR, LPM, "LPM tbl8 memory allocation failed\n");
>                 rte_free(lpm->rules_tbl);
> -               rte_free(lpm);
> -               lpm = NULL;
> +               rte_free(internal_lpm);
> +               internal_lpm = NULL;

Ditto.


>                 rte_free(te);
>                 rte_errno = ENOMEM;
>                 goto exit;

-- 
David Marchand