Netdev List
 help / color / mirror / Atom feed
* [PATCH 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready
From: josua @ 2019-07-06 15:19 UTC (permalink / raw)
  To: netdev; +Cc: Josua Mayer, David S. Miller
In-Reply-To: <20190706151900.14355-1-josua@solid-run.com>

From: Josua Mayer <josua@solid-run.com>

Defer probing of the orion-mdio interface when enabling of either of the
clocks defer probing. This avoids locking up the Armada 8k SoC when mdio
is used before all clocks have been enabled.

Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 89a99bf8e87b..1034013426ad 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -321,6 +321,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
 
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
+		if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
+			ret = -EPROBE_DEFER;
+			goto out_clk;
+		}
 		if (IS_ERR(dev->clk[i]))
 			break;
 		clk_prepare_enable(dev->clk[i]);
@@ -366,6 +370,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	if (dev->err_interrupt > 0)
 		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
 
+out_clk:
 	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
 		if (IS_ERR(dev->clk[i]))
 			break;
-- 
2.16.4


^ permalink raw reply related

* [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio
From: josua @ 2019-07-06 15:18 UTC (permalink / raw)
  To: netdev; +Cc: Josua Mayer, stable, David S. Miller, Rob Herring, Mark Rutland
In-Reply-To: <20190706151900.14355-1-josua@solid-run.com>

From: Josua Mayer <josua@solid-run.com>

Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
Update the binding to allow the extra clock to be specified.

Cc: stable@vger.kernel.org
Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
index 42cd81090a2c..3f3cfc1d8d4d 100644
--- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -16,7 +16,7 @@ Required properties:
 
 Optional properties:
 - interrupts: interrupt line number for the SMI error/done interrupt
-- clocks: phandle for up to three required clocks for the MDIO instance
+- clocks: phandle for up to four required clocks for the MDIO instance
 
 The child nodes of the MDIO driver are the individual PHY devices
 connected to this MDIO bus. They must have a "reg" property given the
-- 
2.16.4


^ permalink raw reply related

* [PATCH 2/4] net: mvmdio: allow up to four clocks to be specified for orion-mdio
From: josua @ 2019-07-06 15:18 UTC (permalink / raw)
  To: netdev; +Cc: Josua Mayer, stable, David S. Miller
In-Reply-To: <20190706151900.14355-1-josua@solid-run.com>

From: Josua Mayer <josua@solid-run.com>

Allow up to four clocks to be specified and enabled for the orion-mdio
interface, which are required by the Armada 8k and defined in
armada-cp110.dtsi.

Fixes a hang in probing the mvmdio driver that was encountered on the
Clearfog GT 8K with all drivers built as modules, but also affects other
boards such as the MacchiatoBIN.

Cc: stable@vger.kernel.org
Fixes: 96cb43423822 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index c5dac6bd2be4..e17d563e97a6 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -64,7 +64,7 @@
 
 struct orion_mdio_dev {
 	void __iomem *regs;
-	struct clk *clk[3];
+	struct clk *clk[4];
 	/*
 	 * If we have access to the error interrupt pin (which is
 	 * somewhat misnamed as it not only reflects internal errors
-- 
2.16.4


^ permalink raw reply related

* Re: [PATCH 1/4] dt-bindings: allow up to four clocks for orion-mdio
From: Andrew Lunn @ 2019-07-06 15:47 UTC (permalink / raw)
  To: josua; +Cc: netdev, stable, David S. Miller, Rob Herring, Mark Rutland
In-Reply-To: <20190706151900.14355-2-josua@solid-run.com>

On Sat, Jul 06, 2019 at 05:18:57PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Armada 8040 needs four clocks to be enabled for MDIO accesses to work.
> Update the binding to allow the extra clock to be specified.
> 
> Cc: stable@vger.kernel.org
> Fixes: 6d6a331f44a1 ("dt-bindings: allow up to three clocks for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 2/4] net: mvmdio: allow up to four clocks to be specified for orion-mdio
From: Andrew Lunn @ 2019-07-06 15:47 UTC (permalink / raw)
  To: josua; +Cc: netdev, stable, David S. Miller
In-Reply-To: <20190706151900.14355-3-josua@solid-run.com>

On Sat, Jul 06, 2019 at 05:18:58PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Allow up to four clocks to be specified and enabled for the orion-mdio
> interface, which are required by the Armada 8k and defined in
> armada-cp110.dtsi.
> 
> Fixes a hang in probing the mvmdio driver that was encountered on the
> Clearfog GT 8K with all drivers built as modules, but also affects other
> boards such as the MacchiatoBIN.
> 
> Cc: stable@vger.kernel.org
> Fixes: 96cb43423822 ("net: mvmdio: allow up to three clocks to be specified for orion-mdio")
> Signed-off-by: Josua Mayer <josua@solid-run.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH 4/4] net: mvmdio: defer probe of orion-mdio if a clock is not ready
From: Andrew Lunn @ 2019-07-06 15:54 UTC (permalink / raw)
  To: josua; +Cc: netdev, David S. Miller
In-Reply-To: <20190706151900.14355-5-josua@solid-run.com>

On Sat, Jul 06, 2019 at 05:19:00PM +0200, josua@solid-run.com wrote:
> From: Josua Mayer <josua@solid-run.com>
> 
> Defer probing of the orion-mdio interface when enabling of either of the
> clocks defer probing.

Hi Josua

I'm having trouble parsing that sentence.

How about:

Defer probing of the orion-mdio interface when getting a clock returns
EPROBE_DEFER.

Otherwise:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew




 This avoids locking up the Armada 8k SoC when mdio
> is used before all clocks have been enabled.
> 
> Signed-off-by: Josua Mayer <josua@solid-run.com>
> ---
>  drivers/net/ethernet/marvell/mvmdio.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
> index 89a99bf8e87b..1034013426ad 100644
> --- a/drivers/net/ethernet/marvell/mvmdio.c
> +++ b/drivers/net/ethernet/marvell/mvmdio.c
> @@ -321,6 +321,10 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>  		dev->clk[i] = of_clk_get(pdev->dev.of_node, i);
> +		if (dev->clk[i] == PTR_ERR(-EPROBE_DEFER)) {
> +			ret = -EPROBE_DEFER;
> +			goto out_clk;
> +		}
>  		if (IS_ERR(dev->clk[i]))
>  			break;
>  		clk_prepare_enable(dev->clk[i]);
> @@ -366,6 +370,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
>  	if (dev->err_interrupt > 0)
>  		writel(0, dev->regs + MVMDIO_ERR_INT_MASK);
>  
> +out_clk:
>  	for (i = 0; i < ARRAY_SIZE(dev->clk); i++) {
>  		if (IS_ERR(dev->clk[i]))
>  			break;
> -- 
> 2.16.4
> 

^ permalink raw reply

* Re: [PATCH v5 bpf-next 1/5] libbpf: add perf buffer API
From: Yonghong Song @ 2019-07-06 15:56 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Kernel Team, bpf@vger.kernel.org,
	netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net
In-Reply-To: <CAEf4BzZYAN5t+6Kkt+W4ee13PL7dR4FG8P71dFnk_CHWqMmHPQ@mail.gmail.com>



On 7/5/19 10:54 PM, Andrii Nakryiko wrote:
> On Fri, Jul 5, 2019 at 10:42 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 7/5/19 9:35 PM, Andrii Nakryiko wrote:
>>> 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>
>>> ---
>>>    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..72149d68b8c1 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 perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
>>> +{
>>> +     int cnt, err;
>>> +
>>> +     cnt = epoll_wait(pb->epoll_fd, pb->events, pb->cpu_cnt, timeout_ms);
>>> +     for (int i = 0; i < cnt; i++) {
>>
>> Find one compilation error here.
>>
>> libbpf.c: In function ‘perf_buffer__poll’:
>> libbpf.c:4728:2: error: ‘for’ loop initial declarations are only allowed
>> in C99 mode
>>     for (int i = 0; i < cnt; i++) {
>>     ^
>>
> 
> Ah... Fixing, thanks!. How did you compile? make -C tools/lib/bpf
> doesn't show this, should we update libbpf Makefile to catch stuff
> like this?

I did not make any code changes. My compiler is gcc 4.8.5. it is 
possible that old compiler less tolerant.

>>> +             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 */
>> [...]

^ permalink raw reply

* [PATCH 3/4] net: mvmdio: print warning when orion-mdio has too many clocks
From: josua @ 2019-07-06 15:18 UTC (permalink / raw)
  To: netdev; +Cc: Josua Mayer, David S. Miller
In-Reply-To: <20190706151900.14355-1-josua@solid-run.com>

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");
+
 	dev->err_interrupt = platform_get_irq(pdev, 0);
 	if (dev->err_interrupt > 0 &&
 	    resource_size(r) < MVMDIO_ERR_INT_MASK + 4) {
-- 
2.16.4


^ permalink raw reply related

* RE: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Saleem, Shiraz @ 2019-07-06 16:03 UTC (permalink / raw)
  To: Greg KH
  Cc: Jason Gunthorpe, Kirsher, Jeffrey T, davem@davemloft.net,
	dledford@redhat.com, Nguyen, Anthony L, netdev@vger.kernel.org,
	linux-rdma@vger.kernel.org, nhorman@redhat.com,
	sassmann@redhat.com, poswald@suse.com, Ismail, Mustafa,
	Ertman, David M, Bowers, AndrewX
In-Reply-To: <20190706082523.GA8727@kroah.com>

> Subject: Re: [net-next 1/3] ice: Initialize and register platform device to provide
> RDMA
> 
> On Fri, Jul 05, 2019 at 04:33:07PM +0000, Saleem, Shiraz wrote:
> > > Subject: Re: [net-next 1/3] ice: Initialize and register platform
> > > device to provide RDMA
> > >
> > > On Thu, Jul 04, 2019 at 12:48:29PM +0000, Jason Gunthorpe wrote:
> > > > On Thu, Jul 04, 2019 at 02:42:47PM +0200, Greg KH wrote:
> > > > > On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> > > > > > On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > > > > > > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > > > > > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > > > > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > > > > > > >
> > > > > > > > > The RDMA block does not advertise on the PCI bus or any other
> bus.
> > > > > > > > > Thus the ice driver needs to provide access to the RDMA
> > > > > > > > > hardware block via a virtual bus; utilize the platform
> > > > > > > > > bus to provide this
> > > access.
> > > > > > > > >
> > > > > > > > > This patch initializes the driver to support RDMA as
> > > > > > > > > well as creates and registers a platform device for the
> > > > > > > > > RDMA driver to register to. At this point the driver is
> > > > > > > > > fully initialized to register a platform driver,
> > > > > > > > > however, can not yet register as the ops have not been
> implemented.
> > > > > > > >
> > > > > > > > I think you need Greg's ack on all this driver stuff -
> > > > > > > > particularly that a platform_device is OK.
> > > > > > >
> > > > > > > A platform_device is almost NEVER ok.
> > > > > > >
> > > > > > > Don't abuse it, make a real device on a real bus.  If you
> > > > > > > don't have a real bus and just need to create a device to
> > > > > > > hang other things off of, then use the virtual one, that's what it is there
> for.
> > > > > >
> > > > > > Ideally I'd like to see all the RDMA drivers that connect to
> > > > > > ethernet drivers use some similar scheme.
> > > > >
> > > > > Why?  They should be attached to a "real" device, why make any up?
> > > >
> > > > ? A "real" device, like struct pci_device, can only bind to one
> > > > driver. How can we bind it concurrently to net, rdma, scsi, etc?
> > >
> > > MFD was designed for this very problem.
> > >
> > > > > > This is for a PCI device that plugs into multiple subsystems
> > > > > > in the kernel, ie it has net driver functionality, rdma
> > > > > > functionality, some even have SCSI functionality
> > > > >
> > > > > Sounds like a MFD device, why aren't you using that
> > > > > functionality instead?
> > > >
> > > > This was also my advice, but in another email Jeff says:
> > > >
> > > >   MFD architecture was also considered, and we selected the simpler
> > > >   platform model. Supporting a MFD architecture would require an
> > > >   additional MFD core driver, individual platform netdev, RDMA function
> > > >   drivers, and stripping a large portion of the netdev drivers into
> > > >   MFD core. The sub-devices registered by MFD core for function
> > > >   drivers are indeed platform devices.
> > >
> > > So, "mfd is too hard, let's abuse a platform device" is ok?
> > >
> > > People have been wanting to do MFD drivers for PCI devices for a
> > > long time, it's about time someone actually did the work for it, I
> > > bet it will not be all that complex if tiny embedded drivers can do
> > > it :)
> > >
> > Hi Greg - Thanks for your feedback!
> >
> > We currently have 2 PCI function netdev drivers in the kernel (i40e &
> > ice) that support devices (x722 & e810) which are RDMA capable. Our
> > objective is to add a single unified RDMA driver (as this a subsystem
> > specific requirement) which needs to access HW resources from the
> > netdev PF drivers. Attaching platform devices from the netdev drivers
> > to the platform bus and having a single RDMA platform driver bind to
> > them and access these resources seemed like a simple approach to realize our
> objective. But seems like attaching platform devices is wrong. I would like to
> understand why.
> 
> Because that is NOT what a platform device is for.
> 
> It was originally created for those types of devices that live on the "platform" bus,
> i.e. things that are hard-wired and you just "know" are in your system because they
> are located at specific locations.  We used to generate them from board files, and
> then when we got DT, we create them from the resources that DT says where the
> locations of the devices are.
> 
> They are NOT to be abused and used whenever someone wants to put them
> somewhere in the "middle" of the device tree because they feel like they are easier
> to use instead of creating a real bus and drivers.
> 
> Yes, they do get abused, and I need to sweep the tree again and fix up all of the
> places where this has crept back in.  But now that I know you are thinking of doing
> this, I'll keep saying to NOT do it for your use case either :)

Thanks Greg for the explanation.
And yes, we went by some example usages in the tree.
> 
> > Are platform sub devices only to be added from an MFD core driver? I
> > am also wondering if MFD arch.  would allow for realizing a single
> > RDMA driver and whether we need an MFD core driver for each device,
> > x722 & e810 or whether it can be a single driver.
> 
> I do not know the details of how MFD works, please ask those developers for
> specifics.  If MFD doesn't work, then create a tiny virtual bus and make sub-
> devices out of that.  If you need a "generic" way to do this for PCI devices, then
> please create that as you are not the only one that keeps wanting this, as for some
> reason PCI hardware vendors don't like dividing up their devices in ways that
> would have made it much simpler to create individual devices (probably saves
> some gates and firmware complexity on the device).
> 

Thank you for laying out options. We will review internally and get back.

Shiraz

^ permalink raw reply

* 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


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