* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28 7:23 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <d0d5dbe3-d2bf-2284-b2a3-667c77487125@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>> indication of whether it can successfully redirect to the map index it was
>> given. Instead, BPF programs have to track this themselves, leading to
>> programs using duplicate maps to track which entries are populated in the
>> devmap.
>>
>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>> helper, which makes it possible to return failure to the eBPF program. The
>> lower bits of the flags argument is used as the return code, which means
>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>>
>> With this, a BPF program can check the return code from the helper call and
>> react by, for instance, substituting a different redirect. This works for
>> any type of map used for redirect.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> [...]
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 183bf4d8e301..a6779e1cc1b8 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -3605,17 +3605,13 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
>> struct bpf_redirect_info *ri)
>> {
>> u32 index = ri->ifindex;
>> - void *fwd = NULL;
>> + void *fwd = ri->item;
>> int err;
>>
>> ri->ifindex = 0;
>> + ri->item = NULL;
>> WRITE_ONCE(ri->map, NULL);
>>
>> - fwd = __xdp_map_lookup_elem(map, index);
>> - if (unlikely(!fwd)) {
>> - err = -EINVAL;
>> - goto err;
>> - }
>
> If you look at the _trace_xdp_redirect{,_err}(), we should also get rid of the
> extra NULL test in devmap_ifindex() which is not under tracepoint static key.
ACK, will add.
-Toke
^ permalink raw reply
* pull-request: wireless-drivers 2019-06-28
From: Kalle Valo @ 2019-06-28 7:18 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel
Hi Dave,
here's a pull request to net tree for 5.2, more info below. Please let
me know if there are any problems.
Kalle
The following changes since commit c356dc4b540edd6c02b409dd8cf3208ba2804c38:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2019-06-21 22:23:35 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git tags/wireless-drivers-for-davem-2019-06-28
for you to fetch changes up to 2a92b08b18553c101115423bd34963b1a59a45a3:
mt76: usb: fix rx A-MSDU support (2019-06-27 19:48:36 +0300)
----------------------------------------------------------------
wireless-drivers fixes for 5.2
Hopefully the last set of fixes for 5.2. Nothing special this around,
only small fixes and support for new cards.
iwlwifi
* add new cards for 22000 series and smaller fixes
wl18xx
* fix a clang warning about unused variables
mwifiex
* properly handle small vendor IEs (a regression from the recent
security fix)
ath
* fix few SPDX tags
mt76
* fix A-MSDU aggregation which got broken in v5.2-rc1
----------------------------------------------------------------
Brian Norris (1):
mwifiex: Don't abort on small, spec-compliant vendor IEs
Ihab Zhaika (3):
iwlwifi: add new cards for 22000 and fix struct name
iwlwifi: add new cards for 22000 and change wrong structs
iwlwifi: change 0x02F0 fw from qu to quz
Kalle Valo (1):
ath: fix SPDX tags
Lorenzo Bianconi (1):
mt76: usb: fix rx A-MSDU support
Nathan Huckleberry (1):
wl18xx: Fix Wunused-const-variable
Oren Givon (1):
iwlwifi: add support for hr1 RF ID
drivers/net/wireless/ath/Kconfig | 2 +-
drivers/net/wireless/ath/Makefile | 2 +-
drivers/net/wireless/ath/ar5523/Kconfig | 2 +-
drivers/net/wireless/ath/ar5523/Makefile | 2 +-
drivers/net/wireless/ath/ath10k/Kconfig | 2 +-
drivers/net/wireless/ath/ath5k/Kconfig | 2 +-
drivers/net/wireless/ath/ath5k/Makefile | 2 +-
drivers/net/wireless/ath/ath6kl/Kconfig | 2 +-
drivers/net/wireless/ath/ath6kl/trace.h | 2 +-
drivers/net/wireless/ath/ath9k/Kconfig | 2 +-
drivers/net/wireless/ath/ath9k/Makefile | 2 +-
drivers/net/wireless/ath/wcn36xx/Kconfig | 2 +-
drivers/net/wireless/ath/wcn36xx/Makefile | 2 +-
drivers/net/wireless/ath/wil6210/Kconfig | 2 +-
drivers/net/wireless/ath/wil6210/Makefile | 2 +-
drivers/net/wireless/intel/iwlwifi/cfg/22000.c | 144 +++++++++++++-
drivers/net/wireless/intel/iwlwifi/iwl-config.h | 14 +-
drivers/net/wireless/intel/iwlwifi/iwl-csr.h | 1 +
drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 241 ++++++++++++-----------
drivers/net/wireless/intel/iwlwifi/pcie/trans.c | 17 +-
drivers/net/wireless/marvell/mwifiex/fw.h | 12 +-
drivers/net/wireless/marvell/mwifiex/scan.c | 18 +-
drivers/net/wireless/marvell/mwifiex/sta_ioctl.c | 4 +-
drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt76.h | 1 +
drivers/net/wireless/mediatek/mt76/usb.c | 46 ++++-
drivers/net/wireless/ti/wl18xx/main.c | 38 ----
27 files changed, 364 insertions(+), 204 deletions(-)
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Daniel Santos @ 2019-06-28 7:16 UTC (permalink / raw)
To: Andrew Lunn
Cc: René van Dorst, sean.wang, f.fainelli, davem, matthias.bgg,
vivien.didelot, frank-w, netdev, linux-mediatek, linux-mips
In-Reply-To: <20190627192806.GQ27733@lunn.ch>
Hello Andrew,
On 6/27/19 2:28 PM, Andrew Lunn wrote:
>>> Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
>>> want the MAC directly talking to the PHY. Bad things will happen.
>> So what exactly do you mean by the MAC directly talking to the PHY? Do
>> you mean setting speed, duplex, etc. via the MAC registers instead of
>> via MDIO to the MII registers of the PHY?
> The data sheet implies the MAC hardware performs reads on the PHY to
> get the status, and then uses that to configure the MAC. This is often
> called PHY polling. The MAC hardware however has no idea what the PHY
> driver is doing. The MAC does not take the PHY mutex. So the PHY
> driver might be doing something at the same time the MAC hardware
> polls the PHY, and it gets odd results. Some PHYs have multiple pages,
> and for example reading the temperature sensor means swapping
> pages. If the MAC hardware was to poll while the sensor is being read,
> it would not get the link status, it would read some random
> temperature register.
>
> So you want the PHY driver to read the results of auto-neg and it then
> tell the MAC the results, so the MAC can be configured correctly.
Thank you, this is very helpful! I finally understand why these
settings are in two different places. :) Unfortunately this driver (in
OpenWRT) does a lot of "magic" during init to registers that I don't
have documentation for, but I see where auto-polling can be disabled now.
>>> Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
>>> phydev->asym_pause.
>>>
>>> The same idea applies when using phylink.
>>>
>>> Andrew
>> You're help is greatly appreciated here. Admittedly, I'm also trying to
>> get this working in the now deprecated swconfig for a 3.18 kernel that's
>> in production.
> I'm not very familiar with swconfig. Is there software driving the
> PHY? If not, it is then safe for the MAC hardware to poll the PHY.
>
> Andrew
swconfig is an netlink-based interface the OpenWRT team developed for
configuring switches before DSA was converted into a vendor-neutral
interface. Now that DSA does what swconfig was designed for it has been
deprecated, although (to my knowledge) we don't yet have DSA for all
devices that OpenWRT supports.
Daniel
^ permalink raw reply
* Re: [PATCH bpf-next v5 2/3] bpf_xdp_redirect_map: Perform map lookup in eBPF helper
From: Toke Høiland-Jørgensen @ 2019-06-28 7:17 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <04a5da1d-6d0e-5963-4622-20cb54285926@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The bpf_redirect_map() helper used by XDP programs doesn't return any
>> indication of whether it can successfully redirect to the map index it was
>> given. Instead, BPF programs have to track this themselves, leading to
>> programs using duplicate maps to track which entries are populated in the
>> devmap.
>>
>> This patch fixes this by moving the map lookup into the bpf_redirect_map()
>> helper, which makes it possible to return failure to the eBPF program. The
>> lower bits of the flags argument is used as the return code, which means
>> that existing users who pass a '0' flag argument will get XDP_ABORTED.
>>
>> With this, a BPF program can check the return code from the helper call and
>> react by, for instance, substituting a different redirect. This works for
>> any type of map used for redirect.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
> Overall series looks good to me. Just very small things inline here & in the
> other two patches:
>
> [...]
>> @@ -3750,9 +3742,16 @@ BPF_CALL_3(bpf_xdp_redirect_map, struct bpf_map *, map, u32, ifindex,
>> {
>> struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
>>
>> - if (unlikely(flags))
>> + /* Lower bits of the flags are used as return code on lookup failure */
>> + if (unlikely(flags > XDP_TX))
>> return XDP_ABORTED;
>>
>> + ri->item = __xdp_map_lookup_elem(map, ifindex);
>> + if (unlikely(!ri->item)) {
>> + WRITE_ONCE(ri->map, NULL);
>
> This WRITE_ONCE() is not needed. We never set it before at this point.
You mean the WRITE_ONCE() wrapper is not needed, or the set-to-NULL is
not needed? The reason I added it is in case an eBPF program calls the
helper twice before returning, where the first lookup succeeds but the
second fails; in that case we want to clear the ->map pointer, no?
-Toke
^ permalink raw reply
* Re: XDP multi-buffer incl. jumbo-frames (Was: [RFC V1 net-next 1/1] net: ena: implement XDP drop support)
From: Eelco Chaudron @ 2019-06-28 7:14 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Machulsky, Zorik, Jubran, Samih, davem, netdev, Woodhouse, David,
Matushevsky, Alexander, Bshara, Saeed, Wilson, Matt,
Liguori, Anthony, Bshara, Nafea, Tzalik, Guy, Belgazal, Netanel,
Saidi, Ali, Herrenschmidt, Benjamin, Kiyanovski, Arthur,
Daniel Borkmann, Toke Høiland-Jørgensen,
Ilias Apalodimas, Alexei Starovoitov, Jakub Kicinski, xdp-newbies
In-Reply-To: <20190626103829.5360ef2d@carbon>
On 26 Jun 2019, at 10:38, Jesper Dangaard Brouer wrote:
> On Tue, 25 Jun 2019 03:19:22 +0000
> "Machulsky, Zorik" <zorik@amazon.com> wrote:
>
>> On 6/23/19, 7:21 AM, "Jesper Dangaard Brouer" <brouer@redhat.com>
>> wrote:
>>
>> On Sun, 23 Jun 2019 10:06:49 +0300 <sameehj@amazon.com> wrote:
>>
>> > This commit implements the basic functionality of drop/pass
>> logic in the
>> > ena driver.
>>
>> Usually we require a driver to implement all the XDP return
>> codes,
>> before we accept it. But as Daniel and I discussed with Zorik
>> during
>> NetConf[1], we are going to make an exception and accept the
>> driver
>> if you also implement XDP_TX.
>>
>> As we trust that Zorik/Amazon will follow and implement
>> XDP_REDIRECT
>> later, given he/you wants AF_XDP support which requires
>> XDP_REDIRECT.
>>
>> Jesper, thanks for your comments and very helpful discussion during
>> NetConf! That's the plan, as we agreed. From our side I would like to
>> reiterate again the importance of multi-buffer support by xdp frame.
>> We would really prefer not to see our MTU shrinking because of xdp
>> support.
>
> Okay we really need to make a serious attempt to find a way to support
> multi-buffer packets with XDP. With the important criteria of not
> hurting performance of the single-buffer per packet design.
>
> I've created a design document[2], that I will update based on our
> discussions: [2]
> https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp-multi-buffer01-design.org
>
> The use-case that really convinced me was Eric's packet header-split.
>
>
> Lets refresh: Why XDP don't have multi-buffer support:
>
> XDP is designed for maximum performance, which is why certain
> driver-level
> use-cases were not supported, like multi-buffer packets (like
> jumbo-frames).
> As it e.g. complicated the driver RX-loop and memory model handling.
>
> The single buffer per packet design, is also tied into eBPF
> Direct-Access
> (DA) to packet data, which can only be allowed if the packet memory is
> in
> contiguous memory. This DA feature is essential for XDP performance.
>
>
> One way forward is to define that XDP only get access to the first
> packet buffer, and it cannot see subsequent buffers. For XDP_TX and
> XDP_REDIRECT to work then XDP still need to carry pointers (plus
> len+offset) to the other buffers, which is 16 bytes per extra buffer.
I’ve seen various network processor HW designs, and they normally get
the first x bytes (128 - 512) which they can manipulate
(append/prepend/insert/modify/delete).
There are designs where they can “page in” the additional fragments
but it’s expensive as it requires additional memory transfers. But the
majority do not care (cannot change) the remaining fragments. Can also
not think of a reason why you might want to remove something at the end
of the frame (thinking about routing/forwarding needs here).
If we do want XDP to access other fragments we could do this through a
helper which swaps the packet context?
//Eelco
^ permalink raw reply
* Re: [PATCH bpf-next v5 1/3] devmap/cpumap: Use flush list instead of bitmap
From: Toke Høiland-Jørgensen @ 2019-06-28 7:14 UTC (permalink / raw)
To: Daniel Borkmann, netdev
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, David Miller,
Jonathan Lemon
In-Reply-To: <ff82dde7-8f31-1ab5-65b8-5e2d5ca5f680@iogearbox.net>
Daniel Borkmann <daniel@iogearbox.net> writes:
> On 06/23/2019 04:17 AM, Toke Høiland-Jørgensen wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The socket map uses a linked list instead of a bitmap to keep track of
>> which entries to flush. Do the same for devmap and cpumap, as this means we
>> don't have to care about the map index when enqueueing things into the
>> map (and so we can cache the map lookup).
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> [...]
>> +static int bq_flush_to_queue(struct xdp_bulk_queue *bq, bool in_napi_ctx)
>> {
>> + struct bpf_cpu_map_entry *rcpu = bq->obj;
>> unsigned int processed = 0, drops = 0;
>> const int to_cpu = rcpu->cpu;
>> struct ptr_ring *q;
>> @@ -621,6 +630,9 @@ static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
>> bq->count = 0;
>> spin_unlock(&q->producer_lock);
>>
>> + __list_del(bq->flush_node.prev, bq->flush_node.next);
>> + bq->flush_node.prev = NULL;
>
> Given this and below is a bit non-standard way of using list API, maybe add
> these as inline helpers to include/linux/list.h to make sure anyone changing
> list API semantics doesn't overlook these in future?
Sure, can do.
-Toke
^ permalink raw reply
* [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Miaohe Lin @ 2019-06-28 9:06 UTC (permalink / raw)
To: pablo, kadlec, fw, davem, kuznet, yoshfuji, netfilter-devel,
coreteam, netdev, linux-kernel
Cc: linmiaohe, mingfangsen
When firewalld is enabled with ipv4/ipv6 rpfilter, vrf
ipv4/ipv6 packets will be dropped. Vrf device will pass
through netfilter hook twice. One with enslaved device
and another one with l3 master device. So in device may
dismatch witch out device because out device is always
enslaved device.So failed with the check of the rpfilter
and drop the packets by mistake.
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
net/ipv4/netfilter/ipt_rpfilter.c | 1 +
net/ipv6/netfilter/ip6t_rpfilter.c | 8 +++++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 59031670b16a..cc23f1ce239c 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -78,6 +78,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
flow.flowi4_tos = RT_TOS(iph->tos);
flow.flowi4_scope = RT_SCOPE_UNIVERSE;
+ flow.flowi4_oif = l3mdev_master_ifindex_rcu(xt_in(par));
return rpfilter_lookup_reverse(xt_net(par), &flow, xt_in(par), info->flags) ^ invert;
}
diff --git a/net/ipv6/netfilter/ip6t_rpfilter.c b/net/ipv6/netfilter/ip6t_rpfilter.c
index 6bcaf7357183..3c4a1772c15f 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -55,6 +55,10 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
if (rpfilter_addr_linklocal(&iph->saddr)) {
lookup_flags |= RT6_LOOKUP_F_IFACE;
fl6.flowi6_oif = dev->ifindex;
+ /* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
+ } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
+ lookup_flags |= FLOWI_FLAG_SKIP_NH_OIF;
+ fl6.flowi6_oif = dev->ifindex;
} else if ((flags & XT_RPFILTER_LOOSE) == 0)
fl6.flowi6_oif = dev->ifindex;
@@ -70,7 +74,9 @@ static bool rpfilter_lookup_reverse6(struct net *net, const struct sk_buff *skb,
goto out;
}
- if (rt->rt6i_idev->dev == dev || (flags & XT_RPFILTER_LOOSE))
+ if (rt->rt6i_idev->dev == dev ||
+ l3mdev_master_ifindex_rcu(rt->rt6i_idev->dev) == dev->ifindex ||
+ (flags & XT_RPFILTER_LOOSE))
ret = true;
out:
ip6_rt_put(rt);
--
2.21.GIT
^ permalink raw reply related
* Re: [RFC, PATCH 2/2, net-next] net: netsec: add XDP support
From: Ilias Apalodimas @ 2019-06-28 6:54 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
magnus.karlsson, brouer, daniel, ast, makita.toshiaki,
jakub.kicinski, john.fastabend, davem
In-Reply-To: <20190627161816.0000645a@gmail.com>
Hi Maciej Fijalkowski,
[...]
> > + tx_ctrl.cksum_offload_flag = false;
> > + tx_ctrl.tcp_seg_offload_flag = false;
> > + tx_ctrl.tcp_seg_len = 0;
>
> Aren't these three lines redundant? tx_ctrl is zero initialized.
>
Yea i think i can remove those
> > +
> > + tx_desc.dma_addr = dma_handle;
> > + tx_desc.addr = xdpf->data;
> > + tx_desc.len = xdpf->len;
> > +
> > + netsec_set_tx_de(priv, tx_ring, &tx_ctrl, &tx_desc, xdpf);
> > +
> > + return NETSEC_XDP_TX;
> > +}
> > +
> > +static u32 netsec_xdp_xmit_back(struct netsec_priv *priv, struct xdp_buff *xdp)
> > +{
> > + struct netsec_desc_ring *tx_ring = &priv->desc_ring[NETSEC_RING_TX];
> > + struct xdp_frame *xdpf = convert_to_xdp_frame(xdp);
> > + u32 ret;
> > +
> > + if (unlikely(!xdpf))
> > + return NETSEC_XDP_CONSUMED;
> > +
> > + spin_lock(&tx_ring->lock);
> > + ret = netsec_xdp_queue_one(priv, xdpf, false);
> > + spin_unlock(&tx_ring->lock);
> > +
> > + return ret;
> > +}
> > +
> > +static u32 netsec_run_xdp(struct netsec_priv *priv, struct bpf_prog *prog,
> > + struct xdp_buff *xdp)
> > +{
> > + u32 ret = NETSEC_XDP_PASS;
> > + int err;
> > + u32 act;
> > +
> > + rcu_read_lock();
> > + act = bpf_prog_run_xdp(prog, xdp);
> > +
> > + switch (act) {
> > + case XDP_PASS:
> > + ret = NETSEC_XDP_PASS;
> > + break;
> > + case XDP_TX:
> > + ret = netsec_xdp_xmit_back(priv, xdp);
> > + if (ret != NETSEC_XDP_TX)
> > + xdp_return_buff(xdp);
> > + break;
> > + case XDP_REDIRECT:
> > + err = xdp_do_redirect(priv->ndev, xdp, prog);
> > + if (!err) {
> > + ret = NETSEC_XDP_REDIR;
> > + } else {
> > + ret = NETSEC_XDP_CONSUMED;
> > + xdp_return_buff(xdp);
> > + }
> > + break;
> > + default:
> > + bpf_warn_invalid_xdp_action(act);
> > + /* fall through */
> > + case XDP_ABORTED:
> > + trace_xdp_exception(priv->ndev, prog, act);
> > + /* fall through -- handle aborts by dropping packet */
> > + case XDP_DROP:
> > + ret = NETSEC_XDP_CONSUMED;
> > + xdp_return_buff(xdp);
> > + break;
> > + }
> > +
> > + rcu_read_unlock();
> > +
> > + return ret;
> > +}
> > +
> > static int netsec_process_rx(struct netsec_priv *priv, int budget)
> > {
> > struct netsec_desc_ring *dring = &priv->desc_ring[NETSEC_RING_RX];
> > + struct bpf_prog *xdp_prog = READ_ONCE(priv->xdp_prog);
>
> Reading BPF prog should be RCU protected. There might be a case where RCU
> callback that destroys BPF prog is executed during the bottom half handling and
> you have the PREEMPT_RCU=y in your kernel config. I've just rephrased Brenden's
> words here, so for further info, see:
>
> https://lore.kernel.org/netdev/20160904042958.8594-1-bblanco@plumgrid.com/
>
> So either expand the RCU section or read prog pointer per each frame, under the
> lock, as it seems that currently we have these two schemes in drivers that
> support XDP.
>
Thanks, i'll fix it
Cheers
/Ilias
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Jesper Dangaard Brouer @ 2019-06-28 6:35 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend, brouer
In-Reply-To: <20190627220245.GA3269@khorivan>
On Fri, 28 Jun 2019 01:02:47 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> Hi Jesper, thanks you remember about it.
>
> >
> >I don't think that "create" and "free" routines paring looks "more
> >correct" together.
> >
> >Maybe we can scale back your solution(?), via creating a page_pool_get()
> >and page_pool_put() API that can be used by your driver, to keep the
> >page_pool object after a xdp_rxq_info_unreg() call. Then you can use
> >it for two xdp_rxq_info structs, and call page_pool_put() after you
> >have unregistered both.
> >
> >The API would basically be:
> >
> >diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> >index b366f59885c1..691ddacfb5a6 100644
> >--- a/net/core/page_pool.c
> >+++ b/net/core/page_pool.c
> >@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
> > void __page_pool_free(struct page_pool *pool)
> > {
> > WARN(pool->alloc.count, "API usage violation");
> >+
> >+ if (atomic_read(&pool->user_cnt) != 0)
> >+ return;
> >+
> > WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
> >
> > /* Can happen due to forced shutdown */
> >@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
> > }
> > EXPORT_SYMBOL(__page_pool_free);
> >
> >+void page_pool_put(struct page_pool *pool)
> >+{
> >+ if (!atomic_dec_and_test(&pool->user_cnt))
> >+ __page_pool_free(pool);
> >+}
> >+EXPORT_SYMBOL(page_pool_put);
> >+
> >+void page_pool_get(struct page_pool *pool)
> >+{
> >+ atomic_inc(&pool->user_cnt);
> >+}
> >+EXPORT_SYMBOL(page_pool_get);
> >+
>
> I have another solution that doesn't touch page pool and adds modifications
> to xdp allocator. As for me it looks better and work wider, I don't need to
> think about this in the driver also.
>
> It's supposed allocator works as before, no any changes to mlx5 and
> page_pool API and its usage and seems like fits your requirements.
> It still supposes that allocator runs under same napi softirq but allows
> to reuse allocator.
>
> I have not verified yet, but looks like:
>
> diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
> index 6a8cba6ea79a..995b21da2f27 100644
> --- a/include/net/xdp_priv.h
> +++ b/include/net/xdp_priv.h
> @@ -18,6 +18,7 @@ struct xdp_mem_allocator {
> struct rcu_head rcu;
> struct delayed_work defer_wq;
> unsigned long defer_warn;
> + unsigned long refcnt;
> };
>
> #endif /* __LINUX_NET_XDP_PRIV_H__ */
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index f98ab6b98674..6239483e3793 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -98,6 +98,12 @@ static bool __mem_id_disconnect(int id, bool force)
> WARN(1, "Request remove non-existing id(%d), driver bug?", id);
> return true;
> }
> +
> + if (--xa->refcnt) {
> + mutex_unlock(&mem_id_lock);
> + return true;
This doesn't work. This function __mem_id_disconnect() can be called
multiple times. E.g. if there are in-flight packets.
> + }
> +
> xa->disconnect_cnt++;
>
> /* Detects in-flight packet-pages for page_pool */
> @@ -312,6 +318,33 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
> return true;
> }
>
> +static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
> +{
> + struct xdp_mem_allocator *xae, *xa == NULL;
> + struct rhashtable_iter iter;
> +
> + mutex_lock(&mem_id_lock);
> + rhashtable_walk_enter(mem_id_ht, &iter);
> + do {
> + rhashtable_walk_start(&iter);
> +
> + while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
> + if (xae->allocator == allocator) {
> + xae->refcnt++;
> + xa = xae;
> + break;
> + }
> + }
> +
> + rhashtable_walk_stop(&iter);
> +
> + } while (xae == ERR_PTR(-EAGAIN));
> + rhashtable_walk_exit(&iter);
> + mutex_unlock(&mem_id_lock);
> +
> + return xa;
> +}
> +
> int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> enum xdp_mem_type type, void *allocator)
> {
> @@ -347,6 +380,9 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> }
> }
>
> + if (xdp_allocator_get(allocator))
> + return 0;
> +
> xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
> if (!xdp_alloc)
> return -ENOMEM;
> @@ -360,6 +396,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
> xdp_rxq->mem.id = id;
> xdp_alloc->mem = xdp_rxq->mem;
> xdp_alloc->allocator = allocator;
> + xdp_alloc->refcnt = 1;
>
> /* Insert allocator into ID lookup table */
> ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
>
>
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH 2/3 nf-next] netfilter:nf_flow_table: Support bridge type flow offload
From: Florian Westphal @ 2019-06-28 6:06 UTC (permalink / raw)
To: wenxu; +Cc: Pablo Neira Ayuso, Florian Westphal, netfilter-devel, netdev
In-Reply-To: <b2a48653-9f30-18a9-d0e1-eaa940a361a9@ucloud.cn>
wenxu <wenxu@ucloud.cn> wrote:
> ns21 iperf to 10.0.0.8 with dport 22 in ns22
> first time with OFFLOAD enable
>
> nft add flowtable bridge firewall fb2 { hook ingress priority 0 \; devices = { veth21, veth22 } \; }
> nft add chain bridge firewall ftb-all {type filter hook forward priority 0 \; policy accept \; }
> nft add rule bridge firewall ftb-all counter ct zone 2 ip protocol tcp flow offload @fb2
>
> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
[..]
> [ 3] 0.0-60.0 sec 353 GBytes 50.5 Gbits/sec
>
> The second time on any offload:
> # iperf -c 10.0.0.8 -p 22 -t 60 -i2
> [ 3] 0.0-60.0 sec 271 GBytes 38.8 Gbits/sec
Wow, this is pretty impressive. Do you have numbers without
offload and no connection tracking?
Is this with CONFIG_RETPOLINE=y (just curious)?
^ permalink raw reply
* Re: [PATCH] netfilter: Fix remainder of pseudo-header protocol 0
From: He Zhe @ 2019-06-28 6:03 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <20190627184903.atdcwk4wnfaayyer@salvia>
On 6/28/19 2:49 AM, Pablo Neira Ayuso wrote:
> On Mon, Jun 24, 2019 at 11:17:38AM +0800, zhe.he@windriver.com wrote:
>> From: He Zhe <zhe.he@windriver.com>
>>
>> Since v5.1-rc1, some types of packets do not get unreachable reply with the
>> following iptables setting. Fox example,
>>
>> $ iptables -A INPUT -p icmp --icmp-type 8 -j REJECT
>> $ ping 127.0.0.1 -c 1
>> PING 127.0.0.1 (127.0.0.1) 56(84) bytes of data.
>> — 127.0.0.1 ping statistics —
>> 1 packets transmitted, 0 received, 100% packet loss, time 0ms
>>
>> We should have got the following reply from command line, but we did not.
>> From 127.0.0.1 icmp_seq=1 Destination Port Unreachable
>>
>> Yi Zhao reported it and narrowed it down to:
>> 7fc38225363d ("netfilter: reject: skip csum verification for protocols that don't support it"),
>>
>> This is because nf_ip_checksum still expects pseudo-header protocol type 0 for
>> packets that are of neither TCP or UDP, and thus ICMP packets are mistakenly
>> treated as TCP/UDP.
>>
>> This patch corrects the conditions in nf_ip_checksum and all other places that
>> still call it with protocol 0.
> Looking at 7fc38225363dd8f19e667ad7c77b63bc4a5c065d, I wonder this can
> be fixed while simplifying it...
>
> I think nf_reject_verify_csum() is useless?
>
> In your patch, now you explicitly check for IPPROTO_TCP and
> IPPROTO_UDP to validate the checksum.
Thanks for your review.
I suppose the two main points of 7fc38225363d are valid and I was trying to
align with them and fix them:
1) Skip csum verification for protocols that don't support it.
2) Remove the protocol 0 used to indicate non-TCP/UDP packets, and use actual
types instead to be clear.
1) uses nf_reject_verify_csum to skip those that should be skipped and leaves
the protocols that support csum to the rest of the logic including
nf_ip_checksum. But 2) removes the "0" transition from the rest of the
logic and thus causes this issue. So I add the explicit check against TCP/UDP to
nf_ip_checksum. And nf_reject_verify_csum is still useful.
Zhe
>
^ permalink raw reply
* [PATCH v3 bpf-next 9/9] selftests/bpf: convert existing tracepoint tests to new APIs
From: Andrii Nakryiko @ 2019-06-28 5:53 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
Convert some existing tests that attach to tracepoints to use
bpf_program__attach_tracepoint API instead.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
.../bpf/prog_tests/stacktrace_build_id.c | 50 ++++---------------
.../selftests/bpf/prog_tests/stacktrace_map.c | 43 ++++------------
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 15 ++++--
3 files changed, 31 insertions(+), 77 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..768883d838ea 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
@@ -4,11 +4,13 @@
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, prog_fd, stack_trace_len;
__u32 key, previous_key, val, duration = 0;
+ struct bpf_program *prog;
struct bpf_object *obj;
+ struct bpf_link *link = NULL;
char buf[256];
int i, j;
struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
@@ -20,42 +22,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))
+ link = bpf_program__attach_tracepoint(prog, "random", "urandom_read");
+ if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
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 +107,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);
+ bpf_link__destroy(link);
bpf_object__close(obj);
printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
__func__);
@@ -152,10 +125,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);
+ bpf_link__destroy(link);
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..fc539335c5b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
@@ -4,50 +4,26 @@
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 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];
+ struct bpf_link *link;
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))
+ link = bpf_program__attach_tracepoint(prog, "sched", "sched_switch");
+ if (CHECK(IS_ERR(link), "attach_tp", "err %ld\n", PTR_ERR(link)))
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 (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 +72,7 @@ void test_stacktrace_map(void)
disable_pmu:
error_cnt++;
disable_pmu_noerr:
- ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
- close(pmu_fd);
+ bpf_link__destroy(link);
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..fbfa8e76cf63 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,25 @@
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 err, prog_fd;
+ struct bpf_program *prog;
struct bpf_object *obj;
+ struct bpf_link *link = NULL;
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;
+
+ link = bpf_program__attach_raw_tracepoint(prog, "sched_switch");
+ if (CHECK(IS_ERR(link), "attach_raw_tp", "err %ld\n", PTR_ERR(link)))
goto close_prog;
/* find map fds */
@@ -55,5 +62,7 @@ void test_stacktrace_map_raw_tp(void)
close_prog:
error_cnt++;
close_prog_noerr:
+ if (!IS_ERR_OR_NULL(link))
+ bpf_link__destroy(link);
bpf_object__close(obj);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-06-28 5:53 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
Add tests verifying kprobe/kretprobe/uprobe/uretprobe APIs work as
expected.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
.../selftests/bpf/prog_tests/attach_probe.c | 155 ++++++++++++++++++
.../selftests/bpf/progs/test_attach_probe.c | 55 +++++++
2 files changed, 210 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..f22929063c58
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,155 @@
+// 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;
+ struct bpf_link *kprobe_link = NULL;
+ struct bpf_link *kretprobe_link = NULL;
+ struct bpf_link *uprobe_link = NULL;
+ struct bpf_link *uretprobe_link = NULL;
+ int results_map_fd;
+ size_t uprobe_offset;
+ ssize_t base_addr;
+
+ base_addr = get_base_addr();
+ if (CHECK(base_addr < 0, "get_base_addr",
+ "failed to find base addr: %zd", base_addr))
+ return;
+ uprobe_offset = (size_t)&get_base_addr - base_addr;
+
+ /* load programs */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
+ if (CHECK(!kprobe_prog, "find_probe",
+ "prog '%s' not found\n", kprobe_name))
+ goto cleanup;
+ kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
+ if (CHECK(!kretprobe_prog, "find_probe",
+ "prog '%s' not found\n", kretprobe_name))
+ goto cleanup;
+ uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
+ if (CHECK(!uprobe_prog, "find_probe",
+ "prog '%s' not found\n", uprobe_name))
+ goto cleanup;
+ uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
+ if (CHECK(!uretprobe_prog, "find_probe",
+ "prog '%s' not found\n", uretprobe_name))
+ goto cleanup;
+
+ /* load maps */
+ results_map_fd = bpf_find_map(__func__, obj, "results_map");
+ if (CHECK(results_map_fd < 0, "find_results_map",
+ "err %d\n", results_map_fd))
+ goto cleanup;
+
+ kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
+ false /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
+ "err %ld\n", PTR_ERR(kprobe_link)))
+ goto cleanup;
+
+ kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
+ true /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
+ "err %ld\n", PTR_ERR(kretprobe_link)))
+ goto cleanup;
+
+ uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
+ false /* retprobe */,
+ 0 /* self pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
+ "err %ld\n", PTR_ERR(uprobe_link)))
+ goto cleanup;
+
+ uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
+ true /* retprobe */,
+ -1 /* any pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
+ "err %ld\n", PTR_ERR(uretprobe_link)))
+ goto cleanup;
+
+ /* trigger & validate kprobe && kretprobe */
+ usleep(1);
+
+ err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
+ if (CHECK(err, "get_kprobe_res",
+ "failed to get kprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
+ "wrong kprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
+ if (CHECK(err, "get_kretprobe_res",
+ "failed to get kretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
+ "wrong kretprobe res: %d\n", res))
+ goto cleanup;
+
+ /* trigger & validate uprobe & uretprobe */
+ get_base_addr();
+
+ err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
+ if (CHECK(err, "get_uprobe_res",
+ "failed to get uprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
+ "wrong uprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
+ if (CHECK(err, "get_uretprobe_res",
+ "failed to get uretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
+ "wrong uretprobe res: %d\n", res))
+ goto cleanup;
+
+cleanup:
+ bpf_link__destroy(kprobe_link);
+ bpf_link__destroy(kretprobe_link);
+ bpf_link__destroy(uprobe_link);
+ bpf_link__destroy(uretprobe_link);
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
new file mode 100644
index 000000000000..7a7c5cd728c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ int type;
+ int max_entries;
+ int *key;
+ int *value;
+} results_map SEC(".maps") = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .max_entries = 4,
+};
+
+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
* [PATCH v3 bpf-next 7/9] selftests/bpf: switch test to new attach_perf_event API
From: Andrii Nakryiko @ 2019-06-28 5:53 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
Use new bpf_program__attach_perf_event() in test previously relying on
direct ioctl manipulations.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 31 +++++++++----------
1 file changed, 15 insertions(+), 16 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..9557b7dfb782 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,7 +26,9 @@ 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;
+ struct bpf_link *link;
char buf[256];
int i, j;
struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH];
@@ -39,6 +42,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,15 +54,12 @@ 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))
- 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;
+ link = bpf_program__attach_perf_event(prog, pmu_fd);
+ if (CHECK(IS_ERR(link), "attach_perf_event",
+ "err %ld\n", PTR_ERR(link))) {
+ close(pmu_fd);
+ goto close_prog;
+ }
/* find map fds */
control_map_fd = bpf_find_map(__func__, obj, "control_map");
@@ -134,8 +138,7 @@ void test_stacktrace_build_id_nmi(void)
* try it one more time.
*/
if (build_id_matches < 1 && retry--) {
- ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
- close(pmu_fd);
+ bpf_link__destroy(link);
bpf_object__close(obj);
printf("%s:WARN:Didn't find expected build ID from the map, retrying\n",
__func__);
@@ -154,11 +157,7 @@ void test_stacktrace_build_id_nmi(void)
*/
disable_pmu:
- ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
- close(pmu_fd);
-
+ bpf_link__destroy(link);
close_prog:
bpf_object__close(obj);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 6/9] libbpf: add raw tracepoint attach API
From: Andrii Nakryiko @ 2019-06-28 5:53 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
Add a wrapper utilizing bpf_link "infrastructure" to allow attaching BPF
programs to raw tracepoints.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 36 ++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 +++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 40 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 76d1599a7d56..a2a98faa9642 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4307,6 +4307,42 @@ struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
return link;
}
+static int bpf_link__destroy_fd(struct bpf_link *link)
+{
+ struct bpf_link_fd *l = (void *)link;
+
+ return close(l->fd);
+}
+
+struct bpf_link *bpf_program__attach_raw_tracepoint(struct bpf_program *prog,
+ const char *tp_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link_fd *link;
+ 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 ERR_PTR(-EINVAL);
+ }
+
+ link = malloc(sizeof(*link));
+ link->link.destroy = &bpf_link__destroy_fd;
+
+ pfd = bpf_raw_tracepoint_open(tp_name, bpf_fd);
+ if (pfd < 0) {
+ free(link);
+ 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 ERR_PTR(pfd);
+ }
+ link->fd = pfd;
+ return (struct bpf_link *)link;
+}
+
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 60611f4b4e1d..f55933784f95 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -182,6 +182,9 @@ LIBBPF_API struct bpf_link *
bpf_program__attach_tracepoint(struct bpf_program *prog,
const char *tp_category,
const char *tp_name);
+LIBBPF_API struct bpf_link *
+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 3c618b75ef65..e6b7d4edbc93 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ 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;
btf_dump__dump_type;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 5/9] libbpf: add tracepoint attach API
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
Allow attaching BPF programs to kernel tracepoint BPF hooks specified by
category and name.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 78 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 4 +++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 83 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 65d2fef41003..76d1599a7d56 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4229,6 +4229,84 @@ struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
return link;
}
+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);
+ 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;
+ }
+
+ 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;
+}
+
+struct bpf_link *bpf_program__attach_tracepoint(struct bpf_program *prog,
+ const char *tp_category,
+ const char *tp_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link *link;
+ 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 ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ 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 link;
+ }
+ return link;
+}
+
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 bd767cc11967..60611f4b4e1d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -178,6 +178,10 @@ LIBBPF_API struct bpf_link *
bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
pid_t pid, const char *binary_path,
size_t func_offset);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_tracepoint(struct bpf_program *prog,
+ const char *tp_category,
+ const char *tp_name);
struct bpf_insn;
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 57a40fb60718..3c618b75ef65 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -171,6 +171,7 @@ LIBBPF_0.0.4 {
bpf_object__load_xattr;
bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
+ bpf_program__attach_tracepoint;
bpf_program__attach_uprobe;
btf_dump__dump_type;
btf_dump__free;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-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 | 213 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 ++
tools/lib/bpf/libbpf.map | 2 +
3 files changed, 222 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 606705f878ba..65d2fef41003 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4016,6 +4016,219 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
return (struct bpf_link *)link;
}
+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));
+ ret = ret < 0 ? -errno : ret;
+ close(fd);
+ if (ret < 0) {
+ 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));
+ ret = ret < 0 ? -errno : ret;
+ close(fd);
+ if (ret < 0) {
+ 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;
+}
+
+struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
+ bool retprobe,
+ const char *func_name)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link *link;
+ 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 ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ 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 link;
+ }
+ return link;
+}
+
+struct bpf_link *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];
+ struct bpf_link *link;
+ 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 ERR_PTR(pfd);
+ }
+ link = bpf_program__attach_perf_event(prog, pfd);
+ if (IS_ERR(link)) {
+ close(pfd);
+ err = PTR_ERR(link);
+ 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 link;
+ }
+ return link;
+}
+
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 1bf66c4a9330..bd767cc11967 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
LIBBPF_API struct bpf_link *
bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
+LIBBPF_API struct bpf_link *
+bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
+ const char *func_name);
+LIBBPF_API struct bpf_link *
+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 756f5aa802e9..57a40fb60718 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
global:
bpf_link__destroy;
bpf_object__load_xattr;
+ bpf_program__attach_kprobe;
bpf_program__attach_perf_event;
+ bpf_program__attach_uprobe;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
bpf_program__attach_perf_event allows to attach BPF program to existing
perf event hook, providing most generic and most low-level way to attach BPF
programs. It returns struct bpf_link, which should be passed to
bpf_link__destroy to detach and free resources, associated with a link.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 58 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 +++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 62 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 455795e6f8af..606705f878ba 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>
@@ -3958,6 +3959,63 @@ int bpf_link__destroy(struct bpf_link *link)
return err;
}
+struct bpf_link_fd {
+ struct bpf_link link; /* has to be at the top of struct */
+ int fd; /* hook FD */
+};
+
+static int bpf_link__destroy_perf_event(struct bpf_link *link)
+{
+ struct bpf_link_fd *l = (void *)link;
+ int err;
+
+ if (l->fd < 0)
+ return 0;
+
+ err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
+ close(l->fd);
+ return err;
+}
+
+struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
+ int pfd)
+{
+ char errmsg[STRERR_BUFSIZE];
+ struct bpf_link_fd *link;
+ 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 ERR_PTR(-EINVAL);
+ }
+
+ link = malloc(sizeof(*link));
+ if (!link)
+ return ERR_PTR(-ENOMEM);
+ link->link.destroy = &bpf_link__destroy_perf_event;
+ link->fd = pfd;
+
+ if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, bpf_fd) < 0) {
+ err = -errno;
+ free(link);
+ 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_PTR(err);
+ }
+ if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ free(link);
+ 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_PTR(err);
+ }
+ return (struct bpf_link *)link;
+}
+
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 5082a5ebb0c2..1bf66c4a9330 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -169,6 +169,9 @@ struct bpf_link;
LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
+LIBBPF_API struct bpf_link *
+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 3cde850fc8da..756f5aa802e9 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
global:
bpf_link__destroy;
bpf_object__load_xattr;
+ bpf_program__attach_perf_event;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
bpf_link is and abstraction of an association of a BPF program and one
of many possible BPF attachment points (hooks). This allows to have
uniform interface for detaching BPF programs regardless of the nature of
link and how it was created. Details of creation and setting up of
a specific bpf_link is handled by corresponding attachment methods
(bpf_program__attach_xxx) added in subsequent commits. Once successfully
created, bpf_link has to be eventually destroyed with
bpf_link__destroy(), at which point BPF program is disassociated from
a hook and all the relevant resources are freed.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
tools/lib/bpf/libbpf.h | 4 ++++
tools/lib/bpf/libbpf.map | 3 ++-
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6e6ebef11ba3..455795e6f8af 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
return 0;
}
+struct bpf_link {
+ int (*destroy)(struct bpf_link *link);
+};
+
+int bpf_link__destroy(struct bpf_link *link)
+{
+ int err;
+
+ if (!link)
+ return 0;
+
+ err = link->destroy(link);
+ free(link);
+
+ return err;
+}
+
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..5082a5ebb0c2 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);
+struct bpf_link;
+
+LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
+
struct bpf_insn;
/*
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..3cde850fc8da 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
LIBBPF_0.0.4 {
global:
+ bpf_link__destroy;
+ bpf_object__load_xattr;
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
btf__parse_elf;
- bpf_object__load_xattr;
libbpf_num_possible_cpus;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 1/9] libbpf: make libbpf_strerror_r agnostic to sign of error
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
In-Reply-To: <20190628055303.1249758-1-andriin@fb.com>
It's often inconvenient to switch sign of error when passing it into
libbpf_strerror_r. It's better for it to handle that automatically.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/str_error.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index 00e48ac5b806..b8064eedc177 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -11,7 +11,7 @@
*/
char *libbpf_strerror_r(int err, char *dst, int len)
{
- int ret = strerror_r(err, dst, len);
+ int ret = strerror_r(err < 0 ? -err : err, dst, len);
if (ret)
snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret);
return dst;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs
From: Andrii Nakryiko @ 2019-06-28 5:52 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, netdev, bpf, sdf, kernel-team
Cc: Andrii Nakryiko
This patchset adds the following APIs to allow attaching BPF programs to
tracing entities:
- bpf_program__attach_perf_event for attaching to any opened perf event FD,
allowing users full control;
- bpf_program__attach_kprobe for attaching to kernel probes (both entry and
return probes);
- bpf_program__attach_uprobe for attaching to user probes (both entry/return);
- bpf_program__attach_tracepoint for attaching to kernel tracepoints;
- bpf_program__attach_raw_tracepoint for attaching to raw kernel tracepoint
(wrapper around bpf_raw_tracepoint_open);
This set of APIs makes libbpf more useful for tracing applications.
All attach APIs return abstract struct bpf_link that encapsulates logic of
detaching BPF program. See patch #2 for details. bpf_assoc was considered as
an alternative name for this opaque "handle", but bpf_link seems to be
appropriate semantically and is nice and short.
Pre-patch #1 makes internal libbpf_strerror_r helper function work w/ negative
error codes, lifting the burder off callers to keep track of error sign.
Patch #2 adds bpf_link abstraction.
Patch #3 adds attach_perf_event, which is the base for all other APIs.
Patch #4 adds kprobe/uprobe APIs.
Patch #5 adds tracepoint API.
Patch #6 adds raw_tracepoint API.
Patch #7 converts one existing test to use attach_perf_event.
Patch #8 adds new kprobe/uprobe tests.
Patch #9 converts some selftests currently using tracepoint to new APIs.
v2->v3:
- added bpf_link concept (Daniel);
- didn't add generic bpf_link__attach_program for reasons described in [0];
- dropped Stanislav's Reviewed-by from patches #2-#6, in case he doesn't like
the change;
v1->v2:
- preserve errno before close() call (Stanislav);
- use libbpf_perf_event_disable_and_close in selftest (Stanislav);
- remove unnecessary memset (Stanislav);
[0] https://lore.kernel.org/bpf/CAEf4BzZ7EM5eP2eaZn7T2Yb5QgVRiwAs+epeLR1g01TTx-6m6Q@mail.gmail.com/
Andrii Nakryiko (9):
libbpf: make libbpf_strerror_r agnostic to sign of error
libbpf: introduce concept of bpf_link
libbpf: add ability to attach/detach BPF program to perf event
libbpf: add kprobe/uprobe attach API
libbpf: add tracepoint attach API
libbpf: add raw tracepoint attach API
selftests/bpf: switch test to new attach_perf_event API
selftests/bpf: add kprobe/uprobe selftests
selftests/bpf: convert existing tracepoint tests to new APIs
tools/lib/bpf/libbpf.c | 402 ++++++++++++++++++
tools/lib/bpf/libbpf.h | 21 +
tools/lib/bpf/libbpf.map | 8 +-
tools/lib/bpf/str_error.c | 2 +-
.../selftests/bpf/prog_tests/attach_probe.c | 155 +++++++
.../bpf/prog_tests/stacktrace_build_id.c | 50 +--
.../bpf/prog_tests/stacktrace_build_id_nmi.c | 31 +-
.../selftests/bpf/prog_tests/stacktrace_map.c | 43 +-
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 15 +-
.../selftests/bpf/progs/test_attach_probe.c | 55 +++
10 files changed, 687 insertions(+), 95 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
--
2.17.1
^ permalink raw reply
* Re: [PATCH] sis900: remove TxIDLE
From: David Miller @ 2019-06-28 5:38 UTC (permalink / raw)
To: sergej.benilov; +Cc: venza, netdev
In-Reply-To: <20190624212102.15844-1-sergej.benilov@googlemail.com>
From: Sergej Benilov <sergej.benilov@googlemail.com>
Date: Mon, 24 Jun 2019 23:21:02 +0200
> Before "sis900: fix TX completion" patch, TX completion was done on TxIDLE interrupt.
> TX completion also was the only thing done on TxIDLE interrupt.
> Since "sis900: fix TX completion", TX completion is done on TxDESC interrupt.
> So it is not necessary any more to set and to check for TxIDLE.
>
> Eliminate TxIDLE from sis900.
> Correct some typos, too.
>
> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net] tipc: add dst_cache support for udp media
From: David Miller @ 2019-06-28 5:37 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, jon.maloy, ying.xue, tipc-discussion, pabeni
In-Reply-To: <0ea2e8519f14d5c9e7bb7ba82a5be371bd4cb9ab.1561028621.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Thu, 20 Jun 2019 19:03:41 +0800
> As other udp/ip tunnels do, tipc udp media should also have a
> lockless dst_cache supported on its tx path.
>
> Here we add dst_cache into udp_replicast to support dst cache
> for both rmcast and rcast, and rmcast uses ub->rcast and each
> rcast uses its own node in ub->rcast.list.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied to net-next.
^ permalink raw reply
* RE: [EXT] [PATCH V4] bnx2x: Prevent ptp_task to be rescheduled indefinitely
From: Sudarsana Reddy Kalluru @ 2019-06-28 5:22 UTC (permalink / raw)
To: Guilherme G. Piccoli, GR-everest-linux-l2, netdev@vger.kernel.org
Cc: Ariel Elior, jay.vosburgh@canonical.com
In-Reply-To: <20190627163133.5990-1-gpiccoli@canonical.com>
> -----Original Message-----
> From: Guilherme G. Piccoli <gpiccoli@canonical.com>
> Sent: Thursday, June 27, 2019 10:02 PM
> To: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>;
> netdev@vger.kernel.org; Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Ariel Elior <aelior@marvell.com>; gpiccoli@canonical.com;
> jay.vosburgh@canonical.com
> Subject: [EXT] [PATCH V4] bnx2x: Prevent ptp_task to be rescheduled
> indefinitely
>
> External Email
>
> ----------------------------------------------------------------------
> Currently bnx2x ptp worker tries to read a register with timestamp
> information in case of TX packet timestamping and in case it fails, the routine
> reschedules itself indefinitely. This was reported as a kworker always at 100%
> of CPU usage, which was narrowed down to be bnx2x ptp_task.
>
> By following the ioctl handler, we could narrow down the problem to an NTP
> tool (chrony) requesting HW timestamping from bnx2x NIC with RX filter
> zeroed; this isn't reproducible for example with ptp4l (from linuxptp) since
> this tool requests a supported RX filter.
> It seems NIC FW timestamp mechanism cannot work well with
> RX_FILTER_NONE - driver's PTP filter init routine skips a register write to the
> adapter if there's not a supported filter request.
>
> This patch addresses the problem of bnx2x ptp thread's everlasting
> reschedule by retrying the register read 10 times; between the read
> attempts the thread sleeps for an increasing amount of time starting in 1ms
> to give FW some time to perform the timestamping. If it still fails after all
> retries, we bail out in order to prevent an unbound resource consumption
> from bnx2x.
>
> The patch also adds an ethtool statistic for accounting the skipped TX
> timestamp packets and it reduces the priority of timestamping error
> messages to prevent log flooding. The code was tested using both linuxptp
> and chrony.
>
> Reported-and-tested-by: Przemyslaw Hausman
> <przemyslaw.hausman@canonical.com>
> Suggested-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@canonical.com>
> ---
>
> Thanks again for your review Sudarsana. I've addressed in this V4 your
> suggestions about removing some debug messages[0].
>
> [0] https://marc.info/?l=linux-netdev&m=156165243804760
>
> .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 5 ++-
> .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 4 ++-
> .../net/ethernet/broadcom/bnx2x/bnx2x_main.c | 33 ++++++++++++++----
> - .../net/ethernet/broadcom/bnx2x/bnx2x_stats.h | 3 ++
> 4 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> index 008ad0ca89ba..c12c1bab0fe4 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
> @@ -3857,9 +3857,12 @@ netdev_tx_t bnx2x_start_xmit(struct sk_buff
> *skb, struct net_device *dev)
>
> if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
> if (!(bp->flags & TX_TIMESTAMPING_EN)) {
> + bp->eth_stats.ptp_skip_tx_ts++;
> BNX2X_ERR("Tx timestamping was not enabled, this
> packet will not be timestamped\n");
> } else if (bp->ptp_tx_skb) {
> - BNX2X_ERR("The device supports only a single
> outstanding packet to timestamp, this packet will not be timestamped\n");
> + bp->eth_stats.ptp_skip_tx_ts++;
> + netdev_err_once(bp->dev,
> + "Device supports only a single
> outstanding packet to timestamp,
> +this packet won't be timestamped\n");
> } else {
> skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> /* schedule check for Tx timestamp */ diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> index 51fc845de31a..4a0ba6801c9e 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
> @@ -182,7 +182,9 @@ static const struct {
> { STATS_OFFSET32(driver_filtered_tx_pkt),
> 4, false, "driver_filtered_tx_pkt" },
> { STATS_OFFSET32(eee_tx_lpi),
> - 4, true, "Tx LPI entry count"}
> + 4, true, "Tx LPI entry count"},
> + { STATS_OFFSET32(ptp_skip_tx_ts),
> + 4, false, "ptp_skipped_tx_tstamp" },
> };
>
> #define BNX2X_NUM_STATS ARRAY_SIZE(bnx2x_stats_arr)
> diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> index 03ac10b1cd1e..2cc14db8f0ec 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
> @@ -15214,11 +15214,24 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
> u32 val_seq;
> u64 timestamp, ns;
> struct skb_shared_hwtstamps shhwtstamps;
> + bool bail = true;
> + int i;
>
> - /* Read Tx timestamp registers */
> - val_seq = REG_RD(bp, port ? NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> - NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> - if (val_seq & 0x10000) {
> + /* FW may take a while to complete timestamping; try a bit and if it's
> + * still not complete, may indicate an error state - bail out then.
> + */
> + for (i = 0; i < 10; i++) {
> + /* Read Tx timestamp registers */
> + val_seq = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_SEQID :
> + NIG_REG_P0_TLLH_PTP_BUF_SEQID);
> + if (val_seq & 0x10000) {
> + bail = false;
> + break;
> + }
> + msleep(1 << i);
> + }
> +
> + if (!bail) {
> /* There is a valid timestamp value */
> timestamp = REG_RD(bp, port ?
> NIG_REG_P1_TLLH_PTP_BUF_TS_MSB :
> NIG_REG_P0_TLLH_PTP_BUF_TS_MSB);
> @@ -15233,16 +15246,18 @@ static void bnx2x_ptp_task(struct work_struct
> *work)
> memset(&shhwtstamps, 0, sizeof(shhwtstamps));
> shhwtstamps.hwtstamp = ns_to_ktime(ns);
> skb_tstamp_tx(bp->ptp_tx_skb, &shhwtstamps);
> - dev_kfree_skb_any(bp->ptp_tx_skb);
> - bp->ptp_tx_skb = NULL;
>
> DP(BNX2X_MSG_PTP, "Tx timestamp, timestamp cycles =
> %llu, ns = %llu\n",
> timestamp, ns);
> } else {
> - DP(BNX2X_MSG_PTP, "There is no valid Tx timestamp
> yet\n");
> - /* Reschedule to keep checking for a valid timestamp value
> */
> - schedule_work(&bp->ptp_task);
> + DP(BNX2X_MSG_PTP,
> + "Tx timestamp is not recorded (register read=%u)\n",
> + val_seq);
> + bp->eth_stats.ptp_skip_tx_ts++;
> }
> +
> + dev_kfree_skb_any(bp->ptp_tx_skb);
> + bp->ptp_tx_skb = NULL;
> }
>
> void bnx2x_set_rx_ts(struct bnx2x *bp, struct sk_buff *skb) diff --git
> a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> index b2644ed13d06..d55e63692cf3 100644
> --- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> +++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h
> @@ -207,6 +207,9 @@ struct bnx2x_eth_stats {
> u32 driver_filtered_tx_pkt;
> /* src: Clear-on-Read register; Will not survive PMF Migration */
> u32 eee_tx_lpi;
> +
> + /* PTP */
> + u32 ptp_skip_tx_ts;
> };
>
> struct bnx2x_eth_q_stats {
> --
> 2.22.0
Thanks for the changes.
Acked-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
^ permalink raw reply
* [PATCH net v3] vxlan: do not destroy fdb if register_netdevice() is failed
From: Taehee Yoo @ 2019-06-28 5:07 UTC (permalink / raw)
To: davem, roopa, petrm, netdev; +Cc: ap420073
__vxlan_dev_create() destroys FDB using specific pointer which indicates
a fdb when error occurs.
But that pointer should not be used when register_netdevice() fails because
register_netdevice() internally destroys fdb when error occurs.
This patch makes vxlan_fdb_create() to do not link fdb entry to vxlan dev
internally.
Instead, a new function vxlan_fdb_insert() is added to link fdb to vxlan
dev.
vxlan_fdb_insert() is called after calling register_netdevice().
This routine can avoid situation that ->ndo_uninit() destroys fdb entry
in error path of register_netdevice().
Hence, error path of __vxlan_dev_create() routine can have an opportunity
to destroy default fdb entry by hand.
Test command
ip link add bonding_masters type vxlan id 0 group 239.1.1.1 \
dev enp0s9 dstport 4789
Splat looks like:
[ 213.392816] kasan: GPF could be caused by NULL-ptr deref or user memory access
[ 213.401257] general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 213.402178] CPU: 0 PID: 1414 Comm: ip Not tainted 5.2.0-rc5+ #256
[ 213.402178] RIP: 0010:vxlan_fdb_destroy+0x120/0x220 [vxlan]
[ 213.402178] Code: df 48 8b 2b 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 06 01 00 00 4c 8b 63 08 48 b8 00 00 00 00 00 fc d
[ 213.402178] RSP: 0018:ffff88810cb9f0a0 EFLAGS: 00010202
[ 213.402178] RAX: dffffc0000000000 RBX: ffff888101d4a8c8 RCX: 0000000000000000
[ 213.402178] RDX: 1bd5a00000000040 RSI: ffff888101d4a8c8 RDI: ffff888101d4a8d0
[ 213.402178] RBP: 0000000000000000 R08: fffffbfff22b72d9 R09: 0000000000000000
[ 213.402178] R10: 00000000ffffffef R11: 0000000000000000 R12: dead000000000200
[ 213.402178] R13: ffff88810cb9f1f8 R14: ffff88810efccda0 R15: ffff88810efccda0
[ 213.402178] FS: 00007f7f6621a0c0(0000) GS:ffff88811b000000(0000) knlGS:0000000000000000
[ 213.402178] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 213.402178] CR2: 000055746f0807d0 CR3: 00000001123e0000 CR4: 00000000001006f0
[ 213.402178] Call Trace:
[ 213.402178] __vxlan_dev_create+0x3a9/0x7d0 [vxlan]
[ 213.402178] ? vxlan_changelink+0x740/0x740 [vxlan]
[ 213.402178] ? rcu_read_unlock+0x60/0x60 [vxlan]
[ 213.402178] ? __kasan_kmalloc.constprop.3+0xa0/0xd0
[ 213.402178] vxlan_newlink+0x8d/0xc0 [vxlan]
[ 213.402178] ? __vxlan_dev_create+0x7d0/0x7d0 [vxlan]
[ 213.554119] ? __netlink_ns_capable+0xc3/0xf0
[ 213.554119] __rtnl_newlink+0xb75/0x1180
[ 213.554119] ? rtnl_link_unregister+0x230/0x230
[ ... ]
Fixes: 0241b836732f ("vxlan: fix default fdb entry netlink notify ordering during netdev create")
Suggested-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v2 -> v3 :
- Use function name vxlan_fdb_insert() instead of vxlan_fdb_link()
- Add and use __vxlan_fdb_free() instead of call_rcu()
v1 -> v2 :
- Add a new function vxlan_fdb_link().
- Fix fdb entry leak.
- Update description.
drivers/net/vxlan.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 083f3f0bf37f..b4283f52a09d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -804,6 +804,14 @@ static struct vxlan_fdb *vxlan_fdb_alloc(struct vxlan_dev *vxlan,
return f;
}
+static void vxlan_fdb_insert(struct vxlan_dev *vxlan, const u8 *mac,
+ __be32 src_vni, struct vxlan_fdb *f)
+{
+ ++vxlan->addrcnt;
+ hlist_add_head_rcu(&f->hlist,
+ vxlan_fdb_head(vxlan, mac, src_vni));
+}
+
static int vxlan_fdb_create(struct vxlan_dev *vxlan,
const u8 *mac, union vxlan_addr *ip,
__u16 state, __be16 port, __be32 src_vni,
@@ -829,18 +837,13 @@ static int vxlan_fdb_create(struct vxlan_dev *vxlan,
return rc;
}
- ++vxlan->addrcnt;
- hlist_add_head_rcu(&f->hlist,
- vxlan_fdb_head(vxlan, mac, src_vni));
-
*fdb = f;
return 0;
}
-static void vxlan_fdb_free(struct rcu_head *head)
+static void __vxlan_fdb_free(struct vxlan_fdb *f)
{
- struct vxlan_fdb *f = container_of(head, struct vxlan_fdb, rcu);
struct vxlan_rdst *rd, *nd;
list_for_each_entry_safe(rd, nd, &f->remotes, list) {
@@ -850,6 +853,13 @@ static void vxlan_fdb_free(struct rcu_head *head)
kfree(f);
}
+static void vxlan_fdb_free(struct rcu_head *head)
+{
+ struct vxlan_fdb *f = container_of(head, struct vxlan_fdb, rcu);
+
+ __vxlan_fdb_free(f);
+}
+
static void vxlan_fdb_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
bool do_notify, bool swdev_notify)
{
@@ -977,6 +987,7 @@ static int vxlan_fdb_update_create(struct vxlan_dev *vxlan,
if (rc < 0)
return rc;
+ vxlan_fdb_insert(vxlan, mac, src_vni, f);
rc = vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f), RTM_NEWNEIGH,
swdev_notify, extack);
if (rc)
@@ -3571,12 +3582,17 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
if (err)
goto errout;
- /* notify default fdb entry */
if (f) {
+ vxlan_fdb_insert(vxlan, all_zeros_mac,
+ vxlan->default_dst.remote_vni, f);
+
+ /* notify default fdb entry */
err = vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f),
RTM_NEWNEIGH, true, extack);
- if (err)
- goto errout;
+ if (err) {
+ vxlan_fdb_destroy(vxlan, f, false, false);
+ goto unregister;
+ }
}
list_add(&vxlan->next, &vn->vxlan_list);
@@ -3588,7 +3604,8 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
* destroy the entry by hand here.
*/
if (f)
- vxlan_fdb_destroy(vxlan, f, false, false);
+ __vxlan_fdb_free(f);
+unregister:
if (unregister)
unregister_netdevice(dev);
return err;
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox