Netdev List
 help / color / mirror / Atom feed
* Re: [Intel-wired-lan] [PATCH net-next v3 1/6] igb: clear out tstamp after sending the packet.
From: Brown, Aaron F @ 2019-06-21  1:50 UTC (permalink / raw)
  To: Patel, Vedang, netdev@vger.kernel.org
  Cc: jiri@resnulli.us, l@dorileo.org, jhs@mojatatu.com,
	intel-wired-lan@lists.osuosl.org, xiyou.wangcong@gmail.com,
	davem@davemloft.net
In-Reply-To: <1560799870-18956-2-git-send-email-vedang.patel@intel.com>

On Mon, 2019-06-17 at 12:31 -0700, Vedang Patel wrote:
> skb->tstamp is being used at multiple places. On the transmit side, it
> is used to determine the launchtime of the packet. It is also used to
> determine the software timestamp after the packet has been transmitted.
> 
> So, clear out the tstamp value after it has been read so that we do not
> report false software timestamp on the receive side.
> 
> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply

* Re: [PATCH 2/2] net: intel: igb: add RR2DCDELAY to ethtool registers dump
From: Brown, Aaron F @ 2019-06-21  1:47 UTC (permalink / raw)
  To: Artem Bityutskiy, David S. Miller
  Cc: Kirsher, Jeffrey T, Fujinaka, Todd,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
In-Reply-To: <20190618115513.99661-2-dedekind1@gmail.com>

On Tue, 2019-06-18 at 14:55 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> This patch adds the RR2DCDELAY register to the ethtool registers dump.
> RR2DCDELAY exists on I210 and I211 Intel Gigabit Ethernet chips and it stands
> for "Read Request To Data Completion Delay". Here is how this register is
> described in the I210 datasheet:
> 
> "This field captures the maximum PCIe split time in 16 ns units, which is the
> maximum delay between the read request to the first data completion. This is
> giving an estimation of the PCIe round trip time."
> 
> In other words, whenever I210 reads from the host memory (e.g., fetches a
> descriptor from the ring), the chip measures every PCI DMA read transaction and
> captures the maximum value. So it ends up containing the longest DMA
> transaction time.
> 
> This register is very useful for troubleshooting and research purposes. If you
> are dealing with time-sensitive networks, this register can help you get
> an idea of your "I210-to-ring" latency. This helps answering questions like
> "should I have PCIe ASPM enabled?" or "should I enable deep C-states?" on
> my system.
> 
> It is safe to read this register at any point, reading it has no effect on
> the I210 chip functionality.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/net/ethernet/intel/igb/e1000_regs.h  | 2 ++
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 5 ++++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply

* Re: [PATCH 1/2] net: intel: igb: minor ethool regdump amendment
From: Brown, Aaron F @ 2019-06-21  1:45 UTC (permalink / raw)
  To: Artem Bityutskiy, David S. Miller
  Cc: Kirsher, Jeffrey T, Fujinaka, Todd,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
In-Reply-To: <20190618115513.99661-1-dedekind1@gmail.com>

On Tue, 2019-06-18 at 14:55 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> 
> This patch has no functional impact and it is just a preparation
> for the following patch. It removes an early return from the
> 'igb_get_regs()' function by moving the 82576-only registers
> dump into an "if" block. With this preparation, we can dump more
> non-82576 registers at the end of this function.
> 
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 70 ++++++++++----------
>  1 file changed, 35 insertions(+), 35 deletions(-)
> 
Aside from teh missing "t" for ethtool in the subject that Andrew Lunn pointed out...

Tested-by: Aaron Brown <aaron.f.brown@intel.com>

^ permalink raw reply

* [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-21  1:19 UTC (permalink / raw)
  To: jmorris
  Cc: linux-security, linux-kernel, linux-api, David Howells,
	Alexei Starovoitov, Matthew Garrett, netdev, Chun-Yi Lee,
	Daniel Borkmann
In-Reply-To: <20190621011941.186255-1-matthewgarrett@google.com>

From: David Howells <dhowells@redhat.com>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program and kernel memory to be altered without
restriction. Disable them if the kernel has been locked down in
confidentiality mode.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
cc: netdev@vger.kernel.org
cc: Chun-Yi Lee <jlee@suse.com>
cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
 include/linux/security.h     |  1 +
 kernel/trace/bpf_trace.c     | 11 +++++++++++
 security/lockdown/lockdown.c |  1 +
 3 files changed, 13 insertions(+)

diff --git a/include/linux/security.h b/include/linux/security.h
index dae4aa83352c..8bf426cdd151 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -97,6 +97,7 @@ enum lockdown_reason {
 	LOCKDOWN_INTEGRITY_MAX,
 	LOCKDOWN_KCORE,
 	LOCKDOWN_KPROBES,
+	LOCKDOWN_BPF,
 	LOCKDOWN_CONFIDENTIALITY_MAX,
 };
 
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d64c00afceb5..6f57485df840 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -137,6 +137,9 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
 {
 	int ret;
 
+	if (security_is_locked_down(LOCKDOWN_BPF))
+		return -EINVAL;
+
 	ret = probe_kernel_read(dst, unsafe_ptr, size);
 	if (unlikely(ret < 0))
 		memset(dst, 0, size);
@@ -156,6 +159,8 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
 BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
 	   u32, size)
 {
+	if (security_is_locked_down(LOCKDOWN_BPF))
+		return -EINVAL;
 	/*
 	 * Ensure we're in user context which is safe for the helper to
 	 * run. This helper has no business in a kthread.
@@ -207,6 +212,9 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	char buf[64];
 	int i;
 
+	if (security_is_locked_down(LOCKDOWN_BPF))
+		return -EINVAL;
+
 	/*
 	 * bpf_check()->check_func_arg()->check_stack_boundary()
 	 * guarantees that fmt points to bpf program stack,
@@ -534,6 +542,9 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
 {
 	int ret;
 
+	if (security_is_locked_down(LOCKDOWN_BPF))
+		return -EINVAL;
+
 	/*
 	 * The strncpy_from_unsafe() call will likely not fill the entire
 	 * buffer, but that's okay in this circumstance as we're probing
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 89ad853daec2..0a3bbf1ba01d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
 	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
 	[LOCKDOWN_KCORE] = "/proc/kcore access",
 	[LOCKDOWN_KPROBES] = "use of kprobes",
+	[LOCKDOWN_BPF] = "use of bpf",
 	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
 };
 
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* Re: [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
From: David Ahern @ 2019-06-21  0:52 UTC (permalink / raw)
  To: Wei Wang, David Ahern
  Cc: David S . Miller, Linux Kernel Network Developers,
	Martin KaFai Lau
In-Reply-To: <CAEA6p_BUSFUCJJ_WsAAM2JRhQBBHjUepNZPpFX6DrTSCancD_g@mail.gmail.com>

On 6/20/19 5:43 PM, Wei Wang wrote:
> I am not very convinced that fib6_lookup() could be equivalent to
> rt6_lookup(). Specifically, rt6_lookup() calls rt6_device_match()
> while fib6_lookup() calls rt6_select() to match the oif. From a brief
> glance, it does seem to be similar, especially considering that saddr
> is NULL. So it probably is OK?

I believe so, but I'll walk the 2 paths again and do the compare based
on the limited flags and inputs for the gateway validation.


^ permalink raw reply

* Re: [PATCH v3 net-next 0/5] ipv6: avoid taking refcnt on dst during route lookup
From: David Ahern @ 2019-06-21  0:50 UTC (permalink / raw)
  To: Wei Wang, David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, Mahesh Bandewar, Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

On 6/20/19 6:36 PM, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
> 
> Ipv6 route lookup code always grabs refcnt on the dst for the caller.
> But for certain cases, grabbing refcnt is not always necessary if the
> call path is rcu protected and the caller does not cache the dst.
> Another issue in the route lookup logic is:
> When there are multiple custom rules, we have to do the lookup into
> each table associated to each rule individually. And when we can't
> find the route in one table, we grab and release refcnt on
> net->ipv6.ip6_null_entry before going to the next table.
> This operation is completely redundant, and causes false issue because
> net->ipv6.ip6_null_entry is a shared object.
> 
> This patch set introduces a new flag RT6_LOOKUP_F_DST_NOREF for route
> lookup callers to set, to avoid any manipulation on the dst refcnt. And
> it converts the major input and output path to use it.
> 
> The performance gain is noticable.
> I ran synflood tests between 2 hosts under the same switch. Both hosts
> have 20G mlx NIC, and 8 tx/rx queues.
> Sender sends pure SYN flood with random src IPs and ports using trafgen.
> Receiver has a simple TCP listener on the target port.
> Both hosts have multiple custom rules:
> - For incoming packets, only local table is traversed.
> - For outgoing packets, 3 tables are traversed to find the route.
> The packet processing rate on the receiver is as follows:
> - Before the fix: 3.78Mpps
> - After the fix:  5.50Mpps
> 

LGTM. Thanks for doing this - big improvement.

Reviewed-by: David Ahern <dsahern@gmail.com>



^ permalink raw reply

* Re: Stats for XDP actions
From: David Ahern @ 2019-06-21  0:42 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Jesper Dangaard Brouer
  Cc: David Miller, mst, makita.toshiaki, jasowang, netdev,
	virtualization, hawk, Jakub Kicinski, John Fastabend,
	Daniel Borkmann, Saeed Mahameed, Tariq Toukan
In-Reply-To: <87sgs46la6.fsf@toke.dk>

On 6/20/19 2:42 PM, Toke Høiland-Jørgensen wrote:
>>> I don't recall seeing any follow-up on this. Did you have a chance to
>>> formulate your ideas? :)
>>>
>>
>> Not yet. Almost done with the nexthop changes. Once that is out of the
>> way I can come back to this.
> 
> Ping? :)
> 

Definitely back to this after the July 4th holiday.

^ permalink raw reply

* Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc
From: Lucas Bates @ 2019-06-21  0:45 UTC (permalink / raw)
  To: Nicolas Dichtel
  Cc: Linux Kernel Network Developers, David Miller, Jamal Hadi Salim,
	kernel, Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner,
	Vlad Buslov, Davide Caratti
In-Reply-To: <0bd19bf9-f499-dc9f-1b26-ee0a075391ac@6wind.com>

On Tue, Jun 18, 2019 at 4:52 AM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> >> From my point of view, if all tests are not successful by default, it scares
> >> users and prevent them to use those tests suite to validate their patches.
> >
> > For me, explicitly telling the user that a test was skipped, and /why/
> > it was skipped, is far better than excluding the test from the
> > results: I don't want to waste someone's time with troubleshooting the
> > script if they're expecting to see results for those tests when
> > running tdc and nothing appears, or worse yet, stop using it because
> > they think it doesn't work properly.
> >
> > I do believe the skip message should be improved so it better
> > indicates why those tests are being skipped.  And the '-d' feature
> > should be documented.  How do these changes sound?
> If the error message is clear enough, I agree with you. The skip message should
> not feel like an error message ;-)

Very true. I think I just put that one in quickly and meant to come
back to it later, but either way it's a bit too vague.

I'll get that corrected, but I believe I'll add it to a separate patch
after the requires functionality goes in.  I want to update some of
the documentation as well.

Thanks,
Lucas

^ permalink raw reply

* [PATCH v3 net-next 1/5] ipv6: introduce RT6_LOOKUP_F_DST_NOREF flag in ip6_pol_route()
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>

This new flag is to instruct the route lookup function to not take
refcnt on the dst entry. The user which does route lookup with this flag
must properly use rcu protection.
ip6_pol_route() is the major route lookup function for both tx and rx
path.
In this function:
Do not take refcnt on dst if RT6_LOOKUP_F_DST_NOREF flag is set, and
directly return the route entry. The caller should be holding rcu lock
when using this flag, and decide whether to take refcnt or not.

One note on the dst cache in the uncached_list:
As uncached_list does not consume refcnt, one refcnt is always returned
back to the caller even if RT6_LOOKUP_F_DST_NOREF flag is set.
Uncached dst is only possible in the output path. So in such call path,
caller MUST check if the dst is in the uncached_list before assuming
that there is no refcnt taken on the returned dst.

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
---
 include/net/ip6_route.h |  1 +
 net/ipv6/route.c        | 73 +++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 43 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 7375a165fd98..82bced2fc1e3 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -36,6 +36,7 @@ struct route_info {
 #define RT6_LOOKUP_F_SRCPREF_PUBLIC	0x00000010
 #define RT6_LOOKUP_F_SRCPREF_COA	0x00000020
 #define RT6_LOOKUP_F_IGNORE_LINKSTATE	0x00000040
+#define RT6_LOOKUP_F_DST_NOREF		0x00000080
 
 /* We do not (yet ?) support IPv6 jumbograms (RFC 2675)
  * Unlike IPv4, hdr->seg_len doesn't include the IPv6 header
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c4d285fe0adc..9dcbc56e4151 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1391,9 +1391,6 @@ static struct rt6_info *rt6_get_pcpu_route(const struct fib6_result *res)
 
 	pcpu_rt = this_cpu_read(*res->nh->rt6i_pcpu);
 
-	if (pcpu_rt)
-		ip6_hold_safe(NULL, &pcpu_rt);
-
 	return pcpu_rt;
 }
 
@@ -1403,12 +1400,9 @@ static struct rt6_info *rt6_make_pcpu_route(struct net *net,
 	struct rt6_info *pcpu_rt, *prev, **p;
 
 	pcpu_rt = ip6_rt_pcpu_alloc(res);
-	if (!pcpu_rt) {
-		dst_hold(&net->ipv6.ip6_null_entry->dst);
-		return net->ipv6.ip6_null_entry;
-	}
+	if (!pcpu_rt)
+		return NULL;
 
-	dst_hold(&pcpu_rt->dst);
 	p = this_cpu_ptr(res->nh->rt6i_pcpu);
 	prev = cmpxchg(p, NULL, pcpu_rt);
 	BUG_ON(prev);
@@ -2189,9 +2183,12 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 			       const struct sk_buff *skb, int flags)
 {
 	struct fib6_result res = {};
-	struct rt6_info *rt;
+	struct rt6_info *rt = NULL;
 	int strict = 0;
 
+	WARN_ON_ONCE((flags & RT6_LOOKUP_F_DST_NOREF) &&
+		     !rcu_read_lock_held());
+
 	strict |= flags & RT6_LOOKUP_F_IFACE;
 	strict |= flags & RT6_LOOKUP_F_IGNORE_LINKSTATE;
 	if (net->ipv6.devconf_all->forwarding == 0)
@@ -2200,23 +2197,15 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 	rcu_read_lock();
 
 	fib6_table_lookup(net, table, oif, fl6, &res, strict);
-	if (res.f6i == net->ipv6.fib6_null_entry) {
-		rt = net->ipv6.ip6_null_entry;
-		rcu_read_unlock();
-		dst_hold(&rt->dst);
-		return rt;
-	}
+	if (res.f6i == net->ipv6.fib6_null_entry)
+		goto out;
 
 	fib6_select_path(net, &res, fl6, oif, false, skb, strict);
 
 	/*Search through exception table */
 	rt = rt6_find_cached_rt(&res, &fl6->daddr, &fl6->saddr);
 	if (rt) {
-		if (ip6_hold_safe(net, &rt))
-			dst_use_noref(&rt->dst, jiffies);
-
-		rcu_read_unlock();
-		return rt;
+		goto out;
 	} else if (unlikely((fl6->flowi6_flags & FLOWI_FLAG_KNOWN_NH) &&
 			    !res.nh->fib_nh_gw_family)) {
 		/* Create a RTF_CACHE clone which will not be
@@ -2224,40 +2213,38 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
 		 * the daddr in the skb during the neighbor look-up is different
 		 * from the fl6->daddr used to look-up route here.
 		 */
-		struct rt6_info *uncached_rt;
+		rt = ip6_rt_cache_alloc(&res, &fl6->daddr, NULL);
 
-		uncached_rt = ip6_rt_cache_alloc(&res, &fl6->daddr, NULL);
-
-		rcu_read_unlock();
-
-		if (uncached_rt) {
-			/* Uncached_rt's refcnt is taken during ip6_rt_cache_alloc()
-			 * No need for another dst_hold()
+		if (rt) {
+			/* 1 refcnt is taken during ip6_rt_cache_alloc().
+			 * As rt6_uncached_list_add() does not consume refcnt,
+			 * this refcnt is always returned to the caller even
+			 * if caller sets RT6_LOOKUP_F_DST_NOREF flag.
 			 */
-			rt6_uncached_list_add(uncached_rt);
+			rt6_uncached_list_add(rt);
 			atomic_inc(&net->ipv6.rt6_stats->fib_rt_uncache);
-		} else {
-			uncached_rt = net->ipv6.ip6_null_entry;
-			dst_hold(&uncached_rt->dst);
-		}
+			rcu_read_unlock();
 
-		return uncached_rt;
+			return rt;
+		}
 	} else {
 		/* Get a percpu copy */
-
-		struct rt6_info *pcpu_rt;
-
 		local_bh_disable();
-		pcpu_rt = rt6_get_pcpu_route(&res);
+		rt = rt6_get_pcpu_route(&res);
 
-		if (!pcpu_rt)
-			pcpu_rt = rt6_make_pcpu_route(net, &res);
+		if (!rt)
+			rt = rt6_make_pcpu_route(net, &res);
 
 		local_bh_enable();
-		rcu_read_unlock();
-
-		return pcpu_rt;
 	}
+out:
+	if (!rt)
+		rt = net->ipv6.ip6_null_entry;
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+		ip6_hold_safe(net, &rt);
+	rcu_read_unlock();
+
+	return rt;
 }
 EXPORT_SYMBOL_GPL(ip6_pol_route);
 
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v3 net-next 5/5] ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>

For tx path, in most cases, we still have to take refcnt on the dst
cause the caller is caching the dst somewhere. But it still is
beneficial to make use of RT6_LOOKUP_F_DST_NOREF flag while doing the
route lookup. It is cause this flag prevents manipulating refcnt on
net->ipv6.ip6_null_entry when doing fib6_rule_lookup() to traverse each
routing table. The null_entry is a shared object and constant updates on
it cause false sharing.

We converted the current major lookup function ip6_route_output_flags()
to make use of RT6_LOOKUP_F_DST_NOREF.

Together with the change in the rx path, we see noticable performance
boost:
I ran synflood tests between 2 hosts under the same switch. Both hosts
have 20G mlx NIC, and 8 tx/rx queues.
Sender sends pure SYN flood with random src IPs and ports using trafgen.
Receiver has a simple TCP listener on the target port.
Both hosts have multiple custom rules:
- For incoming packets, only local table is traversed.
- For outgoing packets, 3 tables are traversed to find the route.
The packet processing rate on the receiver is as follows:
- Before the fix: 3.78Mpps
- After the fix:  5.50Mpps

Signed-off-by: Wei Wang <weiwan@google.com>
---
 drivers/net/vrf.c       |  5 +++--
 include/net/ip6_route.h |  4 ++++
 net/ipv6/route.c        | 29 +++++++++++++++++++++++++++--
 net/l3mdev/l3mdev.c     |  7 +++----
 4 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 11b9525dff27..69ef9cce5858 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1072,12 +1072,14 @@ static struct sk_buff *vrf_l3_rcv(struct net_device *vrf_dev,
 #if IS_ENABLED(CONFIG_IPV6)
 /* send to link-local or multicast address via interface enslaved to
  * VRF device. Force lookup to VRF table without changing flow struct
+ * Note: Caller to this function must hold rcu_read_lock() and no refcnt
+ * is taken on the dst by this function.
  */
 static struct dst_entry *vrf_link_scope_lookup(const struct net_device *dev,
 					      struct flowi6 *fl6)
 {
 	struct net *net = dev_net(dev);
-	int flags = RT6_LOOKUP_F_IFACE;
+	int flags = RT6_LOOKUP_F_IFACE | RT6_LOOKUP_F_DST_NOREF;
 	struct dst_entry *dst = NULL;
 	struct rt6_info *rt;
 
@@ -1087,7 +1089,6 @@ static struct dst_entry *vrf_link_scope_lookup(const struct net_device *dev,
 	 */
 	if (fl6->flowi6_oif == dev->ifindex) {
 		dst = &net->ipv6.ip6_null_entry->dst;
-		dst_hold(dst);
 		return dst;
 	}
 
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 0709835c01ad..89ad7917b98d 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -84,6 +84,10 @@ struct dst_entry *ip6_route_input_lookup(struct net *net,
 					 struct flowi6 *fl6,
 					 const struct sk_buff *skb, int flags);
 
+struct dst_entry *ip6_route_output_flags_noref(struct net *net,
+					       const struct sock *sk,
+					       struct flowi6 *fl6, int flags);
+
 struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
 					 struct flowi6 *fl6, int flags);
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d2b287635aab..8a746001f813 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2415,8 +2415,9 @@ static struct rt6_info *ip6_pol_route_output(struct net *net,
 	return ip6_pol_route(net, table, fl6->flowi6_oif, fl6, skb, flags);
 }
 
-struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
-					 struct flowi6 *fl6, int flags)
+struct dst_entry *ip6_route_output_flags_noref(struct net *net,
+					       const struct sock *sk,
+					       struct flowi6 *fl6, int flags)
 {
 	bool any_src;
 
@@ -2424,6 +2425,7 @@ struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
 	    (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL)) {
 		struct dst_entry *dst;
 
+		/* This function does not take refcnt on the dst */
 		dst = l3mdev_link_scope_lookup(net, fl6);
 		if (dst)
 			return dst;
@@ -2431,6 +2433,7 @@ struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
 
 	fl6->flowi6_iif = LOOPBACK_IFINDEX;
 
+	flags |= RT6_LOOKUP_F_DST_NOREF;
 	any_src = ipv6_addr_any(&fl6->saddr);
 	if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(&fl6->daddr) ||
 	    (fl6->flowi6_oif && any_src))
@@ -2443,6 +2446,28 @@ struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
 
 	return fib6_rule_lookup(net, fl6, NULL, flags, ip6_pol_route_output);
 }
+EXPORT_SYMBOL_GPL(ip6_route_output_flags_noref);
+
+struct dst_entry *ip6_route_output_flags(struct net *net,
+					 const struct sock *sk,
+					 struct flowi6 *fl6,
+					 int flags)
+{
+        struct dst_entry *dst;
+        struct rt6_info *rt6;
+
+        rcu_read_lock();
+        dst = ip6_route_output_flags_noref(net, sk, fl6, flags);
+        rt6 = (struct rt6_info *)dst;
+        /* For dst cached in uncached_list, refcnt is already taken. */
+        if (list_empty(&rt6->rt6i_uncached) && !dst_hold_safe(dst)) {
+                dst = &net->ipv6.ip6_null_entry->dst;
+                dst_hold(dst);
+        }
+        rcu_read_unlock();
+
+        return dst;
+}
 EXPORT_SYMBOL_GPL(ip6_route_output_flags);
 
 struct dst_entry *ip6_blackhole_route(struct net *net, struct dst_entry *dst_orig)
diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
index cfc9fcb97465..f35899d45a9a 100644
--- a/net/l3mdev/l3mdev.c
+++ b/net/l3mdev/l3mdev.c
@@ -118,6 +118,8 @@ EXPORT_SYMBOL_GPL(l3mdev_fib_table_by_index);
  *			     local and multicast addresses
  *	@net: network namespace for device index lookup
  *	@fl6: IPv6 flow struct for lookup
+ *	This function does not hold refcnt on the returned dst.
+ *	Caller must hold rcu_read_lock().
  */
 
 struct dst_entry *l3mdev_link_scope_lookup(struct net *net,
@@ -126,9 +128,8 @@ struct dst_entry *l3mdev_link_scope_lookup(struct net *net,
 	struct dst_entry *dst = NULL;
 	struct net_device *dev;
 
+	WARN_ON_ONCE(!rcu_read_lock_held());
 	if (fl6->flowi6_oif) {
-		rcu_read_lock();
-
 		dev = dev_get_by_index_rcu(net, fl6->flowi6_oif);
 		if (dev && netif_is_l3_slave(dev))
 			dev = netdev_master_upper_dev_get_rcu(dev);
@@ -136,8 +137,6 @@ struct dst_entry *l3mdev_link_scope_lookup(struct net *net,
 		if (dev && netif_is_l3_master(dev) &&
 		    dev->l3mdev_ops->l3mdev_link_scope_lookup)
 			dst = dev->l3mdev_ops->l3mdev_link_scope_lookup(dev, fl6);
-
-		rcu_read_unlock();
 	}
 
 	return dst;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v3 net-next 4/5] ipv6: convert rx data path to not take refcnt on dst
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>

ip6_route_input() is the key function to do the route lookup in the
rx data path. All the callers to this function are already holding rcu
lock. So it is fairly easy to convert it to not take refcnt on the dst:
We pass in flag RT6_LOOKUP_F_DST_NOREF and do skb_dst_set_noref().
This saves a few atomic inc or dec operations and should boost
performance overall.
This also makes the logic more aligned with v4.

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
---
 net/ipv6/route.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 33dc8af9a4bf..d2b287635aab 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2375,11 +2375,12 @@ u32 rt6_multipath_hash(const struct net *net, const struct flowi6 *fl6,
 	return mhash >> 1;
 }
 
+/* Called with rcu held */
 void ip6_route_input(struct sk_buff *skb)
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
 	struct net *net = dev_net(skb->dev);
-	int flags = RT6_LOOKUP_F_HAS_SADDR;
+	int flags = RT6_LOOKUP_F_HAS_SADDR | RT6_LOOKUP_F_DST_NOREF;
 	struct ip_tunnel_info *tun_info;
 	struct flowi6 fl6 = {
 		.flowi6_iif = skb->dev->ifindex,
@@ -2401,8 +2402,8 @@ void ip6_route_input(struct sk_buff *skb)
 	if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6))
 		fl6.mp_hash = rt6_multipath_hash(net, &fl6, skb, flkeys);
 	skb_dst_drop(skb);
-	skb_dst_set(skb,
-		    ip6_route_input_lookup(net, skb->dev, &fl6, skb, flags));
+	skb_dst_set_noref(skb, ip6_route_input_lookup(net, skb->dev,
+						      &fl6, skb, flags));
 }
 
 static struct rt6_info *ip6_pol_route_output(struct net *net,
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v3 net-next 3/5] ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>

This patch specifically converts the rule lookup logic to honor this
flag and not release refcnt when traversing each rule and calling
lookup() on each routing table.
Similar to previous patch, we also need some special handling of dst
entries in uncached list because there is always 1 refcnt taken for them
even if RT6_LOOKUP_F_DST_NOREF flag is set.

Signed-off-by: Wei Wang <weiwan@google.com>
---
 include/net/ip6_route.h | 10 ++++++++++
 net/ipv6/fib6_rules.c   | 12 +++++++-----
 net/ipv6/ip6_fib.c      |  5 +++--
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 82bced2fc1e3..0709835c01ad 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -94,6 +94,16 @@ static inline struct dst_entry *ip6_route_output(struct net *net,
 	return ip6_route_output_flags(net, sk, fl6, 0);
 }
 
+/* Only conditionally release dst if flags indicates
+ * !RT6_LOOKUP_F_DST_NOREF or dst is in uncached_list.
+ */
+static inline void ip6_rt_put_flags(struct rt6_info *rt, int flags)
+{
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF) ||
+	    !list_empty(&rt->rt6i_uncached))
+		ip6_rt_put(rt);
+}
+
 struct dst_entry *ip6_route_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb, int flags);
 struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index bcfae13409b5..d22b6c140f23 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -113,14 +113,15 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 		rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
 		if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
 			return &rt->dst;
-		ip6_rt_put(rt);
+		ip6_rt_put_flags(rt, flags);
 		rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
 		if (rt->dst.error != -EAGAIN)
 			return &rt->dst;
-		ip6_rt_put(rt);
+		ip6_rt_put_flags(rt, flags);
 	}
 
-	dst_hold(&net->ipv6.ip6_null_entry->dst);
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+		dst_hold(&net->ipv6.ip6_null_entry->dst);
 	return &net->ipv6.ip6_null_entry->dst;
 }
 
@@ -237,13 +238,14 @@ static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 			goto out;
 	}
 again:
-	ip6_rt_put(rt);
+	ip6_rt_put_flags(rt, flags);
 	err = -EAGAIN;
 	rt = NULL;
 	goto out;
 
 discard_pkt:
-	dst_hold(&rt->dst);
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+		dst_hold(&rt->dst);
 out:
 	res->rt6 = rt;
 	return err;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 1d16a01eccf5..5b1c9b5b9247 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -316,9 +316,10 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 
 	rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
 	if (rt->dst.error == -EAGAIN) {
-		ip6_rt_put(rt);
+		ip6_rt_put_flags(rt, flags);
 		rt = net->ipv6.ip6_null_entry;
-		dst_hold(&rt->dst);
+		if (!(flags | RT6_LOOKUP_F_DST_NOREF))
+			dst_hold(&rt->dst);
 	}
 
 	return &rt->dst;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v3 net-next 2/5] ipv6: initialize rt6->rt6i_uncached in all pre-allocated dst entries
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang
In-Reply-To: <20190621003641.168591-1-tracywwnj@gmail.com>

From: Wei Wang <weiwan@google.com>

Initialize rt6->rt6i_uncached on the following pre-allocated dsts:
net->ipv6.ip6_null_entry
net->ipv6.ip6_prohibit_entry
net->ipv6.ip6_blk_hole_entry

This is a preparation patch for later commits to be able to distinguish
dst entries in uncached list by doing:
!list_empty(rt6->rt6i_uncached)

Signed-off-by: Wei Wang <weiwan@google.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Acked-by: Mahesh Bandewar <maheshb@google.com>
---
 net/ipv6/route.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9dcbc56e4151..33dc8af9a4bf 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -6010,6 +6010,7 @@ static int __net_init ip6_route_net_init(struct net *net)
 	net->ipv6.ip6_null_entry->dst.ops = &net->ipv6.ip6_dst_ops;
 	dst_init_metrics(&net->ipv6.ip6_null_entry->dst,
 			 ip6_template_metrics, true);
+	INIT_LIST_HEAD(&net->ipv6.ip6_null_entry->rt6i_uncached);
 
 #ifdef CONFIG_IPV6_MULTIPLE_TABLES
 	net->ipv6.fib6_has_custom_rules = false;
@@ -6021,6 +6022,7 @@ static int __net_init ip6_route_net_init(struct net *net)
 	net->ipv6.ip6_prohibit_entry->dst.ops = &net->ipv6.ip6_dst_ops;
 	dst_init_metrics(&net->ipv6.ip6_prohibit_entry->dst,
 			 ip6_template_metrics, true);
+	INIT_LIST_HEAD(&net->ipv6.ip6_prohibit_entry->rt6i_uncached);
 
 	net->ipv6.ip6_blk_hole_entry = kmemdup(&ip6_blk_hole_entry_template,
 					       sizeof(*net->ipv6.ip6_blk_hole_entry),
@@ -6030,6 +6032,7 @@ static int __net_init ip6_route_net_init(struct net *net)
 	net->ipv6.ip6_blk_hole_entry->dst.ops = &net->ipv6.ip6_dst_ops;
 	dst_init_metrics(&net->ipv6.ip6_blk_hole_entry->dst,
 			 ip6_template_metrics, true);
+	INIT_LIST_HEAD(&net->ipv6.ip6_blk_hole_entry->rt6i_uncached);
 #endif
 
 	net->ipv6.sysctl.flush_delay = 0;
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply related

* [PATCH v3 net-next 0/5] ipv6: avoid taking refcnt on dst during route lookup
From: Wei Wang @ 2019-06-21  0:36 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: Eric Dumazet, Martin KaFai Lau, David Ahern, Mahesh Bandewar,
	Wei Wang

From: Wei Wang <weiwan@google.com>

Ipv6 route lookup code always grabs refcnt on the dst for the caller.
But for certain cases, grabbing refcnt is not always necessary if the
call path is rcu protected and the caller does not cache the dst.
Another issue in the route lookup logic is:
When there are multiple custom rules, we have to do the lookup into
each table associated to each rule individually. And when we can't
find the route in one table, we grab and release refcnt on
net->ipv6.ip6_null_entry before going to the next table.
This operation is completely redundant, and causes false issue because
net->ipv6.ip6_null_entry is a shared object.

This patch set introduces a new flag RT6_LOOKUP_F_DST_NOREF for route
lookup callers to set, to avoid any manipulation on the dst refcnt. And
it converts the major input and output path to use it.

The performance gain is noticable.
I ran synflood tests between 2 hosts under the same switch. Both hosts
have 20G mlx NIC, and 8 tx/rx queues.
Sender sends pure SYN flood with random src IPs and ports using trafgen.
Receiver has a simple TCP listener on the target port.
Both hosts have multiple custom rules:
- For incoming packets, only local table is traversed.
- For outgoing packets, 3 tables are traversed to find the route.
The packet processing rate on the receiver is as follows:
- Before the fix: 3.78Mpps
- After the fix:  5.50Mpps

v2->v3:
- Handled fib6_rule_lookup() when CONFIG_IPV6_MULTIPLE_TABLES is not
  configured in patch 03 (suggested by David Ahern)
- Removed the renaming of l3mdev_link_scope_lookup() in patch 05
  (suggested by David Ahern)
- Moved definition of ip6_route_output_flags() from an inline function
  in /net/ipv6/route.c to net/ipv6/route.c in order to address kbuild
  error in patch 05

v1->v2:
- Added a helper ip6_rt_put_flags() in patch 3 suggested by David Miller


Wei Wang (5):
  ipv6: introduce RT6_LOOKUP_F_DST_NOREF flag in ip6_pol_route()
  ipv6: initialize rt6->rt6i_uncached in all pre-allocated dst entries
  ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic
  ipv6: convert rx data path to not take refcnt on dst
  ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF

 drivers/net/vrf.c       |   5 +-
 include/net/ip6_route.h |  15 ++++++
 net/ipv6/fib6_rules.c   |  12 +++--
 net/ipv6/ip6_fib.c      |   5 +-
 net/ipv6/route.c        | 112 +++++++++++++++++++++++-----------------
 net/l3mdev/l3mdev.c     |   7 ++-
 6 files changed, 95 insertions(+), 61 deletions(-)

-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply

* Re: [PATCH] flow_dissector: Fix vlan header offset in __skb_flow_dissect
From: Stanislav Fomichev @ 2019-06-21  0:33 UTC (permalink / raw)
  To: Yuehaibing
  Cc: davem, sdf, jianbol, jiri, mirq-linux, willemb, linux-kernel,
	netdev
In-Reply-To: <00a5d09f-a23e-661f-60c0-75fba6227451@huawei.com>

On 06/20, Yuehaibing wrote:
> On 2019/6/20 2:39, Stanislav Fomichev wrote:
> > On 06/20, YueHaibing wrote:
> >> We build vlan on top of bonding interface, which vlan offload
> >> is off, bond mode is 802.3ad (LACP) and xmit_hash_policy is
> >> BOND_XMIT_POLICY_ENCAP34.
> >>
> >> __skb_flow_dissect() fails to get information from protocol headers
> >> encapsulated within vlan, because 'nhoff' is points to IP header,
> >> so bond hashing is based on layer 2 info, which fails to distribute
> >> packets across slaves.
> >>
> >> Fixes: d5709f7ab776 ("flow_dissector: For stripped vlan, get vlan info from skb->vlan_tci")
> >> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >> ---
> >>  net/core/flow_dissector.c | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> >> index 415b95f..2a52abb 100644
> >> --- a/net/core/flow_dissector.c
> >> +++ b/net/core/flow_dissector.c
> >> @@ -785,6 +785,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> >>  		    skb && skb_vlan_tag_present(skb)) {
> >>  			proto = skb->protocol;
> >>  		} else {
> >> +			if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX)
> >> +				nhoff -=  sizeof(*vlan);
> >> +
> > Should we instead fix the place where the skb is allocated to properly
> > pull vlan (skb_vlan_untag)? I'm not sure this particular place is
> > supposed to work with an skb. Having an skb with nhoff pointing to
> > IP header but missing skb_vlan_tag_present() when with
> > proto==ETH_P_8021xx seems weird.
> 
> The skb is a forwarded vxlan packet, it send through vlan interface like this:
> 
>    vlan_dev_hard_start_xmit
>     --> __vlan_hwaccel_put_tag //vlan_tci and VLAN_TAG_PRESENT is set
>     --> dev_queue_xmit
>         --> validate_xmit_skb
>           --> validate_xmit_vlan // vlan_hw_offload_capable is false
>              --> __vlan_hwaccel_push_inside //here skb_push vlan_hlen, then clear skb->tci
> 
>     --> bond_start_xmit
>        --> bond_xmit_hash
>          --> __skb_flow_dissect // nhoff point to IP header
>             -->  case htons(ETH_P_8021Q)
>             // skb_vlan_tag_present is false, so
>               vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan), //vlan point to ip header wrongly
I see, so bonding device propagates hw VLAN support from the slaves.
If one of the slaves doesn't have it, its disabled for the bond device.
Any idea why we do that? Why not pass skbs to the slave devices
instead and let them handle the hw/sw vlan implementation?
I see the propagation was added in 278339a42a1b 10 years ago and
I don't see any rationale in the commit description.
Somebody with more context should probably chime in :-)

^ permalink raw reply

* Re: [PATCH bpf-next 5/7] selftests/bpf: switch test to new attach_perf_event API
From: Stanislav Fomichev @ 2019-06-21  0:08 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190620230951.3155955-6-andriin@fb.com>

On 06/20, Andrii Nakryiko wrote:
> Use new bpf_program__attach_perf_event() in test previously relying on
> direct ioctl manipulations.
Maybe use new detach/disable routine at the end of the
test_stacktrace_build_id_nmi as well?

> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  .../bpf/prog_tests/stacktrace_build_id_nmi.c     | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> index 1c1a2f75f3d8..1bbdb0b82ac5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
> @@ -17,6 +17,7 @@ static __u64 read_perf_max_sample_freq(void)
>  void test_stacktrace_build_id_nmi(void)
>  {
>  	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> +	const char *prog_name = "tracepoint/random/urandom_read";
>  	const char *file = "./test_stacktrace_build_id.o";
>  	int err, pmu_fd, prog_fd;
>  	struct perf_event_attr attr = {
> @@ -25,6 +26,7 @@ void test_stacktrace_build_id_nmi(void)
>  		.config = PERF_COUNT_HW_CPU_CYCLES,
>  	};
>  	__u32 key, previous_key, val, duration = 0;
> +	struct bpf_program *prog;
>  	struct bpf_object *obj;
>  	char buf[256];
>  	int i, j;
> @@ -39,6 +41,10 @@ void test_stacktrace_build_id_nmi(void)
>  	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
>  		return;
>  
> +	prog = bpf_object__find_program_by_title(obj, prog_name);
> +	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
> +		goto close_prog;
> +
>  	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
>  			 0 /* cpu 0 */, -1 /* group id */,
>  			 0 /* flags */);
> @@ -47,16 +53,10 @@ void test_stacktrace_build_id_nmi(void)
>  		  pmu_fd, errno))
>  		goto close_prog;
>  
> -	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> -	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
> -		  err, errno))
> +	err = bpf_program__attach_perf_event(prog, pmu_fd);
> +	if (CHECK(err, "attach_perf_event", "err %d\n", err))
>  		goto close_pmu;
>  
> -	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> -	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
> -		  err, errno))
> -		goto disable_pmu;
> -
>  	/* find map fds */
>  	control_map_fd = bpf_find_map(__func__, obj, "control_map");
>  	if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH bpf-next 4/7] libbpf: add tracepoint/raw tracepoint attach API
From: Stanislav Fomichev @ 2019-06-21  0:07 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190620230951.3155955-5-andriin@fb.com>

On 06/20, Andrii Nakryiko wrote:
> Add APIs allowing to attach BPF program to kernel tracepoints. Raw
> tracepoint attach API is also added for uniform per-BPF-program API,
> but is mostly a wrapper around existing bpf_raw_tracepoint_open call.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c   | 99 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  5 ++
>  tools/lib/bpf/libbpf.map |  2 +
>  3 files changed, 106 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 11329e05530e..cefe67ba160b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4176,6 +4176,105 @@ int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
>  	return pfd;
>  }
>  
> +static int determine_tracepoint_id(const char* tp_category, const char* tp_name)
> +{
> +	char file[PATH_MAX];
> +	int ret;
> +
> +	ret = snprintf(file, sizeof(file),
> +		       "/sys/kernel/debug/tracing/events/%s/%s/id",
> +		       tp_category, tp_name);
> +	if (ret < 0)
> +		return -errno;
> +	if (ret >= sizeof(file)) {
> +		pr_debug("tracepoint %s/%s path is too long\n",
> +			 tp_category, tp_name);
> +		return -E2BIG;
> +	}
> +	return parse_uint_from_file(file);
> +}
> +
> +static int perf_event_open_tracepoint(const char* tp_category,
> +				      const char* tp_name)
> +{
> +	struct perf_event_attr attr = {};
> +	char errmsg[STRERR_BUFSIZE];
> +	int tp_id, pfd, err;
> +
[..]
> +	tp_id = determine_tracepoint_id(tp_category, tp_name);
Why no assign to attr.config directly here?
You can move all other constants to the initialization as well:

struct perf_event_attr attr = {
	.type = PERF_TYPE_TRACEPON,
	.size = sizeof(struct perf_event_attr),
};

attr.config = determine_tracepoint_id(...);

(I guess that's a matter of style, but something to consider).

> +	if (tp_id < 0){
> +		pr_warning("failed to determine tracepoint '%s/%s' perf ID: %s\n",
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(tp_id, errmsg, sizeof(errmsg)));
> +		return tp_id;
> +	}
> +
[..]
> +	memset(&attr, 0, sizeof(attr));
Not needed since you do attr = {}; above?

> +	attr.type = PERF_TYPE_TRACEPOINT;
> +	attr.size = sizeof(attr);
> +	attr.config = tp_id;
> +
> +	pfd = syscall( __NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu */,
> +			-1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> +	if (pfd < 0) {
> +		err = -errno;
> +		pr_warning("tracepoint '%s/%s' perf_event_open() failed: %s\n",
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +int bpf_program__attach_tracepoint(struct bpf_program *prog,
> +				   const char *tp_category,
> +				   const char *tp_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	int pfd, err;
> +
> +	pfd = perf_event_open_tracepoint(tp_category, tp_name);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create tracepoint '%s/%s' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return pfd;
> +	}
> +	err = bpf_program__attach_perf_event(prog, pfd);
> +	if (err) {
> +		libbpf_perf_event_disable_and_close(pfd);
> +		pr_warning("program '%s': failed to attach to tracepoint '%s/%s': %s\n",
> +			   bpf_program__title(prog, false),
> +			   tp_category, tp_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> +				       const char *tp_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	int bpf_fd, pfd;
> +
> +	bpf_fd = bpf_program__fd(prog);
> +	if (bpf_fd < 0) {
> +		pr_warning("program '%s': can't attach before loaded\n",
> +			   bpf_program__title(prog, false));
> +		return -EINVAL;
> +	}
> +	pfd = bpf_raw_tracepoint_open(tp_name, bpf_fd);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to attach to raw tracepoint '%s': %s\n",
> +			   bpf_program__title(prog, false), tp_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return pfd;
> +	}
> +	return pfd;
> +}
> +
>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>  			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index a7264f06aa5f..bf7020a565c6 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -176,6 +176,11 @@ LIBBPF_API int bpf_program__attach_uprobe(struct bpf_program *prog,
>  					  pid_t pid,
>  					  const char *binary_path,
>  					  size_t func_offset);
> +LIBBPF_API int bpf_program__attach_tracepoint(struct bpf_program *prog,
> +					      const char *tp_category,
> +					      const char *tp_name);
> +LIBBPF_API int bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
> +						  const char *tp_name);
>  
>  struct bpf_insn;
>  
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 1a982c2e1751..2382fbda4cbb 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -174,6 +174,8 @@ LIBBPF_0.0.4 {
>  		bpf_object__load_xattr;
>  		bpf_program__attach_kprobe;
>  		bpf_program__attach_perf_event;
> +		bpf_program__attach_raw_tracepoint;
> +		bpf_program__attach_tracepoint;
>  		bpf_program__attach_uprobe;
>  		libbpf_num_possible_cpus;
>  		libbpf_perf_event_disable_and_close;
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Stanislav Fomichev @ 2019-06-21  0:04 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190620230951.3155955-4-andriin@fb.com>

On 06/20, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c   | 207 +++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |   8 ++
>  tools/lib/bpf/libbpf.map |   2 +
>  3 files changed, 217 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2bb1fa008be3..11329e05530e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3969,6 +3969,213 @@ int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
>  	return 0;
>  }
>  
> +static int parse_uint(const char *buf)
> +{
> +	int ret;
> +
> +	errno = 0;
> +	ret = (int)strtol(buf, NULL, 10);
> +	if (errno) {
> +		ret = -errno;
> +		pr_debug("failed to parse '%s' as unsigned int\n", buf);
> +		return ret;
> +	}
> +	if (ret < 0) {
> +		pr_debug("failed to parse '%s' as unsigned int\n", buf);
> +		return -EINVAL;
> +	}
> +	return ret;
> +}
> +
> +static int parse_uint_from_file(const char* file)
> +{
> +	char buf[STRERR_BUFSIZE];
> +	int fd, ret;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		ret = -errno;
> +		pr_debug("failed to open '%s': %s\n", file,
> +			 libbpf_strerror_r(ret, buf, sizeof(buf)));
> +		return ret;
> +	}
> +	ret = read(fd, buf, sizeof(buf));
> +	close(fd);
> +	if (ret < 0) {
> +		ret = -errno;
Is -errno still valid here after a close(fd) above? Do we have any
guarantee of errno preservation when we do another syscall?

> +		pr_debug("failed to read '%s': %s\n", file,
> +			libbpf_strerror_r(ret, buf, sizeof(buf)));
> +		return ret;
> +	}
> +	if (ret == 0 || ret >= sizeof(buf)) {
> +		buf[sizeof(buf) - 1] = 0;
> +		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> +		return -EINVAL;
> +	}
> +	return parse_uint(buf);
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +	return parse_uint_from_file(file);
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +	return parse_uint_from_file(file);
> +}
> +
> +static int parse_config_from_file(const char *file)
> +{
> +	char buf[STRERR_BUFSIZE];
> +	int fd, ret;
> +
> +	fd = open(file, O_RDONLY);
> +	if (fd < 0) {
> +		ret = -errno;
> +		pr_debug("failed to open '%s': %s\n", file,
> +			 libbpf_strerror_r(ret, buf, sizeof(buf)));
> +		return ret;
> +	}
> +	ret = read(fd, buf, sizeof(buf));
> +	close(fd);
> +	if (ret < 0) {
> +		ret = -errno;
> +		pr_debug("failed to read '%s': %s\n", file,
> +			libbpf_strerror_r(ret, buf, sizeof(buf)));
> +		return ret;
> +	}
> +	if (ret == 0 || ret >= sizeof(buf)) {
> +		buf[sizeof(buf) - 1] = 0;
> +		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
> +		return -EINVAL;
> +	}
> +	if (strncmp(buf, "config:", 7)) {
> +		pr_debug("expected 'config:' prefix, found '%s'\n", buf);
> +		return -EINVAL;
> +	}
> +	return parse_uint(buf + 7);
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +	return parse_config_from_file(file);
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> +	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +	return parse_config_from_file(file);
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char* name,
> +				 uint64_t offset, int pid)
> +{
> +	struct perf_event_attr attr = {};
> +	char errmsg[STRERR_BUFSIZE];
> +	int type, pfd, err;
> +
> +	type = uprobe ? determine_uprobe_perf_type()
> +		      : determine_kprobe_perf_type();
> +	if (type < 0) {
> +		pr_warning("failed to determine %s perf type: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> +		return type;
> +	}
> +	if (retprobe) {
> +		int bit = uprobe ? determine_uprobe_retprobe_bit()
> +				 : determine_kprobe_retprobe_bit();
> +
> +		if (bit < 0) {
> +			pr_warning("failed to determine %s retprobe bit: %s\n",
> +				   uprobe ? "uprobe" : "kprobe",
> +				   libbpf_strerror_r(bit, errmsg,
> +						     sizeof(errmsg)));
> +			return bit;
> +		}
> +		attr.config |= 1 << bit;
> +	}
> +	attr.size = sizeof(attr);
> +	attr.type = type;
> +	attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> +	attr.config2 = offset;		       /* kprobe_addr or probe_offset */
> +
> +	/* pid filter is meaningful only for uprobes */
> +	pfd = syscall(__NR_perf_event_open, &attr,
> +		      pid < 0 ? -1 : pid /* pid */,
> +		      pid == -1 ? 0 : -1 /* cpu */,
> +		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> +	if (pfd < 0) {
> +		err = -errno;
> +		pr_warning("%s perf_event_open() failed: %s\n",
> +			   uprobe ? "uprobe" : "kprobe",
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> +			       const char *func_name)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> +				    0 /* offset */, -1 /* pid */);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return pfd;
> +	}
> +	err = bpf_program__attach_perf_event(prog, pfd);
> +	if (err) {
> +		libbpf_perf_event_disable_and_close(pfd);
> +		pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "kretprobe" : "kprobe", func_name,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
> +int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> +			       pid_t pid, const char *binary_path,
> +			       size_t func_offset)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	int pfd, err;
> +
> +	pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> +				    binary_path, func_offset, pid);
> +	if (pfd < 0) {
> +		pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> +		return pfd;
> +	}
> +	err = bpf_program__attach_perf_event(prog, pfd);
> +	if (err) {
> +		libbpf_perf_event_disable_and_close(pfd);
> +		pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> +			   bpf_program__title(prog, false),
> +			   retprobe ? "uretprobe" : "uprobe",
> +			   binary_path, func_offset,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return pfd;
> +}
> +
>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>  			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 76db1bbc0dac..a7264f06aa5f 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -168,6 +168,14 @@ LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
>  LIBBPF_API int libbpf_perf_event_disable_and_close(int pfd);
>  LIBBPF_API int bpf_program__attach_perf_event(struct bpf_program *prog,
>  					      int pfd);
> +LIBBPF_API int bpf_program__attach_kprobe(struct bpf_program *prog,
> +					  bool retprobe,
> +					  const char *func_name);
> +LIBBPF_API int bpf_program__attach_uprobe(struct bpf_program *prog,
> +					  bool retprobe,
> +					  pid_t pid,
> +					  const char *binary_path,
> +					  size_t func_offset);
>  
>  struct bpf_insn;
>  
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index d27406982b5a..1a982c2e1751 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -172,7 +172,9 @@ LIBBPF_0.0.4 {
>  		btf_dump__new;
>  		btf__parse_elf;
>  		bpf_object__load_xattr;
> +		bpf_program__attach_kprobe;
>  		bpf_program__attach_perf_event;
> +		bpf_program__attach_uprobe;
>  		libbpf_num_possible_cpus;
>  		libbpf_perf_event_disable_and_close;
>  } LIBBPF_0.0.3;
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH bpf-next 2/7] libbpf: add ability to attach/detach BPF to perf event
From: Stanislav Fomichev @ 2019-06-21  0:01 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team
In-Reply-To: <20190620230951.3155955-3-andriin@fb.com>

On 06/20, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event, providing most generic and most low-level way to attach BPF
> programs.
> 
> libbpf_perf_event_disable_and_close API is added to disable and close
> existing perf event by its FD.
> 
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
>  tools/lib/bpf/libbpf.c   | 41 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  4 ++++
>  tools/lib/bpf/libbpf.map |  2 ++
>  3 files changed, 47 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8ce3beba8551..2bb1fa008be3 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
>  #include <linux/limits.h>
>  #include <linux/perf_event.h>
>  #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/vfs.h>
> @@ -3928,6 +3929,46 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
>  	return 0;
>  }
>  
[..]
> +int libbpf_perf_event_disable_and_close(int pfd)
nit: why not call it libbpf_perf_event_detach[_and_close]?
It's usually attach/detach.

> +{
> +	int err;
> +
> +	if (pfd < 0)
> +		return 0;
> +
> +	err = ioctl(pfd, PERF_EVENT_IOC_DISABLE, 0);
> +	close(pfd);
> +	return err;
> +}
> +
> +int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
> +{
> +	char errmsg[STRERR_BUFSIZE];
> +	int bpf_fd, err;
> +
> +	bpf_fd = bpf_program__fd(prog);
> +	if (bpf_fd < 0) {
> +		pr_warning("program '%s': can't attach before loaded\n",
> +			   bpf_program__title(prog, false));
> +		return -EINVAL;
> +	}
> +	if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0) {
> +		err = -errno;
> +		pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> +			   bpf_program__title(prog, false), pfd,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> +		err = -errno;
> +		pr_warning("program '%s': failed to enable pfd %d: %s\n",
> +			   bpf_program__title(prog, false), pfd,
> +			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +		return err;
> +	}
> +	return 0;
> +}
> +
>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>  			   void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d639f47e3110..76db1bbc0dac 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
>  LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
>  LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
>  
> +LIBBPF_API int libbpf_perf_event_disable_and_close(int pfd);
> +LIBBPF_API int bpf_program__attach_perf_event(struct bpf_program *prog,
> +					      int pfd);
> +
>  struct bpf_insn;
>  
>  /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2c6d835620d2..d27406982b5a 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -172,5 +172,7 @@ LIBBPF_0.0.4 {
>  		btf_dump__new;
>  		btf__parse_elf;
>  		bpf_object__load_xattr;
> +		bpf_program__attach_perf_event;
>  		libbpf_num_possible_cpus;
> +		libbpf_perf_event_disable_and_close;
>  } LIBBPF_0.0.3;
> -- 
> 2.17.1
> 

^ permalink raw reply

* [RFC net-next] net: dsa: add support for MC_DISABLED attribute
From: Vivien Didelot @ 2019-06-20 23:56 UTC (permalink / raw)
  To: netdev; +Cc: linux, f.fainelli, idosch, andrew, davem, Vivien Didelot

This patch adds support for enabling or disabling the flooding of
unknown multicast traffic on the CPU ports, depending on the value
of the switchdev SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED attribute.

This allows the user to prevent the CPU to be flooded with a lot of
undesirable traffic that the network stack needs to filter in software.

The bridge has multicast snooping enabled by default, hence CPU ports
aren't bottlenecked with arbitrary network applications anymore.
But this can be an issue in some scenarios such as pinging the bridge's
IPv6 address. Setting /sys/class/net/br0/bridge/multicast_snooping to
0 would restore unknown multicast flooding and thus fix ICMPv6. As
an alternative, enabling multicast_querier would program the bridge
address into the switch.

Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
 net/dsa/dsa_priv.h |  2 ++
 net/dsa/port.c     | 19 +++++++++++++++++++
 net/dsa/slave.c    |  4 ++++
 3 files changed, 25 insertions(+)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index a4853c22c2ff..54d838956499 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -163,6 +163,8 @@ int dsa_port_pre_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 			      struct switchdev_trans *trans);
 int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 			  struct switchdev_trans *trans);
+int dsa_port_bridge_mc_disabled(const struct dsa_port *dp, bool mc_disabled,
+				struct switchdev_trans *trans);
 int dsa_port_vlan_add(struct dsa_port *dp,
 		      const struct switchdev_obj_port_vlan *vlan,
 		      struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index d2b65e8dc60c..79d14c36ef9a 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -261,6 +261,25 @@ int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
 	return err;
 }
 
+int dsa_port_bridge_mc_disabled(const struct dsa_port *dp, bool mc_disabled,
+				struct switchdev_trans *trans)
+{
+	struct dsa_switch *ds = dp->ds;
+	int port = dp->index;
+
+	if (switchdev_trans_ph_prepare(trans)) {
+		if (!ds->ops->port_egress_floods)
+			return -EOPNOTSUPP;
+
+		return 0;
+	}
+
+	/* When multicast snooping is disabled,
+	 * every multicast packet should be flooded to the CPU port.
+         */
+	return ds->ops->port_egress_floods(ds, port, true, mc_disabled);
+}
+
 int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
 		     u16 vid)
 {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index db58e748557d..9308ffa4f22c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -302,6 +302,10 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
 	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
 		ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
 		break;
+	case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED:
+		ret = dsa_port_bridge_mc_disabled(dp->cpu_dp,
+						  attr->u.mc_disabled, trans);
+		break;
 	default:
 		ret = -EOPNOTSUPP;
 		break;
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH net-next] ipv6: Convert gateway validation to use fib6_info
From: Wei Wang @ 2019-06-20 23:43 UTC (permalink / raw)
  To: David Ahern
  Cc: David S . Miller, Linux Kernel Network Developers,
	Martin KaFai Lau, David Ahern
In-Reply-To: <20190620190536.3157-1-dsahern@kernel.org>

On Thu, Jun 20, 2019 at 12:05 PM David Ahern <dsahern@kernel.org> wrote:
>
> From: David Ahern <dsahern@gmail.com>
>
> Gateway validation does not need a dst_entry, it only needs the fib
> entry to validate the gateway resolution and egress device. So,
> convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
> and ip6_route_check_nh to use fib6_lookup over rt6_lookup.
>
> ip6_pol_route is a call to fib6_table_lookup and if successful a call
> to fib6_select_path. From there the exception cache is searched for an
> entry or a dst_entry is created to return to the caller. The exception
> entry is not relevant for gateway validation, so what matters are the
> calls to fib6_table_lookup and then fib6_select_path.
>
> Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
> RT6_LOOKUP_F_IFACE set in flags (saddr is not set for gateway validation,
> so RT6_LOOKUP_F_HAS_SADDR is not relevant). From there ip6_pol_route_lookup
> lookup function is flipped to fib6_table_lookup for the per-table search.
> Again, the exception cache search is not relevant, only the lookup with
> path selection.
>
> Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
> handle a fib6_info vs a rt6_info when performing validation checks.
>
> Existing selftests fib-onlink-tests.sh and fib_tests.sh used to verify
> the changes.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv6/route.c | 119 ++++++++++++++++++++++++++-----------------------------
>  1 file changed, 57 insertions(+), 62 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index c4d285fe0adc..4937084610b5 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3131,10 +3131,9 @@ static int ip6_dst_gc(struct dst_ops *ops)
>         return entries > rt_max_size;
>  }
>
> -static struct rt6_info *ip6_nh_lookup_table(struct net *net,
> -                                           struct fib6_config *cfg,
> -                                           const struct in6_addr *gw_addr,
> -                                           u32 tbid, int flags)
> +static int ip6_nh_lookup_table(struct net *net, struct fib6_config *cfg,
> +                              const struct in6_addr *gw_addr, u32 tbid,
> +                              int flags, struct fib6_result *res)
>  {
>         struct flowi6 fl6 = {
>                 .flowi6_oif = cfg->fc_ifindex,
> @@ -3142,25 +3141,23 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
>                 .saddr = cfg->fc_prefsrc,
>         };
>         struct fib6_table *table;
> -       struct rt6_info *rt;
> +       int err;
>
>         table = fib6_get_table(net, tbid);
>         if (!table)
> -               return NULL;
> +               return -EINVAL;
>
>         if (!ipv6_addr_any(&cfg->fc_prefsrc))
>                 flags |= RT6_LOOKUP_F_HAS_SADDR;
>
>         flags |= RT6_LOOKUP_F_IGNORE_LINKSTATE;
> -       rt = ip6_pol_route(net, table, cfg->fc_ifindex, &fl6, NULL, flags);
>
> -       /* if table lookup failed, fall back to full lookup */
> -       if (rt == net->ipv6.ip6_null_entry) {
> -               ip6_rt_put(rt);
> -               rt = NULL;
> -       }
> +       err = fib6_table_lookup(net, table, cfg->fc_ifindex, &fl6, res, flags);
> +       if (!err && res->f6i != net->ipv6.fib6_null_entry)
> +               fib6_select_path(net, res, &fl6, cfg->fc_ifindex,
> +                                cfg->fc_ifindex != 0, NULL, flags);
>
> -       return rt;
> +       return err;
>  }
>
>  static int ip6_route_check_nh_onlink(struct net *net,
> @@ -3168,29 +3165,19 @@ static int ip6_route_check_nh_onlink(struct net *net,
>                                      const struct net_device *dev,
>                                      struct netlink_ext_ack *extack)
>  {
> -       u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
> +       u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
>         const struct in6_addr *gw_addr = &cfg->fc_gateway;
> -       u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
> -       struct fib6_info *from;
> -       struct rt6_info *grt;
> +       struct fib6_result res = {};
>         int err;
>
> -       err = 0;
> -       grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
> -       if (grt) {
> -               rcu_read_lock();
> -               from = rcu_dereference(grt->from);
> -               if (!grt->dst.error &&
> -                   /* ignore match if it is the default route */
> -                   from && !ipv6_addr_any(&from->fib6_dst.addr) &&
> -                   (grt->rt6i_flags & flags || dev != grt->dst.dev)) {
> -                       NL_SET_ERR_MSG(extack,
> -                                      "Nexthop has invalid gateway or device mismatch");
> -                       err = -EINVAL;
> -               }
> -               rcu_read_unlock();
> -
> -               ip6_rt_put(grt);
> +       err = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0, &res);
> +       if (!err && !(res.fib6_flags & RTF_REJECT) &&
> +           /* ignore match if it is the default route */
> +           !ipv6_addr_any(&res.f6i->fib6_dst.addr) &&
> +           (res.fib6_type != RTN_UNICAST || dev != res.nh->fib_nh_dev)) {
> +               NL_SET_ERR_MSG(extack,
> +                              "Nexthop has invalid gateway or device mismatch");
> +               err = -EINVAL;
>         }
>
>         return err;
> @@ -3203,47 +3190,51 @@ static int ip6_route_check_nh(struct net *net,
>  {
>         const struct in6_addr *gw_addr = &cfg->fc_gateway;
>         struct net_device *dev = _dev ? *_dev : NULL;
> -       struct rt6_info *grt = NULL;
> +       int flags = RT6_LOOKUP_F_IFACE;
> +       struct fib6_result res = {};
>         int err = -EHOSTUNREACH;
>
>         if (cfg->fc_table) {
> -               int flags = RT6_LOOKUP_F_IFACE;
> -
> -               grt = ip6_nh_lookup_table(net, cfg, gw_addr,
> -                                         cfg->fc_table, flags);
> -               if (grt) {
> -                       if (grt->rt6i_flags & RTF_GATEWAY ||
> -                           (dev && dev != grt->dst.dev)) {
> -                               ip6_rt_put(grt);
> -                               grt = NULL;
> -                       }
> -               }
> +               err = ip6_nh_lookup_table(net, cfg, gw_addr,
> +                                         cfg->fc_table, flags, &res);
> +               /* gw_addr can not require a gateway or resolve to a reject
> +                * route. If a device is given, it must match the result.
> +                */
> +               if (err || res.fib6_flags & RTF_REJECT ||
> +                   res.nh->fib_nh_gw_family ||
> +                   (dev && dev != res.nh->fib_nh_dev))
> +                       err = -EHOSTUNREACH;
>         }
>
> -       if (!grt)
> -               grt = rt6_lookup(net, gw_addr, NULL, cfg->fc_ifindex, NULL, 1);
> +       if (err < 0) {
> +               struct flowi6 fl6 = {
> +                       .flowi6_oif = cfg->fc_ifindex,
> +                       .daddr = *gw_addr,
> +               };
>
> -       if (!grt)
> -               goto out;
> +               err = fib6_lookup(net, cfg->fc_ifindex, &fl6, &res, flags);

I am not very convinced that fib6_lookup() could be equivalent to
rt6_lookup(). Specifically, rt6_lookup() calls rt6_device_match()
while fib6_lookup() calls rt6_select() to match the oif. From a brief
glance, it does seem to be similar, especially considering that saddr
is NULL. So it probably is OK?

> +               if (err || res.fib6_flags & RTF_REJECT ||
> +                   res.nh->fib_nh_gw_family)
> +                       err = -EHOSTUNREACH;
> +
> +               if (err)
> +                       return err;
> +
> +               fib6_select_path(net, &res, &fl6, cfg->fc_ifindex,
> +                                cfg->fc_ifindex != 0, NULL, flags);
> +       }
>
> +       err = 0;
>         if (dev) {
> -               if (dev != grt->dst.dev) {
> -                       ip6_rt_put(grt);
> -                       goto out;
> -               }
> +               if (dev != res.nh->fib_nh_dev)
> +                       err = -EHOSTUNREACH;
>         } else {
> -               *_dev = dev = grt->dst.dev;
> -               *idev = grt->rt6i_idev;
> +               *_dev = dev = res.nh->fib_nh_dev;
> +               *idev = __in6_dev_get(dev);
>                 dev_hold(dev);
> -               in6_dev_hold(grt->rt6i_idev);
> +               in6_dev_hold(*idev);

nit: directly do *idev = in6_dev_get(dev) instead of 2 instructions.

>         }
>
> -       if (!(grt->rt6i_flags & RTF_GATEWAY))
> -               err = 0;
> -
> -       ip6_rt_put(grt);
> -
> -out:
>         return err;
>  }
>
> @@ -3284,11 +3275,15 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg,
>                         goto out;
>                 }
>
> +               rcu_read_lock();
> +
>                 if (cfg->fc_flags & RTNH_F_ONLINK)
>                         err = ip6_route_check_nh_onlink(net, cfg, dev, extack);
>                 else
>                         err = ip6_route_check_nh(net, cfg, _dev, idev);
>
> +               rcu_read_unlock();
> +
>                 if (err)
>                         goto out;
>         }
> --
> 2.11.0
>

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH v2] e1000e: Make watchdog use delayed work
From: Brown, Aaron F @ 2019-06-20 23:17 UTC (permalink / raw)
  To: Kirsher, Jeffrey T, Detlev Casanova, David S. Miller,
	intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20190618220846.19486-1-detlev.casanova@gmail.com>

On Tue, 2019-06-18 at 18:08 -0400, Detlev Casanova wrote:
> Use delayed work instead of timers to run the watchdog of the e1000e
> driver.
> 
> Simplify the code with one less middle function.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@gmail.com>
> ---
>  drivers/net/ethernet/intel/e1000e/e1000.h  |  3 +-
>  drivers/net/ethernet/intel/e1000e/netdev.c | 54 ++++++++++++----------
>  2 files changed, 30 insertions(+), 27 deletions(-)
> 

This patch is causing a NULL pointer dereference trace (or just a freeze) on a number of my regression systems when I unload the e1000e driver.  The crashing test systems each have multiple e1000e
based ports, I can pull the systems from the rack and change cards out if necessary, though it's not a trivial task getting to them.

Systems affected had the following e1000e chipsets:
- 82579LM Gigabit Network Connection rev 05) LOM
- 82574L LOM

- I217-LM LOM
- 82572EI (Rimon)

- 80003ES2LAN x2 LOMs
- 82572EI Add in card

- 80003ES2LAN x2 LOMs
- 82571EB (Ophir) copper (dual port card)
- 82571EB (Ophir) fibre  (dual port card)

Here is the trace caputrued from the last system listed, the 80003ES2LAN LOMs and 2 dual port Ophir cards:
-------------------------------------------------------------
u1462:[ttyS1]/root> rmmod e1000e
[  131.799739] BUG: kernel NULL pointer dereference, address: 0000000000000000
[  131.806908] #PF: supervisor write access in kernel mode
[  131.812285] #PF: error_code(0x0002) - not-present page
[  131.817565] PGD 8000000072327067 P4D 8000000072327067 PUD 3050f067 PMD 0
[  131.824540] Oops: 0002 [#1] SMP PTI
[  131.828133] CPU: 3 PID: 4306 Comm: rmmod Not tainted 5.2.0-rc5_next-queue_dev-queue_15bf1c7+ #9
[  131.837073] Hardware name: Supermicro X7DBX/X7DBX, BIOS 2.1 06/23/2008
[  131.843791] RIP: 0010:_raw_spin_lock_irq+0x13/0x30
[  131.848722] Code: 00 75 02 f3 c3 e9 3d 16 8a ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 fa 66 66 90 66 66 90 31 c0 ba 01 00 00 00 <f0> 0f b1 17 0f 94 c2 84 d2 74 02 f3 c3 89 c6 e9 b2
fd 89 ff 66 0f
[  131.867998] RSP: 0018:ffffb85601f87d58 EFLAGS: 00010046
[  131.873377] RAX: 0000000000000000 RBX: ffff9a5a9faa5b90 RCX: 0000000000000003
[  131.880702] RDX: 0000000000000001 RSI: 0000000000000002 RDI: 0000000000000000
[  131.888032] RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
[  131.895356] R10: ffff9a5ab340a550 R11: ffffe13500cd0280 R12: 0000000000000003
[  131.902679] R13: 0000000000000002 R14: ffff9a5a9faa4400 R15: 0000000000000000
[  131.910003] FS:  00007f37948a9740(0000) GS:ffff9a5afcac0000(0000) knlGS:0000000000000000
[  131.918325] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  131.924229] CR2: 0000000000000000 CR3: 0000000020602000 CR4: 00000000000006e0
[  131.931557] Call Trace:
[  131.934093]  flush_workqueue_prep_pwqs+0x55/0x120
[  131.938929]  flush_workqueue+0x1b6/0x460
[  131.942987]  e1000_remove+0x93/0x190 [e1000e]
[  131.947466]  pci_device_remove+0x3b/0xc0
[  131.951511]  device_release_driver_internal+0xdf/0x1a0
[  131.956797]  driver_detach+0x43/0x80
[  131.960487]  bus_remove_driver+0x55/0xd0
[  131.964535]  pci_unregister_driver+0x26/0xa0
[  131.968938]  __x64_sys_delete_module+0x16c/0x250
[  131.973698]  ? exit_to_usermode_loop+0xaa/0xc6
[  131.978270]  do_syscall_64+0x5b/0x1b0
[  131.982041]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  131.987237] RIP: 0033:0x7f3793d82397
[  131.990923] Code: 73 01 c3 48 8b 0d f9 7a 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c9 7a 2c 00 f7
d8 64 89 01 48
[  132.010190] RSP: 002b:00007ffc6b3dea18 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[  132.017972] RAX: ffffffffffffffda RBX: 0000000001a23190 RCX: 00007f3793d82397
[  132.034233] RDX: 00007f3793df6b20 RSI: 0000000000000800 RDI: 0000000001a231f8
[  132.050479] RBP: 0000000000000000 R08: 00007f379404b060 R09: 00007f3793df6b20
[  132.066683] R10: 00007ffc6b3de5e0 R11: 0000000000000206 R12: 00007ffc6b3e0d4a
[  132.082874] R13: 0000000000000000 R14: 0000000001a23190 R15: 0000000001a23010
[  132.098961] Modules linked in: xt_CHECKSUM iptable_mangle xt_MASQUERADE iptable_nat nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ipt_REJECT nf_reject_ipv4 tun bridge stp llc
ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter dm_mirror dm_region_hash dm_log dm_mod coretemp kvm_intel kvm iTCO_wdt i2c_i801 gpio_ich iTCO_vendor_support lpc_ich pcspkr i5000_edac
sg irqbypass acpi_cpufreq i5k_amb nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c radeon sr_mod sd_mod cdrom ata_generic pata_acpi i2c_algo_bit drm_kms_helper syscopyarea
sysfillrect sysimgblt fb_sys_fops ttm drm ata_piix libata serio_raw e1000e(-) e1000
[  132.215961] CR2: 0000000000000000
[  132.229473] ---[ end trace 31a92a65bb543b98 ]---
[  132.244260] RIP: 0010:_raw_spin_lock_irq+0x13/0x30
[  132.259217] Code: 00 75 02 f3 c3 e9 3d 16 8a ff 0f 1f 00 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 90 fa 66 66 90 66 66 90 31 c0 ba 01 00 00 00 <f0> 0f b1 17 0f 94 c2 84 d2 74 02 f3 c3 89 c6 e9 b2
fd 89 ff 66 0f
[  132.299254] RSP: 0018:ffffb85601f87d58 EFLAGS: 00010046
[  132.315166] RAX: 0000000000000000 RBX: ffff9a5a9faa5b90 RCX: 0000000000000003
[  132.333000] RDX: 0000000000000001 RSI: 0000000000000002 RDI: 0000000000000000
[  132.350749] RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
[  132.368150] R10: ffff9a5ab340a550 R11: ffffe13500cd0280 R12: 0000000000000003
[  132.385257] R13: 0000000000000002 R14: ffff9a5a9faa4400 R15: 0000000000000000
[  132.402202] FS:  00007f37948a9740(0000) GS:ffff9a5afcac0000(0000) knlGS:0000000000000000
[  132.420165] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  132.435818] CR2: 0000000000000000 CR3: 0000000020602000 CR4: 00000000000006e0
[  132.453030] Kernel panic - not syncing: Fatal exception
[  132.468323] Kernel Offset: 0x7e00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[  132.489386] ---[ end Kernel panic - not syncing: Fatal exception ]---
                      

^ permalink raw reply

* [PATCH bpf-next 7/7] selftests/bpf: convert existing tracepoint tests to new APIs
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Convert existing tests that attach to tracepoints to use
bpf_program__attach_tracepoint API instead.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../bpf/prog_tests/stacktrace_build_id.c      | 49 +++-------------
 .../selftests/bpf/prog_tests/stacktrace_map.c | 42 +++-----------
 .../bpf/prog_tests/stacktrace_map_raw_tp.c    | 14 ++++-
 .../bpf/prog_tests/task_fd_query_rawtp.c      | 10 +++-
 .../bpf/prog_tests/task_fd_query_tp.c         | 51 +++++------------
 .../bpf/prog_tests/tp_attach_query.c          | 56 ++++++-------------
 6 files changed, 65 insertions(+), 157 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
index 3aab2b083c71..9ef3b66f3644 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -4,10 +4,11 @@
 void test_stacktrace_build_id(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/random/urandom_read";
 	const char *file = "./test_stacktrace_build_id.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
+	int err, pmu_fd, prog_fd, stack_trace_len;
 	__u32 key, previous_key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
 	char buf[256];
 	int i, j;
@@ -20,42 +21,14 @@ void test_stacktrace_build_id(void)
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		goto out;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/random/urandom_read/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (CHECK(bytes <= 0 || bytes >= sizeof(buf),
-		  "read", "bytes %d errno %d\n", bytes, errno))
+	pmu_fd = bpf_program__attach_tracepoint(prog, "random", "urandom_read");
+	if (CHECK(pmu_fd < 0, "attach_tp", "err %d\n", pmu_fd))
 		goto close_prog;
 
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
-		goto close_prog;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-		  err, errno))
-		goto close_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-		  err, errno))
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
@@ -133,8 +106,7 @@ void test_stacktrace_build_id(void)
 	 * try it one more time.
 	 */
 	if (build_id_matches < 1 && retry--) {
-		ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-		close(pmu_fd);
+		libbpf_perf_event_disable_and_close(pmu_fd);
 		bpf_object__close(obj);
 		printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
 		       __func__);
@@ -152,10 +124,7 @@ void test_stacktrace_build_id(void)
 	      "err %d errno %d\n", err, errno);
 
 disable_pmu:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
-	close(pmu_fd);
+	libbpf_perf_event_disable_and_close(pmu_fd);
 
 close_prog:
 	bpf_object__close(obj);
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
index 2bfd50a0d6d1..df0716e69b96 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -4,50 +4,25 @@
 void test_stacktrace_map(void)
 {
 	int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
+	const char *prog_name = "tracepoint/sched/sched_switch";
+	int efd, err, prog_fd, stack_trace_len;
 	const char *file = "./test_stacktrace_map.o";
-	int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
-	struct perf_event_attr attr = {};
 	__u32 key, val, duration = 0;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
-	char buf[256];
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 		return;
 
-	/* Get the ID for the sched/sched_switch tracepoint */
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/sched/sched_switch/id");
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
 
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (bytes <= 0 || bytes >= sizeof(buf))
-		goto close_prog;
-
-	/* Open the perf event and attach bpf progrram */
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n",
-		  pmu_fd, errno))
+	efd = bpf_program__attach_tracepoint(prog, "sched", "sched_switch");
+	if (CHECK(efd < 0, "attach_tp", "err %d\n", efd))
 		goto close_prog;
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (err)
-		goto disable_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (err)
-		goto disable_pmu;
-
 	/* find map fds */
 	control_map_fd = bpf_find_map(__func__, obj, "control_map");
 	if (control_map_fd < 0)
@@ -96,8 +71,7 @@ void test_stacktrace_map(void)
 disable_pmu:
 	error_cnt++;
 disable_pmu_noerr:
-	ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-	close(pmu_fd);
+	close(efd);
 close_prog:
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
index 1f8387d80fd7..4d14a08b1d99 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map_raw_tp.c
@@ -3,18 +3,24 @@
 
 void test_stacktrace_map_raw_tp(void)
 {
+	const char *prog_name = "tracepoint/sched/sched_switch";
 	int control_map_fd, stackid_hmap_fd, stackmap_fd;
 	const char *file = "./test_stacktrace_map.o";
-	int efd, err, prog_fd;
 	__u32 key, val, duration = 0;
+	int efd = -1, err, prog_fd;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
 
 	err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
 	if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
 		return;
 
-	efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
-	if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
+	efd = bpf_program__attach_raw_tracepoint(prog, "sched_switch");
+	if (CHECK(efd < 0, "attach_raw_tp", "err %d\n", efd))
 		goto close_prog;
 
 	/* find map fds */
@@ -55,5 +61,7 @@ void test_stacktrace_map_raw_tp(void)
 close_prog:
 	error_cnt++;
 close_prog_noerr:
+	if (efd >= 0)
+		close(efd);
 	bpf_object__close(obj);
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
index 958a3d88de99..6ad73cb8c7e3 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_rawtp.c
@@ -3,9 +3,11 @@
 
 void test_task_fd_query_rawtp(void)
 {
+	const char *prog_name = "tracepoint/raw_syscalls/sys_enter";
 	const char *file = "./test_get_stack_rawtp.o";
 	__u64 probe_offset, probe_addr;
 	__u32 len, prog_id, fd_type;
+	struct bpf_program *prog;
 	struct bpf_object *obj;
 	int efd, err, prog_fd;
 	__u32 duration = 0;
@@ -15,8 +17,12 @@ void test_task_fd_query_rawtp(void)
 	if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
 		return;
 
-	efd = bpf_raw_tracepoint_open("sys_enter", prog_fd);
-	if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+		goto close_prog;
+
+	efd = bpf_program__attach_raw_tracepoint(prog, "sys_enter");
+	if (CHECK(efd < 0, "attach_raw_tp", "err %d\n", efd))
 		goto close_prog;
 
 	/* query (getpid(), efd) */
diff --git a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
index f9b70e81682b..034870692636 100644
--- a/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
+++ b/tools/testing/selftests/bpf/prog_tests/task_fd_query_tp.c
@@ -1,15 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 
-static void test_task_fd_query_tp_core(const char *probe_name,
+static void test_task_fd_query_tp_core(const char *tp_category,
 				       const char *tp_name)
 {
+	const char *prog_name = "tracepoint/sched/sched_switch";
 	const char *file = "./test_tracepoint.o";
-	int err, bytes, efd, prog_fd, pmu_fd;
-	struct perf_event_attr attr = {};
 	__u64 probe_offset, probe_addr;
 	__u32 len, prog_id, fd_type;
+	int err, prog_fd, pmu_fd;
 	struct bpf_object *obj = NULL;
+	struct bpf_program *prog;
 	__u32 duration = 0;
 	char buf[256];
 
@@ -17,37 +18,13 @@ static void test_task_fd_query_tp_core(const char *probe_name,
 	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
 		goto close_prog;
 
-	snprintf(buf, sizeof(buf),
-		 "/sys/kernel/debug/tracing/events/%s/id", probe_name);
-	efd = open(buf, O_RDONLY, 0);
-	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+	prog = bpf_object__find_program_by_title(obj, prog_name);
+	if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
 		goto close_prog;
-	bytes = read(efd, buf, sizeof(buf));
-	close(efd);
-	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
-		  "bytes %d errno %d\n", bytes, errno))
-		goto close_prog;
-
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-	pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-			 0 /* cpu 0 */, -1 /* group id */,
-			 0 /* flags */);
-	if (CHECK(err, "perf_event_open", "err %d errno %d\n", err, errno))
-		goto close_pmu;
 
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
-	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
-		  errno))
-		goto close_pmu;
-
-	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
-	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
-		  errno))
-		goto close_pmu;
+	pmu_fd = bpf_program__attach_tracepoint(prog, tp_category, tp_name);
+	if (CHECK(pmu_fd < 0, "attach_tp", "err %d\n", pmu_fd))
+		goto close_prog;
 
 	/* query (getpid(), pmu_fd) */
 	len = sizeof(buf);
@@ -62,11 +39,11 @@ static void test_task_fd_query_tp_core(const char *probe_name,
 		  fd_type, buf))
 		goto close_pmu;
 
-	close(pmu_fd);
+	libbpf_perf_event_disable_and_close(pmu_fd);
 	goto close_prog_noerr;
 
 close_pmu:
-	close(pmu_fd);
+	libbpf_perf_event_disable_and_close(pmu_fd);
 close_prog:
 	error_cnt++;
 close_prog_noerr:
@@ -75,8 +52,6 @@ static void test_task_fd_query_tp_core(const char *probe_name,
 
 void test_task_fd_query_tp(void)
 {
-	test_task_fd_query_tp_core("sched/sched_switch",
-				   "sched_switch");
-	test_task_fd_query_tp_core("syscalls/sys_enter_read",
-				   "sys_enter_read");
+	test_task_fd_query_tp_core("sched", "sched_switch");
+	test_task_fd_query_tp_core("syscalls", "sys_enter_read");
 }
diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
index fb095e5cd9af..5e129eb3eb47 100644
--- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
+++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
@@ -6,9 +6,9 @@ void test_tp_attach_query(void)
 	const int num_progs = 3;
 	int i, j, bytes, efd, err, prog_fd[num_progs], pmu_fd[num_progs];
 	__u32 duration = 0, info_len, saved_prog_ids[num_progs];
+	const char *prog_name = "tracepoint/sched/sched_switch";
 	const char *file = "./test_tracepoint.o";
 	struct perf_event_query_bpf *query;
-	struct perf_event_attr attr = {};
 	struct bpf_object *obj[num_progs];
 	struct bpf_prog_info prog_info;
 	char buf[256];
@@ -27,19 +27,19 @@ void test_tp_attach_query(void)
 		  "read", "bytes %d errno %d\n", bytes, errno))
 		return;
 
-	attr.config = strtol(buf, NULL, 0);
-	attr.type = PERF_TYPE_TRACEPOINT;
-	attr.sample_type = PERF_SAMPLE_RAW | PERF_SAMPLE_CALLCHAIN;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
-
 	query = malloc(sizeof(*query) + sizeof(__u32) * num_progs);
 	for (i = 0; i < num_progs; i++) {
+		struct bpf_program *prog;
+
 		err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj[i],
 				    &prog_fd[i]);
 		if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
 			goto cleanup1;
 
+		prog = bpf_object__find_program_by_title(obj[i], prog_name);
+		if (CHECK(!prog, "find_prog", "prog '%s' not found\n", prog_name))
+			goto cleanup1;
+
 		bzero(&prog_info, sizeof(prog_info));
 		prog_info.jited_prog_len = 0;
 		prog_info.xlated_prog_len = 0;
@@ -51,32 +51,10 @@ void test_tp_attach_query(void)
 			goto cleanup1;
 		saved_prog_ids[i] = prog_info.id;
 
-		pmu_fd[i] = syscall(__NR_perf_event_open, &attr, -1 /* pid */,
-				    0 /* cpu 0 */, -1 /* group id */,
-				    0 /* flags */);
-		if (CHECK(pmu_fd[i] < 0, "perf_event_open", "err %d errno %d\n",
-			  pmu_fd[i], errno))
+		pmu_fd[i] = bpf_program__attach_tracepoint(prog, "sched",
+							   "sched_switch");
+		if (CHECK(pmu_fd[i] < 0, "attach_tp", "err %d\n", pmu_fd[i]))
 			goto cleanup2;
-		err = ioctl(pmu_fd[i], PERF_EVENT_IOC_ENABLE, 0);
-		if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
-			  err, errno))
-			goto cleanup3;
-
-		if (i == 0) {
-			/* check NULL prog array query */
-			query->ids_len = num_progs;
-			err = ioctl(pmu_fd[i], PERF_EVENT_IOC_QUERY_BPF, query);
-			if (CHECK(err || query->prog_cnt != 0,
-				  "perf_event_ioc_query_bpf",
-				  "err %d errno %d query->prog_cnt %u\n",
-				  err, errno, query->prog_cnt))
-				goto cleanup3;
-		}
-
-		err = ioctl(pmu_fd[i], PERF_EVENT_IOC_SET_BPF, prog_fd[i]);
-		if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
-			  err, errno))
-			goto cleanup3;
 
 		if (i == 1) {
 			/* try to get # of programs only */
@@ -86,7 +64,7 @@ void test_tp_attach_query(void)
 				  "perf_event_ioc_query_bpf",
 				  "err %d errno %d query->prog_cnt %u\n",
 				  err, errno, query->prog_cnt))
-				goto cleanup3;
+				goto cleanup2;
 
 			/* try a few negative tests */
 			/* invalid query pointer */
@@ -95,7 +73,7 @@ void test_tp_attach_query(void)
 			if (CHECK(!err || errno != EFAULT,
 				  "perf_event_ioc_query_bpf",
 				  "err %d errno %d\n", err, errno))
-				goto cleanup3;
+				goto cleanup2;
 
 			/* no enough space */
 			query->ids_len = 1;
@@ -104,7 +82,7 @@ void test_tp_attach_query(void)
 				  "perf_event_ioc_query_bpf",
 				  "err %d errno %d query->prog_cnt %u\n",
 				  err, errno, query->prog_cnt))
-				goto cleanup3;
+				goto cleanup2;
 		}
 
 		query->ids_len = num_progs;
@@ -113,21 +91,19 @@ void test_tp_attach_query(void)
 			  "perf_event_ioc_query_bpf",
 			  "err %d errno %d query->prog_cnt %u\n",
 			  err, errno, query->prog_cnt))
-			goto cleanup3;
+			goto cleanup2;
 		for (j = 0; j < i + 1; j++)
 			if (CHECK(saved_prog_ids[j] != query->ids[j],
 				  "perf_event_ioc_query_bpf",
 				  "#%d saved_prog_id %x query prog_id %x\n",
 				  j, saved_prog_ids[j], query->ids[j]))
-				goto cleanup3;
+				goto cleanup2;
 	}
 
 	i = num_progs - 1;
 	for (; i >= 0; i--) {
- cleanup3:
-		ioctl(pmu_fd[i], PERF_EVENT_IOC_DISABLE);
  cleanup2:
-		close(pmu_fd[i]);
+		libbpf_perf_event_disable_and_close(pmu_fd[i]);
  cleanup1:
 		bpf_object__close(obj[i]);
 	}
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 3/7] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Add ability to attach to kernel and user probes and retprobes.
Implementation depends on perf event support for kprobes/uprobes.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/lib/bpf/libbpf.c   | 207 +++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |   8 ++
 tools/lib/bpf/libbpf.map |   2 +
 3 files changed, 217 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2bb1fa008be3..11329e05530e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3969,6 +3969,213 @@ int bpf_program__attach_perf_event(struct bpf_program *prog, int pfd)
 	return 0;
 }
 
+static int parse_uint(const char *buf)
+{
+	int ret;
+
+	errno = 0;
+	ret = (int)strtol(buf, NULL, 10);
+	if (errno) {
+		ret = -errno;
+		pr_debug("failed to parse '%s' as unsigned int\n", buf);
+		return ret;
+	}
+	if (ret < 0) {
+		pr_debug("failed to parse '%s' as unsigned int\n", buf);
+		return -EINVAL;
+	}
+	return ret;
+}
+
+static int parse_uint_from_file(const char* file)
+{
+	char buf[STRERR_BUFSIZE];
+	int fd, ret;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		pr_debug("failed to open '%s': %s\n", file,
+			 libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	ret = read(fd, buf, sizeof(buf));
+	close(fd);
+	if (ret < 0) {
+		ret = -errno;
+		pr_debug("failed to read '%s': %s\n", file,
+			libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	if (ret == 0 || ret >= sizeof(buf)) {
+		buf[sizeof(buf) - 1] = 0;
+		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
+		return -EINVAL;
+	}
+	return parse_uint(buf);
+}
+
+static int determine_kprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/type";
+	return parse_uint_from_file(file);
+}
+
+static int determine_uprobe_perf_type(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/type";
+	return parse_uint_from_file(file);
+}
+
+static int parse_config_from_file(const char *file)
+{
+	char buf[STRERR_BUFSIZE];
+	int fd, ret;
+
+	fd = open(file, O_RDONLY);
+	if (fd < 0) {
+		ret = -errno;
+		pr_debug("failed to open '%s': %s\n", file,
+			 libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	ret = read(fd, buf, sizeof(buf));
+	close(fd);
+	if (ret < 0) {
+		ret = -errno;
+		pr_debug("failed to read '%s': %s\n", file,
+			libbpf_strerror_r(ret, buf, sizeof(buf)));
+		return ret;
+	}
+	if (ret == 0 || ret >= sizeof(buf)) {
+		buf[sizeof(buf) - 1] = 0;
+		pr_debug("unexpected input from '%s': '%s'\n", file, buf);
+		return -EINVAL;
+	}
+	if (strncmp(buf, "config:", 7)) {
+		pr_debug("expected 'config:' prefix, found '%s'\n", buf);
+		return -EINVAL;
+	}
+	return parse_uint(buf + 7);
+}
+
+static int determine_kprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
+	return parse_config_from_file(file);
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+	const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+	return parse_config_from_file(file);
+}
+
+static int perf_event_open_probe(bool uprobe, bool retprobe, const char* name,
+				 uint64_t offset, int pid)
+{
+	struct perf_event_attr attr = {};
+	char errmsg[STRERR_BUFSIZE];
+	int type, pfd, err;
+
+	type = uprobe ? determine_uprobe_perf_type()
+		      : determine_kprobe_perf_type();
+	if (type < 0) {
+		pr_warning("failed to determine %s perf type: %s\n",
+			   uprobe ? "uprobe" : "kprobe",
+			   libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
+		return type;
+	}
+	if (retprobe) {
+		int bit = uprobe ? determine_uprobe_retprobe_bit()
+				 : determine_kprobe_retprobe_bit();
+
+		if (bit < 0) {
+			pr_warning("failed to determine %s retprobe bit: %s\n",
+				   uprobe ? "uprobe" : "kprobe",
+				   libbpf_strerror_r(bit, errmsg,
+						     sizeof(errmsg)));
+			return bit;
+		}
+		attr.config |= 1 << bit;
+	}
+	attr.size = sizeof(attr);
+	attr.type = type;
+	attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
+	attr.config2 = offset;		       /* kprobe_addr or probe_offset */
+
+	/* pid filter is meaningful only for uprobes */
+	pfd = syscall(__NR_perf_event_open, &attr,
+		      pid < 0 ? -1 : pid /* pid */,
+		      pid == -1 ? 0 : -1 /* cpu */,
+		      -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
+	if (pfd < 0) {
+		err = -errno;
+		pr_warning("%s perf_event_open() failed: %s\n",
+			   uprobe ? "uprobe" : "kprobe",
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+int bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
+			       const char *func_name)
+{
+	char errmsg[STRERR_BUFSIZE];
+	int pfd, err;
+
+	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
+				    0 /* offset */, -1 /* pid */);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "kretprobe" : "kprobe", func_name,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return pfd;
+	}
+	err = bpf_program__attach_perf_event(prog, pfd);
+	if (err) {
+		libbpf_perf_event_disable_and_close(pfd);
+		pr_warning("program '%s': failed to attach to %s '%s': %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "kretprobe" : "kprobe", func_name,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
+int bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
+			       pid_t pid, const char *binary_path,
+			       size_t func_offset)
+{
+	char errmsg[STRERR_BUFSIZE];
+	int pfd, err;
+
+	pfd = perf_event_open_probe(true /* uprobe */, retprobe,
+				    binary_path, func_offset, pid);
+	if (pfd < 0) {
+		pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "uretprobe" : "uprobe",
+			   binary_path, func_offset,
+			   libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
+		return pfd;
+	}
+	err = bpf_program__attach_perf_event(prog, pfd);
+	if (err) {
+		libbpf_perf_event_disable_and_close(pfd);
+		pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
+			   bpf_program__title(prog, false),
+			   retprobe ? "uretprobe" : "uprobe",
+			   binary_path, func_offset,
+			   libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
+		return err;
+	}
+	return pfd;
+}
+
 enum bpf_perf_event_ret
 bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
 			   void **copy_mem, size_t *copy_size,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 76db1bbc0dac..a7264f06aa5f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -168,6 +168,14 @@ LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
 LIBBPF_API int libbpf_perf_event_disable_and_close(int pfd);
 LIBBPF_API int bpf_program__attach_perf_event(struct bpf_program *prog,
 					      int pfd);
+LIBBPF_API int bpf_program__attach_kprobe(struct bpf_program *prog,
+					  bool retprobe,
+					  const char *func_name);
+LIBBPF_API int bpf_program__attach_uprobe(struct bpf_program *prog,
+					  bool retprobe,
+					  pid_t pid,
+					  const char *binary_path,
+					  size_t func_offset);
 
 struct bpf_insn;
 
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d27406982b5a..1a982c2e1751 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -172,7 +172,9 @@ LIBBPF_0.0.4 {
 		btf_dump__new;
 		btf__parse_elf;
 		bpf_object__load_xattr;
+		bpf_program__attach_kprobe;
 		bpf_program__attach_perf_event;
+		bpf_program__attach_uprobe;
 		libbpf_num_possible_cpus;
 		libbpf_perf_event_disable_and_close;
 } LIBBPF_0.0.3;
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 6/7] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-06-20 23:09 UTC (permalink / raw)
  To: andrii.nakryiko, ast, daniel, netdev, bpf, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190620230951.3155955-1-andriin@fb.com>

Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
expected.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/attach_probe.c   | 151 ++++++++++++++++++
 .../selftests/bpf/progs/test_attach_probe.c   |  55 +++++++
 2 files changed, 206 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..5cc7e674a513
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,151 @@
+// 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;
+}
+
+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;
+	int kprobe_pfd = -1, kretprobe_pfd = -1;
+	int uprobe_pfd = -1, uretprobe_pfd = -1;
+	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_pfd = bpf_program__attach_kprobe(kprobe_prog,
+						false /* retprobe */,
+						"sys_nanosleep");
+	if (CHECK(kprobe_pfd < 0, "attach_kprobe", "err %d\n", kprobe_pfd))
+		goto cleanup;
+
+	kretprobe_pfd = bpf_program__attach_kprobe(kretprobe_prog,
+						   true /* retprobe */,
+						   "sys_nanosleep");
+	if (CHECK(kretprobe_pfd < 0, "attach_kretprobe",
+		  "err %d\n", kretprobe_pfd))
+		goto cleanup;
+
+	uprobe_pfd = bpf_program__attach_uprobe(uprobe_prog,
+						false /* retprobe */,
+						0 /* self pid */,
+						"/proc/self/exe",
+						uprobe_offset);
+	if (CHECK(uprobe_pfd < 0, "attach_uprobe", "err %d\n", uprobe_pfd))
+		goto cleanup;
+
+	uretprobe_pfd = bpf_program__attach_uprobe(uretprobe_prog,
+						   true /* retprobe */,
+						   -1 /* any pid */,
+						   "/proc/self/exe",
+						   uprobe_offset);
+	if (CHECK(uretprobe_pfd < 0, "attach_uretprobe",
+		  "err %d\n", uretprobe_pfd))
+		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:
+	libbpf_perf_event_disable_and_close(kprobe_pfd);
+	libbpf_perf_event_disable_and_close(kretprobe_pfd);
+	libbpf_perf_event_disable_and_close(uprobe_pfd);
+	libbpf_perf_event_disable_and_close(uretprobe_pfd);
+	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,
+};
+
+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;
-- 
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