[PATCH v5 1/5] PID namespace translation support

[Ákos Uzonyi]

On Wed, 5 Aug 2020 at 02:30, Dmitry V. Levin <ldv at altlinux.org> wrote:
> On Wed, Aug 05, 2020 at 02:14:36AM +0200, Ákos Uzonyi wrote:
> > On Wed, 5 Aug 2020 at 02:02, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > On Wed, Aug 05, 2020 at 01:53:37AM +0200, Ákos Uzonyi wrote:
> > > > On Wed, 5 Aug 2020 at 00:31, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > > On Tue, Aug 04, 2020 at 07:52:46PM +0200, Ákos Uzonyi wrote:
> > > > > > On Mon, 3 Aug 2020 at 23:43, Dmitry V. Levin <ldv at altlinux.org> wrote:
> > > > > > > On Mon, Aug 03, 2020 at 09:19:12PM +0200, Ákos Uzonyi wrote:
> > > > > > > [...]
> > > > > > > > +void
> > > > > > > > +pidns_init(void)
> > > > > > > > +{
> > > > > > > > +     if (proc_data_cache)
> > > > > > > > +             return;
> > > > > > > > +
> > > > > > > > +     pid_max = INT_MAX;
> > > > > > > > +     if (read_int_from_file("/proc/sys/kernel/pid_max", &pid_max) < 0)
> > > > > > > > +             debug_func_perror_msg("reading /proc/sys/kernel/pid_max");
> > > > > > > > +     pid_max_size = ilog2_32(pid_max - 1) + 1;
> > > > > > > > +     pid_max_size_lg = ilog2_32(pid_max_size - 1) + 1;
> > > > > > > > +
> > > > > > > > +     for (int i = 0; i < PT_COUNT; i++)
> > > > > > > > +             ns_pid_to_proc_pid[i] = trie_create(64, ptr_sz_lg, 10, 10, 0);
> > > > > > >
> > > > > > > Has the numeric constant 64 been chosen because
> > > > > > > sizeof(struct tcb.pid_ns) * 8 == 64?
> > > > > >
> > > > > > Yes.
> > > > > > For making it clear in the code, which one would be preferred?
> > > > > > - Using a named constant
> > > > >
> > > > > It's always nice to have a good name for a constant.
> > > > >
> > > > > > - Using the expression you wrote above
> > > > >
> > > > > Strictly speaking, that was not a valid C expression, but
> > > > >         sizeof_field(struct tcb, pid_ns) * 8
> > > > > is a valid one.
> > > >
> > > > As tcb.pid_ns is just one of the variables where ns ids are stored, I
> > > > might prefer using sizeof(uint64_t):
> > > > #define NS_ID_SIZE sizeof(uint64_t)
> > > > or maybe just simply:
> > > > #define NS_ID_SIZE 64
> > >
> > > By the way, is it really 64?  Yes, the type of struct tcb.pid_ns is 64-bit,
> > > but any valid pid_ns is 32-bit.
> >
> > I think it was chosen to be 64 bit by Eugene, because it's not
> > documented that it should fit into 32 bit. The documentation only says
> > that it's derived from an inode number. Or is it guaranteed that inode
> > numbers are maximum 32 bit?
>
> If I read the kernel correctly, pid_ns is assigned from struct ns_common.inum
> of type unsigned int.

Yes, I understand. However since it's not documented anywhere (AFAIK),
it might change later, isn't it? But if you say it's highly unlikely
to change, we can use unsigned int for ns id type. I think we should
not use -1U then for indicating error in tcb.pid_ns, as it is a likely
valid ns id. It's not a big problem however, as errors are unlikely,
so there is an option to just leave tcb.pid_ns 0 on error.