From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED,USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85380C43381 for ; Thu, 21 Mar 2019 14:41:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4708B20830 for ; Thu, 21 Mar 2019 14:41:03 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ABWnv6BP" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728138AbfCUOlB (ORCPT ); Thu, 21 Mar 2019 10:41:01 -0400 Received: from mail-qt1-f196.google.com ([209.85.160.196]:34117 "EHLO mail-qt1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726551AbfCUOlB (ORCPT ); Thu, 21 Mar 2019 10:41:01 -0400 Received: by mail-qt1-f196.google.com with SMTP id k2so6881243qtm.1 for ; Thu, 21 Mar 2019 07:41:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=kFZQ6dwFqEe6Uvh007h0MPrC48Rqm53vJIqP3tdFOwA=; b=ABWnv6BPmusTGoWz87GyyZmx6DlWAH7/vIaSo5PYkseE4oDI4Tq9C7eE3iYwPk0r9e A63Pput/E57BjuiCKnHuUUEREUPpK6o1Hn7rYJQ9H25qmoU7EG33ZJ9Dn0H08Wm9k83B QVJ5zglBDS1fw2+2BQ3q7+xeU2EbMMc2nIJiS5mUFnEBsguXo6h3xNqA2zQrp80J3D9/ mVQkdPjtkOCMTLNiHUUoAXSH8dtVIlvOA2f02jIR6V6+UsroAoVbNYuwabGGpZeqsRs4 GVN/C4ERdLFHUoMG718K76QKeVHp0tSA2bL/rZZJL78TOvJYHL55AlGT6vww7QTSJfM3 NXUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=kFZQ6dwFqEe6Uvh007h0MPrC48Rqm53vJIqP3tdFOwA=; b=CBMBAbM7aRf3opNwRQrx0WZCLsA74zYadDBz7LY0ESk/QnUGNY1CdHuMEjojsxA+M/ WsE3zxaJLk7Er6Y40bkqLlwn3Mmbjs1NMh7KTOl7rUvzXzhxDrZ4PvJYnkGBAs3ETmcj 1ojDfAjjMkwXrPd9Uf0NsLeFH80fhRuzObaUnS9ldWUwGhISQK5Ob5ki07RgndHkrnen ZeEeaLkUgDhrJJ2rsVeeLi0S1GZhFkpQAgXmgolqvEWrf1GnJkvP7PH76KhFKen6cOoP SgYYec1+yKA/0+0MvoPykcsc5JEtilRPSrGj8XDLbdleKbJ7vd+bQ79QV+ini2UqE6/L lNhQ== X-Gm-Message-State: APjAAAUmy7EArdc8NmH/nWhtqIxVOq4+5zFl6GK3pYSzYTUtqSZydslH L+wA60WzEKCIaI2wgxmkw4s= X-Google-Smtp-Source: APXvYqzfUxxNJpcNZt0mDcSzvKHC/KqUqLMcFCuNOyLFfdY0+8PSyUtJuMgL0DPskkqF3CLumvuouA== X-Received: by 2002:a0c:9810:: with SMTP id c16mr3467482qvd.192.1553179260256; Thu, 21 Mar 2019 07:41:00 -0700 (PDT) Received: from dev00 ([190.162.109.53]) by smtp.gmail.com with ESMTPSA id l6sm2682871qkc.36.2019.03.21.07.40.58 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 21 Mar 2019 07:40:59 -0700 (PDT) Date: Thu, 21 Mar 2019 11:40:55 -0300 From: Carlos Antonio Neira Bustos To: Alexei Starovoitov Cc: netdev@vger.kernel.org, ys114321@gmail.com Subject: Re: [PATCH v3 bpf-next 1/3] BPF: helpers: New helper to obtain namespace data from current task Message-ID: <20190321144053.zgzb4pwnzarqdjcz@dev00> References: <20190320164920.ha5ne7nlnu7fbx5i@dev00> <20190321012318.yfaeo3fppxm4rtnv@ast-mbp.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190321012318.yfaeo3fppxm4rtnv@ast-mbp.dhcp.thefacebook.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Mar 20, 2019 at 06:23:20PM -0700, Alexei Starovoitov wrote: > On Wed, Mar 20, 2019 at 01:49:22PM -0300, Carlos Antonio Neira Bustos wrote: > > > > This is a series of patches to introduce a new helper called bpf_get_current_pidns_info, > > this change has been splitted into the following patches: > > > > 1- Feature introduction > > 2- Update tools/.../bpf.h > > 3- Self tests and samples > > > > > > From 852a65906122b05b4d1a23af868b2c245d240402 Mon Sep 17 00:00:00 2001 > > From: Carlos > > Date: Tue, 19 Mar 2019 19:38:48 -0300 > > Subject: [PATCH] [PATCH bpf-next 1/3] BPF: New helper to obtain namespace data > > from current task > > > > This helper obtains the active namespace from current and returns pid, tgid, > > device and namespace id as seen from that namespace, allowing to instrument > > a process inside a container. > > Device is read from /proc/self/ns/pid, as in the future it's possible that > > different pid_ns files may belong to different devices, according > > to the discussion between Eric Biederman and Yonghong in 2017 linux plumbers > > conference. > > Currently bpf_get_current_pid_tgid(), is used to do pid filtering in bcc's > > scripts but this helper returns the pid as seen by the root namespace which is > > fine when a bcc script is not executed inside a container. > > When the process of interest is inside a container, pid filtering will not work > > if bpf_get_current_pid_tgid() is used. This helper addresses this limitation > > returning the pid as it's seen by the current namespace where the script is > > executing. > > > > This helper has the same use cases as bpf_get_current_pid_tgid() as it can be > > used to do pid filtering even inside a container. > > > > For example a bcc script using bpf_get_current_pid_tgid() (tools/funccount.py): > > > > u32 pid = bpf_get_current_pid_tgid() >> 32; > > if (pid != ) > > return 0; > > Could be modified to use bpf_get_current_pidns_info() as follows: > > > > struct bpf_pidns pidns; > > bpf_get_current_pidns_info(&pidns, sizeof(struct bpf_pidns)); > > u32 pid = pidns.tgid; > > u32 nsid = pidns.nsid; > > if ((pid != ) && (nsid != )) > > return 0; > > > > To find out the name PID namespace id of a process, you could use this command: > > > > $ ps -h -o pidns -p > > > > Or this other command: > > > > $ ls -Li /proc//ns/pid > > > > Signed-off-by: Carlos Antonio Neira Bustos > > - > > --- > > include/linux/bpf.h | 1 + > > include/uapi/linux/bpf.h | 26 ++++++++++++++++++- > > kernel/bpf/core.c | 1 + > > kernel/bpf/helpers.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ > > kernel/trace/bpf_trace.c | 2 ++ > > 5 files changed, 96 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index a2132e09dc1c..a77f5bd77bd8 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto; > > extern const struct bpf_func_proto bpf_spin_lock_proto; > > extern const struct bpf_func_proto bpf_spin_unlock_proto; > > extern const struct bpf_func_proto bpf_get_local_storage_proto; > > +extern const struct bpf_func_proto bpf_get_current_pidns_info_proto; > > > > /* Shared helpers among cBPF and eBPF. */ > > void bpf_user_rnd_init_once(void); > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 3c38ac9a92a7..facc701c7873 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -2366,6 +2366,18 @@ union bpf_attr { > > * current value is ect (ECN capable). Works with IPv6 and IPv4. > > * Return > > * 1 if set, 0 if not set. > > + * > > + * int bpf_get_current_pidns_info(struct bpf_pidns_info *pidns, u32 size_of_pidns) > > + * Description > > + * Copies into *pidns* pid, namespace id and tgid as seen by the > > + * current namespace and also device from /proc/self/ns/pid. > > + * *size_of_pidns* must be the size of *pidns* > > + * > > + * This helper is used when pid filtering is needed inside a > > + * container as bpf_get_current_tgid() helper returns always the > > + * pid id as seen by the root namespace. > > + * Return > > + * 0 on success -EINVAL on error. > > */ > > #define __BPF_FUNC_MAPPER(FN) \ > > FN(unspec), \ > > @@ -2465,7 +2477,8 @@ union bpf_attr { > > FN(spin_unlock), \ > > FN(sk_fullsock), \ > > FN(tcp_sock), \ > > - FN(skb_ecn_set_ce), > > + FN(skb_ecn_set_ce), \ > > + FN(get_current_pidns_info), > > > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > > * function eBPF program intends to call > > @@ -3152,4 +3165,15 @@ struct bpf_line_info { > > struct bpf_spin_lock { > > __u32 val; > > }; > > + > > +/* helper bpf_get_current_pidns_info will store the following > > + * data, dev will contain major/minor from /proc/self/pid. > > +*/ > > +struct bpf_pidns_info { > > + __u32 dev; > > + __u32 nsid; > > + __u32 tgid; > > + __u32 pid; > > +}; > > + > > #endif /* _UAPI__LINUX_BPF_H__ */ > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > index 3f08c257858e..06329fbed95f 100644 > > --- a/kernel/bpf/core.c > > +++ b/kernel/bpf/core.c > > @@ -2044,6 +2044,7 @@ 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_cgroup_id_proto __weak; > > const struct bpf_func_proto bpf_get_local_storage_proto __weak; > > +const struct bpf_func_proto bpf_get_current_pidns_info __weak; > > > > const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void) > > { > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > > index a411fc17d265..95c3780a6ba7 100644 > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -18,6 +18,11 @@ > > #include > > #include > > #include > > +#include > > +#include > > +#include > > +#include > > +#include > > > > /* If kernel subsystem is allowing eBPF programs to call this function, > > * inside its own verifier_ops->get_func_proto() callback it should return > > @@ -364,3 +369,65 @@ const struct bpf_func_proto bpf_get_local_storage_proto = { > > }; > > #endif > > #endif > > + > > +BPF_CALL_2(bpf_get_current_pidns_info, struct bpf_pidns_info *, pidns_info, u32, > > + size) > > +{ > > + const char *nspid = "/proc/self/ns/pid"; > > + struct pid_namespace *pidns = NULL; > > + struct kstat ks; > > + struct path kp; > > + pid_t tgid = 0; > > + pid_t pid = 0; > > + int res = 0; > > + > > + if (unlikely(size != sizeof(struct bpf_pidns_info))) > > + goto clear; > > + > > + pidns = task_active_pid_ns(current); > > + > > + if (unlikely(!pidns)) > > + goto clear; > > + > > + pidns_info->nsid = pidns->ns.inum; > > + pid = task_pid_nr_ns(current, pidns); > > + > > + if (unlikely(!pid)) > > + goto clear; > > + > > + tgid = task_tgid_nr_ns(current, pidns); > > + > > + if (unlikely(!tgid)) > > + goto clear; > > + > > + pidns_info->tgid = (u32) tgid; > > + pidns_info->pid = (u32) pid; > > + > > + kern_path(nspid, 0, &kp); > > + > > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(4,11,0) > > + res = vfs_getattr(&kp, &ks, STATX_ALL, 0); > > +#else > > + res = vfs_getattr(&kp, &ks); > > +#endif > > Please access namespaces directly. > I suspect bpf helpers cannot do vfs_getattr. Something inside might sleep > while bpf progs are non preemptible. > > Also please run ./scripts/checkpatch.pl on your patches > and read Documentation/bpf/bpf_devel_QA.rst. > Hello Alexei, Thanks for checking this out and also thanks for the documentation link. At this point I have a couple of questions: * What is the reason to not use the current namespace api instead of directly accessing namespaces?. * Regarding bpf programs not being preemptible. If we add spin_locks to the vfs_getattr call, would that be an acceptable solution? spin_lock(&bpf_lock); res = vfs_getattr(&kp, &ks); spin_unlock(&bpf_lock); Is there another way to interact with the vfs layer within a bpf helper?. Bests