This patch limits the number of client connections to the new telemetry socket. The limit is set at 10. Signed-off-by: Ciara Power <ciara.power@intel.com> --- lib/librte_telemetry/telemetry.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c index 56a2fed3f5..5c9d7983e3 100644 --- a/lib/librte_telemetry/telemetry.c +++ b/lib/librte_telemetry/telemetry.c @@ -7,6 +7,7 @@ #include <sys/socket.h> #include <sys/un.h> #include <dlfcn.h> +#include <semaphore.h> /* we won't link against libbsd, so just always use DPDKs-specific strlcpy */ #undef RTE_USE_LIBBSD @@ -23,6 +24,7 @@ #define MAX_CMD_LEN 56 #define MAX_HELP_LEN 64 #define MAX_OUTPUT_LEN (1024 * 16) +#define MAX_CONNECTIONS 10 static void * client_handler(void *socket); @@ -37,6 +39,7 @@ struct socket { int sock; char path[sizeof(((struct sockaddr_un *)0)->sun_path)]; handler fn; + sem_t *limit; }; static struct socket v2_socket; /* socket for v2 telemetry */ static struct socket v1_socket; /* socket for v1 telemetry */ @@ -46,6 +49,7 @@ static struct cmd_callback callbacks[TELEMETRY_MAX_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; +static sem_t v2_limit; int rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help) @@ -263,6 +267,7 @@ client_handler(void *sock_id) bytes = read(s, buffer, sizeof(buffer) - 1); } close(s); + sem_post(&v2_limit); return NULL; } @@ -272,6 +277,8 @@ socket_listener(void *socket) while (1) { pthread_t th; struct socket *s = (struct socket *)socket; + if (s->limit != NULL) + sem_wait(s->limit); int s_accepted = accept(s->sock, NULL, NULL); if (s_accepted < 0) { snprintf(telemetry_log_error, @@ -372,6 +379,9 @@ telemetry_v2_init(const char *runtime_dir) { pthread_t t_new; + RTE_BUILD_BUG_ON(_SC_SEM_VALUE_MAX < MAX_CONNECTIONS); + (void)sem_init(&v2_limit, 0, MAX_CONNECTIONS); + v2_socket.limit = &v2_limit; rte_telemetry_register_cmd("/", list_commands, "Returns list of available commands, Takes no parameters"); rte_telemetry_register_cmd("/info", json_info, -- 2.17.1
On Mon, 18 May 2020 17:12:32 +0100
Ciara Power <ciara.power@intel.com> wrote:
> This patch limits the number of client connections to the new telemetry
> socket. The limit is set at 10.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
The upper limit is fine, but using System V semaphores is not a good
way to do this. Please consider using some other atomic primitive.
19/05/2020 02:07, Stephen Hemminger:
> On Mon, 18 May 2020 17:12:32 +0100
> Ciara Power <ciara.power@intel.com> wrote:
>
> > This patch limits the number of client connections to the new telemetry
> > socket. The limit is set at 10.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> The upper limit is fine, but using System V semaphores is not a good
> way to do this. Please consider using some other atomic primitive.
Why no reply?
>-----Original Message-----
>From: Thomas Monjalon <thomas@monjalon.net>
>Sent: Friday 10 July 2020 21:51
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org; Laatz, Kevin <kevin.laatz@intel.com>; Stephen
>Hemminger <stephen@networkplumber.org>; Richardson, Bruce
><bruce.richardson@intel.com>
>Subject: Re: [dpdk-dev] [PATCH 20.08] telemetry: add upper limit on
>connections
>
>19/05/2020 02:07, Stephen Hemminger:
>> On Mon, 18 May 2020 17:12:32 +0100
>> Ciara Power <ciara.power@intel.com> wrote:
>>
>> > This patch limits the number of client connections to the new
>> > telemetry socket. The limit is set at 10.
>> >
>> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>>
>> The upper limit is fine, but using System V semaphores is not a good
>> way to do this. Please consider using some other atomic primitive.
>
>Why no reply?
>
Hi Thomas,
I had contacted Stephen offline for some detail on his suggestion. I thought I would have a patch sent soon after, but it took longer than expected, and I forgot to reply on list.
I have a patch in progress that will hopefully be sent up soon after internal review.
Thanks,
Ciara
This patch limits the number of client connections to the new telemetry socket. The limit is set to 10. Signed-off-by: Ciara Power <ciara.power@intel.com> --- lib/librte_telemetry/telemetry.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c index e7e3d861d..025228273 100644 --- a/lib/librte_telemetry/telemetry.c +++ b/lib/librte_telemetry/telemetry.c @@ -23,6 +23,7 @@ #define MAX_CMD_LEN 56 #define MAX_HELP_LEN 64 #define MAX_OUTPUT_LEN (1024 * 16) +#define MAX_CONNECTIONS 10 static void * client_handler(void *socket); @@ -37,6 +38,7 @@ struct socket { int sock; char path[sizeof(((struct sockaddr_un *)0)->sun_path)]; handler fn; + uint16_t *num_clients; }; static struct socket v2_socket; /* socket for v2 telemetry */ static struct socket v1_socket; /* socket for v1 telemetry */ @@ -46,6 +48,7 @@ static struct cmd_callback callbacks[TELEMETRY_MAX_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; +static uint16_t v2_clients; int rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char *help) @@ -263,6 +266,7 @@ client_handler(void *sock_id) bytes = read(s, buffer, sizeof(buffer) - 1); } close(s); + __atomic_sub_fetch(&v2_clients, 1, __ATOMIC_RELAXED); return NULL; } @@ -279,6 +283,16 @@ socket_listener(void *socket) "Error with accept, telemetry thread quitting"); return NULL; } + if (s->num_clients != NULL) { + uint16_t conns = __atomic_load_n(s->num_clients, + __ATOMIC_RELAXED); + if (conns >= MAX_CONNECTIONS) { + close(s_accepted); + continue; + } + __atomic_add_fetch(s->num_clients, 1, + __ATOMIC_RELAXED); + } pthread_create(&th, NULL, s->fn, (void *)(uintptr_t)s_accepted); pthread_detach(th); } @@ -373,6 +387,7 @@ telemetry_v2_init(const char *runtime_dir, rte_cpuset_t *cpuset) { pthread_t t_new; + v2_socket.num_clients = &v2_clients; rte_telemetry_register_cmd("/", list_commands, "Returns list of available commands, Takes no parameters"); rte_telemetry_register_cmd("/info", json_info, -- 2.17.1
15/07/2020 17:03, Ciara Power:
> This patch limits the number of client connections to the new telemetry
> socket. The limit is set to 10.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
Not sure I should wait for any review.
Applied, thanks