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 4F91DA0353; Tue, 5 Nov 2019 11:14:15 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 28A192C27; Tue, 5 Nov 2019 11:14:15 +0100 (CET) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id A83772BE5 for ; Tue, 5 Nov 2019 11:14:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572948853; 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=ckiHEl6uGTVq/kcEgGB0FtzJBPQWCgNP1+ZvNzSWFE8=; b=hluoCGI+TyMKdj7EUJfhYIrK+APbmulR15FeP7CKvswzZP8vPvEEoBao4/yNCiR1ywN+Vn BINrVhzTbsxznMjPitB6GHWYA7tpFv7QwAudMfy03YWchr7T3JNzF3NpZPf8IX7RvnkaXQ UyYSoDZmGPQVLgt2G3gplJUYsQP+gYA= Received: from mail-vs1-f70.google.com (mail-vs1-f70.google.com [209.85.217.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-418-Gp1Rba_hO9CwfEEHsaNjuQ-1; Tue, 05 Nov 2019 05:14:09 -0500 Received: by mail-vs1-f70.google.com with SMTP id g126so3343047vsg.7 for ; Tue, 05 Nov 2019 02:14:09 -0800 (PST) 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=GQWLjWn6F7uJ41CczViaQRq50iBSeCwR5cus3sSxTh4=; b=sjiz55aKiumYFjAQqOU/Tm22RCnV8iEkDWCFTrrFxOn5Lz7JbQThAU/Q6BzblkMjjD H4IQuFRC1CW6aiSM0nh2/S/tuShB7qV8EMf0YA2FS5spUoFhVSv+c+30VP/sCFAjGUoh JJAzKpfd61EBDHxCOnHWB+f4G1wdE2WwLC+b2jmOa394tu8RZqjWDo0K2oxxGesLOTgk t8B/scAKJb3sk6BtklFzPgFhzn6g1XuVGSOdUWkYF245yqpKclk5zICzSbMqf+TonTLv DhVHgcJ5I+FS3l7uCoi1oPYj3BXkrpkeyhYXQ0VqnFh0Q4ZrqXj85bKCVFNXWo9MGTYE J95Q== X-Gm-Message-State: APjAAAWZAlUuyKvpV4jxcHfPAfhRfOKVKpjdkPFAU5ENFKelLG7KQFbA 7XZX8+h5fGRwl8z7T4M4mBZI7lQSZf6j58Wq9LrgGoBQcvA38JeRkkOuVNnbDZ1TzNeRWEfmuby lXSNJyNOqCGHtXRZfVqc= X-Received: by 2002:a1f:60ce:: with SMTP id u197mr13899665vkb.80.1572948849373; Tue, 05 Nov 2019 02:14:09 -0800 (PST) X-Google-Smtp-Source: APXvYqxHP/dNn8ukSydcdP0vbpbO2si9Y+THIAbS1uGM53+W5Rl99us2jArZeQ4SDL4aIhfxOP4PfZzy+60Y6hm2ueI= X-Received: by 2002:a1f:60ce:: with SMTP id u197mr13899650vkb.80.1572948848967; Tue, 05 Nov 2019 02:14:08 -0800 (PST) MIME-Version: 1.0 References: <20190724082031.45546-1-yasufum.o@gmail.com> <20191101090413.17997-1-yasufum.o@gmail.com> <20191101090413.17997-2-yasufum.o@gmail.com> <05605cac-693e-042a-a3cb-6506b1ec653e@intel.com> In-Reply-To: <05605cac-693e-042a-a3cb-6506b1ec653e@intel.com> From: David Marchand Date: Tue, 5 Nov 2019 11:13:57 +0100 Message-ID: To: "Burakov, Anatoly" , Yasufumi Ogawa Cc: "Ananyev, Konstantin" , dev , dpdk stable , Yasufumi Ogawa X-MC-Unique: Gp1Rba_hO9CwfEEHsaNjuQ-1 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v6 1/1] fbarray: fix duplicated fbarray file in secondary 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" Hello Anatoly, Yasufumi, On Mon, Nov 4, 2019 at 11:20 AM Burakov, Anatoly wrote: > > On 01-Nov-19 9:04 AM, yasufum.o@gmail.com wrote: > > From: Yasufumi Ogawa > > > > In secondary_msl_create_walk(), it creates a file for fbarrays with its > > PID for reserving unique name among secondary processes. However, it > > does not work if several secondaries run as app containers because each > > of containerized secondary has PID 1, and failed to reserve unique name > > other than first one. To reserve unique name in each of containers, use > > hostname in addition to PID. > > > > Cc: stable@dpdk.org We can't backport this as is, see below. > > > > Signed-off-by: Yasufumi Ogawa > > --- > > lib/librte_eal/common/include/rte_fbarray.h | 2 +- > > lib/librte_eal/linux/eal/eal_memalloc.c | 11 ++++++++--- > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_eal/common/include/rte_fbarray.h b/lib/librte_e= al/common/include/rte_fbarray.h > > index 6dccdbec9..5c2815093 100644 > > --- a/lib/librte_eal/common/include/rte_fbarray.h > > +++ b/lib/librte_eal/common/include/rte_fbarray.h > > @@ -39,7 +39,7 @@ extern "C" { > > #include > > #include > > > > -#define RTE_FBARRAY_NAME_LEN 64 > > +#define RTE_FBARRAY_NAME_LEN NAME_MAX The change on RTE_FBARRAY_NAME_LEN breaks the ABI, so we cannot backport this as is. For 19.11, we can allow this breakage, but we need an update of the release notes. Besides, what is the impact in terms of memory consumption? > > > > struct rte_fbarray { > > char name[RTE_FBARRAY_NAME_LEN]; /**< name associated with an arr= ay */ > > diff --git a/lib/librte_eal/linux/eal/eal_memalloc.c b/lib/librte_eal/l= inux/eal/eal_memalloc.c > > index af6d0d023..24f0275c9 100644 > > --- a/lib/librte_eal/linux/eal/eal_memalloc.c > > +++ b/lib/librte_eal/linux/eal/eal_memalloc.c > > @@ -1365,6 +1365,7 @@ secondary_msl_create_walk(const struct rte_memseg= _list *msl, > > struct rte_memseg_list *primary_msl, *local_msl; > > char name[PATH_MAX]; > > int msl_idx, ret; > > + char hostname[HOST_NAME_MAX] =3D { 0 }; > > > > if (msl->external) > > return 0; > > @@ -1373,9 +1374,13 @@ secondary_msl_create_walk(const struct rte_memse= g_list *msl, > > primary_msl =3D &mcfg->memsegs[msl_idx]; > > local_msl =3D &local_memsegs[msl_idx]; > > > > - /* create distinct fbarrays for each secondary */ > > - snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%i", > > - primary_msl->memseg_arr.name, getpid()); > > + /* Create distinct fbarrays for each secondary by using PID and > > + * hostname. The reason why using hostname is because PID could b= e > > + * duplicated among secondaries if it is launched in a container. > > + */ > > + gethostname(hostname, HOST_NAME_MAX); Personal preference, s/HOST_NAME_MAX/sizeof(hostname)/. hostname[] is HOST_NAME_MAX bytes long. In the worst case, we can get a non NULL terminated hostname string. " gethostname() returns the null-terminated hostname in the character array name, which has a length of len bytes. If the null-terminated hostname is too large to fit, then the name is truncated, and no error is returned (but see NOTES below). POSIX.1-2001 says that if such truncation occurs, then it is unspecified whether the returned buffer includes a terminating null byte. ... NOTES SUSv2 guarantees that "Host names are limited to 255 bytes". POSIX.1-2001 guarantees that "Host names (not including the terminating null byte) are limited to HOST_NAME_MAX bytes". On Linux, HOST_NAME_MAX is defined with the value 64, which has been the limit since Linux 1.0 (earlier kernels imposed a limit of 8 bytes). " How about making hostname[] HOST_NAME_MAX+1 bytes long? > > + snprintf(name, RTE_FBARRAY_NAME_LEN, "%s_%s_%d", > > + primary_msl->memseg_arr.name, hostname, (int)getp= id()); > > > > ret =3D rte_fbarray_init(&local_msl->memseg_arr, name, > > primary_msl->memseg_arr.len, > > > > I think the order should be reversed. Both containers and non-containers > can have their hostname set, and RTE_FBARRAY_NAME_LEN is of fairly > limited length, so if the hostname is long enough, the PID never gets > into the name string, resulting in duplicates. It is better have pid firs= t. Anatoly, On the principle, it seems better, yes. Just the comment on RTE_FBARRAY_NAME_LEN indicates that you missed the change at the top of the patch. What do you think of this change? --=20 David Marchand