Re: Extending bpf_get_ns_current_pid_tgid()

[Yonghong Song <yhs@fb.com>]

On 6/15/21 6:08 PM, carlos antonio neira bustos wrote:
> I'm resuming work on this and would like to know your opinion and 
> concerns about the following proposal:
> 
> - Add  s_dev from  nsfs to ns_common, so now ns_common will have inode 
> and device to identify the namespace, as in the future namespaces will 
> need to match against ino and device.
> 
> - That will allow us to remove the call to ns_match on because the 
> values checked are now present in ns_common (ino and dev_t).

I understand its benefit but I am not 100% sure whether adding s_dev to 
ns_common will be accepted or not by upstream just because of this.

Note that if adding s_dev to ns_common, you then need to ensure s_dev
contains valid value for all usages of ns_common, practically all
namespaces, not just nsfs, otherwise people may argument against this
as existing mechanism works and the change brings little value.
If you go this route, please ensure other namespaces can also
take advantage of this field.

> 
> - Add a new helper named  bpf_get_current_pid_tgid_from_ns that will 
> return pid/tgid from the current task if pid ns matches ino and dev 
> supplied by the user as now both values are in ns_common.

I think current helper get_ns_current_pid_tgid() can already do this.
Did I miss anything?

> 
> 
> 
> 
> 
> On Fri, Nov 13, 2020 at 1:59 PM Yonghong Song <yhs@fb.com 
> <mailto:yhs@fb.com>> wrote:
> 
> 
> 
>     On 11/13/20 6:34 AM, carlos antonio neira bustos wrote:
>      > Hi Blaise and Daniel,
>      >
>      >
>      > I was following a couple of months ago how bpftrace was going to
>     handle
>      > this situation. I thought this PR
>      > https://github.com/iovisor/bpftrace/pull/1602
>     <https://github.com/iovisor/bpftrace/pull/1602>
>      > <https://github.com/iovisor/bpftrace/pull/1602
>     <https://github.com/iovisor/bpftrace/pull/1602>> was going to be merged
>      > but just found today that is not working.
>      >
>      > I agree with Yonghong Song on the approach of using the two helpers
>      > (bpf_get_pid_tgid() and bpf_get_ns_current_pid_tgid()) to move
>     forward
>      > on the short term, bpf_get_ns_current_pid_tgid works as a
>     replacement
>      > to bpf_get_pid_tgid when you are instrumenting inside a container.
>      >
>      > But the use case described by Blaise is one I would like to use
>     bpftrace,
>      >
>      > If nobody is against it, I could start working on a new helper to
>      > address that situation as I need to have bpftrace working in that
>     scenario.
> 
>     Yes, please. Thanks!
> 
>      >
>      > For my understanding of the problem the new helper should be able to
>      > return pid/tgid from a target namespace, is that correct?.
> 
>     Yes. This way, the stack trace can correlate to target namespace for
>     symbolization purpose.
> 
>      >
>      >
>      > Bests
>      >
>      >
>     [...]
>