From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 795CFA0526; Wed, 8 Jul 2020 16:30:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4B73C1DF90; Wed, 8 Jul 2020 16:30:39 +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 F1B7E1DEF3 for ; Wed, 8 Jul 2020 16:30:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594218637; 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=B+U8naIVOKBuB8Sxis+3r9QPIxwFdBVTDtYu9D7X0ek=; b=Tk0PIadzLG4VxSFI3r4Pk/MbZAhIlXMSlRcbw65C5Ow6ZUqM+SJYF7NBucSTjJcs44PB8o A5ZprXCk05wzi6f4ex4VPHZqFYo0Oev6DDUPEJ+7vapahtWf0PihQkRk78kyjlT1k2uS8/ sWOwa4vn8HRo34AzqlDuX0e8B/12hKg= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-457-RNDqqi6GPTWSnaDxYztmWg-1; Wed, 08 Jul 2020 10:30:34 -0400 X-MC-Unique: RNDqqi6GPTWSnaDxYztmWg-1 Received: by mail-ua1-f71.google.com with SMTP id x1so12071877uar.4 for ; Wed, 08 Jul 2020 07:30:33 -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=B+U8naIVOKBuB8Sxis+3r9QPIxwFdBVTDtYu9D7X0ek=; b=PqLzQkVvE2RAw1+PoySO1SGvXOSwu4wQCnD2JEJIeNA2dGBLFOFVfXDHa9i2mWGpm2 ntXei1qns7UIhjRBAZxrePkSux5pByiltUJ9RSL2cCXDdMbjM0UMe0df1QosghWZt2tQ 5K3ernoj5GSvEnlHaO6CO6ausL6HRdzrmBpXUCfY7Mtwhyyzk1vCRP4LX1Bn+o22YicG gaIpp5oQDvR+Su/W8LsDcCPQAulLSI8fvibxz5Qf3c8qaZlOjkRGgzm3UM/RkWwZ6vLG /r77Pdk46PmvLVRvbppSYfHvYjoqzqr26j4zmHfR1x8ZC93RxjNhaKPf+UUCk+Ul0ysd P0sQ== X-Gm-Message-State: AOAM53071IU4robKRFfUxuAw8OHMRcXZ1GUQ2nkRJCnC4cM+2bfHItNg 36GohIhcSug6te9nLng/seajnlHR92dUCRxnRbnE3+cnEPrbvnZojybymGionQm+GFOwg4SL0jQ vQxZxZEOTHxWO/4fS6c8= X-Received: by 2002:ab0:2c3:: with SMTP id 61mr40861921uah.87.1594218633306; Wed, 08 Jul 2020 07:30:33 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxluA5mF2D955UVCj5r3NPF496eCrhNdfrgfuEp7oy6M8Nt7MIJl8GDTMoeumXwS1dyajMCvQRpLOe42xCStNA= X-Received: by 2002:ab0:2c3:: with SMTP id 61mr40861869uah.87.1594218632895; Wed, 08 Jul 2020 07:30:32 -0700 (PDT) MIME-Version: 1.0 References: <20190906094534.36060-1-ruifeng.wang@arm.com> <20200707151554.64431-1-ruifeng.wang@arm.com> <20200707151554.64431-2-ruifeng.wang@arm.com> In-Reply-To: <20200707151554.64431-2-ruifeng.wang@arm.com> From: David Marchand Date: Wed, 8 Jul 2020 16:30:21 +0200 Message-ID: To: Ruifeng Wang Cc: Bruce Richardson , Vladimir Medvedkin , John McNamara , Marko Kovacevic , Ray Kinsella , Neil Horman , dev , "Ananyev, Konstantin" , Honnappa Nagarahalli , nd 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 v7 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Jul 7, 2020 at 5:16 PM Ruifeng Wang wrote: > diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h > index b9d49ac87..7889f21b3 100644 > --- a/lib/librte_lpm/rte_lpm.h > +++ b/lib/librte_lpm/rte_lpm.h > @@ -1,5 +1,6 @@ > /* SPDX-License-Identifier: BSD-3-Clause > * Copyright(c) 2010-2014 Intel Corporation > + * Copyright(c) 2020 Arm Limited > */ > > #ifndef _RTE_LPM_H_ > @@ -20,6 +21,7 @@ > #include > #include > #include > +#include > > #ifdef __cplusplus > extern "C" { > @@ -62,6 +64,17 @@ extern "C" { > /** Bitmask used to indicate successful lookup */ > #define RTE_LPM_LOOKUP_SUCCESS 0x01000000 > > +/** @internal Default RCU defer queue entries to reclaim in one go. */ > +#define RTE_LPM_RCU_DQ_RECLAIM_MAX 16 > + > +/** RCU reclamation modes */ > +enum rte_lpm_qsbr_mode { > + /** Create defer queue for reclaim. */ > + RTE_LPM_QSBR_MODE_DQ = 0, > + /** Use blocking mode reclaim. No defer queue created. */ > + RTE_LPM_QSBR_MODE_SYNC > +}; > + > #if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN > /** @internal Tbl24 entry structure. */ > __extension__ > @@ -130,6 +143,28 @@ struct rte_lpm { > __rte_cache_aligned; /**< LPM tbl24 table. */ > struct rte_lpm_tbl_entry *tbl8; /**< LPM tbl8 table. */ > struct rte_lpm_rule *rules_tbl; /**< LPM rules. */ > +#ifdef ALLOW_EXPERIMENTAL_API > + /* 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. */ > +#endif > +}; I can see failures in travis reports for v7 and v6. I reproduced them in my env. 1 function with some indirect sub-type change: [C]'function int rte_lpm_add(rte_lpm*, uint32_t, uint8_t, uint32_t)' at rte_lpm.c:764:1 has some indirect sub-type changes: parameter 1 of type 'rte_lpm*' has sub-type changes: in pointed to type 'struct rte_lpm' at rte_lpm.h:134:1: type size hasn't changed 3 data member insertions: 'rte_rcu_qsbr* rte_lpm::v', at offset 536873600 (in bits) at rte_lpm.h:148:1 'rte_lpm_qsbr_mode rte_lpm::rcu_mode', at offset 536873664 (in bits) at rte_lpm.h:149:1 'rte_rcu_qsbr_dq* rte_lpm::dq', at offset 536873728 (in bits) at rte_lpm.h:150:1 Going back to my proposal of hiding what does not need to be seen. Disclaimer, *this is quick & dirty* but it builds and passes ABI check: $ git diff diff --git a/lib/librte_lpm/rte_lpm.c b/lib/librte_lpm/rte_lpm.c index d498ba761..7109aef6a 100644 --- a/lib/librte_lpm/rte_lpm.c +++ b/lib/librte_lpm/rte_lpm.c @@ -115,6 +115,15 @@ rte_lpm_find_existing(const char *name) return l; } +struct internal_lpm { + /* Public object */ + 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. */ +}; + /* * Allocates memory for LPM object */ @@ -123,6 +132,7 @@ rte_lpm_create(const char *name, int socket_id, const struct rte_lpm_config *config) { char mem_name[RTE_LPM_NAMESIZE]; + struct internal_lpm *internal = NULL; struct rte_lpm *lpm = NULL; struct rte_tailq_entry *te; uint32_t mem_size, rules_size, tbl8s_size; @@ -141,12 +151,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 */ @@ -170,16 +174,23 @@ rte_lpm_create(const char *name, int socket_id, goto exit; } + /* Determine the amount of memory to allocate. */ + mem_size = sizeof(*internal); + 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 memory to store the LPM data structures. */ - lpm = rte_zmalloc_socket(mem_name, mem_size, + internal = rte_zmalloc_socket(mem_name, mem_size, RTE_CACHE_LINE_SIZE, socket_id); - if (lpm == NULL) { + if (internal == NULL) { RTE_LOG(ERR, LPM, "LPM memory allocation failed\n"); rte_free(te); rte_errno = ENOMEM; goto exit; } + lpm = &internal->lpm; lpm->rules_tbl = rte_zmalloc_socket(NULL, (size_t)rules_size, RTE_CACHE_LINE_SIZE, socket_id); @@ -226,6 +237,7 @@ rte_lpm_create(const char *name, int socket_id, void rte_lpm_free(struct rte_lpm *lpm) { + struct internal_lpm *internal; struct rte_lpm_list *lpm_list; struct rte_tailq_entry *te; @@ -247,8 +259,9 @@ rte_lpm_free(struct rte_lpm *lpm) rte_mcfg_tailq_write_unlock(); - if (lpm->dq) - rte_rcu_qsbr_dq_delete(lpm->dq); + internal = container_of(lpm, struct internal_lpm, lpm); + if (internal->dq != NULL) + rte_rcu_qsbr_dq_delete(internal->dq); rte_free(lpm->tbl8); rte_free(lpm->rules_tbl); rte_free(lpm); @@ -276,13 +289,15 @@ rte_lpm_rcu_qsbr_add(struct rte_lpm *lpm, struct rte_lpm_rcu_config *cfg, { char rcu_dq_name[RTE_RCU_QSBR_DQ_NAMESIZE]; struct rte_rcu_qsbr_dq_parameters params = {0}; + struct internal_lpm *internal; - if ((lpm == NULL) || (cfg == NULL)) { + if (lpm == NULL || cfg == NULL) { rte_errno = EINVAL; return 1; } - if (lpm->v) { + internal = container_of(lpm, struct internal_lpm, lpm); + if (internal->v != NULL) { rte_errno = EEXIST; return 1; } @@ -305,20 +320,19 @@ rte_lpm_rcu_qsbr_add(struct rte_lpm *lpm, struct rte_lpm_rcu_config *cfg, params.free_fn = __lpm_rcu_qsbr_free_resource; params.p = lpm; params.v = cfg->v; - lpm->dq = rte_rcu_qsbr_dq_create(¶ms); - if (lpm->dq == NULL) { - RTE_LOG(ERR, LPM, - "LPM QS defer queue creation failed\n"); + internal->dq = rte_rcu_qsbr_dq_create(¶ms); + if (internal->dq == NULL) { + RTE_LOG(ERR, LPM, "LPM QS defer queue creation failed\n"); return 1; } if (dq) - *dq = lpm->dq; + *dq = internal->dq; } else { rte_errno = EINVAL; return 1; } - lpm->rcu_mode = cfg->mode; - lpm->v = cfg->v; + internal->rcu_mode = cfg->mode; + internal->v = cfg->v; return 0; } @@ -502,12 +516,13 @@ _tbl8_alloc(struct rte_lpm *lpm) static int32_t tbl8_alloc(struct rte_lpm *lpm) { + struct internal_lpm *internal = container_of(lpm, struct internal_lpm, lpm); int32_t group_idx; /* tbl8 group index. */ group_idx = _tbl8_alloc(lpm); - if ((group_idx == -ENOSPC) && (lpm->dq != NULL)) { + if (group_idx == -ENOSPC && internal->dq != NULL) { /* If there are no tbl8 groups try to reclaim one. */ - if (rte_rcu_qsbr_dq_reclaim(lpm->dq, 1, NULL, NULL, NULL) == 0) + if (rte_rcu_qsbr_dq_reclaim(internal->dq, 1, NULL, NULL, NULL) == 0) group_idx = _tbl8_alloc(lpm); } @@ -518,20 +533,21 @@ static void tbl8_free(struct rte_lpm *lpm, uint32_t tbl8_group_start) { struct rte_lpm_tbl_entry zero_tbl8_entry = {0}; + struct internal_lpm *internal = container_of(lpm, struct internal_lpm, lpm); - if (!lpm->v) { + if (internal->v == NULL) { /* Set tbl8 group invalid*/ __atomic_store(&lpm->tbl8[tbl8_group_start], &zero_tbl8_entry, __ATOMIC_RELAXED); - } else if (lpm->rcu_mode == RTE_LPM_QSBR_MODE_SYNC) { + } else if (internal->rcu_mode == RTE_LPM_QSBR_MODE_SYNC) { /* Wait for quiescent state change. */ - rte_rcu_qsbr_synchronize(lpm->v, RTE_QSBR_THRID_INVALID); + rte_rcu_qsbr_synchronize(internal->v, RTE_QSBR_THRID_INVALID); /* Set tbl8 group invalid*/ __atomic_store(&lpm->tbl8[tbl8_group_start], &zero_tbl8_entry, __ATOMIC_RELAXED); - } else if (lpm->rcu_mode == RTE_LPM_QSBR_MODE_DQ) { + } else if (internal->rcu_mode == RTE_LPM_QSBR_MODE_DQ) { /* Push into QSBR defer queue. */ - rte_rcu_qsbr_dq_enqueue(lpm->dq, (void *)&tbl8_group_start); + rte_rcu_qsbr_dq_enqueue(internal->dq, (void *)&tbl8_group_start); } } diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h index 7889f21b3..a9568fcdd 100644 --- a/lib/librte_lpm/rte_lpm.h +++ b/lib/librte_lpm/rte_lpm.h @@ -143,12 +143,6 @@ struct rte_lpm { __rte_cache_aligned; /**< LPM tbl24 table. */ struct rte_lpm_tbl_entry *tbl8; /**< LPM tbl8 table. */ struct rte_lpm_rule *rules_tbl; /**< LPM rules. */ -#ifdef ALLOW_EXPERIMENTAL_API - /* 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. */ -#endif }; /** LPM RCU QSBR configuration structure. */ -- David Marchand