Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/1] fbarray: get fbarrays from containerized secondary

[Yasufumi Ogawa <yasufum.o@gmail.com>]

On 2019/07/09 19:26, Burakov, Anatoly wrote:
> On 09-Jul-19 11:24 AM, Burakov, Anatoly wrote:
>> On 09-Jul-19 11:22 AM, Yasufumi Ogawa wrote:
>>> Hi Anatoly,
>>>
>>> On 2019/07/05 17:53, Burakov, Anatoly wrote:
>>>> On 16-Apr-19 4:43 AM, ogawa.yasufumi@lab.ntt.co.jp wrote:
>>>>> From: Yasufumi Ogawa <ogawa.yasufumi@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 secondary is run as app container because each of
>>>>> containerized secondary has PID 1. To reserve unique name, use 
>>>>> hostname
>>>>> instead of PID if the value is 1.
>>>>>
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Yasufumi Ogawa <ogawa.yasufumi@lab.ntt.co.jp>
>>>>> ---
>>>>
>>>> I'm not too well versed in containers - is this hostname 1) always 
>>>> set, and 2) always unique?
>>> For docker, 1) hostname is always set. 2) The hostname is decided as 
>>> short form of container ID, so it might not be unique even though 
>>> very low possibility.
>>>
>>> I found that we can get whole container ID in `/proc/self/cgroup` as 
>>> discussed [1]. I think using hostname is reasonable way without 
>>> running many secondary processes. However, it might be better to use 
>>> 64 digits full container ID instead of 12 digits short ID if ensure 
>>> uniqueness strongly. What do yo think?
>>>
>>> [1] 
>>> https://forums.docker.com/t/get-a-containers-full-id-from-inside-of-itself/37237 
>>>
>>
>> I think it's better to err on the side of caution and guarantee better 
>> uniqueness. This code will get into an LTS and will be used for years 
>> to come :)
>>
> 
> ...however, i think a full 64-digit ID won't even fit into the fbarray 
> filename, as i believe it's limited to something like 64 chars. Perhaps 
> hostname would be enough after all... or we can increase fbarray name 
> length - that would require ABI breakage but the ABI is already broken 
> in this release, so it's OK i think.
OK.

 >> Wouldn't an error in fscanf() leak the file handle? I think you need 
to fclose() before checking the result.
 > I would like to fix it.
I would like send v3 patch for fixing for fclose().

Thanks,
Yasufumi