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 B6FAFA0C4C; Thu, 14 Oct 2021 11:40:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 724EA40042; Thu, 14 Oct 2021 11:40:13 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id D88F740041 for ; Thu, 14 Oct 2021 11:40:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1634204411; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1TZ1TG6xGpZWHju98lCv1TrZqDJQXT5reRN0kDBKFeA=; b=RC1OusshXxxE6I6z9XxI3pPmK59t8l9biLQMl9m+jBCPkri4LgZ3v5x03l83myOAbVsf4L PfvT4fXKu/E86sJOHevc45Il5KLm+YdG7XXKhCpQgp7sSkCetCeVjZdiuPI7flkqWzUmzk nyC4MLB2AHCnSd92Z3y9SHDAxFir9jI= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-493-rEz2VHF3NOuo95FFi5lSDQ-1; Thu, 14 Oct 2021 05:40:10 -0400 X-MC-Unique: rEz2VHF3NOuo95FFi5lSDQ-1 Received: by mail-wr1-f69.google.com with SMTP id h99-20020adf906c000000b001644add8925so925802wrh.0 for ; Thu, 14 Oct 2021 02:40:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:subject:in-reply-to :content-transfer-encoding; bh=1TZ1TG6xGpZWHju98lCv1TrZqDJQXT5reRN0kDBKFeA=; b=UFXqUZWLzK0T2w3KA0wIuXLFOWomcvg81T73x0hWUKSF3zBRrrzCVbdMYfvQrzWeIT yC5Ea0SINKJ7qilBVBkkBzqI0quNvyZacoF/BKYKDbLmtnBOwNgpZo5D3Clg92CQCGQr hTz5FJ4u2/EOanAotE1TSN5dDrzpcpXL/KyfRkDLH95pA2wpvuOFet/1lYvQbfZttwFp QDnpoewVJHzTKAQSZxIZmd7LYI3BSPDK3PWpnE/guAGevkr9MVLo1rDx1CUChLMMteZ6 vMidvzj7sTdB7nTd6LAfoNj/WOTxY5Us27NxQk4BuS/T6BdEgc4RJ5R7R2xTSs65iIBP Pjbw== X-Gm-Message-State: AOAM533Uo9/rU9VOIV4zxeR4iLRvU69EfaLC1yh4y1YDm8DzBvPxsYy7 9ynGIQ5ykPReBv8O4nttKPEyuc9b6Y0a2P8tqQZSFWoZGdIpT7E8OmvoJV7y1NAOFl0kowuvCte sJZA= X-Received: by 2002:adf:9b97:: with SMTP id d23mr5222287wrc.53.1634204409081; Thu, 14 Oct 2021 02:40:09 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwBHefO21jrIlYEOgBv5xNLUsFlPxZF/V5ssSTL6PdqacVgEGJ6nQCf7TcdNzOilhyANXuN/A== X-Received: by 2002:adf:9b97:: with SMTP id d23mr5222254wrc.53.1634204408884; Thu, 14 Oct 2021 02:40:08 -0700 (PDT) Received: from [192.168.0.36] ([78.19.105.235]) by smtp.gmail.com with ESMTPSA id j1sm1966521wrd.28.2021.10.14.02.40.08 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Oct 2021 02:40:08 -0700 (PDT) Message-ID: Date: Thu, 14 Oct 2021 10:40:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0 To: Bruce Richardson , dev@dpdk.org Cc: Ciara Power , David Marchand , Anatoly Burakov References: <20210915141030.23514-1-bruce.richardson@intel.com> <20211012163908.758767-1-bruce.richardson@intel.com> <20211012163908.758767-3-bruce.richardson@intel.com> From: Kevin Traynor In-Reply-To: <20211012163908.758767-3-bruce.richardson@intel.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ktraynor@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 2/4] telemetry: fix socket path conflicts for in-memory mode 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 12/10/2021 17:39, Bruce Richardson wrote: > When running using in-memory mode, multiple processes can use the same > runtime dir, leading to conflicts with the telemetry sockets in that > directory. We can resolve this by appending a suffix to each socket > beyond the first, with the suffix being an increasing counter value. > Each process uses the first unused socket counter value. > > Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality") > > Reported-by: David Marchand > Signed-off-by: Bruce Richardson > Acked-by: Ciara Power Thanks Bruce, lgtm. Nice solution to keep existing name where possible and to give as predictable as you can names for others. Acked-by: Kevin Traynor > --- > lib/telemetry/telemetry.c | 65 +++++++++++++++++++++++++++++---------- > 1 file changed, 49 insertions(+), 16 deletions(-) > > diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c > index 48f4c7ba46..a7483167d4 100644 > --- a/lib/telemetry/telemetry.c > +++ b/lib/telemetry/telemetry.c > @@ -457,28 +457,45 @@ create_socket(char *path) > > struct sockaddr_un sun = {.sun_family = AF_UNIX}; > strlcpy(sun.sun_path, path, sizeof(sun.sun_path)); > - unlink(sun.sun_path); > + TMTY_LOG(DEBUG, "Attempting socket bind to path '%s'\n", path); > + > if (bind(sock, (void *) &sun, sizeof(sun)) < 0) { > struct stat st; > > - TMTY_LOG(ERR, "Error binding socket: %s\n", strerror(errno)); > - if (stat(socket_dir, &st) < 0 || !S_ISDIR(st.st_mode)) > + TMTY_LOG(DEBUG, "Initial bind to socket '%s' failed.\n", path); > + > + /* first check if we have a runtime dir */ > + if (stat(socket_dir, &st) < 0 || !S_ISDIR(st.st_mode)) { > TMTY_LOG(ERR, "Cannot access DPDK runtime directory: %s\n", socket_dir); > - sun.sun_path[0] = 0; > - goto error; > + close(sock); > + return -ENOENT; > + } > + > + /* check if current socket is active */ > + if (connect(sock, (void *)&sun, sizeof(sun)) == 0) { > + close(sock); > + return -EADDRINUSE; > + } > + > + /* socket is not active, delete and attempt rebind */ > + TMTY_LOG(DEBUG, "Attempting unlink and retrying bind\n"); > + unlink(sun.sun_path); > + if (bind(sock, (void *) &sun, sizeof(sun)) < 0) { > + TMTY_LOG(ERR, "Error binding socket: %s\n", strerror(errno)); > + close(sock); > + return -errno; /* if unlink failed, this will be -EADDRINUSE as above */ > + } > } > > if (listen(sock, 1) < 0) { > TMTY_LOG(ERR, "Error calling listen for socket: %s\n", strerror(errno)); > - goto error; > + unlink(sun.sun_path); > + close(sock); > + return -errno; > } > + TMTY_LOG(DEBUG, "Socket creation and binding ok\n"); > > return sock; > - > -error: > - close(sock); > - unlink_sockets(); > - return -1; > } > > static void > @@ -511,8 +528,10 @@ telemetry_legacy_init(void) > return -1; > } > v1_socket.sock = create_socket(v1_socket.path); > - if (v1_socket.sock < 0) > + if (v1_socket.sock < 0) { > + v1_socket.path[0] = '\0'; > return -1; > + } > rc = pthread_create(&t_old, NULL, socket_listener, &v1_socket); > if (rc != 0) { > TMTY_LOG(ERR, "Error with create legcay socket thread: %s\n", > @@ -533,7 +552,9 @@ telemetry_legacy_init(void) > static int > telemetry_v2_init(void) > { > + char spath[sizeof(v2_socket.path)]; > pthread_t t_new; > + short suffix = 0; > int rc; > > v2_socket.num_clients = &v2_clients; > @@ -544,15 +565,27 @@ telemetry_v2_init(void) > rte_telemetry_register_cmd("/help", command_help, > "Returns help text for a command. Parameters: string command"); > v2_socket.fn = client_handler; > - if (strlcpy(v2_socket.path, get_socket_path(socket_dir, 2), > - sizeof(v2_socket.path)) >= sizeof(v2_socket.path)) { > + if (strlcpy(spath, get_socket_path(socket_dir, 2), sizeof(spath)) >= sizeof(spath)) { > TMTY_LOG(ERR, "Error with socket binding, path too long\n"); > return -1; > } > + memcpy(v2_socket.path, spath, sizeof(v2_socket.path)); > > v2_socket.sock = create_socket(v2_socket.path); > - if (v2_socket.sock < 0) > - return -1; > + while (v2_socket.sock < 0) { > + /* bail out on unexpected error, or suffix wrap-around */ > + if (v2_socket.sock != -EADDRINUSE || suffix < 0) { > + v2_socket.path[0] = '\0'; /* clear socket path */ > + return -1; > + } > + /* add a suffix to the path if the basic version fails */ > + if (snprintf(v2_socket.path, sizeof(v2_socket.path), "%s:%d", > + spath, ++suffix) >= (int)sizeof(v2_socket.path)) { > + TMTY_LOG(ERR, "Error with socket binding, path too long\n"); > + return -1; > + } > + v2_socket.sock = create_socket(v2_socket.path); > + } > rc = pthread_create(&t_new, NULL, socket_listener, &v2_socket); > if (rc != 0) { > TMTY_LOG(ERR, "Error with create socket thread: %s\n", >