Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks
From: Andrew Lunn @ 2019-07-06 16:09 UTC (permalink / raw)
  To: josua; +Cc: netdev, David S. Miller
In-Reply-To: <20190706151900.14355-4-josua@solid-run.com>

On Sat, Jul 06, 2019 at 05:18:59PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Print a warning when device tree specifies more than the maximum of four
> clocks supported by orion-mdio. Because reading from mdio can lock up
> the Armada 8k when a required clock is not initialized, it is important
> to notify the user when a specified clock is ignored.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index e17d563e97a6..89a99bf8e87b 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  		clk_prepare_enable(dev->clk[i]);
>  	}
>  
> +	if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
> +		dev_warn(dev, "unsupported number of clocks, limiting to the first "
> +			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
> +

Hi Josua

Humm. Say getting clock 0 returned -EINVAL, or some other error code.
We break out of the loop, since such errors are being ignored. We then
hit this new code. Getting clock 1 works, and then we incorrectly
print this message.

Rather than getting the i'th clock, get ARRAY_SIZE(dev->clk)'th clock.

       Andrew

^ permalink raw reply

* RE: [rdma 14/16] RDMA/irdma: Add ABI definitions
From: Saleem, Shiraz @ 2019-07-06 16:15 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Kirsher, Jeffrey T, dledford@redhat.com,
	davem@davemloft.net, Ismail, Mustafa, linux-rdma@vger.kernel.org,
	netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
	poswald@suse.com, Ertman, David M
In-Reply-To: <20190705171650.GI31525@mellanox.com>

> Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> 
> On Fri, Jul 05, 2019 at 04:42:19PM +0000, Saleem, Shiraz wrote:
> > > Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> > >
> > > On Thu, Jul 04, 2019 at 10:40:21AM +0300, Leon Romanovsky wrote:
> > > > On Wed, Jul 03, 2019 at 07:12:57PM -0700, Jeff Kirsher wrote:
> > > > > From: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > >
> > > > > Add ABI definitions for irdma.
> > > > >
> > > > > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > > > include/uapi/rdma/irdma-abi.h | 130
> > > > > ++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 130 insertions(+)  create mode 100644
> > > > > include/uapi/rdma/irdma-abi.h
> > > > >
> > > > > diff --git a/include/uapi/rdma/irdma-abi.h
> > > > > b/include/uapi/rdma/irdma-abi.h new file mode 100644 index
> > > > > 000000000000..bdfbda4c829e
> > > > > +++ b/include/uapi/rdma/irdma-abi.h
> > > > > @@ -0,0 +1,130 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> > > > > +/* Copyright (c) 2006 - 2019 Intel Corporation.  All rights reserved.
> > > > > + * Copyright (c) 2005 Topspin Communications.  All rights reserved.
> > > > > + * Copyright (c) 2005 Cisco Systems.  All rights reserved.
> > > > > + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> > > > > + */
> > > > > +
> > > > > +#ifndef IRDMA_ABI_H
> > > > > +#define IRDMA_ABI_H
> > > > > +
> > > > > +#include <linux/types.h>
> > > > > +
> > > > > +/* irdma must support legacy GEN_1 i40iw kernel
> > > > > + * and user-space whose last ABI ver is 5  */ #define
> > > > > +IRDMA_ABI_VER
> > > > > +6
> > > >
> > > > Can you please elaborate about it more?
> > > > There is no irdma code in RDMA yet, so it makes me wonder why new
> > > > define shouldn't start from 1.
> > >
> > > It is because they are ABI compatible with the current user space,
> > > which raises the question why we even have this confusing header file..
> >
> > It is because we need to support current providers/i40iw user-space.
> > Our user-space patch series will introduce a new provider (irdma)
> > whose ABI ver. is also 6 (capable of supporting X722 and which will
> > work with i40iw driver on older kernels) and removes providers/i40iw from rdma-
> core.
> 
> Why on earth would we do that?
> 
A unified library providers/irdma to go in hand with the driver irdma and uses the ABI header.
It can support the new network device e810 and existing x722 iWARP device. It obsoletes
providers/i40iw and extends its ABI. So why keep providers/i40iw around in rdma-core?

Shiraz 


^ permalink raw reply

* Re: Kernel BUG: epoll_wait() (and epoll_pwait) stall for 206 ms per call on sockets with a small-ish snd/rcv buffer.
From: Carlo Wood @ 2019-07-06 16:16 UTC (permalink / raw)
  To: davem, netdev
In-Reply-To: <20190706034508.43aabff0@hikaru>

I improved the test case a bit:

https://github.com/CarloWood/ai-evio-testsuite/blob/2a9ae49e3ae39eea7cb1d105883254370f53831b/src/epoll_bug.c

If the bug doesn't show, please increase burst_size and/or decrease
sndbuf_size and rcvbuf_size.

The output that I get with VERBOSE defined is for example:

[...snip...]
wrote 34784 bytes to 6 (total written now 9665792), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9665792), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9700576), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9700576), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9735360), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9735360), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9770144), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9770144), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9804928), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9804928), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9839712), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9839712), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9874496), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9874496), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9909280), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9909280), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9944064), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9944064), left in pipe line: 0
wrote 34784 bytes to 6 (total written now 9978848), now in pipe line: 34784
Read 34784 bytes from fd 5 (total read now 9978848), left in pipe line: 0
wrote 21152 bytes to 6 (total written now 10000000), now in pipe line: 21152
Read 21152 bytes from fd 5 (total read now 10000000), left in pipe line: 0
epoll_wait() stalled 193 milliseconds!
Read 21888 bytes from fd 6 (total read now 70912), left in pipe line: 21888
epoll_wait() stalled 255 milliseconds!
Read 21888 bytes from fd 6 (total read now 92800), left in pipe line: 0
write(5, buf, 9907200) = 43776 (total written now 136576), now in pipe line: 43776
epoll_wait() stalled 211 milliseconds!
Read 21888 bytes from fd 6 (total read now 114688), left in pipe line: 21888
epoll_wait() stalled 207 milliseconds!
write(5, buf, 9863424) = 38272 (total written now 174848), now in pipe line: 60160
Read 16384 bytes from fd 6 (total read now 131072), left in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 152960), left in pipe line: 21888
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 174848), left in pipe line: 0
write(5, buf, 9825152) = 43776 (total written now 218624), now in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 196736), left in pipe line: 21888
epoll_wait() stalled 211 milliseconds!
Read 21888 bytes from fd 6 (total read now 218624), left in pipe line: 0
write(5, buf, 9781376) = 43776 (total written now 262400), now in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 240512), left in pipe line: 21888
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 262400), left in pipe line: 0
write(5, buf, 9737600) = 43776 (total written now 306176), now in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 284288), left in pipe line: 21888
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 306176), left in pipe line: 0
write(5, buf, 9693824) = 43776 (total written now 349952), now in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
Read 21888 bytes from fd 6 (total read now 328064), left in pipe line: 21888
epoll_wait() stalled 207 milliseconds!
write(5, buf, 9650048) = 38272 (total written now 388224), now in pipe line: 60160
Read 16384 bytes from fd 6 (total read now 344448), left in pipe line: 43776
epoll_wait() stalled 207 milliseconds!
... etc. etc.


It seems that the problem always occur right after stopping to write data in one
direction, and then happens for the way back.

In the case above the burst_size is set to 10000000 bytes, and it writes
that amount and reads it on the other side successfully.

What I think is going on however is that the data on the way back is stalling,
during which the "forward" burst finishes (without the epoll_wait stalling it
is VERY fast). Above you see:

Read 21152 bytes from fd 5 (total read now 10000000), left in pipe line: 0
epoll_wait() stalled 193 milliseconds!

But since we know that the stall always seems to be 207ms, I'm pretty sure
that actually it stalled 14 ms before that, and needed 14 ms to finish the
complete burst in one direction.

-- 
Carlo Wood <carlo@alinoe.com>

^ permalink raw reply

* Re: [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks
From: Josua Mayer @ 2019-07-06 16:21 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, David S. Miller
In-Reply-To: <20190706160925.GI4428@lunn.ch>

Hi Andrew,

Am 06.07.19 um 18:09 schrieb Andrew Lunn:
> On Sat, Jul 06, 2019 at 05:18:59PM +0200, josua@solid-run.com wrote:
>> From: Josua Mayer <josua@solid-run.com>
>>
>> Print a warning when device tree specifies more than the maximum of four
>> clocks supported by orion-mdio. Because reading from mdio can lock up
>> the Armada 8k when a required clock is not initialized, it is important
>> to notify the user when a specified clock is ignored.
>>
>> Signed-off-by: Josua Mayer <josua@solid-run.com>
>> ---
>>  drivers/net/ethernet/marvell/mvmdio.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
>> index e17d563e97a6..89a99bf8e87b 100644
>> --- a/drivers/net/ethernet/marvell/mvmdio.c
>> +++ b/drivers/net/ethernet/marvell/mvmdio.c
>> @@ -326,6 +326,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>>  		clk_prepare_enable(dev->clk[i]);
>>  	}
>>  
>> +	if (!IS_ERR(of_clk_get(pdev->dev.of_node, i)))
>> +		dev_warn(dev, "unsupported number of clocks, limiting to the first "
>> +			 __stringify(ARRAY_SIZE(dev->clk)) "\n");
>> +
> Hi Josua
>
> Humm. Say getting clock 0 returned -EINVAL, or some other error code.
> We break out of the loop, since such errors are being ignored. We then
> hit this new code. Getting clock 1 works, and then we incorrectly
> print this message.

Good point about breaking out of the loop! I did indeed not take the
break condition of the loop into account.

I can not exactly follow your example though. The first failure of
of_clk_get will trigger break and exit the loop,
and then we would indeed print the wrong message ... .

>
> Rather than getting the i'th clock, get ARRAY_SIZE(dev->clk)'th clock.
I will gladly make the change in a v2.
>        Andrew

-
Josua


^ permalink raw reply

* [PATCH 1/2] proc: revalidate directories created with proc_net_mkdir()
From: Alexey Dobriyan @ 2019-07-06 16:52 UTC (permalink / raw)
  To: davem; +Cc: netdev, Per.Hallsmark

/proc/net directories may contain content which depends on net namespace.
Such dentries should be revalidated after setns(CLONE_NEWNET).

See
	commit 1fde6f21d90f8ba5da3cb9c54ca991ed72696c43
	proc: fix /proc/net/* after setns(2)

The patch is all about "pde_force_lookup()" line.

	[redid original patch --adobriyan]

Reported-by: "Hallsmark, Per" <Per.Hallsmark@windriver.com>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 fs/proc/generic.c       |   25 ++++++++++++++++++-------
 fs/proc/internal.h      |    3 +++
 fs/proc/proc_net.c      |   17 +++++++++++++++++
 include/linux/proc_fs.h |   16 ++++++++--------
 4 files changed, 46 insertions(+), 15 deletions(-)

--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -459,26 +459,37 @@ struct proc_dir_entry *proc_symlink(const char *name,
 }
 EXPORT_SYMBOL(proc_symlink);
 
-struct proc_dir_entry *proc_mkdir_data(const char *name, umode_t mode,
-		struct proc_dir_entry *parent, void *data)
+struct proc_dir_entry *_proc_mkdir(const char *name, umode_t mode,
+				   struct proc_dir_entry **parent, void *data)
 {
 	struct proc_dir_entry *ent;
 
 	if (mode == 0)
 		mode = S_IRUGO | S_IXUGO;
 
-	ent = __proc_create(&parent, name, S_IFDIR | mode, 2);
+	ent = __proc_create(parent, name, S_IFDIR | mode, 2);
 	if (ent) {
 		ent->data = data;
 		ent->proc_fops = &proc_dir_operations;
 		ent->proc_iops = &proc_dir_inode_operations;
-		parent->nlink++;
-		ent = proc_register(parent, ent);
-		if (!ent)
-			parent->nlink--;
 	}
 	return ent;
 }
+
+struct proc_dir_entry *proc_mkdir_data(const char *name, umode_t mode,
+		struct proc_dir_entry *parent, void *data)
+{
+	struct proc_dir_entry *pde;
+
+	pde = _proc_mkdir(name, mode, &parent, data);
+	if (!pde)
+		return NULL;
+	parent->nlink++;
+	pde = proc_register(parent, pde);
+	if (!pde)
+		parent->nlink--;
+	return pde;
+}
 EXPORT_SYMBOL_GPL(proc_mkdir_data);
 
 struct proc_dir_entry *proc_mkdir_mode(const char *name, umode_t mode,
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -299,3 +299,6 @@ extern unsigned long task_statm(struct mm_struct *,
 				unsigned long *, unsigned long *,
 				unsigned long *, unsigned long *);
 extern void task_mem(struct seq_file *, struct mm_struct *);
+
+struct proc_dir_entry *_proc_mkdir(const char *name, umode_t mode,
+				   struct proc_dir_entry **parent, void *data);
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -251,6 +251,23 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
 }
 EXPORT_SYMBOL_GPL(proc_create_net_single_write);
 
+struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name,
+				      struct proc_dir_entry *parent)
+{
+	struct proc_dir_entry *pde;
+
+	pde = _proc_mkdir(name, 0, &parent, net);
+	if (!pde)
+		return NULL;
+	pde_force_lookup(pde);
+	parent->nlink++;
+	pde = proc_register(parent, pde);
+	if (!pde)
+		parent->nlink++;
+	return pde;
+}
+EXPORT_SYMBOL_GPL(proc_net_mkdir);
+
 static struct net *get_proc_task_net(struct inode *dir)
 {
 	struct task_struct *task;
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -8,6 +8,7 @@
 #include <linux/types.h>
 #include <linux/fs.h>
 
+struct net;
 struct proc_dir_entry;
 struct seq_file;
 struct seq_operations;
@@ -73,6 +74,8 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
 						    int (*show)(struct seq_file *, void *),
 						    proc_write_t write,
 						    void *data);
+struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name, struct proc_dir_entry *parent);
+
 extern struct pid *tgid_pidfd_to_pid(const struct file *file);
 
 #else /* CONFIG_PROC_FS */
@@ -115,6 +118,11 @@ static inline int remove_proc_subtree(const char *name, struct proc_dir_entry *p
 #define proc_create_net(name, mode, parent, state_size, ops) ({NULL;})
 #define proc_create_net_single(name, mode, parent, show, data) ({NULL;})
 
+static inline struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name, struct proc_dir_entry *parent)
+{
+	return NULL;
+}
+
 static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
 {
 	return ERR_PTR(-EBADF);
@@ -122,14 +130,6 @@ static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
 
 #endif /* CONFIG_PROC_FS */
 
-struct net;
-
-static inline struct proc_dir_entry *proc_net_mkdir(
-	struct net *net, const char *name, struct proc_dir_entry *parent)
-{
-	return proc_mkdir_data(name, 0, parent, net);
-}
-
 struct ns_common;
 int open_related_ns(struct ns_common *ns,
 		   struct ns_common *(*get_ns)(struct ns_common *ns));

^ permalink raw reply

* [PATCH 2/2] net: apply proc_net_mkdir() harder
From: Alexey Dobriyan @ 2019-07-06 16:55 UTC (permalink / raw)
  To: davem
  Cc: netdev, netfilter-devel, linux-nfs, j.vosburgh, vfalico, andy,
	pablo, kadlec, fw, bfields, chuck.lever

From: "Hallsmark, Per" <Per.Hallsmark@windriver.com>

proc_net_mkdir() should be used to create stuff under /proc/net,
so that dentry revalidation kicks in.

See

	commit 1fde6f21d90f8ba5da3cb9c54ca991ed72696c43
	proc: fix /proc/net/* after setns(2)

	[added more chunks --adobriyan]

Signed-off-by: "Hallsmark, Per" <Per.Hallsmark@windriver.com>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/net/bonding/bond_procfs.c  |    2 +-
 net/core/pktgen.c                  |    2 +-
 net/ipv4/netfilter/ipt_CLUSTERIP.c |    2 +-
 net/ipv6/proc.c                    |    2 +-
 net/netfilter/xt_hashlimit.c       |    4 ++--
 net/netfilter/xt_recent.c          |    2 +-
 net/sunrpc/stats.c                 |    2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -293,7 +293,7 @@ void bond_remove_proc_entry(struct bonding *bond)
 void __net_init bond_create_proc_dir(struct bond_net *bn)
 {
 	if (!bn->proc_dir) {
-		bn->proc_dir = proc_mkdir(DRV_NAME, bn->net->proc_net);
+		bn->proc_dir = proc_net_mkdir(bn->net, DRV_NAME, bn->net->proc_net);
 		if (!bn->proc_dir)
 			pr_warn("Warning: Cannot create /proc/net/%s\n",
 				DRV_NAME);
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3791,7 +3791,7 @@ static int __net_init pg_net_init(struct net *net)
 	pn->net = net;
 	INIT_LIST_HEAD(&pn->pktgen_threads);
 	pn->pktgen_exiting = false;
-	pn->proc_dir = proc_mkdir(PG_PROC_DIR, pn->net->proc_net);
+	pn->proc_dir = proc_net_mkdir(pn->net, PG_PROC_DIR, pn->net->proc_net);
 	if (!pn->proc_dir) {
 		pr_warn("cannot create /proc/net/%s\n", PG_PROC_DIR);
 		return -ENODEV;
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -828,7 +828,7 @@ static int clusterip_net_init(struct net *net)
 		return ret;
 
 #ifdef CONFIG_PROC_FS
-	cn->procdir = proc_mkdir("ipt_CLUSTERIP", net->proc_net);
+	cn->procdir = proc_net_mkdir(net, "ipt_CLUSTERIP", net->proc_net);
 	if (!cn->procdir) {
 		nf_unregister_net_hook(net, &cip_arp_ops);
 		pr_err("Unable to proc dir entry\n");
--- a/net/ipv6/proc.c
+++ b/net/ipv6/proc.c
@@ -282,7 +282,7 @@ static int __net_init ipv6_proc_init_net(struct net *net)
 			snmp6_seq_show, NULL))
 		goto proc_snmp6_fail;
 
-	net->mib.proc_net_devsnmp6 = proc_mkdir("dev_snmp6", net->proc_net);
+	net->mib.proc_net_devsnmp6 = proc_net_mkdir(net, "dev_snmp6", net->proc_net);
 	if (!net->mib.proc_net_devsnmp6)
 		goto proc_dev_snmp6_fail;
 	return 0;
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -1237,11 +1237,11 @@ static int __net_init hashlimit_proc_net_init(struct net *net)
 {
 	struct hashlimit_net *hashlimit_net = hashlimit_pernet(net);
 
-	hashlimit_net->ipt_hashlimit = proc_mkdir("ipt_hashlimit", net->proc_net);
+	hashlimit_net->ipt_hashlimit = proc_net_mkdir(net, "ipt_hashlimit", net->proc_net);
 	if (!hashlimit_net->ipt_hashlimit)
 		return -ENOMEM;
 #if IS_ENABLED(CONFIG_IP6_NF_IPTABLES)
-	hashlimit_net->ip6t_hashlimit = proc_mkdir("ip6t_hashlimit", net->proc_net);
+	hashlimit_net->ip6t_hashlimit = proc_net_mkdir(net, "ip6t_hashlimit", net->proc_net);
 	if (!hashlimit_net->ip6t_hashlimit) {
 		remove_proc_entry("ipt_hashlimit", net->proc_net);
 		return -ENOMEM;
--- a/net/netfilter/xt_recent.c
+++ b/net/netfilter/xt_recent.c
@@ -629,7 +629,7 @@ static int __net_init recent_proc_net_init(struct net *net)
 {
 	struct recent_net *recent_net = recent_pernet(net);
 
-	recent_net->xt_recent = proc_mkdir("xt_recent", net->proc_net);
+	recent_net->xt_recent = proc_net_mkdir(net, "xt_recent", net->proc_net);
 	if (!recent_net->xt_recent)
 		return -ENOMEM;
 	return 0;
--- a/net/sunrpc/stats.c
+++ b/net/sunrpc/stats.c
@@ -323,7 +323,7 @@ int rpc_proc_init(struct net *net)
 
 	dprintk("RPC:       registering /proc/net/rpc\n");
 	sn = net_generic(net, sunrpc_net_id);
-	sn->proc_net_rpc = proc_mkdir("rpc", net->proc_net);
+	sn->proc_net_rpc = proc_net_mkdir(net, "rpc", net->proc_net);
 	if (sn->proc_net_rpc == NULL)
 		return -ENOMEM;
 

^ permalink raw reply

* Re: [PATCH v6 bpf-next 3/5] selftests/bpf: test perf buffer API
From: Yonghong Song @ 2019-07-06 17:18 UTC (permalink / raw)
  To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
	netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team
In-Reply-To: <20190706060220.1801632-4-andriin@fb.com>



On 7/5/19 11:02 PM, Andrii Nakryiko wrote:
> Add test verifying perf buffer API functionality.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   .../selftests/bpf/prog_tests/perf_buffer.c    | 94 +++++++++++++++++++
>   .../selftests/bpf/progs/test_perf_buffer.c    | 25 +++++
>   2 files changed, 119 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> new file mode 100644
> index 000000000000..64556ab0d1a9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <pthread.h>
> +#include <sched.h>
> +#include <sys/socket.h>
> +#include <test_progs.h>
> +
> +static void on_sample(void *ctx, int cpu, void *data, __u32 size)
> +{
> +	int cpu_data = *(int *)data, duration = 0;
> +	cpu_set_t *cpu_seen = ctx;
> +
> +	if (cpu_data != cpu)
> +		CHECK(cpu_data != cpu, "check_cpu_data",
> +		      "cpu_data %d != cpu %d\n", cpu_data, cpu);
> +
> +	CPU_SET(cpu, cpu_seen);
> +}
> +
> +void test_perf_buffer(void)
> +{
> +	int err, prog_fd, nr_cpus, i, duration = 0;
> +	const char *prog_name = "kprobe/sys_nanosleep";
> +	const char *file = "./test_perf_buffer.o";
> +	struct perf_buffer_opts pb_opts = {};
> +	struct bpf_map *perf_buf_map;
> +	cpu_set_t cpu_set, cpu_seen;
> +	struct bpf_program *prog;
> +	struct bpf_object *obj;
> +	struct perf_buffer *pb;
> +	struct bpf_link *link;
> +
> +	nr_cpus = libbpf_num_possible_cpus();
> +	if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
> +		return;
> +
> +	/* load program */
> +	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> +	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> +		return;
> +
> +	prog = bpf_object__find_program_by_title(obj, prog_name);
> +	if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
> +		goto out_close;
> +
> +	/* load map */
> +	perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
> +	if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
> +		goto out_close;
> +
> +	/* attach kprobe */
> +	link = bpf_program__attach_kprobe(prog, false /* retprobe */,
> +					      "sys_nanosleep");

The attach function "sys_nanosleep" won't work. You can have something
similar to attach_probe.c.

#ifdef __x86_64__
#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
#else
#define SYS_KPROBE_NAME "sys_nanosleep"
#endif


> +	if (CHECK(IS_ERR(link), "attach_kprobe", "err %ld\n", PTR_ERR(link)))
> +		goto out_close;
> +
> +	/* set up perf buffer */
> +	pb_opts.sample_cb = on_sample;
> +	pb_opts.ctx = &cpu_seen;
> +	pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts);
> +	if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
> +		goto out_detach;
> +
> +	/* trigger kprobe on every CPU */
> +	CPU_ZERO(&cpu_seen);
> +	for (i = 0; i < nr_cpus; i++) {
> +		CPU_ZERO(&cpu_set);
> +		CPU_SET(i, &cpu_set);
> +
> +		err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
> +					     &cpu_set);
> +		if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
> +				 i, err))
> +			goto out_detach;
> +
> +		usleep(1);
> +	}
> +
> +	/* read perf buffer */
> +	err = perf_buffer__poll(pb, 100);
> +	if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
> +		goto out_free_pb;
> +
> +	if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
> +		  "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
> +		goto out_free_pb;
> +
> +out_free_pb:
> +	perf_buffer__free(pb);
> +out_detach:
> +	bpf_link__destroy(link);
> +out_close:
> +	bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> new file mode 100644
> index 000000000000..876c27deb65a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> +	__uint(key_size, sizeof(int));
> +	__uint(value_size, sizeof(int));
> +} perf_buf_map SEC(".maps");
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> +	int cpu = bpf_get_smp_processor_id();
> +
> +	bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
> +			      &cpu, sizeof(cpu));
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> 

^ permalink raw reply

* Re: [PATCH v5 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Yonghong Song @ 2019-07-06 17:21 UTC (permalink / raw)
  To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
	netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team
In-Reply-To: <20190701235903.660141-9-andriin@fb.com>



On 7/1/19 4:59 PM, Andrii Nakryiko wrote:
> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> expected.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Reviewed-by: Stanislav Fomichev <sdf@google.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
>   .../selftests/bpf/prog_tests/attach_probe.c   | 166 ++++++++++++++++++
>   .../selftests/bpf/progs/test_attach_probe.c   |  55 ++++++
>   2 files changed, 221 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> new file mode 100644
> index 000000000000..a4686395522c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +
> +ssize_t get_base_addr() {
> +	size_t start;
> +	char buf[256];
> +	FILE *f;
> +
> +	f = fopen("/proc/self/maps", "r");
> +	if (!f)
> +		return -errno;
> +
> +	while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> +		if (strcmp(buf, "r-xp") == 0) {
> +			fclose(f);
> +			return start;
> +		}
> +	}
> +
> +	fclose(f);
> +	return -EINVAL;
> +}
> +
> +#ifdef __x86_64__
> +#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
> +#else
> +#define SYS_KPROBE_NAME "sys_nanosleep"
> +#endif
> +
> +void test_attach_probe(void)
> +{
> +	const char *kprobe_name = "kprobe/sys_nanosleep";
> +	const char *kretprobe_name = "kretprobe/sys_nanosleep";
> +	const char *uprobe_name = "uprobe/trigger_func";
> +	const char *uretprobe_name = "uretprobe/trigger_func";
> +	const int kprobe_idx = 0, kretprobe_idx = 1;
> +	const int uprobe_idx = 2, uretprobe_idx = 3;
> +	const char *file = "./test_attach_probe.o";
> +	struct bpf_program *kprobe_prog, *kretprobe_prog;
> +	struct bpf_program *uprobe_prog, *uretprobe_prog;
> +	struct bpf_object *obj;
> +	int err, prog_fd, duration = 0, res;
> +	struct bpf_link *kprobe_link = NULL;
> +	struct bpf_link *kretprobe_link = NULL;
> +	struct bpf_link *uprobe_link = NULL;
> +	struct bpf_link *uretprobe_link = NULL;
> +	int results_map_fd;
> +	size_t uprobe_offset;
> +	ssize_t base_addr;
> +
> +	base_addr = get_base_addr();
> +	if (CHECK(base_addr < 0, "get_base_addr",
> +		  "failed to find base addr: %zd", base_addr))
> +		return;
> +	uprobe_offset = (size_t)&get_base_addr - base_addr;
> +
> +	/* load programs */
> +	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> +	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> +		return;
> +
> +	kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> +	if (CHECK(!kprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kprobe_name))
> +		goto cleanup;
> +	kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> +	if (CHECK(!kretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", kretprobe_name))
> +		goto cleanup;
> +	uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> +	if (CHECK(!uprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uprobe_name))
> +		goto cleanup;
> +	uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> +	if (CHECK(!uretprobe_prog, "find_probe",
> +		  "prog '%s' not found\n", uretprobe_name))
> +		goto cleanup;
> +
> +	/* load maps */
> +	results_map_fd = bpf_find_map(__func__, obj, "results_map");
> +	if (CHECK(results_map_fd < 0, "find_results_map",
> +		  "err %d\n", results_map_fd))
> +		goto cleanup;
> +
> +	kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> +						 false /* retprobe */,
> +						 SYS_KPROBE_NAME);
> +	if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> +		  "err %ld\n", PTR_ERR(kprobe_link))) {
> +		kprobe_link = NULL;
> +		goto cleanup;
> +	}
> +	kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> +						    true /* retprobe */,
> +						    SYS_KPROBE_NAME);
> +	if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> +		  "err %ld\n", PTR_ERR(kretprobe_link))) {
> +		kretprobe_link = NULL;
> +		goto cleanup;
> +	}
> +	uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> +						 false /* retprobe */,
> +						 0 /* self pid */,
> +						 "/proc/self/exe",
> +						 uprobe_offset);
> +	if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> +		  "err %ld\n", PTR_ERR(uprobe_link))) {
> +		uprobe_link = NULL;
> +		goto cleanup;
> +	}
> +	uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> +						    true /* retprobe */,
> +						    -1 /* any pid */,
> +						    "/proc/self/exe",
> +						    uprobe_offset);
> +	if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> +		  "err %ld\n", PTR_ERR(uretprobe_link))) {
> +		uretprobe_link = NULL;
> +		goto cleanup;
> +	}
> +
> +	/* trigger & validate kprobe && kretprobe */
> +	usleep(1);
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> +	if (CHECK(err, "get_kprobe_res",
> +		  "failed to get kprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> +		  "wrong kprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> +	if (CHECK(err, "get_kretprobe_res",
> +		  "failed to get kretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> +		  "wrong kretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	/* trigger & validate uprobe & uretprobe */
> +	get_base_addr();
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> +	if (CHECK(err, "get_uprobe_res",
> +		  "failed to get uprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> +		  "wrong uprobe res: %d\n", res))
> +		goto cleanup;
> +
> +	err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> +	if (CHECK(err, "get_uretprobe_res",
> +		  "failed to get uretprobe res: %d\n", err))
> +		goto cleanup;
> +	if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> +		  "wrong uretprobe res: %d\n", res))
> +		goto cleanup;
> +
> +cleanup:
> +	bpf_link__destroy(kprobe_link);
> +	bpf_link__destroy(kretprobe_link);
> +	bpf_link__destroy(uprobe_link);
> +	bpf_link__destroy(uretprobe_link);
> +	bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> new file mode 100644
> index 000000000000..7a7c5cd728c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> +	int type;
> +	int max_entries;
> +	int *key;
> +	int *value;
> +} results_map SEC(".maps") = {
> +	.type = BPF_MAP_TYPE_ARRAY,
> +	.max_entries = 4,
> +};

After the new .maps convention patch is merged, test_progs is broken due 
to this. The above .maps definition needs to be updated to

struct {
	__uint(type, BPF_MAP_TYPE_ARRAY);
	__uint(max_entries, 4);
	__type(key, int);
	__type(value, int);
} results_map SEC(".maps");

> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> +	const int key = 0, value = 1;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("kretprobe/sys_nanosleep")
> +int handle_sys_getpid_return(struct pt_regs *ctx)
> +{
> +	const int key = 1, value = 2;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("uprobe/trigger_func")
> +int handle_uprobe_entry(struct pt_regs *ctx)
> +{
> +	const int key = 2, value = 3;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +SEC("uretprobe/trigger_func")
> +int handle_uprobe_return(struct pt_regs *ctx)
> +{
> +	const int key = 3, value = 4;
> +
> +	bpf_map_update_elem(&results_map, &key, &value, 0);
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> 

^ permalink raw reply

* Re: [PATCH v6 bpf-next 0/5] libbpf: add perf buffer abstraction and API
From: Yonghong Song @ 2019-07-06 17:24 UTC (permalink / raw)
  To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
	netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
	Kernel Team
In-Reply-To: <20190706060220.1801632-1-andriin@fb.com>



On 7/5/19 11:02 PM, Andrii Nakryiko wrote:
> This patchset adds a high-level API for setting up and polling perf buffers
> associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
> described in corresponding commit.
> 
> Patch #1 adds a set of APIs to set up and work with perf buffer.
> Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
> Patch #3 adds test.
> Patch #4 converts bpftool map event_pipe to new API.
> Patch #5 updates README to mention perf_buffer_ prefix.
> 
> v5->v6:
> - fix C99 for loop variable initialization usage (Yonghong);
> v4->v5:
> - initialize perf_buffer_raw_opts in bpftool map event_pipe (Jakub);
> - add perf_buffer_ to README;
> v3->v4:
> - fixed bpftool event_pipe cmd error handling (Jakub);
> v2->v3:
> - added perf_buffer__new_raw for more low-level control;
> - converted bpftool map event_pipe to new API (Daniel);
> - fixed bug with error handling in create_maps (Song);
> v1->v2:
> - add auto-sizing of PERF_EVENT_ARRAY maps;
> 
> Andrii Nakryiko (5):
>    libbpf: add perf buffer API
>    libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
>    selftests/bpf: test perf buffer API
>    tools/bpftool: switch map event_pipe to libbpf's perf_buffer
>    libbpf: add perf_buffer_ prefix to README
> 
>   tools/bpf/bpftool/map_perf_ring.c             | 201 +++------
>   tools/lib/bpf/README.rst                      |   3 +-
>   tools/lib/bpf/libbpf.c                        | 397 +++++++++++++++++-
>   tools/lib/bpf/libbpf.h                        |  49 +++
>   tools/lib/bpf/libbpf.map                      |   4 +
>   .../selftests/bpf/prog_tests/perf_buffer.c    |  94 +++++
>   .../selftests/bpf/progs/test_perf_buffer.c    |  25 ++
>   7 files changed, 628 insertions(+), 145 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c

With a minor comment on patch 3/5 in a different thread,
LGTM. Ack for the whole series.
Acked-by: Yonghong Song <yhs@fb.com>


^ permalink raw reply

* Re: [PATCH v5 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-07-06 17:42 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Andrii Nakryiko, bpf@vger.kernel.org, netdev@vger.kernel.org,
	Alexei Starovoitov, daniel@iogearbox.net, Kernel Team
In-Reply-To: <ed0d9c3d-da7c-b925-e3a6-767098765850@fb.com>

On Sat, Jul 6, 2019 at 10:21 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/1/19 4:59 PM, Andrii Nakryiko wrote:
> > Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
> > expected.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Reviewed-by: Stanislav Fomichev <sdf@google.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > ---
> >   .../selftests/bpf/prog_tests/attach_probe.c   | 166 ++++++++++++++++++
> >   .../selftests/bpf/progs/test_attach_probe.c   |  55 ++++++
> >   2 files changed, 221 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > new file mode 100644
> > index 000000000000..a4686395522c
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
> > @@ -0,0 +1,166 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <test_progs.h>
> > +
> > +ssize_t get_base_addr() {
> > +     size_t start;
> > +     char buf[256];
> > +     FILE *f;
> > +
> > +     f = fopen("/proc/self/maps", "r");
> > +     if (!f)
> > +             return -errno;
> > +
> > +     while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
> > +             if (strcmp(buf, "r-xp") == 0) {
> > +                     fclose(f);
> > +                     return start;
> > +             }
> > +     }
> > +
> > +     fclose(f);
> > +     return -EINVAL;
> > +}
> > +
> > +#ifdef __x86_64__
> > +#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
> > +#else
> > +#define SYS_KPROBE_NAME "sys_nanosleep"
> > +#endif
> > +
> > +void test_attach_probe(void)
> > +{
> > +     const char *kprobe_name = "kprobe/sys_nanosleep";
> > +     const char *kretprobe_name = "kretprobe/sys_nanosleep";
> > +     const char *uprobe_name = "uprobe/trigger_func";
> > +     const char *uretprobe_name = "uretprobe/trigger_func";
> > +     const int kprobe_idx = 0, kretprobe_idx = 1;
> > +     const int uprobe_idx = 2, uretprobe_idx = 3;
> > +     const char *file = "./test_attach_probe.o";
> > +     struct bpf_program *kprobe_prog, *kretprobe_prog;
> > +     struct bpf_program *uprobe_prog, *uretprobe_prog;
> > +     struct bpf_object *obj;
> > +     int err, prog_fd, duration = 0, res;
> > +     struct bpf_link *kprobe_link = NULL;
> > +     struct bpf_link *kretprobe_link = NULL;
> > +     struct bpf_link *uprobe_link = NULL;
> > +     struct bpf_link *uretprobe_link = NULL;
> > +     int results_map_fd;
> > +     size_t uprobe_offset;
> > +     ssize_t base_addr;
> > +
> > +     base_addr = get_base_addr();
> > +     if (CHECK(base_addr < 0, "get_base_addr",
> > +               "failed to find base addr: %zd", base_addr))
> > +             return;
> > +     uprobe_offset = (size_t)&get_base_addr - base_addr;
> > +
> > +     /* load programs */
> > +     err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> > +     if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> > +             return;
> > +
> > +     kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
> > +     if (CHECK(!kprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", kprobe_name))
> > +             goto cleanup;
> > +     kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
> > +     if (CHECK(!kretprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", kretprobe_name))
> > +             goto cleanup;
> > +     uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
> > +     if (CHECK(!uprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", uprobe_name))
> > +             goto cleanup;
> > +     uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
> > +     if (CHECK(!uretprobe_prog, "find_probe",
> > +               "prog '%s' not found\n", uretprobe_name))
> > +             goto cleanup;
> > +
> > +     /* load maps */
> > +     results_map_fd = bpf_find_map(__func__, obj, "results_map");
> > +     if (CHECK(results_map_fd < 0, "find_results_map",
> > +               "err %d\n", results_map_fd))
> > +             goto cleanup;
> > +
> > +     kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
> > +                                              false /* retprobe */,
> > +                                              SYS_KPROBE_NAME);
> > +     if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
> > +               "err %ld\n", PTR_ERR(kprobe_link))) {
> > +             kprobe_link = NULL;
> > +             goto cleanup;
> > +     }
> > +     kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
> > +                                                 true /* retprobe */,
> > +                                                 SYS_KPROBE_NAME);
> > +     if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
> > +               "err %ld\n", PTR_ERR(kretprobe_link))) {
> > +             kretprobe_link = NULL;
> > +             goto cleanup;
> > +     }
> > +     uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
> > +                                              false /* retprobe */,
> > +                                              0 /* self pid */,
> > +                                              "/proc/self/exe",
> > +                                              uprobe_offset);
> > +     if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
> > +               "err %ld\n", PTR_ERR(uprobe_link))) {
> > +             uprobe_link = NULL;
> > +             goto cleanup;
> > +     }
> > +     uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
> > +                                                 true /* retprobe */,
> > +                                                 -1 /* any pid */,
> > +                                                 "/proc/self/exe",
> > +                                                 uprobe_offset);
> > +     if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
> > +               "err %ld\n", PTR_ERR(uretprobe_link))) {
> > +             uretprobe_link = NULL;
> > +             goto cleanup;
> > +     }
> > +
> > +     /* trigger & validate kprobe && kretprobe */
> > +     usleep(1);
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
> > +     if (CHECK(err, "get_kprobe_res",
> > +               "failed to get kprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
> > +               "wrong kprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
> > +     if (CHECK(err, "get_kretprobe_res",
> > +               "failed to get kretprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
> > +               "wrong kretprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     /* trigger & validate uprobe & uretprobe */
> > +     get_base_addr();
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
> > +     if (CHECK(err, "get_uprobe_res",
> > +               "failed to get uprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
> > +               "wrong uprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +     err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
> > +     if (CHECK(err, "get_uretprobe_res",
> > +               "failed to get uretprobe res: %d\n", err))
> > +             goto cleanup;
> > +     if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
> > +               "wrong uretprobe res: %d\n", res))
> > +             goto cleanup;
> > +
> > +cleanup:
> > +     bpf_link__destroy(kprobe_link);
> > +     bpf_link__destroy(kretprobe_link);
> > +     bpf_link__destroy(uprobe_link);
> > +     bpf_link__destroy(uretprobe_link);
> > +     bpf_object__close(obj);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> > new file mode 100644
> > index 000000000000..7a7c5cd728c8
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
> > @@ -0,0 +1,55 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2017 Facebook
> > +
> > +#include <linux/ptrace.h>
> > +#include <linux/bpf.h>
> > +#include "bpf_helpers.h"
> > +
> > +struct {
> > +     int type;
> > +     int max_entries;
> > +     int *key;
> > +     int *value;
> > +} results_map SEC(".maps") = {
> > +     .type = BPF_MAP_TYPE_ARRAY,
> > +     .max_entries = 4,
> > +};
>
> After the new .maps convention patch is merged, test_progs is broken due
> to this. The above .maps definition needs to be updated to
>
> struct {
>         __uint(type, BPF_MAP_TYPE_ARRAY);
>         __uint(max_entries, 4);
>         __type(key, int);
>         __type(value, int);
> } results_map SEC(".maps");
>

Yep, noticed that yesterday. Fixed in [0].

  [0] https://patchwork.ozlabs.org/patch/1128383/

> > +
> > +SEC("kprobe/sys_nanosleep")
> > +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> > +{
> > +     const int key = 0, value = 1;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("kretprobe/sys_nanosleep")
> > +int handle_sys_getpid_return(struct pt_regs *ctx)
> > +{
> > +     const int key = 1, value = 2;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("uprobe/trigger_func")
> > +int handle_uprobe_entry(struct pt_regs *ctx)
> > +{
> > +     const int key = 2, value = 3;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +SEC("uretprobe/trigger_func")
> > +int handle_uprobe_return(struct pt_regs *ctx)
> > +{
> > +     const int key = 3, value = 4;
> > +
> > +     bpf_map_update_elem(&results_map, &key, &value, 0);
> > +     return 0;
> > +}
> > +
> > +char _license[] SEC("license") = "GPL";
> > +__u32 _version SEC("version") = 1;
> >

^ permalink raw reply

* Re: [PATCH v6 bpf-next 3/5] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-07-06 17:44 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Andrii Nakryiko, bpf@vger.kernel.org, netdev@vger.kernel.org,
	Alexei Starovoitov, daniel@iogearbox.net, Kernel Team
In-Reply-To: <a04cc2f1-a107-221e-4ea3-a4650826f325@fb.com>

On Sat, Jul 6, 2019 at 10:18 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 7/5/19 11:02 PM, Andrii Nakryiko wrote:
> > Add test verifying perf buffer API functionality.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > ---
> >   .../selftests/bpf/prog_tests/perf_buffer.c    | 94 +++++++++++++++++++
> >   .../selftests/bpf/progs/test_perf_buffer.c    | 25 +++++
> >   2 files changed, 119 insertions(+)
> >   create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> >   create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> > new file mode 100644
> > index 000000000000..64556ab0d1a9
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> > @@ -0,0 +1,94 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#define _GNU_SOURCE
> > +#include <pthread.h>
> > +#include <sched.h>
> > +#include <sys/socket.h>
> > +#include <test_progs.h>
> > +
> > +static void on_sample(void *ctx, int cpu, void *data, __u32 size)
> > +{
> > +     int cpu_data = *(int *)data, duration = 0;
> > +     cpu_set_t *cpu_seen = ctx;
> > +
> > +     if (cpu_data != cpu)
> > +             CHECK(cpu_data != cpu, "check_cpu_data",
> > +                   "cpu_data %d != cpu %d\n", cpu_data, cpu);
> > +
> > +     CPU_SET(cpu, cpu_seen);
> > +}
> > +
> > +void test_perf_buffer(void)
> > +{
> > +     int err, prog_fd, nr_cpus, i, duration = 0;
> > +     const char *prog_name = "kprobe/sys_nanosleep";
> > +     const char *file = "./test_perf_buffer.o";
> > +     struct perf_buffer_opts pb_opts = {};
> > +     struct bpf_map *perf_buf_map;
> > +     cpu_set_t cpu_set, cpu_seen;
> > +     struct bpf_program *prog;
> > +     struct bpf_object *obj;
> > +     struct perf_buffer *pb;
> > +     struct bpf_link *link;
> > +
> > +     nr_cpus = libbpf_num_possible_cpus();
> > +     if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
> > +             return;
> > +
> > +     /* load program */
> > +     err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> > +     if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> > +             return;
> > +
> > +     prog = bpf_object__find_program_by_title(obj, prog_name);
> > +     if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
> > +             goto out_close;
> > +
> > +     /* load map */
> > +     perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
> > +     if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
> > +             goto out_close;
> > +
> > +     /* attach kprobe */
> > +     link = bpf_program__attach_kprobe(prog, false /* retprobe */,
> > +                                           "sys_nanosleep");
>
> The attach function "sys_nanosleep" won't work. You can have something
> similar to attach_probe.c.
>
> #ifdef __x86_64__
> #define SYS_KPROBE_NAME "__x64_sys_nanosleep"
> #else
> #define SYS_KPROBE_NAME "sys_nanosleep"
> #endif
>

Yeah, this is going to be a nightmare with those arch-specific names.
I'm wondering if it's worth it to automatically do that in libbpf for
users...

But anyway, will fix in v7, thanks!

>
> > +     if (CHECK(IS_ERR(link), "attach_kprobe", "err %ld\n", PTR_ERR(link)))
> > +             goto out_close;
> > +
> > +     /* set up perf buffer */
> > +     pb_opts.sample_cb = on_sample;
> > +     pb_opts.ctx = &cpu_seen;
> > +     pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts);
> > +     if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
> > +             goto out_detach;
> > +
> > +     /* trigger kprobe on every CPU */
> > +     CPU_ZERO(&cpu_seen);
> > +     for (i = 0; i < nr_cpus; i++) {
> > +             CPU_ZERO(&cpu_set);
> > +             CPU_SET(i, &cpu_set);
> > +
> > +             err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
> > +                                          &cpu_set);
> > +             if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
> > +                              i, err))
> > +                     goto out_detach;
> > +
> > +             usleep(1);
> > +     }
> > +
> > +     /* read perf buffer */
> > +     err = perf_buffer__poll(pb, 100);
> > +     if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
> > +             goto out_free_pb;
> > +
> > +     if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
> > +               "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
> > +             goto out_free_pb;
> > +
> > +out_free_pb:
> > +     perf_buffer__free(pb);
> > +out_detach:
> > +     bpf_link__destroy(link);
> > +out_close:
> > +     bpf_object__close(obj);
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> > new file mode 100644
> > index 000000000000..876c27deb65a
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> > @@ -0,0 +1,25 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (c) 2019 Facebook
> > +
> > +#include <linux/ptrace.h>
> > +#include <linux/bpf.h>
> > +#include "bpf_helpers.h"
> > +
> > +struct {
> > +     __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> > +     __uint(key_size, sizeof(int));
> > +     __uint(value_size, sizeof(int));
> > +} perf_buf_map SEC(".maps");
> > +
> > +SEC("kprobe/sys_nanosleep")
> > +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> > +{
> > +     int cpu = bpf_get_smp_processor_id();
> > +
> > +     bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
> > +                           &cpu, sizeof(cpu));
> > +     return 0;
> > +}
> > +
> > +char _license[] SEC("license") = "GPL";
> > +__u32 _version SEC("version") = 1;
> >

^ permalink raw reply

* Re: [PATCH v5 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Yonghong Song @ 2019-07-06 17:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf@vger.kernel.org, netdev@vger.kernel.org,
	Alexei Starovoitov, daniel@iogearbox.net, Kernel Team
In-Reply-To: <CAEf4BzZhXS1q_tLGdbwarRRQMx0YfhugYwCKZM=7RUKa=2uHMA@mail.gmail.com>



On 7/6/19 10:42 AM, Andrii Nakryiko wrote:
> On Sat, Jul 6, 2019 at 10:21 AM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 7/1/19 4:59 PM, Andrii Nakryiko wrote:
>>> Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
>>> expected.
>>>
>>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>>> Reviewed-by: Stanislav Fomichev <sdf@google.com>
>>> Acked-by: Song Liu <songliubraving@fb.com>
>>> ---
>>>    .../selftests/bpf/prog_tests/attach_probe.c   | 166 ++++++++++++++++++
>>>    .../selftests/bpf/progs/test_attach_probe.c   |  55 ++++++
>>>    2 files changed, 221 insertions(+)
>>>    create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
>>>    create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
>>> new file mode 100644
>>> index 000000000000..a4686395522c
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
>>> @@ -0,0 +1,166 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +#include <test_progs.h>
>>> +
>>> +ssize_t get_base_addr() {
>>> +     size_t start;
>>> +     char buf[256];
>>> +     FILE *f;
>>> +
>>> +     f = fopen("/proc/self/maps", "r");
>>> +     if (!f)
>>> +             return -errno;
>>> +
>>> +     while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
>>> +             if (strcmp(buf, "r-xp") == 0) {
>>> +                     fclose(f);
>>> +                     return start;
>>> +             }
>>> +     }
>>> +
>>> +     fclose(f);
>>> +     return -EINVAL;
>>> +}
>>> +
>>> +#ifdef __x86_64__
>>> +#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
>>> +#else
>>> +#define SYS_KPROBE_NAME "sys_nanosleep"
>>> +#endif
>>> +
>>> +void test_attach_probe(void)
>>> +{
>>> +     const char *kprobe_name = "kprobe/sys_nanosleep";
>>> +     const char *kretprobe_name = "kretprobe/sys_nanosleep";
>>> +     const char *uprobe_name = "uprobe/trigger_func";
>>> +     const char *uretprobe_name = "uretprobe/trigger_func";
>>> +     const int kprobe_idx = 0, kretprobe_idx = 1;
>>> +     const int uprobe_idx = 2, uretprobe_idx = 3;
>>> +     const char *file = "./test_attach_probe.o";
>>> +     struct bpf_program *kprobe_prog, *kretprobe_prog;
>>> +     struct bpf_program *uprobe_prog, *uretprobe_prog;
>>> +     struct bpf_object *obj;
>>> +     int err, prog_fd, duration = 0, res;
>>> +     struct bpf_link *kprobe_link = NULL;
>>> +     struct bpf_link *kretprobe_link = NULL;
>>> +     struct bpf_link *uprobe_link = NULL;
>>> +     struct bpf_link *uretprobe_link = NULL;
>>> +     int results_map_fd;
>>> +     size_t uprobe_offset;
>>> +     ssize_t base_addr;
>>> +
>>> +     base_addr = get_base_addr();
>>> +     if (CHECK(base_addr < 0, "get_base_addr",
>>> +               "failed to find base addr: %zd", base_addr))
>>> +             return;
>>> +     uprobe_offset = (size_t)&get_base_addr - base_addr;
>>> +
>>> +     /* load programs */
>>> +     err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
>>> +     if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
>>> +             return;
>>> +
>>> +     kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
>>> +     if (CHECK(!kprobe_prog, "find_probe",
>>> +               "prog '%s' not found\n", kprobe_name))
>>> +             goto cleanup;
>>> +     kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
>>> +     if (CHECK(!kretprobe_prog, "find_probe",
>>> +               "prog '%s' not found\n", kretprobe_name))
>>> +             goto cleanup;
>>> +     uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
>>> +     if (CHECK(!uprobe_prog, "find_probe",
>>> +               "prog '%s' not found\n", uprobe_name))
>>> +             goto cleanup;
>>> +     uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
>>> +     if (CHECK(!uretprobe_prog, "find_probe",
>>> +               "prog '%s' not found\n", uretprobe_name))
>>> +             goto cleanup;
>>> +
>>> +     /* load maps */
>>> +     results_map_fd = bpf_find_map(__func__, obj, "results_map");
>>> +     if (CHECK(results_map_fd < 0, "find_results_map",
>>> +               "err %d\n", results_map_fd))
>>> +             goto cleanup;
>>> +
>>> +     kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
>>> +                                              false /* retprobe */,
>>> +                                              SYS_KPROBE_NAME);
>>> +     if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
>>> +               "err %ld\n", PTR_ERR(kprobe_link))) {
>>> +             kprobe_link = NULL;
>>> +             goto cleanup;
>>> +     }
>>> +     kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
>>> +                                                 true /* retprobe */,
>>> +                                                 SYS_KPROBE_NAME);
>>> +     if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
>>> +               "err %ld\n", PTR_ERR(kretprobe_link))) {
>>> +             kretprobe_link = NULL;
>>> +             goto cleanup;
>>> +     }
>>> +     uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
>>> +                                              false /* retprobe */,
>>> +                                              0 /* self pid */,
>>> +                                              "/proc/self/exe",
>>> +                                              uprobe_offset);
>>> +     if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
>>> +               "err %ld\n", PTR_ERR(uprobe_link))) {
>>> +             uprobe_link = NULL;
>>> +             goto cleanup;
>>> +     }
>>> +     uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
>>> +                                                 true /* retprobe */,
>>> +                                                 -1 /* any pid */,
>>> +                                                 "/proc/self/exe",
>>> +                                                 uprobe_offset);
>>> +     if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
>>> +               "err %ld\n", PTR_ERR(uretprobe_link))) {
>>> +             uretprobe_link = NULL;
>>> +             goto cleanup;
>>> +     }
>>> +
>>> +     /* trigger & validate kprobe && kretprobe */
>>> +     usleep(1);
>>> +
>>> +     err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
>>> +     if (CHECK(err, "get_kprobe_res",
>>> +               "failed to get kprobe res: %d\n", err))
>>> +             goto cleanup;
>>> +     if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
>>> +               "wrong kprobe res: %d\n", res))
>>> +             goto cleanup;
>>> +
>>> +     err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
>>> +     if (CHECK(err, "get_kretprobe_res",
>>> +               "failed to get kretprobe res: %d\n", err))
>>> +             goto cleanup;
>>> +     if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
>>> +               "wrong kretprobe res: %d\n", res))
>>> +             goto cleanup;
>>> +
>>> +     /* trigger & validate uprobe & uretprobe */
>>> +     get_base_addr();
>>> +
>>> +     err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
>>> +     if (CHECK(err, "get_uprobe_res",
>>> +               "failed to get uprobe res: %d\n", err))
>>> +             goto cleanup;
>>> +     if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
>>> +               "wrong uprobe res: %d\n", res))
>>> +             goto cleanup;
>>> +
>>> +     err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
>>> +     if (CHECK(err, "get_uretprobe_res",
>>> +               "failed to get uretprobe res: %d\n", err))
>>> +             goto cleanup;
>>> +     if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
>>> +               "wrong uretprobe res: %d\n", res))
>>> +             goto cleanup;
>>> +
>>> +cleanup:
>>> +     bpf_link__destroy(kprobe_link);
>>> +     bpf_link__destroy(kretprobe_link);
>>> +     bpf_link__destroy(uprobe_link);
>>> +     bpf_link__destroy(uretprobe_link);
>>> +     bpf_object__close(obj);
>>> +}
>>> diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
>>> new file mode 100644
>>> index 000000000000..7a7c5cd728c8
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
>>> @@ -0,0 +1,55 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +// Copyright (c) 2017 Facebook
>>> +
>>> +#include <linux/ptrace.h>
>>> +#include <linux/bpf.h>
>>> +#include "bpf_helpers.h"
>>> +
>>> +struct {
>>> +     int type;
>>> +     int max_entries;
>>> +     int *key;
>>> +     int *value;
>>> +} results_map SEC(".maps") = {
>>> +     .type = BPF_MAP_TYPE_ARRAY,
>>> +     .max_entries = 4,
>>> +};
>>
>> After the new .maps convention patch is merged, test_progs is broken due
>> to this. The above .maps definition needs to be updated to
>>
>> struct {
>>          __uint(type, BPF_MAP_TYPE_ARRAY);
>>          __uint(max_entries, 4);
>>          __type(key, int);
>>          __type(value, int);
>> } results_map SEC(".maps");
>>
> 
> Yep, noticed that yesterday. Fixed in [0].
> 
>    [0] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_1128383_&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=HOjHmyyEXoOHq5292pOm6Ai8KJSyeLjdFOHQ_T8oAdI&s=vRc6OsEL3WigbFGNcfJUWeBCPbbyxn8g_r2SeTgoM-I&e=
> 

Oh, sorry. forgot this patch. Indeed, it fixed the issue.

>>> +
>>> +SEC("kprobe/sys_nanosleep")
>>> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
>>> +{
>>> +     const int key = 0, value = 1;
>>> +
>>> +     bpf_map_update_elem(&results_map, &key, &value, 0);
>>> +     return 0;
>>> +}
>>> +
>>> +SEC("kretprobe/sys_nanosleep")
>>> +int handle_sys_getpid_return(struct pt_regs *ctx)
>>> +{
>>> +     const int key = 1, value = 2;
>>> +
>>> +     bpf_map_update_elem(&results_map, &key, &value, 0);
>>> +     return 0;
>>> +}
>>> +
>>> +SEC("uprobe/trigger_func")
>>> +int handle_uprobe_entry(struct pt_regs *ctx)
>>> +{
>>> +     const int key = 2, value = 3;
>>> +
>>> +     bpf_map_update_elem(&results_map, &key, &value, 0);
>>> +     return 0;
>>> +}
>>> +
>>> +SEC("uretprobe/trigger_func")
>>> +int handle_uprobe_return(struct pt_regs *ctx)
>>> +{
>>> +     const int key = 3, value = 4;
>>> +
>>> +     bpf_map_update_elem(&results_map, &key, &value, 0);
>>> +     return 0;
>>> +}
>>> +
>>> +char _license[] SEC("license") = "GPL";
>>> +__u32 _version SEC("version") = 1;
>>>

^ permalink raw reply

* Re: INFO: rcu detected stall in ext4_write_checks
From: Paul E. McKenney @ 2019-07-06 18:03 UTC (permalink / raw)
  To: Theodore Ts'o, Dmitry Vyukov, syzbot, Andreas Dilger,
	David Miller, eladr, Ido Schimmel, Jiri Pirko, John Stultz,
	linux-ext4, LKML, netdev, syzkaller-bugs, Thomas Gleixner,
	Peter Zijlstra, Ingo Molnar
In-Reply-To: <20190706150226.GG11665@mit.edu>

On Sat, Jul 06, 2019 at 11:02:26AM -0400, Theodore Ts'o wrote:
> On Fri, Jul 05, 2019 at 11:16:31PM -0700, Paul E. McKenney wrote:
> > I suppose RCU could take the dueling-banjos approach and use increasingly
> > aggressive scheduler policies itself, up to and including SCHED_DEADLINE,
> > until it started getting decent forward progress.  However, that
> > sounds like the something that just might have unintended consequences,
> > particularly if other kernel subsystems were to also play similar
> > games of dueling banjos.
> 
> So long as the RCU threads are well-behaved, using SCHED_DEADLINE
> shouldn't have much of an impact on the system --- and the scheduling
> parameters that you can specify on SCHED_DEADLINE allows you to
> specify the worst-case impact on the system while also guaranteeing
> that the SCHED_DEADLINE tasks will urn in the first place.  After all,
> that's the whole point of SCHED_DEADLINE.
> 
> So I wonder if the right approach is during the the first userspace
> system call to shced_setattr to enable a (any) real-time priority
> scheduler (SCHED_DEADLINE, SCHED_FIFO or SCHED_RR) on a userspace
> thread, before that's allowed to proceed, the RCU kernel threads are
> promoted to be SCHED_DEADLINE with appropriately set deadline
> parameters.  That way, a root user won't be able to shoot the system
> in the foot, and since the vast majority of the time, there shouldn't
> be any processes running with real-time priorities, we won't be
> changing the behavior of a normal server system.

It might well be.  However, running the RCU kthreads at real-time
priority does not come for free.  For example, it tends to crank up the
context-switch rate.

Plus I have taken several runs at computing SCHED_DEADLINE parameters,
but things like the rcuo callback-offload threads have computational
requirements that are controlled not by RCU, and not just by the rest of
the kernel, but also by userspace (keeping in mind the example of opening
and closing a file in a tight loop, each pass of which queues a callback).
I suspect that RCU is not the only kernel subsystem whose computational
requirements are set not by the subsystem, but rather by external code.

OK, OK, I suppose I could just set insanely large SCHED_DEADLINE
parameters, following syzkaller's example, and then trust my ability to
keep the RCU code from abusing the resulting awesome power.  But wouldn't
a much nicer approach be to put SCHED_DEADLINE between SCHED_RR/SCHED_FIFO
priorities 98 and 99 or some such?  Then the same (admittedly somewhat
scary) result could be obtained much more simply via SCHED_FIFO or
SCHED_RR priority 99.

Some might argue that this is one of those situations where simplicity
is not necessarily an advantage, but then again, you can find someone
who will complain about almost anything.  ;-)

> (I suspect there might be some audio applications that might try to
> set real-time priorities, but for desktop systems, it's probably more
> important that the system not tie its self into knots since the
> average desktop user isn't going to be well equipped to debug the
> problem.)

Not only that, but if core counts continue to increase, and if reliance
on cloud computing continues to grow, there are going to be an increasing
variety of mixed workloads in increasingly less-controlled environments.

So, yes, it would be good to solve this problem in some reasonable way.

I don't see this as urgent just yet, but I am sure you all will let
me know if I am mistaken on that point.

> > Alternatively, is it possible to provide stricter admission control?
> 
> I think that's an orthogonal issue; better admission control would be
> nice, but it looks to me that it's going to be fundamentally an issue
> of tweaking hueristics, and a fool-proof solution that will protect
> against all malicious userspace applications (including syzkaller) is
> going to require solving the halting problem.  So while it would be
> nice to improve the admission control, I don't think that's a going to
> be a general solution.

Agreed, and my earlier point about the need to trust the coding abilities
of those writing ultimate-priority code is all too consistent with your
point about needing to solve the halting problem.  Nevertheless,  I believe
that we could make something that worked reasonably well in practice.

Here are a few components of a possible solution, in practice, but
of course not in theory:

1.	We set limits to SCHED_DEADLINE parameters, perhaps novel ones.
	For one example, insist on (say) 10 milliseconds of idle time
	every second on each CPU.  Yes, you can configure beyond that
	given sufficient permissions, but if you do so, you just voided
	your warranty.

2.	Only allow SCHED_DEADLINE on nohz_full CPUs.  (Partial solution,
	given that such a CPU might be running in the kernel or have
	more than one runnable task.  Just for fun, I will suggest the
	option of disabling SCHED_DEADLINE during such times.)

3.	RCU detects slowdowns, and does something TBD to increase its
	priority, but only while the slowdown persists.  This likely
	relies on scheduling-clock interrupts to detect the slowdowns,
	so there might be additional challenges on a fully nohz_full
	system.

4.	Your idea here.

							Thanx, Paul


^ permalink raw reply

* [PATCH v7 bpf-next 0/5] libbpf: add perf buffer abstraction and API
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko

This patchset adds a high-level API for setting up and polling perf buffers
associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
described in corresponding commit.

Patch #1 adds a set of APIs to set up and work with perf buffer.
Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
Patch #3 adds test.
Patch #4 converts bpftool map event_pipe to new API.
Patch #5 updates README to mention perf_buffer_ prefix.

v6->v7:
- __x64_ syscall prefix (Yonghong);
v5->v6:
- fix C99 for loop variable initialization usage (Yonghong);
v4->v5:
- initialize perf_buffer_raw_opts in bpftool map event_pipe (Jakub);
- add perf_buffer_ to README;
v3->v4:
- fixed bpftool event_pipe cmd error handling (Jakub);
v2->v3:
- added perf_buffer__new_raw for more low-level control;
- converted bpftool map event_pipe to new API (Daniel);
- fixed bug with error handling in create_maps (Song);
v1->v2:
- add auto-sizing of PERF_EVENT_ARRAY maps;

Andrii Nakryiko (5):
  libbpf: add perf buffer API
  libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
  selftests/bpf: test perf buffer API
  tools/bpftool: switch map event_pipe to libbpf's perf_buffer
  libbpf: add perf_buffer_ prefix to README

 tools/bpf/bpftool/map_perf_ring.c             | 201 +++------
 tools/lib/bpf/README.rst                      |   3 +-
 tools/lib/bpf/libbpf.c                        | 397 +++++++++++++++++-
 tools/lib/bpf/libbpf.h                        |  49 +++
 tools/lib/bpf/libbpf.map                      |   4 +
 .../selftests/bpf/prog_tests/perf_buffer.c    | 100 +++++
 .../selftests/bpf/progs/test_perf_buffer.c    |  25 ++
 7 files changed, 634 insertions(+), 145 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c

-- 
2.17.1


^ permalink raw reply

* [PATCH v7 bpf-next 2/5] libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>

For BPF_MAP_TYPE_PERF_EVENT_ARRAY typically correct size is number of
possible CPUs. This is impossible to specify at compilation time. This
change adds automatic setting of PERF_EVENT_ARRAY size to number of
system CPUs, unless non-zero size is specified explicitly. This allows
to adjust size for advanced specific cases, while providing convenient
and logical defaults.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/lib/bpf/libbpf.c | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ae569b50e2e0..ed07789b3e62 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2116,6 +2116,7 @@ static int
 bpf_object__create_maps(struct bpf_object *obj)
 {
 	struct bpf_create_map_attr create_attr = {};
+	int nr_cpus = 0;
 	unsigned int i;
 	int err;
 
@@ -2138,7 +2139,22 @@ bpf_object__create_maps(struct bpf_object *obj)
 		create_attr.map_flags = def->map_flags;
 		create_attr.key_size = def->key_size;
 		create_attr.value_size = def->value_size;
-		create_attr.max_entries = def->max_entries;
+		if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY &&
+		    !def->max_entries) {
+			if (!nr_cpus)
+				nr_cpus = libbpf_num_possible_cpus();
+			if (nr_cpus < 0) {
+				pr_warning("failed to determine number of system CPUs: %d\n",
+					   nr_cpus);
+				err = nr_cpus;
+				goto err_out;
+			}
+			pr_debug("map '%s': setting size to %d\n",
+				 map->name, nr_cpus);
+			create_attr.max_entries = nr_cpus;
+		} else {
+			create_attr.max_entries = def->max_entries;
+		}
 		create_attr.btf_fd = 0;
 		create_attr.btf_key_type_id = 0;
 		create_attr.btf_value_type_id = 0;
@@ -2155,9 +2171,10 @@ bpf_object__create_maps(struct bpf_object *obj)
 		*pfd = bpf_create_map_xattr(&create_attr);
 		if (*pfd < 0 && (create_attr.btf_key_type_id ||
 				 create_attr.btf_value_type_id)) {
-			cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+			err = -errno;
+			cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
 			pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
-				   map->name, cp, errno);
+				   map->name, cp, err);
 			create_attr.btf_fd = 0;
 			create_attr.btf_key_type_id = 0;
 			create_attr.btf_value_type_id = 0;
@@ -2169,11 +2186,11 @@ bpf_object__create_maps(struct bpf_object *obj)
 		if (*pfd < 0) {
 			size_t j;
 
-			err = *pfd;
+			err = -errno;
 err_out:
-			cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
-			pr_warning("failed to create map (name: '%s'): %s\n",
-				   map->name, cp);
+			cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
+			pr_warning("failed to create map (name: '%s'): %s(%d)\n",
+				   map->name, cp, err);
 			for (j = 0; j < i; j++)
 				zclose(obj->maps[j].fd);
 			return err;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 bpf-next 1/5] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>

BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
to user space for additional processing. libbpf already has very low-level API
to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
use and requires a lot of code to set everything up. This patch adds
perf_buffer abstraction on top of it, abstracting setting up and polling
per-CPU logic into simple and convenient API, similar to what BCC provides.

perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
map entries. It accepts two user-provided callbacks: one for handling raw
samples and one for get notifications of lost samples due to buffer overflow.

perf_buffer__new_raw() is similar, but provides more control over how
perf events are set up (by accepting user-provided perf_event_attr), how
they are handled (perf_event_header pointer is passed directly to
user-provided callback), and on which CPUs ring buffers are created
(it's possible to provide a list of CPUs and corresponding map keys to
update). This API allows advanced users fuller control.

perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
utilizing epoll instance.

perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.

All APIs are not thread-safe. User should ensure proper locking/coordination if
used in multi-threaded set up.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/lib/bpf/libbpf.c   | 366 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  49 ++++++
 tools/lib/bpf/libbpf.map |   4 +
 3 files changed, 419 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2a08eb106221..ae569b50e2e0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,7 +32,9 @@
 #include <linux/limits.h>
 #include <linux/perf_event.h>
 #include <linux/ring_buffer.h>
+#include <sys/epoll.h>
 #include <sys/ioctl.h>
+#include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
@@ -4354,6 +4356,370 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 	return ret;
 }
 
+struct perf_buffer;
+
+struct perf_buffer_params {
+	struct perf_event_attr *attr;
+	/* if event_cb is specified, it takes precendence */
+	perf_buffer_event_fn event_cb;
+	/* sample_cb and lost_cb are higher-level common-case callbacks */
+	perf_buffer_sample_fn sample_cb;
+	perf_buffer_lost_fn lost_cb;
+	void *ctx;
+	int cpu_cnt;
+	int *cpus;
+	int *map_keys;
+};
+
+struct perf_cpu_buf {
+	struct perf_buffer *pb;
+	void *base; /* mmap()'ed memory */
+	void *buf; /* for reconstructing segmented data */
+	size_t buf_size;
+	int fd;
+	int cpu;
+	int map_key;
+};
+
+struct perf_buffer {
+	perf_buffer_event_fn event_cb;
+	perf_buffer_sample_fn sample_cb;
+	perf_buffer_lost_fn lost_cb;
+	void *ctx; /* passed into callbacks */
+
+	size_t page_size;
+	size_t mmap_size;
+	struct perf_cpu_buf **cpu_bufs;
+	struct epoll_event *events;
+	int cpu_cnt;
+	int epoll_fd; /* perf event FD */
+	int map_fd; /* BPF_MAP_TYPE_PERF_EVENT_ARRAY BPF map FD */
+};
+
+static void perf_buffer__free_cpu_buf(struct perf_buffer *pb,
+				      struct perf_cpu_buf *cpu_buf)
+{
+	if (!cpu_buf)
+		return;
+	if (cpu_buf->base &&
+	    munmap(cpu_buf->base, pb->mmap_size + pb->page_size))
+		pr_warning("failed to munmap cpu_buf #%d\n", cpu_buf->cpu);
+	if (cpu_buf->fd >= 0) {
+		ioctl(cpu_buf->fd, PERF_EVENT_IOC_DISABLE, 0);
+		close(cpu_buf->fd);
+	}
+	free(cpu_buf->buf);
+	free(cpu_buf);
+}
+
+void perf_buffer__free(struct perf_buffer *pb)
+{
+	int i;
+
+	if (!pb)
+		return;
+	if (pb->cpu_bufs) {
+		for (i = 0; i < pb->cpu_cnt && pb->cpu_bufs[i]; i++) {
+			struct perf_cpu_buf *cpu_buf = pb->cpu_bufs[i];
+
+			bpf_map_delete_elem(pb->map_fd, &cpu_buf->map_key);
+			perf_buffer__free_cpu_buf(pb, cpu_buf);
+		}
+		free(pb->cpu_bufs);
+	}
+	if (pb->epoll_fd >= 0)
+		close(pb->epoll_fd);
+	free(pb->events);
+	free(pb);
+}
+
+static struct perf_cpu_buf *
+perf_buffer__open_cpu_buf(struct perf_buffer *pb, struct perf_event_attr *attr,
+			  int cpu, int map_key)
+{
+	struct perf_cpu_buf *cpu_buf;
+	char msg[STRERR_BUFSIZE];
+	int err;
+
+	cpu_buf = calloc(1, sizeof(*cpu_buf));
+	if (!cpu_buf)
+		return ERR_PTR(-ENOMEM);
+
+	cpu_buf->pb = pb;
+	cpu_buf->cpu = cpu;
+	cpu_buf->map_key = map_key;
+
+	cpu_buf->fd = syscall(__NR_perf_event_open, attr, -1 /* pid */, cpu,
+			      -1, PERF_FLAG_FD_CLOEXEC);
+	if (cpu_buf->fd < 0) {
+		err = -errno;
+		pr_warning("failed to open perf buffer event on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	cpu_buf->base = mmap(NULL, pb->mmap_size + pb->page_size,
+			     PROT_READ | PROT_WRITE, MAP_SHARED,
+			     cpu_buf->fd, 0);
+	if (cpu_buf->base == MAP_FAILED) {
+		cpu_buf->base = NULL;
+		err = -errno;
+		pr_warning("failed to mmap perf buffer on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	if (ioctl(cpu_buf->fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+		err = -errno;
+		pr_warning("failed to enable perf buffer event on cpu #%d: %s\n",
+			   cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	return cpu_buf;
+
+error:
+	perf_buffer__free_cpu_buf(pb, cpu_buf);
+	return (struct perf_cpu_buf *)ERR_PTR(err);
+}
+
+static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
+					      struct perf_buffer_params *p);
+
+struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
+				     const struct perf_buffer_opts *opts)
+{
+	struct perf_buffer_params p = {};
+	struct perf_event_attr attr = {
+		.config = PERF_COUNT_SW_BPF_OUTPUT,
+		.type = PERF_TYPE_SOFTWARE,
+		.sample_type = PERF_SAMPLE_RAW,
+		.sample_period = 1,
+		.wakeup_events = 1,
+	};
+
+	p.attr = &attr;
+	p.sample_cb = opts ? opts->sample_cb : NULL;
+	p.lost_cb = opts ? opts->lost_cb : NULL;
+	p.ctx = opts ? opts->ctx : NULL;
+
+	return __perf_buffer__new(map_fd, page_cnt, &p);
+}
+
+struct perf_buffer *
+perf_buffer__new_raw(int map_fd, size_t page_cnt,
+		     const struct perf_buffer_raw_opts *opts)
+{
+	struct perf_buffer_params p = {};
+
+	p.attr = opts->attr;
+	p.event_cb = opts->event_cb;
+	p.ctx = opts->ctx;
+	p.cpu_cnt = opts->cpu_cnt;
+	p.cpus = opts->cpus;
+	p.map_keys = opts->map_keys;
+
+	return __perf_buffer__new(map_fd, page_cnt, &p);
+}
+
+static struct perf_buffer *__perf_buffer__new(int map_fd, size_t page_cnt,
+					      struct perf_buffer_params *p)
+{
+	struct bpf_map_info map = {};
+	char msg[STRERR_BUFSIZE];
+	struct perf_buffer *pb;
+	__u32 map_info_len;
+	int err, i;
+
+	if (page_cnt & (page_cnt - 1)) {
+		pr_warning("page count should be power of two, but is %zu\n",
+			   page_cnt);
+		return ERR_PTR(-EINVAL);
+	}
+
+	map_info_len = sizeof(map);
+	err = bpf_obj_get_info_by_fd(map_fd, &map, &map_info_len);
+	if (err) {
+		err = -errno;
+		pr_warning("failed to get map info for map FD %d: %s\n",
+			   map_fd, libbpf_strerror_r(err, msg, sizeof(msg)));
+		return ERR_PTR(err);
+	}
+
+	if (map.type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+		pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+			   map.name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	pb = calloc(1, sizeof(*pb));
+	if (!pb)
+		return ERR_PTR(-ENOMEM);
+
+	pb->event_cb = p->event_cb;
+	pb->sample_cb = p->sample_cb;
+	pb->lost_cb = p->lost_cb;
+	pb->ctx = p->ctx;
+
+	pb->page_size = getpagesize();
+	pb->mmap_size = pb->page_size * page_cnt;
+	pb->map_fd = map_fd;
+
+	pb->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
+	if (pb->epoll_fd < 0) {
+		err = -errno;
+		pr_warning("failed to create epoll instance: %s\n",
+			   libbpf_strerror_r(err, msg, sizeof(msg)));
+		goto error;
+	}
+
+	if (p->cpu_cnt > 0) {
+		pb->cpu_cnt = p->cpu_cnt;
+	} else {
+		pb->cpu_cnt = libbpf_num_possible_cpus();
+		if (pb->cpu_cnt < 0) {
+			err = pb->cpu_cnt;
+			goto error;
+		}
+		if (map.max_entries < pb->cpu_cnt)
+			pb->cpu_cnt = map.max_entries;
+	}
+
+	pb->events = calloc(pb->cpu_cnt, sizeof(*pb->events));
+	if (!pb->events) {
+		err = -ENOMEM;
+		pr_warning("failed to allocate events: out of memory\n");
+		goto error;
+	}
+	pb->cpu_bufs = calloc(pb->cpu_cnt, sizeof(*pb->cpu_bufs));
+	if (!pb->cpu_bufs) {
+		err = -ENOMEM;
+		pr_warning("failed to allocate buffers: out of memory\n");
+		goto error;
+	}
+
+	for (i = 0; i < pb->cpu_cnt; i++) {
+		struct perf_cpu_buf *cpu_buf;
+		int cpu, map_key;
+
+		cpu = p->cpu_cnt > 0 ? p->cpus[i] : i;
+		map_key = p->cpu_cnt > 0 ? p->map_keys[i] : i;
+
+		cpu_buf = perf_buffer__open_cpu_buf(pb, p->attr, cpu, map_key);
+		if (IS_ERR(cpu_buf)) {
+			err = PTR_ERR(cpu_buf);
+			goto error;
+		}
+
+		pb->cpu_bufs[i] = cpu_buf;
+
+		err = bpf_map_update_elem(pb->map_fd, &map_key,
+					  &cpu_buf->fd, 0);
+		if (err) {
+			err = -errno;
+			pr_warning("failed to set cpu #%d, key %d -> perf FD %d: %s\n",
+				   cpu, map_key, cpu_buf->fd,
+				   libbpf_strerror_r(err, msg, sizeof(msg)));
+			goto error;
+		}
+
+		pb->events[i].events = EPOLLIN;
+		pb->events[i].data.ptr = cpu_buf;
+		if (epoll_ctl(pb->epoll_fd, EPOLL_CTL_ADD, cpu_buf->fd,
+			      &pb->events[i]) < 0) {
+			err = -errno;
+			pr_warning("failed to epoll_ctl cpu #%d perf FD %d: %s\n",
+				   cpu, cpu_buf->fd,
+				   libbpf_strerror_r(err, msg, sizeof(msg)));
+			goto error;
+		}
+	}
+
+	return pb;
+
+error:
+	if (pb)
+		perf_buffer__free(pb);
+	return ERR_PTR(err);
+}
+
+struct perf_sample_raw {
+	struct perf_event_header header;
+	uint32_t size;
+	char data[0];
+};
+
+struct perf_sample_lost {
+	struct perf_event_header header;
+	uint64_t id;
+	uint64_t lost;
+	uint64_t sample_id;
+};
+
+static enum bpf_perf_event_ret
+perf_buffer__process_record(struct perf_event_header *e, void *ctx)
+{
+	struct perf_cpu_buf *cpu_buf = ctx;
+	struct perf_buffer *pb = cpu_buf->pb;
+	void *data = e;
+
+	/* user wants full control over parsing perf event */
+	if (pb->event_cb)
+		return pb->event_cb(pb->ctx, cpu_buf->cpu, e);
+
+	switch (e->type) {
+	case PERF_RECORD_SAMPLE: {
+		struct perf_sample_raw *s = data;
+
+		if (pb->sample_cb)
+			pb->sample_cb(pb->ctx, cpu_buf->cpu, s->data, s->size);
+		break;
+	}
+	case PERF_RECORD_LOST: {
+		struct perf_sample_lost *s = data;
+
+		if (pb->lost_cb)
+			pb->lost_cb(pb->ctx, cpu_buf->cpu, s->lost);
+		break;
+	}
+	default:
+		pr_warning("unknown perf sample type %d\n", e->type);
+		return LIBBPF_PERF_EVENT_ERROR;
+	}
+	return LIBBPF_PERF_EVENT_CONT;
+}
+
+static int perf_buffer__process_records(struct perf_buffer *pb,
+					struct perf_cpu_buf *cpu_buf)
+{
+	enum bpf_perf_event_ret ret;
+
+	ret = bpf_perf_event_read_simple(cpu_buf->base, pb->mmap_size,
+					 pb->page_size, &cpu_buf->buf,
+					 &cpu_buf->buf_size,
+					 perf_buffer__process_record, cpu_buf);
+	if (ret != LIBBPF_PERF_EVENT_CONT)
+		return ret;
+	return 0;
+}
+
+int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
+{
+	int i, cnt, err;
+
+	cnt = epoll_wait(pb->epoll_fd, pb->events, pb->cpu_cnt, timeout_ms);
+	for (i = 0; i < cnt; i++) {
+		struct perf_cpu_buf *cpu_buf = pb->events[i].data.ptr;
+
+		err = perf_buffer__process_records(pb, cpu_buf);
+		if (err) {
+			pr_warning("error while processing records: %d\n", err);
+			return err;
+		}
+	}
+	return cnt < 0 ? -errno : cnt;
+}
+
 struct bpf_prog_info_array_desc {
 	int	array_offset;	/* e.g. offset of jited_prog_insns */
 	int	count_offset;	/* e.g. offset of jited_prog_len */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index f55933784f95..5cbf459ece0b 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -358,6 +358,26 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
 LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
 LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
 
+struct perf_buffer;
+
+typedef void (*perf_buffer_sample_fn)(void *ctx, int cpu,
+				      void *data, __u32 size);
+typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
+
+/* common use perf buffer options */
+struct perf_buffer_opts {
+	/* if specified, sample_cb is called for each sample */
+	perf_buffer_sample_fn sample_cb;
+	/* if specified, lost_cb is called for each batch of lost samples */
+	perf_buffer_lost_fn lost_cb;
+	/* ctx is provided to sample_cb and lost_cb */
+	void *ctx;
+};
+
+LIBBPF_API struct perf_buffer *
+perf_buffer__new(int map_fd, size_t page_cnt,
+		 const struct perf_buffer_opts *opts);
+
 enum bpf_perf_event_ret {
 	LIBBPF_PERF_EVENT_DONE	= 0,
 	LIBBPF_PERF_EVENT_ERROR	= -1,
@@ -365,6 +385,35 @@ enum bpf_perf_event_ret {
 };
 
 struct perf_event_header;
+
+typedef enum bpf_perf_event_ret
+(*perf_buffer_event_fn)(void *ctx, int cpu, struct perf_event_header *event);
+
+/* raw perf buffer options, giving most power and control */
+struct perf_buffer_raw_opts {
+	/* perf event attrs passed directly into perf_event_open() */
+	struct perf_event_attr *attr;
+	/* raw event callback */
+	perf_buffer_event_fn event_cb;
+	/* ctx is provided to event_cb */
+	void *ctx;
+	/* if cpu_cnt == 0, open all on all possible CPUs (up to the number of
+	 * max_entries of given PERF_EVENT_ARRAY map)
+	 */
+	int cpu_cnt;
+	/* if cpu_cnt > 0, cpus is an array of CPUs to open ring buffers on */
+	int *cpus;
+	/* if cpu_cnt > 0, map_keys specify map keys to set per-CPU FDs for */
+	int *map_keys;
+};
+
+LIBBPF_API struct perf_buffer *
+perf_buffer__new_raw(int map_fd, size_t page_cnt,
+		     const struct perf_buffer_raw_opts *opts);
+
+LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
+LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
+
 typedef enum bpf_perf_event_ret
 	(*bpf_perf_event_print_t)(struct perf_event_header *hdr,
 				  void *private_data);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index e6b7d4edbc93..f9d316e873d8 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -179,4 +179,8 @@ LIBBPF_0.0.4 {
 		btf_dump__new;
 		btf__parse_elf;
 		libbpf_num_possible_cpus;
+		perf_buffer__free;
+		perf_buffer__new;
+		perf_buffer__new_raw;
+		perf_buffer__poll;
 } LIBBPF_0.0.3;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 bpf-next 5/5] libbpf: add perf_buffer_ prefix to README
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>

perf_buffer "object" is part of libbpf API now, add it to the list of
libbpf function prefixes.

Suggested-by: Daniel Borkman <daniel@iogearbox.net>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/lib/bpf/README.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/README.rst b/tools/lib/bpf/README.rst
index cef7b77eab69..8928f7787f2d 100644
--- a/tools/lib/bpf/README.rst
+++ b/tools/lib/bpf/README.rst
@@ -9,7 +9,8 @@ described here. It's recommended to follow these conventions whenever a
 new function or type is added to keep libbpf API clean and consistent.
 
 All types and functions provided by libbpf API should have one of the
-following prefixes: ``bpf_``, ``btf_``, ``libbpf_``, ``xsk_``.
+following prefixes: ``bpf_``, ``btf_``, ``libbpf_``, ``xsk_``,
+``perf_buffer_``.
 
 System call wrappers
 --------------------
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 bpf-next 3/5] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>

Add test verifying perf buffer API functionality.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 .../selftests/bpf/prog_tests/perf_buffer.c    | 100 ++++++++++++++++++
 .../selftests/bpf/progs/test_perf_buffer.c    |  25 +++++
 2 files changed, 125 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c

diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
new file mode 100644
index 000000000000..3f1ef95865ff
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sched.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+#ifdef __x86_64__
+#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
+#else
+#define SYS_KPROBE_NAME "sys_nanosleep"
+#endif
+
+static void on_sample(void *ctx, int cpu, void *data, __u32 size)
+{
+	int cpu_data = *(int *)data, duration = 0;
+	cpu_set_t *cpu_seen = ctx;
+
+	if (cpu_data != cpu)
+		CHECK(cpu_data != cpu, "check_cpu_data",
+		      "cpu_data %d != cpu %d\n", cpu_data, cpu);
+
+	CPU_SET(cpu, cpu_seen);
+}
+
+void test_perf_buffer(void)
+{
+	int err, prog_fd, nr_cpus, i, duration = 0;
+	const char *prog_name = "kprobe/sys_nanosleep";
+	const char *file = "./test_perf_buffer.o";
+	struct perf_buffer_opts pb_opts = {};
+	struct bpf_map *perf_buf_map;
+	cpu_set_t cpu_set, cpu_seen;
+	struct bpf_program *prog;
+	struct bpf_object *obj;
+	struct perf_buffer *pb;
+	struct bpf_link *link;
+
+	nr_cpus = libbpf_num_possible_cpus();
+	if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
+		return;
+
+	/* load program */
+	err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+	if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+		return;
+
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
+		goto out_close;
+
+	/* load map */
+	perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
+	if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
+		goto out_close;
+
+	/* attach kprobe */
+	link = bpf_program__attach_kprobe(prog, false /* retprobe */,
+					  SYS_KPROBE_NAME);
+	if (CHECK(IS_ERR(link), "attach_kprobe", "err %ld\n", PTR_ERR(link)))
+		goto out_close;
+
+	/* set up perf buffer */
+	pb_opts.sample_cb = on_sample;
+	pb_opts.ctx = &cpu_seen;
+	pb = perf_buffer__new(bpf_map__fd(perf_buf_map), 1, &pb_opts);
+	if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
+		goto out_detach;
+
+	/* trigger kprobe on every CPU */
+	CPU_ZERO(&cpu_seen);
+	for (i = 0; i < nr_cpus; i++) {
+		CPU_ZERO(&cpu_set);
+		CPU_SET(i, &cpu_set);
+
+		err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
+					     &cpu_set);
+		if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
+				 i, err))
+			goto out_detach;
+
+		usleep(1);
+	}
+
+	/* read perf buffer */
+	err = perf_buffer__poll(pb, 100);
+	if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
+		goto out_free_pb;
+
+	if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
+		  "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
+		goto out_free_pb;
+
+out_free_pb:
+	perf_buffer__free(pb);
+out_detach:
+	bpf_link__destroy(link);
+out_close:
+	bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
new file mode 100644
index 000000000000..876c27deb65a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
+	__uint(key_size, sizeof(int));
+	__uint(value_size, sizeof(int));
+} perf_buf_map SEC(".maps");
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+	int cpu = bpf_get_smp_processor_id();
+
+	bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
+			      &cpu, sizeof(cpu));
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
-- 
2.17.1


^ permalink raw reply related

* [PATCH v7 bpf-next 4/5] tools/bpftool: switch map event_pipe to libbpf's perf_buffer
From: Andrii Nakryiko @ 2019-07-06 18:06 UTC (permalink / raw)
  To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>

Switch event_pipe implementation to rely on new libbpf perf buffer API
(it's raw low-level variant).

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Yonghong Song <yhs@fb.com>
---
 tools/bpf/bpftool/map_perf_ring.c | 201 ++++++++++--------------------
 1 file changed, 64 insertions(+), 137 deletions(-)

diff --git a/tools/bpf/bpftool/map_perf_ring.c b/tools/bpf/bpftool/map_perf_ring.c
index 0507dfaf7a8f..3f108ab17797 100644
--- a/tools/bpf/bpftool/map_perf_ring.c
+++ b/tools/bpf/bpftool/map_perf_ring.c
@@ -28,7 +28,7 @@
 
 #define MMAP_PAGE_CNT	16
 
-static bool stop;
+static volatile bool stop;
 
 struct event_ring_info {
 	int fd;
@@ -44,32 +44,44 @@ struct perf_event_sample {
 	unsigned char data[];
 };
 
+struct perf_event_lost {
+	struct perf_event_header header;
+	__u64 id;
+	__u64 lost;
+};
+
 static void int_exit(int signo)
 {
 	fprintf(stderr, "Stopping...\n");
 	stop = true;
 }
 
+struct event_pipe_ctx {
+	bool all_cpus;
+	int cpu;
+	int idx;
+};
+
 static enum bpf_perf_event_ret
-print_bpf_output(struct perf_event_header *event, void *private_data)
+print_bpf_output(void *private_data, int cpu, struct perf_event_header *event)
 {
-	struct perf_event_sample *e = container_of(event, struct perf_event_sample,
+	struct perf_event_sample *e = container_of(event,
+						   struct perf_event_sample,
 						   header);
-	struct event_ring_info *ring = private_data;
-	struct {
-		struct perf_event_header header;
-		__u64 id;
-		__u64 lost;
-	} *lost = (typeof(lost))event;
+	struct perf_event_lost *lost = container_of(event,
+						    struct perf_event_lost,
+						    header);
+	struct event_pipe_ctx *ctx = private_data;
+	int idx = ctx->all_cpus ? cpu : ctx->idx;
 
 	if (json_output) {
 		jsonw_start_object(json_wtr);
 		jsonw_name(json_wtr, "type");
 		jsonw_uint(json_wtr, e->header.type);
 		jsonw_name(json_wtr, "cpu");
-		jsonw_uint(json_wtr, ring->cpu);
+		jsonw_uint(json_wtr, cpu);
 		jsonw_name(json_wtr, "index");
-		jsonw_uint(json_wtr, ring->key);
+		jsonw_uint(json_wtr, idx);
 		if (e->header.type == PERF_RECORD_SAMPLE) {
 			jsonw_name(json_wtr, "timestamp");
 			jsonw_uint(json_wtr, e->time);
@@ -89,7 +101,7 @@ print_bpf_output(struct perf_event_header *event, void *private_data)
 		if (e->header.type == PERF_RECORD_SAMPLE) {
 			printf("== @%lld.%09lld CPU: %d index: %d =====\n",
 			       e->time / 1000000000ULL, e->time % 1000000000ULL,
-			       ring->cpu, ring->key);
+			       cpu, idx);
 			fprint_hex(stdout, e->data, e->size, " ");
 			printf("\n");
 		} else if (e->header.type == PERF_RECORD_LOST) {
@@ -103,87 +115,25 @@ print_bpf_output(struct perf_event_header *event, void *private_data)
 	return LIBBPF_PERF_EVENT_CONT;
 }
 
-static void
-perf_event_read(struct event_ring_info *ring, void **buf, size_t *buf_len)
-{
-	enum bpf_perf_event_ret ret;
-
-	ret = bpf_perf_event_read_simple(ring->mem,
-					 MMAP_PAGE_CNT * get_page_size(),
-					 get_page_size(), buf, buf_len,
-					 print_bpf_output, ring);
-	if (ret != LIBBPF_PERF_EVENT_CONT) {
-		fprintf(stderr, "perf read loop failed with %d\n", ret);
-		stop = true;
-	}
-}
-
-static int perf_mmap_size(void)
-{
-	return get_page_size() * (MMAP_PAGE_CNT + 1);
-}
-
-static void *perf_event_mmap(int fd)
-{
-	int mmap_size = perf_mmap_size();
-	void *base;
-
-	base = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
-	if (base == MAP_FAILED) {
-		p_err("event mmap failed: %s\n", strerror(errno));
-		return NULL;
-	}
-
-	return base;
-}
-
-static void perf_event_unmap(void *mem)
-{
-	if (munmap(mem, perf_mmap_size()))
-		fprintf(stderr, "Can't unmap ring memory!\n");
-}
-
-static int bpf_perf_event_open(int map_fd, int key, int cpu)
+int do_event_pipe(int argc, char **argv)
 {
-	struct perf_event_attr attr = {
+	struct perf_event_attr perf_attr = {
 		.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_TIME,
 		.type = PERF_TYPE_SOFTWARE,
 		.config = PERF_COUNT_SW_BPF_OUTPUT,
+		.sample_period = 1,
+		.wakeup_events = 1,
 	};
-	int pmu_fd;
-
-	pmu_fd = sys_perf_event_open(&attr, -1, cpu, -1, 0);
-	if (pmu_fd < 0) {
-		p_err("failed to open perf event %d for CPU %d", key, cpu);
-		return -1;
-	}
-
-	if (bpf_map_update_elem(map_fd, &key, &pmu_fd, BPF_ANY)) {
-		p_err("failed to update map for event %d for CPU %d", key, cpu);
-		goto err_close;
-	}
-	if (ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0)) {
-		p_err("failed to enable event %d for CPU %d", key, cpu);
-		goto err_close;
-	}
-
-	return pmu_fd;
-
-err_close:
-	close(pmu_fd);
-	return -1;
-}
-
-int do_event_pipe(int argc, char **argv)
-{
-	int i, nfds, map_fd, index = -1, cpu = -1;
 	struct bpf_map_info map_info = {};
-	struct event_ring_info *rings;
-	size_t tmp_buf_sz = 0;
-	void *tmp_buf = NULL;
-	struct pollfd *pfds;
+	struct perf_buffer_raw_opts opts = {};
+	struct event_pipe_ctx ctx = {
+		.all_cpus = true,
+		.cpu = -1,
+		.idx = -1,
+	};
+	struct perf_buffer *pb;
 	__u32 map_info_len;
-	bool do_all = true;
+	int err, map_fd;
 
 	map_info_len = sizeof(map_info);
 	map_fd = map_parse_fd_and_info(&argc, &argv, &map_info, &map_info_len);
@@ -205,7 +155,7 @@ int do_event_pipe(int argc, char **argv)
 			char *endptr;
 
 			NEXT_ARG();
-			cpu = strtoul(*argv, &endptr, 0);
+			ctx.cpu = strtoul(*argv, &endptr, 0);
 			if (*endptr) {
 				p_err("can't parse %s as CPU ID", **argv);
 				goto err_close_map;
@@ -216,7 +166,7 @@ int do_event_pipe(int argc, char **argv)
 			char *endptr;
 
 			NEXT_ARG();
-			index = strtoul(*argv, &endptr, 0);
+			ctx.idx = strtoul(*argv, &endptr, 0);
 			if (*endptr) {
 				p_err("can't parse %s as index", **argv);
 				goto err_close_map;
@@ -228,45 +178,32 @@ int do_event_pipe(int argc, char **argv)
 			goto err_close_map;
 		}
 
-		do_all = false;
+		ctx.all_cpus = false;
 	}
 
-	if (!do_all) {
-		if (index == -1 || cpu == -1) {
+	if (!ctx.all_cpus) {
+		if (ctx.idx == -1 || ctx.cpu == -1) {
 			p_err("cpu and index must be specified together");
 			goto err_close_map;
 		}
-
-		nfds = 1;
 	} else {
-		nfds = min(get_possible_cpus(), map_info.max_entries);
-		cpu = 0;
-		index = 0;
+		ctx.cpu = 0;
+		ctx.idx = 0;
 	}
 
-	rings = calloc(nfds, sizeof(rings[0]));
-	if (!rings)
+	opts.attr = &perf_attr;
+	opts.event_cb = print_bpf_output;
+	opts.ctx = &ctx;
+	opts.cpu_cnt = ctx.all_cpus ? 0 : 1;
+	opts.cpus = &ctx.cpu;
+	opts.map_keys = &ctx.idx;
+
+	pb = perf_buffer__new_raw(map_fd, MMAP_PAGE_CNT, &opts);
+	err = libbpf_get_error(pb);
+	if (err) {
+		p_err("failed to create perf buffer: %s (%d)",
+		      strerror(err), err);
 		goto err_close_map;
-
-	pfds = calloc(nfds, sizeof(pfds[0]));
-	if (!pfds)
-		goto err_free_rings;
-
-	for (i = 0; i < nfds; i++) {
-		rings[i].cpu = cpu + i;
-		rings[i].key = index + i;
-
-		rings[i].fd = bpf_perf_event_open(map_fd, rings[i].key,
-						  rings[i].cpu);
-		if (rings[i].fd < 0)
-			goto err_close_fds_prev;
-
-		rings[i].mem = perf_event_mmap(rings[i].fd);
-		if (!rings[i].mem)
-			goto err_close_fds_current;
-
-		pfds[i].fd = rings[i].fd;
-		pfds[i].events = POLLIN;
 	}
 
 	signal(SIGINT, int_exit);
@@ -277,34 +214,24 @@ int do_event_pipe(int argc, char **argv)
 		jsonw_start_array(json_wtr);
 
 	while (!stop) {
-		poll(pfds, nfds, 200);
-		for (i = 0; i < nfds; i++)
-			perf_event_read(&rings[i], &tmp_buf, &tmp_buf_sz);
+		err = perf_buffer__poll(pb, 200);
+		if (err < 0 && err != -EINTR) {
+			p_err("perf buffer polling failed: %s (%d)",
+			      strerror(err), err);
+			goto err_close_pb;
+		}
 	}
-	free(tmp_buf);
 
 	if (json_output)
 		jsonw_end_array(json_wtr);
 
-	for (i = 0; i < nfds; i++) {
-		perf_event_unmap(rings[i].mem);
-		close(rings[i].fd);
-	}
-	free(pfds);
-	free(rings);
+	perf_buffer__free(pb);
 	close(map_fd);
 
 	return 0;
 
-err_close_fds_prev:
-	while (i--) {
-		perf_event_unmap(rings[i].mem);
-err_close_fds_current:
-		close(rings[i].fd);
-	}
-	free(pfds);
-err_free_rings:
-	free(rings);
+err_close_pb:
+	perf_buffer__free(pb);
 err_close_map:
 	close(map_fd);
 	return -1;
-- 
2.17.1


^ permalink raw reply related

* Re: Kernel BUG: epoll_wait() (and epoll_pwait) stall for 206 ms per call on sockets with a small-ish snd/rcv buffer.
From: Carlo Wood @ 2019-07-06 18:19 UTC (permalink / raw)
  To: davem, netdev
In-Reply-To: <20190706181657.7ff57395@hikaru>

While investigating this further, I read on
http://www.masterraghu.com/subjects/np/introduction/unix_network_programming_v1.3/ch07lev1sec5.html
under "SO_RCVBUF and SO_SNDBUF Socket Options":

    When setting the size of the TCP socket receive buffer, the
    ordering of the function calls is important. This is because of
    TCP's window scale option (Section 2.6), which is exchanged with
    the peer on the SYN segments when the connection is established.
    For a client, this means the SO_RCVBUF socket option must be set
    before calling connect. For a server, this means the socket option
    must be set for the listening socket before calling listen. Setting
    this option for the connected socket will have no effect whatsoever
    on the possible window scale option because accept does not return
    with the connected socket until TCP's three-way handshake is
    complete. That is why this option must be set for the listening
    socket. (The sizes of the socket buffers are always inherited from
    the listening socket by the newly created connected socket: pp.
    462–463 of TCPv2.)

As mentioned in a previous post, I had already discovered about
needing to set the socket buffers before connect, but I didn't know
about setting them before the call to listen() in order to get the
buffer sizes inherited by the accepted sockets.

After fixing this in my test program, all problems disappeared when
keeping the send and receive buffers the same on both sides.

However, when only setting the send and receive buffers on the client
socket (not on the (accepted or) listen socket), epoll_wait() still
stalls 43ms. When the SO_SNDBUF is smaller than 33182 bytes.

Here is the latest version of my test program:

https://github.com/CarloWood/ai-evio-testsuite/blob/master/src/epoll_bug.c

I have to retract most of my "bug" report, it might even not really be
a bug then... but nevertheless, what remains strange is the fact
that setting the socket buffer sizes on the accepted sockets can lead
to so much crippling effect, while the quoted website states:

    Setting this option for the connected socket will have no effect
    whatsoever on the possible window scale option because accept does
    not return with the connected socket until TCP's three-way
    handshake is complete.

And when only setting the socket buffer sizes for the client socket
(that I use to send back received data; so this is the sending
side now) then why does epoll_wait() stall 43 ms per call when the
receiving side is using the default (much larger) socket buffer sizes?

That 43 ms is STILL crippling-- slowing down the transmission of the
data to a trickling speed compared to what it should be.
 
-- 
Carlo Wood <carlo@alinoe.com>

^ permalink raw reply

* [PATCH net-next v4 0/4] devlink: Introduce PCI PF, VF ports and attributes
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
  To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190701122734.18770-1-parav@mellanox.com>

This patchset carry forwards the work initiated in [1] and discussion
futher concluded at [2].

To improve visibility of representor netdevice, its association with
PF or VF, physical port, two new devlink port flavours are added as
PCI PF and PCI VF ports.

A sample eswitch view can be seen below, which will be futher extended to
mdev subdevices of a PCI function in future.

Patch-1,2 extends devlink port attributes and port flavour.
Patch-3 extends mlx5 driver to register devlink ports for PF, VF and
physical link.

                                +---+      +---+
                              vf|   |      |   | pf
                                +-+-+      +-+-+
physical link <---------+         |          |
                        |         |          |
                        |         |          |
                      +-+-+     +-+-+      +-+-+
                      | 1 |     | 2 |      | 3 |
                   +--+---+-----+---+------+---+--+
                   |  physical   vf         pf    |
                   |  port       port       port  |
                   |                              |
                   |             eswitch          |
                   |                              |
                   +------------------------------+

[1] https://www.spinics.net/lists/netdev/msg555797.html
[2] https://marc.info/?l=linux-netdev&m=155354609408485&w=2

---
Changelog:
v3->v4:
 - Addressed comments from Jiri.
 - Split first patch to two patches.
 - Renamed phys_port to physical to be consistent with pci_pf.
 - Removed port_number from __devlink_port_attrs_set and moved
   assignment to caller function.
 - Used capital letter while moving old comment to new structure.
 - Removed helper function is_devlink_phy_port_num_supported().

v2->v3:
 - Made port_number and split_port_number applicable only to
   physical port flavours.
v1->v2:
 - Updated new APIs and mlx5 driver to drop port_number for PF, VF
   attributes
 - Updated port_number comment for its usage
 - Limited putting port_number to physical ports

Parav Pandit (4):
  devlink: Refactor physical port attributes
  devlink: Introduce PCI PF port flavour and port attribute
  devlink: Introduce PCI VF port flavour and port attribute
  net/mlx5e: Register devlink ports for physical link, PCI PF, VFs

 .../net/ethernet/mellanox/mlx5/core/en_rep.c  | 108 ++++++++++----
 .../net/ethernet/mellanox/mlx5/core/en_rep.h  |   1 +
 include/net/devlink.h                         |  31 +++-
 include/uapi/linux/devlink.h                  |  11 ++
 net/core/devlink.c                            | 135 +++++++++++++++---
 5 files changed, 233 insertions(+), 53 deletions(-)

-- 
2.19.2


^ permalink raw reply

* [PATCH net-next v4 1/4] devlink: Refactor physical port attributes
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
  To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>

To support additional devlink port flavours and to support few common
and few different port attributes, make following changes.

1. Move physical port attributes to a different structure
2. Return such attritubes in netlink response only for physical ports
(PHYSICAL, CPU and DSA)

Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 include/net/devlink.h | 13 +++++++--
 net/core/devlink.c    | 63 +++++++++++++++++++++++++++++--------------
 2 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6625ea068d5e..c79a1370867a 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -38,14 +38,23 @@ struct devlink {
 	char priv[0] __aligned(NETDEV_ALIGN);
 };
 
+struct devlink_port_phys_attrs {
+	u32 port_number; /* Same value as "split group".
+			  * A physical port which is visible to the user
+			  * for a given port flavour.
+			  */
+	u32 split_subport_number;
+};
+
 struct devlink_port_attrs {
 	u8 set:1,
 	   split:1,
 	   switch_port:1;
 	enum devlink_port_flavour flavour;
-	u32 port_number; /* same value as "split group" */
-	u32 split_subport_number;
 	struct netdev_phys_item_id switch_id;
+	union {
+		struct devlink_port_phys_attrs physical;
+	};
 };
 
 struct devlink_port {
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 89c533778135..db6fa6bb9b33 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -515,14 +515,20 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		return 0;
 	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
 		return -EMSGSIZE;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
+	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
+	    devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
+	    devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA)
+		return 0;
+	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
+			attrs->physical.port_number))
 		return -EMSGSIZE;
 	if (!attrs->split)
 		return 0;
-	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
+	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
+			attrs->physical.port_number))
 		return -EMSGSIZE;
 	if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
-			attrs->split_subport_number))
+			attrs->physical.split_subport_number))
 		return -EMSGSIZE;
 	return 0;
 }
@@ -5738,6 +5744,29 @@ void devlink_port_type_clear(struct devlink_port *devlink_port)
 }
 EXPORT_SYMBOL_GPL(devlink_port_type_clear);
 
+static int __devlink_port_attrs_set(struct devlink_port *devlink_port,
+				    enum devlink_port_flavour flavour,
+				    const unsigned char *switch_id,
+				    unsigned char switch_id_len)
+{
+	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+
+	if (WARN_ON(devlink_port->registered))
+		return -EEXIST;
+	attrs->set = true;
+	attrs->flavour = flavour;
+	if (switch_id) {
+		attrs->switch_port = true;
+		if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
+			switch_id_len = MAX_PHYS_ITEM_ID_LEN;
+		memcpy(attrs->switch_id.id, switch_id, switch_id_len);
+		attrs->switch_id.id_len = switch_id_len;
+	} else {
+		attrs->switch_port = false;
+	}
+	return 0;
+}
+
 /**
  *	devlink_port_attrs_set - Set port attributes
  *
@@ -5760,23 +5789,15 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    unsigned char switch_id_len)
 {
 	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+	int ret;
 
-	if (WARN_ON(devlink_port->registered))
+	ret = __devlink_port_attrs_set(devlink_port, flavour,
+				       switch_id, switch_id_len);
+	if (ret)
 		return;
-	attrs->set = true;
-	attrs->flavour = flavour;
-	attrs->port_number = port_number;
 	attrs->split = split;
-	attrs->split_subport_number = split_subport_number;
-	if (switch_id) {
-		attrs->switch_port = true;
-		if (WARN_ON(switch_id_len > MAX_PHYS_ITEM_ID_LEN))
-			switch_id_len = MAX_PHYS_ITEM_ID_LEN;
-		memcpy(attrs->switch_id.id, switch_id, switch_id_len);
-		attrs->switch_id.id_len = switch_id_len;
-	} else {
-		attrs->switch_port = false;
-	}
+	attrs->physical.port_number = port_number;
+	attrs->physical.split_subport_number = split_subport_number;
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
 
@@ -5792,10 +5813,12 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 	switch (attrs->flavour) {
 	case DEVLINK_PORT_FLAVOUR_PHYSICAL:
 		if (!attrs->split)
-			n = snprintf(name, len, "p%u", attrs->port_number);
+			n = snprintf(name, len, "p%u",
+				     attrs->physical.port_number);
 		else
-			n = snprintf(name, len, "p%us%u", attrs->port_number,
-				     attrs->split_subport_number);
+			n = snprintf(name, len, "p%us%u",
+				     attrs->physical.port_number,
+				     attrs->physical.split_subport_number);
 		break;
 	case DEVLINK_PORT_FLAVOUR_CPU:
 	case DEVLINK_PORT_FLAVOUR_DSA:
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next v4 2/4] devlink: Introduce PCI PF port flavour and port attribute
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
  To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>

In an eswitch, PCI PF may have port which is normally represented
using a representor netdevice.
To have better visibility of eswitch port, its association with
PF and a representor netdevice, introduce a PCI PF port
flavour and port attriute.

When devlink port flavour is PCI PF, fill up PCI PF attributes of the
port.

Extend port name creation using PCI PF number on best effort basis.
So that vendor drivers can skip defining their own scheme.

$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0

Signed-off-by: Parav Pandit <parav@mellanox.com>
---
Changelog:
v3->v4:
 - Addressed comments from Jiri.
 - Renamed phys_port to physical to be consistent with pci_pf.
 - Removed port_number from __devlink_port_attrs_set and moved
   assigment to caller function.
 - Used capital letter while moving old comment to new structure.
 - Removed helper function is_devlink_phy_port_num_supported().
v2->v3:
 - Address comments from Jakub.
 - Made port_number and split_port_number applicable only to
   physical port flavours by having in union.
v1->v2:
 - Limited port_num attribute to physical ports
 - Updated PCI PF attribute set API to not have port_number
---
 include/net/devlink.h        |  8 ++++++++
 include/uapi/linux/devlink.h |  5 +++++
 net/core/devlink.c           | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index c79a1370867a..2a8eaaff3d4b 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -46,6 +46,10 @@ struct devlink_port_phys_attrs {
 	u32 split_subport_number;
 };
 
+struct devlink_port_pci_pf_attrs {
+	u16 pf;	/* Associated PCI PF for this port. */
+};
+
 struct devlink_port_attrs {
 	u8 set:1,
 	   split:1,
@@ -54,6 +58,7 @@ struct devlink_port_attrs {
 	struct netdev_phys_item_id switch_id;
 	union {
 		struct devlink_port_phys_attrs physical;
+		struct devlink_port_pci_pf_attrs pci_pf;
 	};
 };
 
@@ -599,6 +604,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 			    u32 split_subport_number,
 			    const unsigned char *switch_id,
 			    unsigned char switch_id_len);
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len, u16 pf);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5287b42c181f..f7323884c3fe 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -169,6 +169,10 @@ enum devlink_port_flavour {
 	DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
 				   * interconnect port.
 				   */
+	DEVLINK_PORT_FLAVOUR_PCI_PF, /* Represents eswitch port for
+				      * the PCI PF. It is an internal
+				      * port that faces the PCI PF.
+				      */
 };
 
 enum devlink_param_cmode {
@@ -337,6 +341,7 @@ enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,	/* u64 */
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,	/* u64 */
 
+	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index db6fa6bb9b33..3717eae8a502 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -515,6 +515,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		return 0;
 	if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
 		return -EMSGSIZE;
+	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
+		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
+				attrs->pci_pf.pf))
+			return -EMSGSIZE;
+	}
 	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
 	    devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
 	    devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_DSA)
@@ -5801,6 +5806,32 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
 
+/**
+ *	devlink_port_attrs_pci_pf_set - Set PCI PF port attributes
+ *
+ *	@devlink_port: devlink port
+ *	@pf: associated PF for the devlink port instance
+ *	@switch_id: if the port is part of switch, this is buffer with ID,
+ *	            otwerwise this is NULL
+ *	@switch_id_len: length of the switch_id buffer
+ */
+void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len, u16 pf)
+{
+	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+	int ret;
+
+	ret = __devlink_port_attrs_set(devlink_port,
+				       DEVLINK_PORT_FLAVOUR_PCI_PF,
+				       switch_id, switch_id_len);
+	if (ret)
+		return;
+
+	attrs->pci_pf.pf = pf;
+}
+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
+
 static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 					     char *name, size_t len)
 {
@@ -5827,6 +5858,9 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 		 */
 		WARN_ON(1);
 		return -EINVAL;
+	case DEVLINK_PORT_FLAVOUR_PCI_PF:
+		n = snprintf(name, len, "pf%u", attrs->pci_pf.pf);
+		break;
 	}
 
 	if (n >= len)
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next v4 3/4] devlink: Introduce PCI VF port flavour and port attribute
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
  To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>

In an eswitch, PCI VF may have port which is normally represented using
a representor netdevice.
To have better visibility of eswitch port, its association with VF,
and its representor netdevice, introduce a PCI VF port flavour.

When devlink port flavour is PCI VF, fill up PCI VF attributes of
the port.

Extend port name creation using PCI PF and VF number scheme on best
effort basis, so that vendor drivers can skip defining their own scheme.

$ devlink port show
pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1

Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 include/net/devlink.h        | 10 ++++++++++
 include/uapi/linux/devlink.h |  6 ++++++
 net/core/devlink.c           | 38 ++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 2a8eaaff3d4b..a02f639ad519 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -50,6 +50,11 @@ struct devlink_port_pci_pf_attrs {
 	u16 pf;	/* Associated PCI PF for this port. */
 };
 
+struct devlink_port_pci_vf_attrs {
+	u16 pf;	/* Associated PCI PF for this port. */
+	u16 vf;	/* Associated PCI VF for of the PCI PF for this port. */
+};
+
 struct devlink_port_attrs {
 	u8 set:1,
 	   split:1,
@@ -59,6 +64,7 @@ struct devlink_port_attrs {
 	union {
 		struct devlink_port_phys_attrs physical;
 		struct devlink_port_pci_pf_attrs pci_pf;
+		struct devlink_port_pci_vf_attrs pci_vf;
 	};
 };
 
@@ -607,6 +613,10 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
 void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
 				   const unsigned char *switch_id,
 				   unsigned char switch_id_len, u16 pf);
+void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len,
+				   u16 pf, u16 vf);
 int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
 			u32 size, u16 ingress_pools_count,
 			u16 egress_pools_count, u16 ingress_tc_count,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index f7323884c3fe..ffc993256527 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -173,6 +173,10 @@ enum devlink_port_flavour {
 				      * the PCI PF. It is an internal
 				      * port that faces the PCI PF.
 				      */
+	DEVLINK_PORT_FLAVOUR_PCI_VF, /* Represents eswitch port
+				      * for the PCI VF. It is an internal
+				      * port that faces the PCI VF.
+				      */
 };
 
 enum devlink_param_cmode {
@@ -342,6 +346,8 @@ enum devlink_attr {
 	DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,	/* u64 */
 
 	DEVLINK_ATTR_PORT_PCI_PF_NUMBER,	/* u16 */
+	DEVLINK_ATTR_PORT_PCI_VF_NUMBER,	/* u16 */
+
 	/* add new attributes above here, update the policy in devlink.c */
 
 	__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3717eae8a502..5a1887c1b3c5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -519,6 +519,12 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
 		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
 				attrs->pci_pf.pf))
 			return -EMSGSIZE;
+	} else if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+		if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
+				attrs->pci_vf.pf) ||
+		    nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_VF_NUMBER,
+				attrs->pci_vf.vf))
+			return -EMSGSIZE;
 	}
 	if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PHYSICAL ||
 	    devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_CPU ||
@@ -5832,6 +5838,34 @@ void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
 }
 EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_pf_set);
 
+/**
+ *	devlink_port_attrs_pci_vf_set - Set PCI VF port attributes
+ *
+ *	@devlink_port: devlink port
+ *	@pf: associated PF for the devlink port instance
+ *	@vf: associated VF of a PF for the devlink port instance
+ *	@switch_id: if the port is part of switch, this is buffer with ID,
+ *	            otwerwise this is NULL
+ *	@switch_id_len: length of the switch_id buffer
+ */
+void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
+				   const unsigned char *switch_id,
+				   unsigned char switch_id_len,
+				   u16 pf, u16 vf)
+{
+	struct devlink_port_attrs *attrs = &devlink_port->attrs;
+	int ret;
+
+	ret = __devlink_port_attrs_set(devlink_port,
+				       DEVLINK_PORT_FLAVOUR_PCI_VF,
+				       switch_id, switch_id_len);
+	if (ret)
+		return;
+	attrs->pci_vf.pf = pf;
+	attrs->pci_vf.vf = vf;
+}
+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_vf_set);
+
 static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 					     char *name, size_t len)
 {
@@ -5861,6 +5895,10 @@ static int __devlink_port_phys_port_name_get(struct devlink_port *devlink_port,
 	case DEVLINK_PORT_FLAVOUR_PCI_PF:
 		n = snprintf(name, len, "pf%u", attrs->pci_pf.pf);
 		break;
+	case DEVLINK_PORT_FLAVOUR_PCI_VF:
+		n = snprintf(name, len, "pf%uvf%u",
+			     attrs->pci_vf.pf, attrs->pci_vf.vf);
+		break;
 	}
 
 	if (n >= len)
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next v4 4/4] net/mlx5e: Register devlink ports for physical link, PCI PF, VFs
From: Parav Pandit @ 2019-07-06 18:23 UTC (permalink / raw)
  To: netdev; +Cc: jiri, saeedm, jakub.kicinski, Parav Pandit
In-Reply-To: <20190706182350.11929-1-parav@mellanox.com>

Register devlink port of physical port, PCI PF and PCI VF flavour
for each PF, VF when a given devlink instance is in switchdev mode.

Implement ndo_get_devlink_port callback API to make use of registered
devlink ports.
This eliminates ndo_get_phys_port_name() and ndo_get_port_parent_id()
callbacks. Hence, remove them.

An example output with 2 VFs, without a PF and single uplink port is
below.

$devlink port show
pci/0000:06:00.0/65535: type eth netdev ens2f0 flavour physical
pci/0000:05:00.0/1: type eth netdev eth1 flavour pcivf pfnum 0 vfnum 0
pci/0000:05:00.0/2: type eth netdev eth2 flavour pcivf pfnum 0 vfnum 1

Reviewed-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  | 108 +++++++++++++-----
 .../net/ethernet/mellanox/mlx5/core/en_rep.h  |   1 +
 2 files changed, 78 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 529f8e4b32c6..6810b9fa0705 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -37,6 +37,7 @@
 #include <net/act_api.h>
 #include <net/netevent.h>
 #include <net/arp.h>
+#include <net/devlink.h>
 
 #include "eswitch.h"
 #include "en.h"
@@ -1119,32 +1120,6 @@ static int mlx5e_rep_close(struct net_device *dev)
 	return ret;
 }
 
-static int mlx5e_rep_get_phys_port_name(struct net_device *dev,
-					char *buf, size_t len)
-{
-	struct mlx5e_priv *priv = netdev_priv(dev);
-	struct mlx5e_rep_priv *rpriv = priv->ppriv;
-	struct mlx5_eswitch_rep *rep = rpriv->rep;
-	unsigned int fn;
-	int ret;
-
-	fn = PCI_FUNC(priv->mdev->pdev->devfn);
-	if (fn >= MLX5_MAX_PORTS)
-		return -EOPNOTSUPP;
-
-	if (rep->vport == MLX5_VPORT_UPLINK)
-		ret = snprintf(buf, len, "p%d", fn);
-	else if (rep->vport == MLX5_VPORT_PF)
-		ret = snprintf(buf, len, "pf%d", fn);
-	else
-		ret = snprintf(buf, len, "pf%dvf%d", fn, rep->vport - 1);
-
-	if (ret >= len)
-		return -EOPNOTSUPP;
-
-	return 0;
-}
-
 static int
 mlx5e_rep_setup_tc_cls_flower(struct mlx5e_priv *priv,
 			      struct tc_cls_flower_offload *cls_flower, int flags)
@@ -1298,17 +1273,24 @@ static int mlx5e_uplink_rep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan
 	return 0;
 }
 
+static struct devlink_port *mlx5e_get_devlink_port(struct net_device *dev)
+{
+	struct mlx5e_priv *priv = netdev_priv(dev);
+	struct mlx5e_rep_priv *rpriv = priv->ppriv;
+
+	return &rpriv->dl_port;
+}
+
 static const struct net_device_ops mlx5e_netdev_ops_rep = {
 	.ndo_open                = mlx5e_rep_open,
 	.ndo_stop                = mlx5e_rep_close,
 	.ndo_start_xmit          = mlx5e_xmit,
-	.ndo_get_phys_port_name  = mlx5e_rep_get_phys_port_name,
 	.ndo_setup_tc            = mlx5e_rep_setup_tc,
+	.ndo_get_devlink_port = mlx5e_get_devlink_port,
 	.ndo_get_stats64         = mlx5e_rep_get_stats,
 	.ndo_has_offload_stats	 = mlx5e_rep_has_offload_stats,
 	.ndo_get_offload_stats	 = mlx5e_rep_get_offload_stats,
 	.ndo_change_mtu          = mlx5e_rep_change_mtu,
-	.ndo_get_port_parent_id	 = mlx5e_rep_get_port_parent_id,
 };
 
 static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
@@ -1316,8 +1298,8 @@ static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
 	.ndo_stop                = mlx5e_close,
 	.ndo_start_xmit          = mlx5e_xmit,
 	.ndo_set_mac_address     = mlx5e_uplink_rep_set_mac,
-	.ndo_get_phys_port_name  = mlx5e_rep_get_phys_port_name,
 	.ndo_setup_tc            = mlx5e_rep_setup_tc,
+	.ndo_get_devlink_port = mlx5e_get_devlink_port,
 	.ndo_get_stats64         = mlx5e_get_stats,
 	.ndo_has_offload_stats	 = mlx5e_rep_has_offload_stats,
 	.ndo_get_offload_stats	 = mlx5e_rep_get_offload_stats,
@@ -1330,7 +1312,6 @@ static const struct net_device_ops mlx5e_netdev_ops_uplink_rep = {
 	.ndo_get_vf_config       = mlx5e_get_vf_config,
 	.ndo_get_vf_stats        = mlx5e_get_vf_stats,
 	.ndo_set_vf_vlan         = mlx5e_uplink_rep_set_vf_vlan,
-	.ndo_get_port_parent_id	 = mlx5e_rep_get_port_parent_id,
 	.ndo_set_features        = mlx5e_set_features,
 };
 
@@ -1731,6 +1712,55 @@ static const struct mlx5e_profile mlx5e_uplink_rep_profile = {
 	.max_tc			= MLX5E_MAX_NUM_TC,
 };
 
+static bool
+is_devlink_port_supported(const struct mlx5_core_dev *dev,
+			  const struct mlx5e_rep_priv *rpriv)
+{
+	return rpriv->rep->vport == MLX5_VPORT_UPLINK ||
+	       rpriv->rep->vport == MLX5_VPORT_PF ||
+	       mlx5_eswitch_is_vf_vport(dev->priv.eswitch, rpriv->rep->vport);
+}
+
+static int register_devlink_port(struct mlx5_core_dev *dev,
+				 struct mlx5e_rep_priv *rpriv)
+{
+	struct devlink *devlink = priv_to_devlink(dev);
+	struct mlx5_eswitch_rep *rep = rpriv->rep;
+	struct netdev_phys_item_id ppid = {};
+	int ret;
+
+	if (!is_devlink_port_supported(dev, rpriv))
+		return 0;
+
+	ret = mlx5e_rep_get_port_parent_id(rpriv->netdev, &ppid);
+	if (ret)
+		return ret;
+
+	if (rep->vport == MLX5_VPORT_UPLINK)
+		devlink_port_attrs_set(&rpriv->dl_port,
+				       DEVLINK_PORT_FLAVOUR_PHYSICAL,
+				       PCI_FUNC(dev->pdev->devfn), false, 0,
+				       &ppid.id[0], ppid.id_len);
+	else if (rep->vport == MLX5_VPORT_PF)
+		devlink_port_attrs_pci_pf_set(&rpriv->dl_port,
+					      &ppid.id[0], ppid.id_len,
+					      dev->pdev->devfn);
+	else if (mlx5_eswitch_is_vf_vport(dev->priv.eswitch, rpriv->rep->vport))
+		devlink_port_attrs_pci_vf_set(&rpriv->dl_port,
+					      &ppid.id[0], ppid.id_len,
+					      dev->pdev->devfn,
+					      rep->vport - 1);
+
+	return devlink_port_register(devlink, &rpriv->dl_port, rep->vport);
+}
+
+static void unregister_devlink_port(struct mlx5_core_dev *dev,
+				    struct mlx5e_rep_priv *rpriv)
+{
+	if (is_devlink_port_supported(dev, rpriv))
+		devlink_port_unregister(&rpriv->dl_port);
+}
+
 /* e-Switch vport representors */
 static int
 mlx5e_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
@@ -1782,15 +1812,27 @@ mlx5e_vport_rep_load(struct mlx5_core_dev *dev, struct mlx5_eswitch_rep *rep)
 		goto err_detach_netdev;
 	}
 
+	err = register_devlink_port(dev, rpriv);
+	if (err) {
+		esw_warn(dev, "Failed to register devlink port %d\n",
+			 rep->vport);
+		goto err_neigh_cleanup;
+	}
+
 	err = register_netdev(netdev);
 	if (err) {
 		pr_warn("Failed to register representor netdev for vport %d\n",
 			rep->vport);
-		goto err_neigh_cleanup;
+		goto err_devlink_cleanup;
 	}
 
+	if (is_devlink_port_supported(dev, rpriv))
+		devlink_port_type_eth_set(&rpriv->dl_port, netdev);
 	return 0;
 
+err_devlink_cleanup:
+	unregister_devlink_port(dev, rpriv);
+
 err_neigh_cleanup:
 	mlx5e_rep_neigh_cleanup(rpriv);
 
@@ -1813,9 +1855,13 @@ mlx5e_vport_rep_unload(struct mlx5_eswitch_rep *rep)
 	struct mlx5e_rep_priv *rpriv = mlx5e_rep_to_rep_priv(rep);
 	struct net_device *netdev = rpriv->netdev;
 	struct mlx5e_priv *priv = netdev_priv(netdev);
+	struct mlx5_core_dev *dev = priv->mdev;
 	void *ppriv = priv->ppriv;
 
+	if (is_devlink_port_supported(dev, rpriv))
+		devlink_port_type_clear(&rpriv->dl_port);
 	unregister_netdev(netdev);
+	unregister_devlink_port(dev, rpriv);
 	mlx5e_rep_neigh_cleanup(rpriv);
 	mlx5e_detach_netdev(priv);
 	if (rep->vport == MLX5_VPORT_UPLINK)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index d4585f3b8cb2..c56e6ee4350c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -86,6 +86,7 @@ struct mlx5e_rep_priv {
 	struct mlx5_flow_handle *vport_rx_rule;
 	struct list_head       vport_sqs_list;
 	struct mlx5_rep_uplink_priv uplink_priv; /* valid for uplink rep */
+	struct devlink_port dl_port;
 };
 
 static inline
-- 
2.19.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox