On 29-Nov-19 5:44 AM, Yasufumi Ogawa wrote:
Hi Anatoly,
On 2019/11/27 19:26, Burakov, Anatoly wrote:
On 26-Nov-19 7:40 PM, Yasufumi Ogawa wrote:
Hi David,
Sorry for slow reply.
On 2019/11/14 21:27, David Marchand wrote:
On Thu, Nov 14, 2019 at 12:42 PM Yasufumi Ogawa
<yasufu...@gmail.com> wrote:
On 2019/11/14 2:01, Burakov, Anatoly wrote:
On 13-Nov-19 9:43 PM, yasufu...@gmail.com wrote:
From: Yasufumi Ogawa <ogawa.yasuf...@lab.ntt.co.jp>
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: sta...@dpdk.org
Signed-off-by: Yasufumi Ogawa <yasufu...@gmail.com>
---
lib/librte_eal/linux/eal/eal_memalloc.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/lib/librte_eal/linux/eal/eal_memalloc.c
b/lib/librte_eal/linux/eal/eal_memalloc.c
index af6d0d023..11de6d4d6 100644
--- a/lib/librte_eal/linux/eal/eal_memalloc.c
+++ b/lib/librte_eal/linux/eal/eal_memalloc.c
@@ -1365,6 +1365,12 @@ 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+1] = { 0 };
+ /* filename of secondary's fbarray is defined such as
+ * "fbarray_memseg-1048576k-0-0_PID_HOSTNAME" and length of PID
+ * can be 7 digits maximumly.
+ */
+ int fbarray_sec_name_len = 32 + 7 + 1 + HOST_NAME_MAX + 1;
What does 32 stand for? Maybe #define both 32 and 7 values?
Hi Anatoly,
Thank you for your comments! If my understanding is correct, the
prefix
"fbarray_memseg-1048576k-0-0_" is 28 digits and it could be larger if
using the size of hugepage or the number of NUMA nodes are larger
possibly. However, I think 32 digits is still enough.
> Maybe #define both 32 and 7 values?
Yes. I think it should be better to use #define if this values are
referred several times.
We can truncate to RTE_FBARRAY_NAME_LEN in all cases.
And iiuc, rte_fbarray_init will refuse any longer name anyway.
Could I confirm the issue? I've understood that it is failed to
validate the name of fbarray in fully_validate() at
"lib/librte_eal/common/eal_common_fbarray.c:697".
static int
fully_validate(const char *name, unsigned int elt_sz, unsigned int len)
{
if (name == NULL || elt_sz == 0 || len == 0 || len > INT_MAX) {
rte_errno = EINVAL;
return -1;
}
if (strnlen(name, RTE_FBARRAY_NAME_LEN) ==
RTE_FBARRAY_NAME_LEN) {
rte_errno = ENAMETOOLONG;
return -1;
}
return 0;
}
I should overwrite the definition of RTE_FBARRAY_NAME_LEN as previous
patch in this case, and it causes an ABI breakage, right? If so, I
would like to make the change and give up to update stable release.
Thanks,
Yasufumi
It seems we're getting into bikeshedding...
We can do this without ABI breakage. You could have just used
RTE_FBARRAY_NAME_LEN as max fbarray name length for
fbarray_sec_name_len (i.e. that would include hostname + pid +
whatever else there is). The name, as David has pointed out, would be
truncated to RTE_FBARRAY_NAME_LEN anyway (or, more precisely, it will
be refused if it's longer than that), so this is the most you can have
- so you can just use that as the maximum.
I sent v8 patch to change the size of RTE_FBARRAY_NAME_LEN itself to be
allowed the size of secondary's fbarray over 64 bytes. I appreciate if
you agree that.
Why not just limit the name to RTE_FBARRAY_NAME_LEN instead of changing
the definition of RTE_FBARRAY_NAME_LEN?
One the other hand, technically, fbarray API is experimental. The only
structure that uses rte_fbarray is rte_memseg_list, but API's using
either rte_fbarray or rte_memseg_list are either internal (memory/VFIO
subsystem), or are marked as experimental (walk functions).
So i *think* we're actually OK with changing the length of
RTE_FBARRAY_NAME_LEN as far as ABI policy goes: nothing in the stable
ABI gets affected. David, thoughts?
(i think it's probably time to make experimental memory/fbarray stuff
stable, but that's a different conversation...)
Thanks,
Yasufumi
--
Thanks,
Anatoly
