Thank you very much for your comments, after reading them, I realized that
my change just added too much unneeded code, when we could just return the
values of interest in one call. I'll delete the unneeded helpers and work
on this proposed helper :
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)
Thanks again for your help. I'm just starting to dig in the ebpf and bcc
code and it's really awesome the tools you could build with it.
Bests
On Sat, Sep 9, 2017 at 10:12 AM, Y Song <ys114...@gmail.com> wrote:
> 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@lists.iovisor.org> 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
> > <cneirabus...@gmail.com> wrote:
> >>
> >> Thank you very much.
> >>
> >> On Sep 8, 2017 6:35 PM, "Y Song" <ys114...@gmail.com> wrote:
> >>>
> >>>
> >>>
> >>> On Fri, Sep 8, 2017 at 12:21 PM, carlos antonio neira bustos via
> >>> iovisor-dev <iovisor-dev@lists.iovisor.org> 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@lists.iovisor.org
> >>>> https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> >>>>
> >>>
> >
> >
> > _______________________________________________
> > iovisor-dev mailing list
> > iovisor-dev@lists.iovisor.org
> > https://lists.iovisor.org/mailman/listinfo/iovisor-dev
> >
>
_______________________________________________
iovisor-dev mailing list
iovisor-dev@lists.iovisor.org
https://lists.iovisor.org/mailman/listinfo/iovisor-dev
