Hi, Carlos,

Thanks for the prototyping. See comments below.

On Sat, Sep 9, 2017 at 6:47 AM, carlos antonio neira bustos via
iovisor-dev <iovisor-dev@...> wrote:
> Hi All,
>
> I was working on this bcc issue https://github.com/iovisor/bcc/issues/1329
> (PID filtering issues when running bpf script inside container). The current
> issue is that bpf_get_current_pid_tgid() gets the pid outside the container.
> I have created a couple of helpers that could help on this issue, I have add
> them to bcc  but currently I'm testing them.
> I would like to know if my current approach is correct.
>
> These are the helpers implemented in this patch.
>
>  int bpf_get_current_ns_id(void)
>      Return namespace id associated with current task
>      Return: ts->nsproxy->pid_ns_for_children->ns.inum

We already have helper to get current task structure. From there, bpf_probe_read
should get you to read namespace ID.

>
>  u64 bpf_get_current_pid_ns(void)
>      Return pid_namespace struct
>      Return: struct pid_namespace

Do you have a use case for this?

>
>  u64 bpf_get_current_pid(void)
>       Returns pid of current task as seen from pid namespace
>       Return: (u64) ts->tgid << 32 | task_pid_vnr(current);

This is useful, but need extension. But in typical use case, a helper
to get nsid, ns_tgid and ns_pid should be good
enough. Maybe something like:

struct bpf_current_ns_info {
     u64 ns_id;  /* namespace id */
     u32 tgid;     /* tgid inside namespace */
     u32 gid;      /* gid inside namespace */
}
int bpf_get_current_ns_info(void *buf, int size)

(1). In filter case, user can call this helper to get ns_nsid and
ns_tgid/ns_pid. user can already get its own
from getpid() and /proc/<pid>/ns/pid. They can compare the values
returned from the helper to the value
currently in the container (or even the host), for filtering purpose.
(2). For map key purpose, the helper returned values can be a key in
the map to differentiate between different
process instances.

>
>
> cnb@Debian9:~/ebpf-backports/new-bcc-helpers/linux-4.13$ cat
> new-helpers.patch
> diff -uN /home/cnb/linux/linux-4.13/kernel/bpf/core.c
> /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/core.c
> --- /home/cnb/linux/linux-4.13/kernel/bpf/core.c        2017-09-03
> 13:56:17.000000000 -0700
> +++ /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/core.c
> 2017-09-07 18:50:13.956874952 -0700
> @@ -1379,6 +1379,10 @@
>  const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> +const struct bpf_func_proto bpf_get_current_pid_ns_proto __weak;
> +const struct bpf_func_proto bpf_get_current_ns_id_proto __weak;
> +const struct bpf_func_proto bpf_get_current_pid_proto __weak;
> +
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
>         return NULL;
> diff -uN /home/cnb/linux/linux-4.13/kernel/bpf/helpers.c
> /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/helpers.c
> --- /home/cnb/linux/linux-4.13/kernel/bpf/helpers.c     2017-09-03
> 13:56:17.000000000 -0700
> +++ /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/helpers.c
> 2017-09-09 05:57:27.970448102 -0700
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/pid_namespace.h>
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -179,3 +180,64 @@
>         .arg1_type      = ARG_PTR_TO_UNINIT_MEM,
>         .arg2_type      = ARG_CONST_SIZE,
>  };
> +
> +BPF_CALL_0(bpf_get_current_pid_ns)
> +{
> +#ifdef CONFIG_PID_NS
> +       struct pid_namespace *current_ns =
> +               task_active_pid_ns(current);
> +
> +       if (unlikely(!current_ns))
> +               return -EINVAL;
> +
> +       return (u64) current_ns;
> +#else
> +
> +       return 0;
> +#endif
> +
> +}
> +
> +const struct bpf_func_proto bpf_get_current_pid_ns_proto = {
> +       .func           = bpf_get_current_pid_ns,
> +       .gpl_only       = false,
> +       .ret_type       = RET_INTEGER,
> +};
> +
> +BPF_CALL_0(bpf_get_current_ns_id)
> +{
> +       struct task_struct *ts = current;
> +
> +       if (unlikely(!ts))
> +               return -EINVAL;
> +
> +       return (unsigned int)
> +               ts->nsproxy->pid_ns_for_children->ns.inum;
> +
> +}
> +
> +const struct bpf_func_proto bpf_get_current_ns_id_proto = {
> +       .func           = bpf_get_current_ns_id,
> +       .gpl_only       = false,
> +       .ret_type       = RET_INTEGER,
> +};
> +
> +BPF_CALL_0(bpf_get_current_pid)
> +{
> +       struct task_struct *ts = current;
> +       pid_t pid;
> +       if (unlikely(!ts))
> +               return -EINVAL;
> +
> +       pid = task_pid_vnr(ts);
> +
> +       return (u64) ts->tgid << 32 | pid;
> +}
> +
> +const struct bpf_func_proto bpf_get_current_pid_proto = {
> +       .func           = bpf_get_current_pid,
> +       .gpl_only       = false,
> +       .ret_type       = RET_INTEGER,
> +};
> +
> +
> --- /home/cnb/linux/linux-4.13/include/uapi/linux/bpf.h 2017-09-03
> 13:56:17.000000000 -0700
> +++
> /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/include/uapi/linux/bpf.h
> 2017-09-09 06:22:46.763652066 -0700
> @@ -539,6 +539,19 @@
>   *     @mode: operation mode (enum bpf_adj_room_mode)
>   *     @flags: reserved for future use
>   *     Return: 0 on success or negative error code
> + *
> + * int bpf_get_current_ns_id(void)
> + *     Return namespace id associated with current task
> + *     Return: ts->nsproxy->pid_ns_for_children->ns.inum
> + *
> + * u64 bpf_get_current_pid_ns(void)
> + *     Return pid_namespace struct
> + *     Return: struct pid_namespace
> + *
> + * u64 bpf_get_current_pid(void)
> + *      Returns pid of current task as seen from pid namespace
> + *     return (u64) ts->tgid << 32 | task_pid_vnr(current);
> + *
>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -591,7 +604,11 @@
>         FN(get_socket_uid),             \
>         FN(set_hash),                   \
>         FN(setsockopt),                 \
> -       FN(skb_adjust_room),
> +       FN(skb_adjust_room),            \
> +       FN(get_current_pid_ns),         \
> +       FN(get_current_ns_id),          \
> +       FN(get_current_pid),
> +
>  /* integer value in 'imm' field of BPF_CALL instruction selects which
> helper
>   * function eBPF program intends to call
>
>
>
>
> Bests
>
> On Fri, Sep 8, 2017 at 2:39 PM, carlos antonio neira bustos
> <cneirabustos@...> wrote:
>>
>> Thank you very much.
>>
>> On Sep 8, 2017 6:35 PM, "Y Song" <ys114321@...> wrote:
>>>
>>>
>>>
>>> On Fri, Sep 8, 2017 at 12:21 PM, carlos antonio neira bustos via
>>> iovisor-dev <iovisor-dev@...> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I'm trying to add new helpers to obtain a pid namespace, I'm working on
>>>> kernel 4.13
>>>>
>>>> --- linux/linux-4.13/kernel/bpf/helpers.c 2017-09-03 13:56:17.000000000
>>>> -0700
>>>> +++
>>>> /home/cnb/ebpf-backports/new-bcc-helpers/linux-4.13/kernel/bpf/helpers.c
>>>> 2017-09-07 18:52:40.839525862 -0700
>>>> @@ -18,6 +18,7 @@
>>>>  #include <linux/sched.h>
>>>>  #include <linux/uidgid.h>
>>>>  #include <linux/filter.h>
>>>> +#include <linux/pid_namespace.h>
>>>>
>>>>  /* If kernel subsystem is allowing eBPF programs to call this function,
>>>>   * inside its own verifier_ops->get_func_proto() callback it should
>>>> return
>>>> @@ -179,3 +180,64 @@
>>>>   .arg1_type = ARG_PTR_TO_UNINIT_MEM,
>>>>   .arg2_type = ARG_CONST_SIZE,
>>>>  };
>>>> +
>>>> +BPF_CALL_0(bpf_get_current_pid_ns)
>>>> +{
>>>> +#ifdef CONFIG_PID_NS
>>>> + struct pid_namespace *current_ns =
>>>> +  task_active_pid_ns(current);
>>>> +
>>>> + if (unlikely(!current_ns))
>>>> +  return -EINVAL;
>>>> +
>>>> + return (long) current_ns;
>>>> +#else
>>>> +
>>>> + return 0;
>>>> +#endif
>>>> +
>>>> +}
>>>> +
>>>> +const struct bpf_func_proto bpf_get_current_pid_ns_proto = {
>>>> + .func  = bpf_get_current_pid_ns,
>>>> + .gpl_only = false,
>>>> + .ret_type = RET_INTEGER,
>>>> +};
>>>> +
>>>> +BPF_CALL_0(bpf_get_current_ns_id)
>>>> +{
>>>> + struct task_struct *ts = current;
>>>> +
>>>> + if (unlikely(!ts))
>>>> +  return -EINVAL;
>>>> +
>>>> + return (unsigned int)
>>>> +  ts->nsproxy->pid_ns_for_children->ns.inum;
>>>> +
>>>> +}
>>>> +
>>>> +const struct bpf_func_proto bpf_get_current_ns_id_proto = {
>>>> + .func  = bpf_get_current_ns_id,
>>>> + .gpl_only = false,
>>>> + .ret_type = RET_INTEGER,
>>>> +};
>>>> +
>>>> +BPF_CALL_0(bpf_get_current_pid)
>>>> +{
>>>> + struct task_struct *ts = current;
>>>> +
>>>> + if (unlikely(!ts))
>>>> +  return -EINVAL;
>>>> +
>>>> + pid_t pid = task_pid_vnr(ts);
>>>> +
>>>> + return (u64) ts->tgid << 32 | pid;
>>>> +}
>>>> +
>>>> +const struct bpf_func_proto bpf_get_current_pid_proto = {
>>>> + .func  = bpf_get_current_pid,
>>>> + .gpl_only = false,
>>>> + .ret_type = RET_INTEGER,
>>>> +};
>>>> +
>>>> +
>>>> I wanted to integrate this on bcc tools, so I added these helpers on
>>>> bcc/src/cc/compat/linux/virtual_bpf.h
>>>> bcc/src/cc/compat/linux/bpf.h
>>>> bcc/src/cc/export/helpers.h
>>>> bcc/src/cc/export/helpers.h
>>>>
>>>> then just  to test one of them I modified bcc/tools/funccount.py
>>>>
>>>> --- funccount.py 2017-09-08 12:14:57.601604654 -0700
>>>> +++ /home/cnb/bcc-new-helpers/bcc/tools/funccount.py 2017-09-07
>>>> 20:27:32.982815146 -0700
>>>> @@ -185,7 +185,7 @@
>>>>          # the top 32 bits of bpf_get_current_pid_tgid().
>>>>          if self.pid:
>>>>              trace_count_text = trace_count_text.replace('FILTER',
>>>> -                """u32 pid = bpf_get_current_pid_tgid() >> 32;
>>>> +                """u32 pid = bpf_get_current_pid() >> 32;
>>>>                     if (pid != %d) { return 0; }""" % self.pid)
>>>>          else:
>>>>              trace_count_text = trace_count_text.replace('FILTER', '')
>>>>
>>>>
>>>> but I'm getting this error
>>>>
>>>> cnb@Debian9:~/bcc/tools$ sudo /usr/share/bcc/tools/funccount -p 385
>>>> c:malloc
>>>> bpf: Invalid argument
>>>> 0: (85) call unknown#51
>>>> invalid func unknown#51
>>>> Failed to load BPF program trace_count_0: Invalid argument
>>>>
>>>>
>>>> Is something that I'm missing on the bcc side or on bpf side ?
>>>
>>>
>>> In kernel, you need to add your function proto to kprobe_prog_func_proto
>>> in kernel/trace/bpf_trace.c
>>>
>>>>
>>>>
>>>> Bests
>>>>
>>>>
>>>> _______________________________________________
>>>> iovisor-dev mailing list
>>>> iovisor-dev@...
>>>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>>>>
>>>
>
>
> _______________________________________________
> iovisor-dev mailing list
> iovisor-dev@...
> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
>