* Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace [not found] ` <20220221032202.1925507-1-xu.xin16@zte.com.cn> @ 2022-02-23 0:00 ` Eric W. Biederman 2022-02-23 4:56 ` cgel.zte 0 siblings, 1 reply; 3+ messages in thread From: Eric W. Biederman @ 2022-02-23 0:00 UTC (permalink / raw) To: xu xin Cc: bsingharora, akpm, deng.changcheng, linux-kernel, balbir, xu.xin16, Linux Containers xu xin <cgel.zte@gmail.com> writes: > Currently, the application getdelays cannot get taskstats in a net > namespace. The returned error is just like the following: > -sh-4.4# ps -ef | tail -5 > root 186 2 0 09:23 ? 00:00:00 [kworker/2:1H] > root 187 2 0 09:23 ? 00:00:00 [kworker/0:2-eve] > root 190 183 0 09:23 ? 00:00:00 -sh > root 198 190 0 09:25 ? 00:00:00 ps -ef > root 199 190 0 09:25 ? 00:00:00 tail -5 > -sh-4.4# > -sh-4.4# ./getdelays -d -p 186 -v > print delayacct stats ON > debug on > Error getting family id, errno 0 > > As more and more applications are deployed in containers like Docker, > it is necessary to support getdelays to be used in net namespace. > Taskstats is safe for use per namespace as genetlink checks the > capability of namespace message by netlink_ns_capable(). > > Make taskstats available via genetlink per namespace. Let me add a polite nack to this patch. Taskstats is completely senseless in a network namespace. There is no translation of identifiers into the context of the receiver of the message. As such taskstats can not be meaningfully used in a container. To make this work requires updating the taskstats code to do something sensible when in a pid namespace, as well as when in a network namespace. I would like to give a suggest on how to do something sensible but I don't have any idea at this point. The code would have been converted long ago on general principles if this was a straight forward thing to do. The taskstats interface only makes sense when you are within all of the initial namespaces. Eric > Reported-by: Changcheng Deng <deng.changcheng@zte.com.cn> > Signed-off-by: xu xin <xu.xin16@zte.com.cn> > --- > kernel/taskstats.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/kernel/taskstats.c b/kernel/taskstats.c > index 2b4898b4752e..4d6bcaaf52a0 100644 > --- a/kernel/taskstats.c > +++ b/kernel/taskstats.c > @@ -664,6 +664,7 @@ static struct genl_family family __ro_after_init = { > .module = THIS_MODULE, > .ops = taskstats_ops, > .n_ops = ARRAY_SIZE(taskstats_ops), > + .netnsok = true, > }; > > /* Needed early in initialization */ ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace 2022-02-23 0:00 ` [PATCH resend] kernel: Make taskstats available via genetlink per namespace Eric W. Biederman @ 2022-02-23 4:56 ` cgel.zte 2022-02-27 11:16 ` cgel.zte 0 siblings, 1 reply; 3+ messages in thread From: cgel.zte @ 2022-02-23 4:56 UTC (permalink / raw) To: ebiederm Cc: akpm, balbir, bsingharora, cgel.zte, containers, deng.changcheng, linux-kernel, xu.xin16 >> -sh-4.4# ./getdelays -d -p 186 -v >> print delayacct stats ON >> debug on >> Error getting family id, errno 0 >> >> As more and more applications are deployed in containers like Docker, >> it is necessary to support getdelays to be used in net namespace. >> Taskstats is safe for use per namespace as genetlink checks the >> capability of namespace message by netlink_ns_capable(). >> >> Make taskstats available via genetlink per namespace. > > Let me add a polite nack to this patch. > Taskstats is completely senseless in a network namespace. There is no > translation of identifiers into the context of the receiver of the > message. The interface of taskstats is genetlink that is sensible in net namsespace. > To make this work requires updating the taskstats code to do something > sensible when in a pid namespace, as well as when in a network > namespace. Yes. Taskstats already does convert the input process ID into the task in the correspoding pid namsespace. Do you mean to add some check of current user's capability like SYS_ADMIN or else? ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re:Re: [PATCH resend] kernel: Make taskstats available via genetlink per namespace 2022-02-23 4:56 ` cgel.zte @ 2022-02-27 11:16 ` cgel.zte 0 siblings, 0 replies; 3+ messages in thread From: cgel.zte @ 2022-02-27 11:16 UTC (permalink / raw) To: cgel.zte, ebiederm Cc: akpm, balbir, bsingharora, containers, deng.changcheng, linux-kernel, xu.xin16 >>> -sh-4.4# ./getdelays -d -p 186 -v >>> print delayacct stats ON >>> debug on >>> Error getting family id, errno 0 >>> >>> As more and more applications are deployed in containers like Docker, >>> it is necessary to support getdelays to be used in net namespace. >>> Taskstats is safe for use per namespace as genetlink checks the >>> capability of namespace message by netlink_ns_capable(). >>> >>> Make taskstats available via genetlink per namespace. >> >> Let me add a polite nack to this patch. > >> Taskstats is completely senseless in a network namespace. There is no >> translation of identifiers into the context of the receiver of the >> message. > >The interface of taskstats is genetlink that is sensible in net namsespace. > >> To make this work requires updating the taskstats code to do something >> sensible when in a pid namespace, as well as when in a network >> namespace. > > Yes. Taskstats already does convert the input process ID into the task in the > correspoding pid namsespace. Do you mean to add some check of current user's > capability like SYS_ADMIN or else? Actually, here, I think it's meaningful to set the genl_family's netnsok of Taskstat as true. As you said, Taskstats itself is senseless in a network namespace. So, we don't have to limit it to the only init_net_ns, it is basically okay to make it available in all network namespace. Certainly, maybe taskstats itself also needs to updated, because it does seem to be missing something to just use CAP_NET_ADMIN as the acquisition restriction of taskstat. Thanks, xu xin ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-02-27 11:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220217093945.1904085-1-xu.xin16@zte.com.cn>
[not found] ` <20220221032202.1925507-1-xu.xin16@zte.com.cn>
2022-02-23 0:00 ` [PATCH resend] kernel: Make taskstats available via genetlink per namespace Eric W. Biederman
2022-02-23 4:56 ` cgel.zte
2022-02-27 11:16 ` cgel.zte
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox