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 32151A0524; Wed, 5 May 2021 12:25:02 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E59C840040; Wed, 5 May 2021 12:25:01 +0200 (CEST) Received: from mail-il1-f182.google.com (mail-il1-f182.google.com [209.85.166.182]) by mails.dpdk.org (Postfix) with ESMTP id 3754F4003C for ; Wed, 5 May 2021 12:25:00 +0200 (CEST) Received: by mail-il1-f182.google.com with SMTP id h6so1279609ila.7 for ; Wed, 05 May 2021 03:25:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=v7R9NHbt6d0to/A6bnO+qsoO5SozEzdMVplzKO1w+kc=; b=FXGi8TKSFeMXAPQH3kIBTZleXrcK+Cy7PlULT5oHTLjBLHB4Fa0xBHhWpFGFK/EvoY 2yF+4qIUe8c3b+vD8vnlcaLoY4+Px0U35cKsJ//TJrVQtytYqYC2edWgsF5RKzRvjbhz W9b0h1dSCtrHGugtUR7O+2dVCWaIyRZbOB1q1GBv19YN7AyfA/Vrms6mZuzswhpW94EF l7UTziIoXXDKbrm6fvtkT9WEzB7vy0I5ZUVIARhXGsYLdx7GwIIQ/zD/eHQrvaONjnte 5kBnZqruAZO5dfYcVktO+KwsPXYpgOHzuceqKRq3Q0Hil7y56gLpRZOwcJInoslSHRHk OSXQ== 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=v7R9NHbt6d0to/A6bnO+qsoO5SozEzdMVplzKO1w+kc=; b=nuxRVe11lAXXF23VtQJlsNntTy575h1Yz4iCgFwrp8sg0gxd6w+0TRL7OS/eUaBR6w ujLPF/aAUx2dgPg4cVW3ZfjVJSJPnDNYwDV6y4s9OBW6/8bSh6NNEpldu/MF0jWj22d+ jRt8DzMiU9796HC2q1Nqlqcx89N1lfRiGXB+2KQzxpaB6Mkg9ERMtjMYkuPhZie9JM0R 0UY4SWHhVahMaBewbP4Z40qxbYQyg9FZ3O3kI3kBWaQ5GOha2igWudKAnSe7pQ1w4REU Hi8uFXD7vQAgJeZItlkaV8ITAp3LrOehZV2ihW0BhpD/i3eD0kELqytx9rOHj47F8FEX bmnA== X-Gm-Message-State: AOAM530wQZoI1fDQb2NN+5n9BdJzEtho+VLOxr62z0DJyWdZWU0tFk57 nG5Th/yf5dda3CdDynZ24+TiyN6H8tQF8O7HSWQ= X-Google-Smtp-Source: ABdhPJyMMZINP3CJUaiZzPCvGFWruBhuvFk1amlyD0E0CiXnA0sOzmofgYmpbQsVQvS79seWN3PzYHWTkkVeFNGX7lQ= X-Received: by 2002:a92:c566:: with SMTP id b6mr24835066ilj.162.1620210299651; Wed, 05 May 2021 03:24:59 -0700 (PDT) MIME-Version: 1.0 References: <20210503163429.3222198-1-jerinj@marvell.com> In-Reply-To: From: Jerin Jacob Date: Wed, 5 May 2021 15:54:43 +0530 Message-ID: To: David Marchand Cc: "Power, Ciara" , "Richardson, Bruce" , "jerinj@marvell.com" , "dev@dpdk.org" , "thomas@monjalon.net" 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 Wed, May 5, 2021 at 2:14 PM David Marchand wrote: > > 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? I think, that may be better. Probably this patch can go in rc2. The dynamic feature can be pushed to the next release. I will leave it to the maintainers to decide. I am fine with any scheme. > > > 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 >