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 08DBBA0524; Wed, 5 May 2021 10:44:35 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E51FB40040; Wed, 5 May 2021 10:44:34 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 635C04003C for ; Wed, 5 May 2021 10:44:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620204272; 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=fXFxPsKOBNikyoi9kbfsGCRZ7Vk8KkGjw83G6UzT+tM=; b=LHPejk2hAbTwBFRI1Ml0BVoDHeaY8PxFG284yDV33hrZa3ec0lOm9Hj8py4/SSXWCk75xb nHaYgyjxjPRpao8pZD+mHPhqnphMStkAXQ5ATNeAtCSHQEjWFF9htcKMgjt4nQmc4/x/Vb ebKWAv/OEnbgnPozB8/iBY3SVYM4B4I= Received: from mail-vs1-f71.google.com (mail-vs1-f71.google.com [209.85.217.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-168-D_V1FNFYMBe2ddTYg5L7MA-1; Wed, 05 May 2021 04:44:29 -0400 X-MC-Unique: D_V1FNFYMBe2ddTYg5L7MA-1 Received: by mail-vs1-f71.google.com with SMTP id v27-20020a67c89b0000b02902227f70aa8fso745605vsk.6 for ; Wed, 05 May 2021 01:44:29 -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=fXFxPsKOBNikyoi9kbfsGCRZ7Vk8KkGjw83G6UzT+tM=; b=tZNXwPEfzoTdrwWA45yoUlIqUQ2KjmpgioBhJPez+JE/dYCcxdPZ7acst0GZS2+Y6p GuhvHGhv35x1Ljgjw358nQsZlLCkEBJnO8FzPilOB8q0lTnZXZRelH+Tu54xwRC82SLv YsA69oZQNO8hNEm80qQdCcpmigVLT9pHs04+DO/CquEa9rKEg5VNclFvZoRgs6xzFKiW w2kPYFykG7KS6Os7sc/oQoLcsj93Bcu/Z5vGEO64yubshM5XRLGp4/vuni8bO+Qxm2mZ MV2njbxmB3Yeq7vjLU48UD/QwW7mGim0iAIvGNiG0nOYdtrrQ48E5KWnWsa1UDBm2Y3N 7zcg== X-Gm-Message-State: AOAM533HjqYG7oZ6cdJdwSuVxHQavhscvtdBFGD4NSYZVUzRPIZlwAV9 yec9EHY78COBm5bbpI5KWKear8p//tvGw9syjU11n7VUFuYFTPtdnyXa1AvRWDPyHXSJpdwGEeH +118AA/OVORlkRcrv4KE= X-Received: by 2002:a1f:3285:: with SMTP id y127mr1048338vky.17.1620204268659; Wed, 05 May 2021 01:44:28 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw98UftEZ8R+dIToidOrl6Ery4vewoaYa/HrWijGzSom7T+NMCbe1Oe8DIugC4Ql5ASzIT31A35qSfsmgn+hSQ= X-Received: by 2002:a1f:3285:: with SMTP id y127mr1048327vky.17.1620204268438; Wed, 05 May 2021 01:44:28 -0700 (PDT) MIME-Version: 1.0 References: <20210503163429.3222198-1-jerinj@marvell.com> In-Reply-To: From: David Marchand Date: Wed, 5 May 2021 10:44:17 +0200 Message-ID: To: "Power, Ciara" , "Richardson, Bruce" Cc: "jerinj@marvell.com" , "dev@dpdk.org" , "thomas@monjalon.net" 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] telemetry: remove internal symbol from public header 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" On Tue, May 4, 2021 at 6:19 PM Power, Ciara wrote: > > >-----Original Message----- > >From: Richardson, Bruce > >Sent: Tuesday 4 May 2021 13:14 > >To: jerinj@marvell.com > >Cc: Power, Ciara ; dev@dpdk.org; > >thomas@monjalon.net > >Subject: Re: [PATCH] telemetry: remove internal symbol from public header > > > >On Mon, May 03, 2021 at 10:04:28PM +0530, jerinj@marvell.com wrote: > >> From: Jerin Jacob > >> > >> Remove TELEMETRY_MAX_CALLBACKS symbol from public rte_telemetry.h > >> header file. > >> > >> Signed-off-by: Jerin Jacob > >> --- > >Acked-by: Bruce Richardson > > Thanks, > Acked-by: Ciara Power I agree this define should be hidden. Just, what do you think of using a dynamic allocation and remove the limitation entirely? diff --git a/lib/telemetry/rte_telemetry.h b/lib/telemetry/rte_telemetry.h index 031db9e968..8776998b54 100644 --- a/lib/telemetry/rte_telemetry.h +++ b/lib/telemetry/rte_telemetry.h @@ -10,8 +10,6 @@ #ifndef _RTE_TELEMETRY_H_ #define _RTE_TELEMETRY_H_ -/** Maximum number of telemetry callbacks. */ -#define TELEMETRY_MAX_CALLBACKS 64 /** Maximum length for string used in object. */ #define RTE_TEL_MAX_STRING_LEN 64 /** Maximum length of string. */ @@ -285,7 +283,7 @@ typedef void * (*handler)(void *sock_id); * @return * -EINVAL for invalid parameters failure. * @return - * -ENOENT if max callbacks limit has been reached. + * -ENOMEM for mem allocation failure. */ __rte_experimental int diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c index 68b479e0e4..6baba57ec2 100644 --- a/lib/telemetry/telemetry.c +++ b/lib/telemetry/telemetry.c @@ -59,7 +59,7 @@ static uint32_t logtype; rte_log_ptr(RTE_LOG_ ## l, logtype, "TELEMETRY: " __VA_ARGS__) /* list of command callbacks, with one command registered by default */ -static struct cmd_callback callbacks[TELEMETRY_MAX_CALLBACKS]; +static struct cmd_callback *callbacks; static int num_callbacks; /* How many commands are registered */ /* Used when accessing or modifying list of command callbacks */ static rte_spinlock_t callback_sl = RTE_SPINLOCK_INITIALIZER; @@ -70,15 +70,21 @@ static uint16_t v2_clients; int rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help) { + struct cmd_callback *new_callbacks; int i = 0; if (strlen(cmd) >= MAX_CMD_LEN || fn == NULL || cmd[0] != '/' || strlen(help) >= MAX_HELP_LEN) return -EINVAL; - if (num_callbacks >= TELEMETRY_MAX_CALLBACKS) - return -ENOENT; rte_spinlock_lock(&callback_sl); + new_callbacks = realloc(callbacks, sizeof(callbacks[0]) * (num_callbacks + 1)); + if (new_callbacks == NULL) { + rte_spinlock_unlock(&callback_sl); + return -ENOMEM; + } + callbacks = new_callbacks; + while (i < num_callbacks && strcmp(cmd, callbacks[i].cmd) > 0) i++; if (i != num_callbacks) And there is a race to fix in list_commands() (which accesses the callbacks array without taking the lock). -- David Marchand