* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
From: David Miller @ 2019-07-02 2:02 UTC (permalink / raw)
To: marcelo.leitner; +Cc: netdev, lucien.xin, nhorman, linux-sctp, hdanton
In-Reply-To: <bcbc85604e53843a731a79df620d5f92b194d085.1561675505.git.marcelo.leitner@gmail.com>
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 27 Jun 2019 19:48:10 -0300
> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated. When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail. In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state. On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
>
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
>
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
>
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [Patch net] netrom: fix a memory leak in nr_rx_frame()
From: David Miller @ 2019-07-02 2:01 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, syzbot+d6636a36d3c34bd88938
In-Reply-To: <20190627213058.3028-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 27 Jun 2019 14:30:58 -0700
> When the skb is associated with a new sock, just assigning
> it to skb->sk is not sufficient, we have to set its destructor
> to free the sock properly too.
>
> Reported-by: syzbot+d6636a36d3c34bd88938@syzkaller.appspotmail.com
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 00/16] mlxsw: PTP timestamping support
From: David Miller @ 2019-07-02 1:59 UTC (permalink / raw)
To: idosch; +Cc: netdev, richardcochran, jiri, petrm, mlxsw, idosch
In-Reply-To: <20190630060500.7882-1-idosch@idosch.org>
From: Ido Schimmel <idosch@idosch.org>
Date: Sun, 30 Jun 2019 09:04:44 +0300
> This is the second patchset adding PTP support in mlxsw. Next patchset
> will add PTP shapers which are required to maintain accuracy under rates
> lower than 40Gb/s, while subsequent patchsets will add tracepoints and
> selftests.
...
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Andy Lutomirski @ 2019-07-02 1:59 UTC (permalink / raw)
To: Song Liu
Cc: Andy Lutomirski, linux-security@vger.kernel.org, Networking, bpf,
Alexei Starovoitov, Daniel Borkmann, Kernel Team, Lorenz Bauer,
Jann Horn, Greg KH, Linux API, Kees Cook
In-Reply-To: <0DE7F23E-9CD2-4F03-82B5-835506B59056@fb.com>
On Mon, Jul 1, 2019 at 2:03 AM Song Liu <songliubraving@fb.com> wrote:
>
> Hi Andy,
>
> Thanks for these detailed analysis.
>
> > On Jun 30, 2019, at 8:12 AM, Andy Lutomirski <luto@kernel.org> wrote:
> >
> > On Fri, Jun 28, 2019 at 12:05 PM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> Hi Andy,
> >>
> >>> On Jun 27, 2019, at 4:40 PM, Andy Lutomirski <luto@kernel.org> wrote:
> >>>
> >>> On 6/27/19 1:19 PM, Song Liu wrote:
> >>>> This patch introduce unprivileged BPF access. The access control is
> >>>> achieved via device /dev/bpf. Users with write access to /dev/bpf are able
> >>>> to call sys_bpf().
> >>>> Two ioctl command are added to /dev/bpf:
> >>>> The two commands enable/disable permission to call sys_bpf() for current
> >>>> task. This permission is noted by bpf_permitted in task_struct. This
> >>>> permission is inherited during clone(CLONE_THREAD).
> >>>> Helper function bpf_capable() is added to check whether the task has got
> >>>> permission via /dev/bpf.
> >>>
> >>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >>>> index 0e079b2298f8..79dc4d641cf3 100644
> >>>> --- a/kernel/bpf/verifier.c
> >>>> +++ b/kernel/bpf/verifier.c
> >>>> @@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
> >>>> env->insn_aux_data[i].orig_idx = i;
> >>>> env->prog = *prog;
> >>>> env->ops = bpf_verifier_ops[env->prog->type];
> >>>> - is_priv = capable(CAP_SYS_ADMIN);
> >>>> + is_priv = bpf_capable(CAP_SYS_ADMIN);
> >>>
> >>> Huh? This isn't a hardening measure -- the "is_priv" verifier mode allows straight-up leaks of private kernel state to user mode.
> >>>
> >>> (For that matter, the pending lockdown stuff should possibly consider this a "confidentiality" issue.)
> >>>
> >>>
> >>> I have a bigger issue with this patch, though: it's a really awkward way to pretend to have capabilities. For bpf, it seems like you could make this be a *real* capability without too much pain since there's only one syscall there. Just find a way to pass an fd to /dev/bpf into the syscall. If this means you need a new bpf_with_cap() syscall that takes an extra argument, so be it. The old bpf() syscall can just translate to bpf_with_cap(..., -1).
> >>>
> >>> For a while, I've considered a scheme I call "implicit rights". There would be a directory in /dev called /dev/implicit_rights. This would either be part of devtmpfs or a whole new filesystem -- it would *not* be any other filesystem. The contents would be files that can't be read or written and exist only in memory. You create them with a privileged syscall. Certain actions that are sensitive but not at the level of CAP_SYS_ADMIN (use of large-attack-surface bpf stuff, creation of user namespaces, profiling the kernel, etc) could require an "implicit right". When you do them, if you don't have CAP_SYS_ADMIN, the kernel would do a path walk for, say, /dev/implicit_rights/bpf and, if the object exists, can be opened, and actually refers to the "bpf" rights object, then the action is allowed. Otherwise it's denied.
> >>>
> >>> This is extensible, and it doesn't require the rather ugly per-task state of whether it's enabled.
> >>>
> >>> For things like creation of user namespaces, there's an existing API, and the default is that it works without privilege. Switching it to an implicit right has the benefit of not requiring code changes to programs that already work as non-root.
> >>>
> >>> But, for BPF in particular, this type of compatibility issue doesn't exist now. You already can't use most eBPF functionality without privilege. New bpf-using programs meant to run without privilege are *new*, so they can use a new improved API. So, rather than adding this obnoxious ioctl, just make the API explicit, please.
> >>>
> >>> Also, please cc: linux-abi next time.
> >>
> >> Thanks for your inputs.
> >>
> >> I think we need to clarify the use case here. In this case, we are NOT
> >> thinking about creating new tools for unprivileged users. Instead, we
> >> would like to use existing tools without root.
> >
> > I read patch 4, and I interpret it very differently. Patches 2-4 are
> > creating a new version of libbpf and a new version of bpftool. Given
> > this, I see no real justification for adding a new in-kernel per-task
> > state instead of just pushing the complexity into libbpf.
>
> I am not sure whether we are on the same page. Let me try an example,
> say we have application A, which calls sys_bpf().
>
> Before the series: we have to run A with root;
> After the series: we add a special user with access to /dev/bpf, and
> run A with this special user.
>
> If we look at the whole system, I would say we are more secure after
> the series.
>
> I am not trying to make an extreme example here, because this use case
> is the motivation here.
>
> To stay safe, we have to properly manage the permission of /dev/bpf.
> This is just like we need to properly manage access to /etc/sudoers and
> /dev/mem.
>
> Does this make sense?
>
I think I'm understanding your motivation. You're not trying to make
bpf() generically usable without privilege -- you're trying to create
a way to allow certain users to access dangerous bpf functionality
within some limits.
That's a perfectly fine goal, but I think you're reinventing the
wheel, and the wheel you're reinventing is quite complicated and
already exists. I think you should teach bpftool to be secure when
installed setuid root or with fscaps enabled and put your policy in
bpftool. If you want to harden this a little bit, it would seem
entirely reasonable to add a new CAP_BPF_ADMIN and change some, but
not all, of the capable() checks to check CAP_BPF_ADMIN instead of the
capabilities that they currently check.
Your example of /etc/sudoers is apt, and it does not involve any
kernel support :)
^ permalink raw reply
* Re: [PATCH RESEND 4.9.y] net: check before dereferencing netdev_ops during busy poll
From: Josh Elsasser @ 2019-07-02 1:56 UTC (permalink / raw)
To: David Miller; +Cc: stable, gregkh, netdev, edumazet, mcroce
In-Reply-To: <20190701.185156.2142325894415755085.davem@davemloft.net>
On Jul 1, 2019, at 6:51 PM, David Miller <davem@davemloft.net> wrote:
> I just tried to apply this with "git am" to the current v4.19 -stable
> branch and it failed.
This is only needed for the v4.9 stable kernel, ndo_busy_poll (and this NPE)
went away in kernel 4.11.
Sorry, I probably should have called that out more explicitly.
^ permalink raw reply
* [PATCH v5] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: Miaohe Lin @ 2019-07-02 3:59 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, 7 insertions(+), 2 deletions(-)
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..d800801a5dd2 100644
--- a/net/ipv6/netfilter/ip6t_rpfilter.c
+++ b/net/ipv6/netfilter/ip6t_rpfilter.c
@@ -55,7 +55,9 @@ 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;
- } else if ((flags & XT_RPFILTER_LOOSE) == 0)
+ /* Set flowi6_oif for vrf devices to lookup route in l3mdev domain. */
+ } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev) ||
+ (flags & XT_RPFILTER_LOOSE) == 0)
fl6.flowi6_oif = dev->ifindex;
rt = (void *)ip6_route_lookup(net, &fl6, skb, lookup_flags);
@@ -70,7 +72,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: [PATCH RESEND 4.9.y] net: check before dereferencing netdev_ops during busy poll
From: David Miller @ 2019-07-02 1:51 UTC (permalink / raw)
To: jelsasser; +Cc: stable, gregkh, netdev, edumazet, mcroce
In-Reply-To: <20190701234143.72631-1-jelsasser@appneta.com>
From: Josh Elsasser <jelsasser@appneta.com>
Date: Mon, 1 Jul 2019 16:41:43 -0700
> No changes since V2[1], resent as per discussiond on -stable[2]. I hope
> this is the correct way to send net fixes for older LTS releases, I'm
> going off of the latest netdev FAQ:
I just tried to apply this with "git am" to the current v4.19 -stable
branch and it failed.
[davem@localhost linux-stable]$ git am --signoff diff
Applying: net: check before dereferencing netdev_ops during busy poll
error: patch failed: net/core/dev.c:5083
error: net/core/dev.c: patch does not apply
Patch failed at 0001 net: check before dereferencing netdev_ops during busy poll
hint: Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
^ permalink raw reply
* [PATCH V4] can: flexcan: fix stop mode acknowledgment
From: Joakim Zhang @ 2019-07-02 1:45 UTC (permalink / raw)
To: mkl@pengutronix.de, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org, dl-linux-imx,
Joakim Zhang
To enter stop mode, the CPU should manually assert a global Stop Mode
request and check the acknowledgment asserted by FlexCAN. The CPU must
only consider the FlexCAN in stop mode when both request and
acknowledgment conditions are satisfied.
Fixes: de3578c198c6 ("can: flexcan: add self wakeup support")
Reported-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
ChangeLog:
V1->V2:
* regmap_read()-->regmap_read_poll_timeout()
V2->V3:
* change the way of error return, it will make easy for function
extension.
V3->V4:
* rebase to linux-next/master, as this is a fix.
---
drivers/net/can/flexcan.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 1c66fb2ad76b..bf1bd6f5dbb1 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -400,9 +400,10 @@ static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
priv->write(reg_mcr, ®s->mcr);
}
-static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
+static inline int flexcan_enter_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
+ unsigned int ackval;
u32 reg_mcr;
reg_mcr = priv->read(®s->mcr);
@@ -412,20 +413,37 @@ static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
/* enable stop request */
regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+
+ /* get stop acknowledgment */
+ if (regmap_read_poll_timeout(priv->stm.gpr, priv->stm.ack_gpr,
+ ackval, ackval & (1 << priv->stm.ack_bit),
+ 0, FLEXCAN_TIMEOUT_US))
+ return -ETIMEDOUT;
+
+ return 0;
}
-static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
+static inline int flexcan_exit_stop_mode(struct flexcan_priv *priv)
{
struct flexcan_regs __iomem *regs = priv->regs;
+ unsigned int ackval;
u32 reg_mcr;
/* remove stop request */
regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
1 << priv->stm.req_bit, 0);
+ /* get stop acknowledgment */
+ if (regmap_read_poll_timeout(priv->stm.gpr, priv->stm.ack_gpr,
+ ackval, !(ackval & (1 << priv->stm.ack_bit)),
+ 0, FLEXCAN_TIMEOUT_US))
+ return -ETIMEDOUT;
+
reg_mcr = priv->read(®s->mcr);
reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
priv->write(reg_mcr, ®s->mcr);
+
+ return 0;
}
static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
@@ -1615,7 +1633,9 @@ static int __maybe_unused flexcan_suspend(struct device *device)
*/
if (device_may_wakeup(device)) {
enable_irq_wake(dev->irq);
- flexcan_enter_stop_mode(priv);
+ err = flexcan_enter_stop_mode(priv);
+ if (err)
+ return err;
} else {
err = flexcan_chip_disable(priv);
if (err)
@@ -1665,10 +1685,13 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
{
struct net_device *dev = dev_get_drvdata(device);
struct flexcan_priv *priv = netdev_priv(dev);
+ int err;
if (netif_running(dev) && device_may_wakeup(device)) {
flexcan_enable_wakeup_irq(priv, false);
- flexcan_exit_stop_mode(priv);
+ err = flexcan_exit_stop_mode(priv);
+ if (err)
+ return err;
}
return 0;
--
2.17.1
^ permalink raw reply related
* Re: Re: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: linmiaohe @ 2019-07-02 1:36 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David Ahern, kadlec@blackhole.kfki.hu, fw@strlen.de,
davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org, netfilter-devel@vger.kernel.org,
coreteam@netfilter.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Mingfangsen
On Sat, July 2, 2019 at 02:02:59AM, Pablo wrote:
>
> Probably this?
>
> } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev) ||
> (flags & XT_RPFILTER_LOOSE) == 0) {
> fl6.flowi6_oif = dev->ifindex;
> }
>
> Thanks.
I would send patch v5 according to this. Many Thanks.
Have a nice day.
Best wishes.
^ permalink raw reply
* Re: [PATCH bpf-next] virtio_net: add XDP meta data support
From: Yuya Kusakabe @ 2019-07-02 1:00 UTC (permalink / raw)
To: Jason Wang, davem
Cc: Michael S. Tsirkin, Alexei Starovoitov, Daniel Borkmann,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
Martin KaFai Lau, Song Liu, Yonghong Song, netdev
In-Reply-To: <fe3070db-ec3a-9c7c-e15e-93032811767f@redhat.com>
On 7/1/19 6:30 PM, Jason Wang wrote:
>
> On 2019/6/27 下午4:06, Yuya Kusakabe wrote:
>> This adds XDP meta data support to both receive_small() and
>> receive_mergeable().
>>
>> Fixes: de8f3a83b0a0 ("bpf: add meta pointer for direct access")
>> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
>> ---
>> drivers/net/virtio_net.c | 40 +++++++++++++++++++++++++++++-----------
>> 1 file changed, 29 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 4f3de0ac8b0b..e787657fc568 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -371,7 +371,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> struct receive_queue *rq,
>> struct page *page, unsigned int offset,
>> unsigned int len, unsigned int truesize,
>> - bool hdr_valid)
>> + bool hdr_valid, unsigned int metasize)
>> {
>> struct sk_buff *skb;
>> struct virtio_net_hdr_mrg_rxbuf *hdr;
>> @@ -393,17 +393,25 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>> else
>> hdr_padded_len = sizeof(struct padded_vnet_hdr);
>> - if (hdr_valid)
>> + if (hdr_valid && !metasize)
>> memcpy(hdr, p, hdr_len);
>> len -= hdr_len;
>> offset += hdr_padded_len;
>> p += hdr_padded_len;
>> - copy = len;
>> + copy = len + metasize;
>> if (copy > skb_tailroom(skb))
>> copy = skb_tailroom(skb);
>> - skb_put_data(skb, p, copy);
>> +
>> + if (metasize) {
>> + skb_put_data(skb, p - metasize, copy);
>
>
> I would rather keep copy untouched above, and use copy + metasize here, then you can save the following decrement as well. Or tweak the caller the count the meta in to offset, then we need only deal with skb_pull() and skb_metadata_set() here.
I think the latter is better, because copy + metasize must be smaller than skb tailroom size.
>
>> + __skb_pull(skb, metasize);
>> + skb_metadata_set(skb, metasize);
>> + copy -= metasize;
>> + } else {
>> + skb_put_data(skb, p, copy);
>> + }
>> len -= copy;
>> offset += copy;
>> @@ -644,6 +652,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> unsigned int delta = 0;
>> struct page *xdp_page;
>> int err;
>> + unsigned int metasize = 0;
>> len -= vi->hdr_len;
>> stats->bytes += len;
>> @@ -683,8 +692,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> xdp.data_hard_start = buf + VIRTNET_RX_PAD + vi->hdr_len;
>> xdp.data = xdp.data_hard_start + xdp_headroom;
>> - xdp_set_data_meta_invalid(&xdp);
>> xdp.data_end = xdp.data + len;
>> + xdp.data_meta = xdp.data;
>> xdp.rxq = &rq->xdp_rxq;
>> orig_data = xdp.data;
>> act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> @@ -695,9 +704,11 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> /* Recalculate length in case bpf program changed it */
>> delta = orig_data - xdp.data;
>> len = xdp.data_end - xdp.data;
>> + metasize = xdp.data - xdp.data_meta;
>> break;
>> case XDP_TX:
>> stats->xdp_tx++;
>> + xdp.data_meta = xdp.data;
>
>
> Why need this?
Because virtnet_xdp_xmit() doesn't support XDP meta data as below. And I suppose that we don't need to support XDP metadata for XDP_TX.
static int __virtnet_xdp_xmit_one(struct virtnet_info *vi,
struct send_queue *sq,
struct xdp_frame *xdpf)
{
struct virtio_net_hdr_mrg_rxbuf *hdr;
int err;
/* virtqueue want to use data area in-front of packet */
if (unlikely(xdpf->metasize > 0))
return -EOPNOTSUPP;
>
>> xdpf = convert_to_xdp_frame(&xdp);
>> if (unlikely(!xdpf))
>> goto err_xdp;
>> @@ -735,11 +746,14 @@ static struct sk_buff *receive_small(struct net_device *dev,
>> }
>> skb_reserve(skb, headroom - delta);
>> skb_put(skb, len);
>> - if (!delta) {
>> + if (!delta && !metasize) {
>> buf += header_offset;
>> memcpy(skb_vnet_hdr(skb), buf, vi->hdr_len);
>> } /* keep zeroed vnet hdr since packet was changed by bpf */
>
>
> Is there any method to preserve the vnet header here? We probably don't want to lose it for XDP_PASS when packet is not modified.
I'll try to keep the vnet header with moving the meta data to the front of the vnet header.
>
>> + if (metasize)
>> + skb_metadata_set(skb, metasize);
>> +
>> err:
>> return skb;
>> @@ -761,7 +775,7 @@ static struct sk_buff *receive_big(struct net_device *dev,
>> {
>> struct page *page = buf;
>> struct sk_buff *skb = page_to_skb(vi, rq, page, 0, len,
>> - PAGE_SIZE, true);
>> + PAGE_SIZE, true, 0);
>> stats->bytes += len - vi->hdr_len;
>> if (unlikely(!skb))
>> @@ -793,6 +807,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> unsigned int truesize;
>> unsigned int headroom = mergeable_ctx_to_headroom(ctx);
>> int err;
>> + unsigned int metasize = 0;
>> head_skb = NULL;
>> stats->bytes += len - vi->hdr_len;
>> @@ -839,8 +854,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> data = page_address(xdp_page) + offset;
>> xdp.data_hard_start = data - VIRTIO_XDP_HEADROOM + vi->hdr_len;
>> xdp.data = data + vi->hdr_len;
>> - xdp_set_data_meta_invalid(&xdp);
>> xdp.data_end = xdp.data + (len - vi->hdr_len);
>> + xdp.data_meta = xdp.data;
>> xdp.rxq = &rq->xdp_rxq;
>> act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> @@ -859,18 +874,20 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> * adjusted
>> */
>> len = xdp.data_end - xdp.data + vi->hdr_len;
>> + metasize = xdp.data - xdp.data_meta;
>> /* We can only create skb based on xdp_page. */
>> if (unlikely(xdp_page != page)) {
>> rcu_read_unlock();
>> put_page(page);
>> head_skb = page_to_skb(vi, rq, xdp_page,
>> - offset, len,
>> - PAGE_SIZE, false);
>> + offset, len,
>> + PAGE_SIZE, false, metasize);
>
>
> Indentation is wired.
Sorry. I'll fix it.
>
> Thanks
>
>
>> return head_skb;
>> }
>> break;
>> case XDP_TX:
>> stats->xdp_tx++;
>> + xdp.data_meta = xdp.data;
>> xdpf = convert_to_xdp_frame(&xdp);
>> if (unlikely(!xdpf))
>> goto err_xdp;
>> @@ -921,7 +938,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
>> goto err_skb;
>> }
>> - head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog);
>> + head_skb = page_to_skb(vi, rq, page, offset, len, truesize, !xdp_prog,
>> + metasize);
>> curr_skb = head_skb;
>> if (unlikely(!curr_skb))
^ permalink raw reply
* Re: [PATCH bpf-next 7/8] samples/bpf: add sample program that periodically dumps TCP stats
From: Y Song @ 2019-07-02 0:39 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, bpf, David Miller, Alexei Starovoitov,
Daniel Borkmann, Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <20190702003156.GI6757@mini-arch>
On Mon, Jul 1, 2019 at 5:31 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/01, Y Song wrote:
> > On Mon, Jul 1, 2019 at 1:49 PM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Uses new RTT callback to dump stats every second.
> > >
> > > $ mkdir -p /tmp/cgroupv2
> > > $ mount -t cgroup2 none /tmp/cgroupv2
> > > $ mkdir -p /tmp/cgroupv2/foo
> > > $ echo $$ >> /tmp/cgroupv2/foo/cgroup.procs
> > > $ bpftool prog load ./tcp_dumpstats_kern.o /sys/fs/bpf/tcp_prog
> > > $ bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
> > > $ bpftool prog tracelog
> > > $ # run neper/netperf/etc
> > >
> > > Used neper to compare performance with and without this program attached
> > > and didn't see any noticeable performance impact.
> > >
> > > Sample output:
> > > <idle>-0 [015] ..s. 2074.128800: 0: dsack_dups=0 delivered=242526
> > > <idle>-0 [015] ..s. 2074.128808: 0: delivered_ce=0 icsk_retransmits=0
> > > <idle>-0 [015] ..s. 2075.130133: 0: dsack_dups=0 delivered=323599
> > > <idle>-0 [015] ..s. 2075.130138: 0: delivered_ce=0 icsk_retransmits=0
> > > <idle>-0 [005] .Ns. 2076.131440: 0: dsack_dups=0 delivered=404648
> > > <idle>-0 [005] .Ns. 2076.131447: 0: delivered_ce=0 icsk_retransmits=0
> > >
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Priyaranjan Jha <priyarjha@google.com>
> > > Cc: Yuchung Cheng <ycheng@google.com>
> > > Cc: Soheil Hassas Yeganeh <soheil@google.com>
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> > > samples/bpf/Makefile | 1 +
> > > samples/bpf/tcp_dumpstats_kern.c | 65 ++++++++++++++++++++++++++++++++
> > > 2 files changed, 66 insertions(+)
> > > create mode 100644 samples/bpf/tcp_dumpstats_kern.c
> >
> > Currently, the bpf program into the repo. If we do not have another
> > script to use
> > this program for testing, the instructions in the commit message should be
> > added to the bpf program as comments so people know what to do with this file
> > without going through git commit message.
> >
> > Is it possible to create a script to run with this bpf program?
> There is a general instruction in samples/bpf/tcp_bpf.readme
> with bpftool examples/etc. Should I just a comment at the top
> of the BPF program to point people to that .readme file instead?
Referring to tcp_bpf.readme should work. Even simpler :-)
>
> > >
> > > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > > index 0917f8cf4fab..eaebbeead42f 100644
> > > --- a/samples/bpf/Makefile
> > > +++ b/samples/bpf/Makefile
> > > @@ -154,6 +154,7 @@ always += tcp_iw_kern.o
> > > always += tcp_clamp_kern.o
> > > always += tcp_basertt_kern.o
> > > always += tcp_tos_reflect_kern.o
> > > +always += tcp_dumpstats_kern.o
> > > always += xdp_redirect_kern.o
> > > always += xdp_redirect_map_kern.o
> > > always += xdp_redirect_cpu_kern.o
> > > diff --git a/samples/bpf/tcp_dumpstats_kern.c b/samples/bpf/tcp_dumpstats_kern.c
> > > new file mode 100644
> > > index 000000000000..5d22bf61db65
> > > --- /dev/null
> > > +++ b/samples/bpf/tcp_dumpstats_kern.c
> > > @@ -0,0 +1,65 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/bpf.h>
> > > +
> > > +#include "bpf_helpers.h"
> > > +#include "bpf_endian.h"
> > > +
> > > +#define INTERVAL 1000000000ULL
> > > +
> > > +int _version SEC("version") = 1;
> > > +char _license[] SEC("license") = "GPL";
[...]
^ permalink raw reply
* Re: [PATCH v5 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs
From: Yonghong Song @ 2019-07-02 0:36 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team
In-Reply-To: <20190701235903.660141-1-andriin@fb.com>
On 7/1/19 4:58 PM, Andrii Nakryiko wrote:
> 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.
>
> v4->v5:
> - typo and small nits (Yonghong);
> - validate pfd in attach_perf_event (Yonghong);
> - parse_uint_from_file fixes (Yonghong);
> - check for malloc failure in attach_raw_tracepoint (Yonghong);
> - attach_probes selftests clean up fixes (Yonghong);
> v3->v4:
> - proper errno handling (Stanislav);
> - bpf_fd -> prog_fd (Stanislav);
> - switch to fprintf (Song);
> 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 | 367 ++++++++++++++++++
> 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 | 166 ++++++++
> .../bpf/prog_tests/stacktrace_build_id.c | 55 +--
> .../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, 664 insertions(+), 99 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
Looks good to me. Ack for the whole series.
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH bpf-next 7/8] samples/bpf: add sample program that periodically dumps TCP stats
From: Stanislav Fomichev @ 2019-07-02 0:31 UTC (permalink / raw)
To: Y Song
Cc: Stanislav Fomichev, netdev, bpf, David Miller, Alexei Starovoitov,
Daniel Borkmann, Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <CAH3MdRX+utr3w1gC537ui7nLOZ+b8yrSKeO3CMuszXG5sGg3NA@mail.gmail.com>
On 07/01, Y Song wrote:
> On Mon, Jul 1, 2019 at 1:49 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Uses new RTT callback to dump stats every second.
> >
> > $ mkdir -p /tmp/cgroupv2
> > $ mount -t cgroup2 none /tmp/cgroupv2
> > $ mkdir -p /tmp/cgroupv2/foo
> > $ echo $$ >> /tmp/cgroupv2/foo/cgroup.procs
> > $ bpftool prog load ./tcp_dumpstats_kern.o /sys/fs/bpf/tcp_prog
> > $ bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
> > $ bpftool prog tracelog
> > $ # run neper/netperf/etc
> >
> > Used neper to compare performance with and without this program attached
> > and didn't see any noticeable performance impact.
> >
> > Sample output:
> > <idle>-0 [015] ..s. 2074.128800: 0: dsack_dups=0 delivered=242526
> > <idle>-0 [015] ..s. 2074.128808: 0: delivered_ce=0 icsk_retransmits=0
> > <idle>-0 [015] ..s. 2075.130133: 0: dsack_dups=0 delivered=323599
> > <idle>-0 [015] ..s. 2075.130138: 0: delivered_ce=0 icsk_retransmits=0
> > <idle>-0 [005] .Ns. 2076.131440: 0: dsack_dups=0 delivered=404648
> > <idle>-0 [005] .Ns. 2076.131447: 0: delivered_ce=0 icsk_retransmits=0
> >
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Priyaranjan Jha <priyarjha@google.com>
> > Cc: Yuchung Cheng <ycheng@google.com>
> > Cc: Soheil Hassas Yeganeh <soheil@google.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > samples/bpf/Makefile | 1 +
> > samples/bpf/tcp_dumpstats_kern.c | 65 ++++++++++++++++++++++++++++++++
> > 2 files changed, 66 insertions(+)
> > create mode 100644 samples/bpf/tcp_dumpstats_kern.c
>
> Currently, the bpf program into the repo. If we do not have another
> script to use
> this program for testing, the instructions in the commit message should be
> added to the bpf program as comments so people know what to do with this file
> without going through git commit message.
>
> Is it possible to create a script to run with this bpf program?
There is a general instruction in samples/bpf/tcp_bpf.readme
with bpftool examples/etc. Should I just a comment at the top
of the BPF program to point people to that .readme file instead?
> >
> > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > index 0917f8cf4fab..eaebbeead42f 100644
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -154,6 +154,7 @@ always += tcp_iw_kern.o
> > always += tcp_clamp_kern.o
> > always += tcp_basertt_kern.o
> > always += tcp_tos_reflect_kern.o
> > +always += tcp_dumpstats_kern.o
> > always += xdp_redirect_kern.o
> > always += xdp_redirect_map_kern.o
> > always += xdp_redirect_cpu_kern.o
> > diff --git a/samples/bpf/tcp_dumpstats_kern.c b/samples/bpf/tcp_dumpstats_kern.c
> > new file mode 100644
> > index 000000000000..5d22bf61db65
> > --- /dev/null
> > +++ b/samples/bpf/tcp_dumpstats_kern.c
> > @@ -0,0 +1,65 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +
> > +#include "bpf_helpers.h"
> > +#include "bpf_endian.h"
> > +
> > +#define INTERVAL 1000000000ULL
> > +
> > +int _version SEC("version") = 1;
> > +char _license[] SEC("license") = "GPL";
> > +
> > +struct {
> > + __u32 type;
> > + __u32 map_flags;
> > + int *key;
> > + __u64 *value;
> > +} bpf_next_dump SEC(".maps") = {
> > + .type = BPF_MAP_TYPE_SK_STORAGE,
> > + .map_flags = BPF_F_NO_PREALLOC,
> > +};
> > +
> > +SEC("sockops")
> > +int _sockops(struct bpf_sock_ops *ctx)
> > +{
> > + struct bpf_tcp_sock *tcp_sk;
> > + struct bpf_sock *sk;
> > + __u64 *next_dump;
> > + __u64 now;
> > +
> > + switch (ctx->op) {
> > + case BPF_SOCK_OPS_TCP_CONNECT_CB:
> > + bpf_sock_ops_cb_flags_set(ctx, BPF_SOCK_OPS_RTT_CB_FLAG);
> > + return 1;
> > + case BPF_SOCK_OPS_RTT_CB:
> > + break;
> > + default:
> > + return 1;
> > + }
> > +
> > + sk = ctx->sk;
> > + if (!sk)
> > + return 1;
> > +
> > + next_dump = bpf_sk_storage_get(&bpf_next_dump, sk, 0,
> > + BPF_SK_STORAGE_GET_F_CREATE);
> > + if (!next_dump)
> > + return 1;
> > +
> > + now = bpf_ktime_get_ns();
> > + if (now < *next_dump)
> > + return 1;
> > +
> > + tcp_sk = bpf_tcp_sock(sk);
> > + if (!tcp_sk)
> > + return 1;
> > +
> > + *next_dump = now + INTERVAL;
> > +
> > + bpf_printk("dsack_dups=%u delivered=%u\n",
> > + tcp_sk->dsack_dups, tcp_sk->delivered);
> > + bpf_printk("delivered_ce=%u icsk_retransmits=%u\n",
> > + tcp_sk->delivered_ce, tcp_sk->icsk_retransmits);
> > +
> > + return 1;
> > +}
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
> >
^ permalink raw reply
* Re: [PATCH bpf-next 6/8] selftests/bpf: test BPF_SOCK_OPS_RTT_CB
From: Y Song @ 2019-07-02 0:26 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Stanislav Fomichev, netdev, bpf, David Miller, Alexei Starovoitov,
Daniel Borkmann, Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <20190702000736.GH6757@mini-arch>
On Mon, Jul 1, 2019 at 5:07 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 07/01, Y Song wrote:
> > On Mon, Jul 1, 2019 at 1:49 PM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Make sure the callback is invoked for syn-ack and data packet.
> > >
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Priyaranjan Jha <priyarjha@google.com>
> > > Cc: Yuchung Cheng <ycheng@google.com>
> > > Cc: Soheil Hassas Yeganeh <soheil@google.com>
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 3 +-
> > > tools/testing/selftests/bpf/progs/tcp_rtt.c | 61 +++++
> > > tools/testing/selftests/bpf/test_tcp_rtt.c | 253 ++++++++++++++++++++
> > > 3 files changed, 316 insertions(+), 1 deletion(-)
> > > create mode 100644 tools/testing/selftests/bpf/progs/tcp_rtt.c
> > > create mode 100644 tools/testing/selftests/bpf/test_tcp_rtt.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index de1754a8f5fe..2620406a53ec 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -27,7 +27,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
> > > test_cgroup_storage test_select_reuseport test_section_names \
> > > test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
> > > test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
> > > - test_sockopt_multi
> > > + test_sockopt_multi test_tcp_rtt
> > >
> > > BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
> > > TEST_GEN_FILES = $(BPF_OBJ_FILES)
> > > @@ -107,6 +107,7 @@ $(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
> > > $(OUTPUT)/test_sockopt: cgroup_helpers.c
> > > $(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
> > > $(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
> > > +$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
> > >
> > > .PHONY: force
> > >
> > > diff --git a/tools/testing/selftests/bpf/progs/tcp_rtt.c b/tools/testing/selftests/bpf/progs/tcp_rtt.c
> > > new file mode 100644
> > > index 000000000000..233bdcb1659e
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/progs/tcp_rtt.c
> > > @@ -0,0 +1,61 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include <linux/bpf.h>
> > > +#include "bpf_helpers.h"
> > > +
> > > +char _license[] SEC("license") = "GPL";
> > > +__u32 _version SEC("version") = 1;
> > > +
> > > +struct tcp_rtt_storage {
> > > + __u32 invoked;
> > > + __u32 dsack_dups;
> > > + __u32 delivered;
> > > + __u32 delivered_ce;
> > > + __u32 icsk_retransmits;
> > > +};
[...]
> > > +
> > > +static void *server_thread(void *arg)
> > > +{
> > > + struct sockaddr_storage addr;
> > > + socklen_t len = sizeof(addr);
> > > + int fd = *(int *)arg;
> > > + int client_fd;
> > > +
> > > + if (listen(fd, 1) < 0)
> > > + error(1, errno, "Failed to listed on socket");
> >
> > The error() here only reports the error, right? In case of error,
> > should the control jumps to the end of this function and return?
> > The same for several error() calls below.
> No, error() calls exit(), so the whole process should die. Do you think
> it's better to gracefully handle that with pthread_join?
Thanks for explanation of error() semantics.
test_tcp_rtt is a standalone a program, so exiting
with a meaningful error message is fine to me. No need to change then.
>
> > > +
> > > + client_fd = accept(fd, (struct sockaddr *)&addr, &len);
> > > + if (client_fd < 0)
> > > + error(1, errno, "Failed to accept client");
> > > +
> > > + if (accept(fd, (struct sockaddr *)&addr, &len) >= 0)
> > > + error(1, errno, "Unexpected success in second accept");
> >
> > What is the purpose of this second default to-be-failed accept() call?
> So the server_thread waits here for the next client (that never arrives)
> and doesn't exit and call close(client_fd). I can add a comment here to
> clarify. Alternatively, I can just drop close(client_fd) and let
> the thread exit. WDYT?
Adding a comment to explain should be good enough. Thanks!
>
> > > +
> > > + close(client_fd);
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +int main(int args, char **argv)
> > > +{
> > > + int server_fd, cgroup_fd;
> > > + int err = EXIT_SUCCESS;
> > > + pthread_t tid;
> > > +
> > > + if (setup_cgroup_environment())
> > > + goto cleanup_obj;
> > > +
> > > + cgroup_fd = create_and_get_cgroup(CG_PATH);
> > > + if (cgroup_fd < 0)
> > > + goto cleanup_cgroup_env;
> > > +
> > > + if (join_cgroup(CG_PATH))
> > > + goto cleanup_cgroup;
> > > +
> > > + server_fd = start_server();
> > > + if (server_fd < 0) {
> > > + err = EXIT_FAILURE;
> > > + goto cleanup_cgroup;
> > > + }
> > > +
> > > + pthread_create(&tid, NULL, server_thread, (void *)&server_fd);
> > > +
> > > + if (run_test(cgroup_fd, server_fd))
> > > + err = EXIT_FAILURE;
> > > +
> > > + close(server_fd);
> > > +
> > > + printf("test_sockopt_sk: %s\n",
> > > + err == EXIT_SUCCESS ? "PASSED" : "FAILED");
> > > +
> > > +cleanup_cgroup:
> > > + close(cgroup_fd);
> > > +cleanup_cgroup_env:
> > > + cleanup_cgroup_environment();
> > > +cleanup_obj:
> > > + return err;
> > > +}
> > > --
> > > 2.22.0.410.gd8fdbe21b5-goog
> > >
^ permalink raw reply
* Re: [PATCH bpf-next 7/8] samples/bpf: add sample program that periodically dumps TCP stats
From: Y Song @ 2019-07-02 0:15 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, bpf, David Miller, Alexei Starovoitov, Daniel Borkmann,
Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <20190701204821.44230-8-sdf@google.com>
On Mon, Jul 1, 2019 at 1:49 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Uses new RTT callback to dump stats every second.
>
> $ mkdir -p /tmp/cgroupv2
> $ mount -t cgroup2 none /tmp/cgroupv2
> $ mkdir -p /tmp/cgroupv2/foo
> $ echo $$ >> /tmp/cgroupv2/foo/cgroup.procs
> $ bpftool prog load ./tcp_dumpstats_kern.o /sys/fs/bpf/tcp_prog
> $ bpftool cgroup attach /tmp/cgroupv2/foo sock_ops pinned /sys/fs/bpf/tcp_prog
> $ bpftool prog tracelog
> $ # run neper/netperf/etc
>
> Used neper to compare performance with and without this program attached
> and didn't see any noticeable performance impact.
>
> Sample output:
> <idle>-0 [015] ..s. 2074.128800: 0: dsack_dups=0 delivered=242526
> <idle>-0 [015] ..s. 2074.128808: 0: delivered_ce=0 icsk_retransmits=0
> <idle>-0 [015] ..s. 2075.130133: 0: dsack_dups=0 delivered=323599
> <idle>-0 [015] ..s. 2075.130138: 0: delivered_ce=0 icsk_retransmits=0
> <idle>-0 [005] .Ns. 2076.131440: 0: dsack_dups=0 delivered=404648
> <idle>-0 [005] .Ns. 2076.131447: 0: delivered_ce=0 icsk_retransmits=0
>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Priyaranjan Jha <priyarjha@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> samples/bpf/Makefile | 1 +
> samples/bpf/tcp_dumpstats_kern.c | 65 ++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+)
> create mode 100644 samples/bpf/tcp_dumpstats_kern.c
Currently, the bpf program into the repo. If we do not have another
script to use
this program for testing, the instructions in the commit message should be
added to the bpf program as comments so people know what to do with this file
without going through git commit message.
Is it possible to create a script to run with this bpf program?
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index 0917f8cf4fab..eaebbeead42f 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -154,6 +154,7 @@ always += tcp_iw_kern.o
> always += tcp_clamp_kern.o
> always += tcp_basertt_kern.o
> always += tcp_tos_reflect_kern.o
> +always += tcp_dumpstats_kern.o
> always += xdp_redirect_kern.o
> always += xdp_redirect_map_kern.o
> always += xdp_redirect_cpu_kern.o
> diff --git a/samples/bpf/tcp_dumpstats_kern.c b/samples/bpf/tcp_dumpstats_kern.c
> new file mode 100644
> index 000000000000..5d22bf61db65
> --- /dev/null
> +++ b/samples/bpf/tcp_dumpstats_kern.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +
> +#include "bpf_helpers.h"
> +#include "bpf_endian.h"
> +
> +#define INTERVAL 1000000000ULL
> +
> +int _version SEC("version") = 1;
> +char _license[] SEC("license") = "GPL";
> +
> +struct {
> + __u32 type;
> + __u32 map_flags;
> + int *key;
> + __u64 *value;
> +} bpf_next_dump SEC(".maps") = {
> + .type = BPF_MAP_TYPE_SK_STORAGE,
> + .map_flags = BPF_F_NO_PREALLOC,
> +};
> +
> +SEC("sockops")
> +int _sockops(struct bpf_sock_ops *ctx)
> +{
> + struct bpf_tcp_sock *tcp_sk;
> + struct bpf_sock *sk;
> + __u64 *next_dump;
> + __u64 now;
> +
> + switch (ctx->op) {
> + case BPF_SOCK_OPS_TCP_CONNECT_CB:
> + bpf_sock_ops_cb_flags_set(ctx, BPF_SOCK_OPS_RTT_CB_FLAG);
> + return 1;
> + case BPF_SOCK_OPS_RTT_CB:
> + break;
> + default:
> + return 1;
> + }
> +
> + sk = ctx->sk;
> + if (!sk)
> + return 1;
> +
> + next_dump = bpf_sk_storage_get(&bpf_next_dump, sk, 0,
> + BPF_SK_STORAGE_GET_F_CREATE);
> + if (!next_dump)
> + return 1;
> +
> + now = bpf_ktime_get_ns();
> + if (now < *next_dump)
> + return 1;
> +
> + tcp_sk = bpf_tcp_sock(sk);
> + if (!tcp_sk)
> + return 1;
> +
> + *next_dump = now + INTERVAL;
> +
> + bpf_printk("dsack_dups=%u delivered=%u\n",
> + tcp_sk->dsack_dups, tcp_sk->delivered);
> + bpf_printk("delivered_ce=%u icsk_retransmits=%u\n",
> + tcp_sk->delivered_ce, tcp_sk->icsk_retransmits);
> +
> + return 1;
> +}
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* [PATCH V2] net: usb: asix: init MAC address buffers
From: Phong Tran @ 2019-07-02 0:10 UTC (permalink / raw)
To: davem, dcbw, netdev, linux-usb
Cc: glider, linux-kernel-mentees, linux-kernel, lynxis,
marcel.ziswiler, skhan, syzbot+8a3fc6674bbc3978ed4e,
syzkaller-bugs, yang.wei9, zhang.run, tranmanphong
In-Reply-To: <20190630234533.15089-1-tranmanphong@gmail.com>
This is for fixing bug KMSAN: uninit-value in ax88772_bind
Tested by
https://groups.google.com/d/msg/syzkaller-bugs/aFQurGotng4/eB_HlNhhCwAJ
Reported-by: syzbot+8a3fc6674bbc3978ed4e@syzkaller.appspotmail.com
syzbot found the following crash on:
HEAD commit: f75e4cfe kmsan: use kmsan_handle_urb() in urb.c
git tree: kmsan
console output: https://syzkaller.appspot.com/x/log.txt?x=136d720ea00000
kernel config:
https://syzkaller.appspot.com/x/.config?x=602468164ccdc30a
dashboard link:
https://syzkaller.appspot.com/bug?extid=8a3fc6674bbc3978ed4e
compiler: clang version 9.0.0 (/home/glider/llvm/clang
06d00afa61eef8f7f501ebdb4e8612ea43ec2d78)
syz repro:
https://syzkaller.appspot.com/x/repro.syz?x=12788316a00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=120359aaa00000
==================================================================
BUG: KMSAN: uninit-value in is_valid_ether_addr
include/linux/etherdevice.h:200 [inline]
BUG: KMSAN: uninit-value in asix_set_netdev_dev_addr
drivers/net/usb/asix_devices.c:73 [inline]
BUG: KMSAN: uninit-value in ax88772_bind+0x93d/0x11e0
drivers/net/usb/asix_devices.c:724
CPU: 0 PID: 3348 Comm: kworker/0:2 Not tainted 5.1.0+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x191/0x1f0 lib/dump_stack.c:113
kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
__msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
is_valid_ether_addr include/linux/etherdevice.h:200 [inline]
asix_set_netdev_dev_addr drivers/net/usb/asix_devices.c:73 [inline]
ax88772_bind+0x93d/0x11e0 drivers/net/usb/asix_devices.c:724
usbnet_probe+0x10f5/0x3940 drivers/net/usb/usbnet.c:1728
usb_probe_interface+0xd66/0x1320 drivers/usb/core/driver.c:361
really_probe+0xdae/0x1d80 drivers/base/dd.c:513
driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x454/0x730 drivers/base/dd.c:844
device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
bus_probe_device+0x137/0x390 drivers/base/bus.c:514
device_add+0x288d/0x30e0 drivers/base/core.c:2106
usb_set_configuration+0x30dc/0x3750 drivers/usb/core/message.c:2027
generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
usb_probe_device+0x14c/0x200 drivers/usb/core/driver.c:266
really_probe+0xdae/0x1d80 drivers/base/dd.c:513
driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
__device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
__device_attach+0x454/0x730 drivers/base/dd.c:844
device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
bus_probe_device+0x137/0x390 drivers/base/bus.c:514
device_add+0x288d/0x30e0 drivers/base/core.c:2106
usb_new_device+0x23e5/0x2ff0 drivers/usb/core/hub.c:2534
hub_port_connect drivers/usb/core/hub.c:5089 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
port_event drivers/usb/core/hub.c:5350 [inline]
hub_event+0x48d1/0x7290 drivers/usb/core/hub.c:5432
process_one_work+0x1572/0x1f00 kernel/workqueue.c:2269
process_scheduled_works kernel/workqueue.c:2331 [inline]
worker_thread+0x189c/0x2460 kernel/workqueue.c:2417
kthread+0x4b5/0x4f0 kernel/kthread.c:254
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355
Signed-off-by: Phong Tran <tranmanphong@gmail.com>
---
Changes since v1:
- replace memset() by array init
---
drivers/net/usb/asix_devices.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index c9bc96310ed4..ef548beba684 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -226,7 +226,7 @@ static void asix_phy_reset(struct usbnet *dev, unsigned int reset_bits)
static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
{
int ret = 0;
- u8 buf[ETH_ALEN];
+ u8 buf[ETH_ALEN] = {0};
int i;
unsigned long gpio_bits = dev->driver_info->data;
@@ -677,7 +677,7 @@ static int asix_resume(struct usb_interface *intf)
static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
{
int ret, i;
- u8 buf[ETH_ALEN], chipcode = 0;
+ u8 buf[ETH_ALEN] = {0}, chipcode = 0;
u32 phyid;
struct asix_common_private *priv;
@@ -1061,7 +1061,7 @@ static const struct net_device_ops ax88178_netdev_ops = {
static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
{
int ret;
- u8 buf[ETH_ALEN];
+ u8 buf[ETH_ALEN] = {0};
usbnet_get_endpoints(dev,intf);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 2/3] net: phy: realtek: Enable accessing RTL8211E extension pages
From: Matthias Kaehlcke @ 2019-07-02 0:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: Heiner Kallweit, David S . Miller, Rob Herring, Mark Rutland,
Florian Fainelli, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190701210902.GL30468@lunn.ch>
On Mon, Jul 01, 2019 at 11:09:02PM +0200, Andrew Lunn wrote:
> On Mon, Jul 01, 2019 at 10:37:16PM +0200, Heiner Kallweit wrote:
> > On 01.07.2019 22:02, Andrew Lunn wrote:
> > > On Mon, Jul 01, 2019 at 12:52:24PM -0700, Matthias Kaehlcke wrote:
> > >> The RTL8211E has extension pages, which can be accessed after
> > >> selecting a page through a custom method. Add a function to
> > >> modify bits in a register of an extension page and a few
> > >> helpers for dealing with ext pages.
> > >>
> > >> rtl8211e_modify_ext_paged() and rtl821e_restore_page() are
> > >> inspired by their counterparts phy_modify_paged() and
> > >> phy_restore_page().
> > >
> > > Hi Matthias
> > >
> > > While an extended page is selected, what happens to the normal
> > > registers in the range 0-0x1c? Are they still accessible?
> > >
> > AFAIK: no
>
> This it would be better to make use of the core paged access support,
> so that locking is done correctly.
Do I understand correctly that this would involve assigning
.read/write_page and use phy_select_page() and phy_restore_page()?
Besides the benefit of locking this would also result in less code and
we could get rid of the custom _restore_page().
^ permalink raw reply
* Re: [PATCH bpf-next 6/8] selftests/bpf: test BPF_SOCK_OPS_RTT_CB
From: Stanislav Fomichev @ 2019-07-02 0:07 UTC (permalink / raw)
To: Y Song
Cc: Stanislav Fomichev, netdev, bpf, David Miller, Alexei Starovoitov,
Daniel Borkmann, Eric Dumazet, Priyaranjan Jha, Yuchung Cheng,
Soheil Hassas Yeganeh
In-Reply-To: <CAH3MdRXx4uO3pTFiLZk8j9ooO0gd1ppbSyT8zHMsVs01P6wKpA@mail.gmail.com>
On 07/01, Y Song wrote:
> On Mon, Jul 1, 2019 at 1:49 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Make sure the callback is invoked for syn-ack and data packet.
> >
> > Cc: Eric Dumazet <edumazet@google.com>
> > Cc: Priyaranjan Jha <priyarjha@google.com>
> > Cc: Yuchung Cheng <ycheng@google.com>
> > Cc: Soheil Hassas Yeganeh <soheil@google.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 3 +-
> > tools/testing/selftests/bpf/progs/tcp_rtt.c | 61 +++++
> > tools/testing/selftests/bpf/test_tcp_rtt.c | 253 ++++++++++++++++++++
> > 3 files changed, 316 insertions(+), 1 deletion(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/tcp_rtt.c
> > create mode 100644 tools/testing/selftests/bpf/test_tcp_rtt.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index de1754a8f5fe..2620406a53ec 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -27,7 +27,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
> > test_cgroup_storage test_select_reuseport test_section_names \
> > test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
> > test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
> > - test_sockopt_multi
> > + test_sockopt_multi test_tcp_rtt
> >
> > BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
> > TEST_GEN_FILES = $(BPF_OBJ_FILES)
> > @@ -107,6 +107,7 @@ $(OUTPUT)/test_cgroup_attach: cgroup_helpers.c
> > $(OUTPUT)/test_sockopt: cgroup_helpers.c
> > $(OUTPUT)/test_sockopt_sk: cgroup_helpers.c
> > $(OUTPUT)/test_sockopt_multi: cgroup_helpers.c
> > +$(OUTPUT)/test_tcp_rtt: cgroup_helpers.c
> >
> > .PHONY: force
> >
> > diff --git a/tools/testing/selftests/bpf/progs/tcp_rtt.c b/tools/testing/selftests/bpf/progs/tcp_rtt.c
> > new file mode 100644
> > index 000000000000..233bdcb1659e
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/tcp_rtt.c
> > @@ -0,0 +1,61 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +#include "bpf_helpers.h"
> > +
> > +char _license[] SEC("license") = "GPL";
> > +__u32 _version SEC("version") = 1;
> > +
> > +struct tcp_rtt_storage {
> > + __u32 invoked;
> > + __u32 dsack_dups;
> > + __u32 delivered;
> > + __u32 delivered_ce;
> > + __u32 icsk_retransmits;
> > +};
> > +
> > +struct bpf_map_def SEC("maps") socket_storage_map = {
> > + .type = BPF_MAP_TYPE_SK_STORAGE,
> > + .key_size = sizeof(int),
> > + .value_size = sizeof(struct tcp_rtt_storage),
> > + .map_flags = BPF_F_NO_PREALLOC,
> > +};
> > +BPF_ANNOTATE_KV_PAIR(socket_storage_map, int, struct tcp_rtt_storage);
> > +
> > +SEC("sockops")
> > +int _sockops(struct bpf_sock_ops *ctx)
> > +{
> > + struct tcp_rtt_storage *storage;
> > + struct bpf_tcp_sock *tcp_sk;
> > + int op = (int) ctx->op;
> > + struct bpf_sock *sk;
> > +
> > + sk = ctx->sk;
> > + if (!sk)
> > + return 1;
> > +
> > + storage = bpf_sk_storage_get(&socket_storage_map, sk, 0,
> > + BPF_SK_STORAGE_GET_F_CREATE);
> > + if (!storage)
> > + return 1;
> > +
> > + if (op == BPF_SOCK_OPS_TCP_CONNECT_CB) {
> > + bpf_sock_ops_cb_flags_set(ctx, BPF_SOCK_OPS_RTT_CB_FLAG);
> > + return 1;
> > + }
> > +
> > + if (op != BPF_SOCK_OPS_RTT_CB)
> > + return 1;
> > +
> > + tcp_sk = bpf_tcp_sock(sk);
> > + if (!tcp_sk)
> > + return 1;
> > +
> > + storage->invoked++;
> > +
> > + storage->dsack_dups = tcp_sk->dsack_dups;
> > + storage->delivered = tcp_sk->delivered;
> > + storage->delivered_ce = tcp_sk->delivered_ce;
> > + storage->icsk_retransmits = tcp_sk->icsk_retransmits;
> > +
> > + return 1;
> > +}
> > diff --git a/tools/testing/selftests/bpf/test_tcp_rtt.c b/tools/testing/selftests/bpf/test_tcp_rtt.c
> > new file mode 100644
> > index 000000000000..413fd8514adc
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/test_tcp_rtt.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <error.h>
> > +#include <errno.h>
> > +#include <stdio.h>
> > +#include <unistd.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +#include <netinet/in.h>
> > +#include <pthread.h>
> > +
> > +#include <linux/filter.h>
> > +#include <bpf/bpf.h>
> > +#include <bpf/libbpf.h>
> > +
> > +#include "bpf_rlimit.h"
> > +#include "bpf_util.h"
> > +#include "cgroup_helpers.h"
> > +
> > +#define CG_PATH "/tcp_rtt"
> > +
> > +struct tcp_rtt_storage {
> > + __u32 invoked;
> > + __u32 dsack_dups;
> > + __u32 delivered;
> > + __u32 delivered_ce;
> > + __u32 icsk_retransmits;
> > +};
> > +
> > +static void send_byte(int fd)
> > +{
> > + char b = 0x55;
> > +
> > + if (write(fd, &b, sizeof(b)) != 1)
> > + error(1, errno, "Failed to send single byte");
> > +}
> > +
> > +static int verify_sk(int map_fd, int client_fd, const char *msg, __u32 invoked,
> > + __u32 dsack_dups, __u32 delivered, __u32 delivered_ce,
> > + __u32 icsk_retransmits)
> > +{
> > + int err = 0;
> > + struct tcp_rtt_storage val;
> > +
> > + if (bpf_map_lookup_elem(map_fd, &client_fd, &val) < 0)
> > + error(1, errno, "Failed to read socket storage");
> > +
> > + if (val.invoked != invoked) {
> > + log_err("%s: unexpected bpf_tcp_sock.invoked %d != %d",
> > + msg, val.invoked, invoked);
> > + err++;
> > + }
> > +
> > + if (val.dsack_dups != dsack_dups) {
> > + log_err("%s: unexpected bpf_tcp_sock.dsack_dups %d != %d",
> > + msg, val.dsack_dups, dsack_dups);
> > + err++;
> > + }
> > +
> > + if (val.delivered != delivered) {
> > + log_err("%s: unexpected bpf_tcp_sock.delivered %d != %d",
> > + msg, val.delivered, delivered);
> > + err++;
> > + }
> > +
> > + if (val.delivered_ce != delivered_ce) {
> > + log_err("%s: unexpected bpf_tcp_sock.delivered_ce %d != %d",
> > + msg, val.delivered_ce, delivered_ce);
> > + err++;
> > + }
> > +
> > + if (val.icsk_retransmits != icsk_retransmits) {
> > + log_err("%s: unexpected bpf_tcp_sock.icsk_retransmits %d != %d",
> > + msg, val.icsk_retransmits, icsk_retransmits);
> > + err++;
> > + }
> > +
> > + return err;
> > +}
> > +
> > +static int connect_to_server(int server_fd)
> > +{
> > + struct sockaddr_storage addr;
> > + socklen_t len = sizeof(addr);
> > + int fd;
> > +
> > + fd = socket(AF_INET, SOCK_STREAM, 0);
> > + if (fd < 0) {
> > + log_err("Failed to create client socket");
> > + return -1;
> > + }
> > +
> > + if (getsockname(server_fd, (struct sockaddr *)&addr, &len)) {
> > + log_err("Failed to get server addr");
> > + goto out;
> > + }
> > +
> > + if (connect(fd, (const struct sockaddr *)&addr, len) < 0) {
> > + log_err("Fail to connect to server");
> > + goto out;
> > + }
> > +
> > + return fd;
> > +
> > +out:
> > + close(fd);
> > + return -1;
> > +}
> > +
> > +static int run_test(int cgroup_fd, int server_fd)
> > +{
> > + struct bpf_prog_load_attr attr = {
> > + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> > + .file = "./tcp_rtt.o",
> > + .expected_attach_type = BPF_CGROUP_SOCK_OPS,
> > + };
> > + struct bpf_program *prog;
> > + struct bpf_object *obj;
> > + struct bpf_map *map;
> > + int client_fd;
> > + int ignored;
> > + int map_fd;
> > + int err;
> > +
> > + err = bpf_prog_load_xattr(&attr, &obj, &ignored);
> > + if (err) {
> > + log_err("Failed to load BPF object");
> > + return -1;
> > + }
>
> The third argument of bpf_prog_load_xattr is prog_fd.
> If you have it, you do not need the below retrieving prog_fd
> by bpf_program__fd(prog).
Ack. I think I copy-pasted it from my other test where
I have multiple progs in the object file and attach them
manually by name. Will s/ignored/prog_fd/ in the v2.
> > +
> > + map = bpf_map__next(NULL, obj);
> > + map_fd = bpf_map__fd(map);
> > +
> > + prog = bpf_program__next(NULL, obj);
> > + err = bpf_prog_attach(bpf_program__fd(prog), cgroup_fd,
> > + BPF_CGROUP_SOCK_OPS, 0);
> > + if (err) {
> > + log_err("Failed to attach BPF program");
> > + goto close_bpf_object;
> > + }
> > +
> > + client_fd = connect_to_server(server_fd);
> > + if (client_fd < 0) {
> > + err = -1;
> > + goto close_bpf_object;
> > + }
> > +
> > + err += verify_sk(map_fd, client_fd, "syn-ack",
> > + /*invoked=*/1,
> > + /*dsack_dups=*/0,
> > + /*delivered=*/1,
> > + /*delivered_ce=*/0,
> > + /*icsk_retransmits=*/0);
> > +
> > + send_byte(client_fd);
> > +
> > + err += verify_sk(map_fd, client_fd, "first payload byte",
> > + /*invoked=*/2,
> > + /*dsack_dups=*/0,
> > + /*delivered=*/2,
> > + /*delivered_ce=*/0,
> > + /*icsk_retransmits=*/0);
> > +
> > + close(client_fd);
> > +
> > +close_bpf_object:
> > + bpf_object__close(obj);
> > + return err;
> > +}
> > +
> > +static int start_server(void)
> > +{
> > + struct sockaddr_in addr = {
> > + .sin_family = AF_INET,
> > + .sin_addr.s_addr = htonl(INADDR_LOOPBACK),
> > + };
> > + int fd;
> > +
> > + fd = socket(AF_INET, SOCK_STREAM, 0);
> > + if (fd < 0) {
> > + log_err("Failed to create server socket");
> > + return -1;
> > + }
> > +
> > + if (bind(fd, (const struct sockaddr *)&addr, sizeof(addr)) < 0) {
> > + log_err("Failed to bind socket");
> > + close(fd);
> > + return -1;
> > + }
> > +
> > + return fd;
> > +}
> > +
> > +static void *server_thread(void *arg)
> > +{
> > + struct sockaddr_storage addr;
> > + socklen_t len = sizeof(addr);
> > + int fd = *(int *)arg;
> > + int client_fd;
> > +
> > + if (listen(fd, 1) < 0)
> > + error(1, errno, "Failed to listed on socket");
>
> The error() here only reports the error, right? In case of error,
> should the control jumps to the end of this function and return?
> The same for several error() calls below.
No, error() calls exit(), so the whole process should die. Do you think
it's better to gracefully handle that with pthread_join?
> > +
> > + client_fd = accept(fd, (struct sockaddr *)&addr, &len);
> > + if (client_fd < 0)
> > + error(1, errno, "Failed to accept client");
> > +
> > + if (accept(fd, (struct sockaddr *)&addr, &len) >= 0)
> > + error(1, errno, "Unexpected success in second accept");
>
> What is the purpose of this second default to-be-failed accept() call?
So the server_thread waits here for the next client (that never arrives)
and doesn't exit and call close(client_fd). I can add a comment here to
clarify. Alternatively, I can just drop close(client_fd) and let
the thread exit. WDYT?
> > +
> > + close(client_fd);
> > +
> > + return NULL;
> > +}
> > +
> > +int main(int args, char **argv)
> > +{
> > + int server_fd, cgroup_fd;
> > + int err = EXIT_SUCCESS;
> > + pthread_t tid;
> > +
> > + if (setup_cgroup_environment())
> > + goto cleanup_obj;
> > +
> > + cgroup_fd = create_and_get_cgroup(CG_PATH);
> > + if (cgroup_fd < 0)
> > + goto cleanup_cgroup_env;
> > +
> > + if (join_cgroup(CG_PATH))
> > + goto cleanup_cgroup;
> > +
> > + server_fd = start_server();
> > + if (server_fd < 0) {
> > + err = EXIT_FAILURE;
> > + goto cleanup_cgroup;
> > + }
> > +
> > + pthread_create(&tid, NULL, server_thread, (void *)&server_fd);
> > +
> > + if (run_test(cgroup_fd, server_fd))
> > + err = EXIT_FAILURE;
> > +
> > + close(server_fd);
> > +
> > + printf("test_sockopt_sk: %s\n",
> > + err == EXIT_SUCCESS ? "PASSED" : "FAILED");
> > +
> > +cleanup_cgroup:
> > + close(cgroup_fd);
> > +cleanup_cgroup_env:
> > + cleanup_cgroup_environment();
> > +cleanup_obj:
> > + return err;
> > +}
> > --
> > 2.22.0.410.gd8fdbe21b5-goog
> >
^ permalink raw reply
* Re: [PATCH] net: usb: asix: init MAC address buffers
From: Phong Tran @ 2019-07-02 0:02 UTC (permalink / raw)
To: Dan Williams
Cc: syzbot, davem, Alexander Potapenko, linux-kernel, linux-usb,
lynxis, marcel.ziswiler, netdev, syzkaller-bugs, yang.wei9,
zhang.run, Shuah Khan, linux-kernel-mentees
In-Reply-To: <279519d5386680b3353b994a02475df08df13e29.camel@redhat.com>
Hello Dan,
On Mon, Jul 1, 2019 at 10:30 PM Dan Williams <dcbw@redhat.com> wrote:
>
> On Mon, 2019-07-01 at 06:45 +0700, Phong Tran wrote:
> > This is for fixing bug KMSAN: uninit-value in ax88772_bind
> >
> > Tested by
> > https://groups.google.com/d/msg/syzkaller-bugs/aFQurGotng4/cFe9nxMCCwAJ
> >
> > Reported-by: syzbot+8a3fc6674bbc3978ed4e@syzkaller.appspotmail.com
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: f75e4cfe kmsan: use kmsan_handle_urb() in urb.c
> > git tree: kmsan
> > console output:
> > https://syzkaller.appspot.com/x/log.txt?x=136d720ea00000
> > kernel config:
> > https://syzkaller.appspot.com/x/.config?x=602468164ccdc30a
> > dashboard link:
> > https://syzkaller.appspot.com/bug?extid=8a3fc6674bbc3978ed4e
> > compiler: clang version 9.0.0 (/home/glider/llvm/clang
> > 06d00afa61eef8f7f501ebdb4e8612ea43ec2d78)
> > syz repro:
> > https://syzkaller.appspot.com/x/repro.syz?x=12788316a00000
> > C reproducer:
> > https://syzkaller.appspot.com/x/repro.c?x=120359aaa00000
> >
> > ==================================================================
> > BUG: KMSAN: uninit-value in is_valid_ether_addr
> > include/linux/etherdevice.h:200 [inline]
> > BUG: KMSAN: uninit-value in asix_set_netdev_dev_addr
> > drivers/net/usb/asix_devices.c:73 [inline]
> > BUG: KMSAN: uninit-value in ax88772_bind+0x93d/0x11e0
> > drivers/net/usb/asix_devices.c:724
> > CPU: 0 PID: 3348 Comm: kworker/0:2 Not tainted 5.1.0+ #1
> > Hardware name: Google Google Compute Engine/Google Compute Engine,
> > BIOS
> > Google 01/01/2011
> > Workqueue: usb_hub_wq hub_event
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x191/0x1f0 lib/dump_stack.c:113
> > kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
> > __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
> > is_valid_ether_addr include/linux/etherdevice.h:200 [inline]
> > asix_set_netdev_dev_addr drivers/net/usb/asix_devices.c:73 [inline]
> > ax88772_bind+0x93d/0x11e0 drivers/net/usb/asix_devices.c:724
> > usbnet_probe+0x10f5/0x3940 drivers/net/usb/usbnet.c:1728
> > usb_probe_interface+0xd66/0x1320 drivers/usb/core/driver.c:361
> > really_probe+0xdae/0x1d80 drivers/base/dd.c:513
> > driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
> > __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
> > bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
> > __device_attach+0x454/0x730 drivers/base/dd.c:844
> > device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
> > bus_probe_device+0x137/0x390 drivers/base/bus.c:514
> > device_add+0x288d/0x30e0 drivers/base/core.c:2106
> > usb_set_configuration+0x30dc/0x3750 drivers/usb/core/message.c:2027
> > generic_probe+0xe7/0x280 drivers/usb/core/generic.c:210
> > usb_probe_device+0x14c/0x200 drivers/usb/core/driver.c:266
> > really_probe+0xdae/0x1d80 drivers/base/dd.c:513
> > driver_probe_device+0x1b3/0x4f0 drivers/base/dd.c:671
> > __device_attach_driver+0x5b8/0x790 drivers/base/dd.c:778
> > bus_for_each_drv+0x28e/0x3b0 drivers/base/bus.c:454
> > __device_attach+0x454/0x730 drivers/base/dd.c:844
> > device_initial_probe+0x4a/0x60 drivers/base/dd.c:891
> > bus_probe_device+0x137/0x390 drivers/base/bus.c:514
> > device_add+0x288d/0x30e0 drivers/base/core.c:2106
> > usb_new_device+0x23e5/0x2ff0 drivers/usb/core/hub.c:2534
> > hub_port_connect drivers/usb/core/hub.c:5089 [inline]
> > hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
> > port_event drivers/usb/core/hub.c:5350 [inline]
> > hub_event+0x48d1/0x7290 drivers/usb/core/hub.c:5432
> > process_one_work+0x1572/0x1f00 kernel/workqueue.c:2269
> > process_scheduled_works kernel/workqueue.c:2331 [inline]
> > worker_thread+0x189c/0x2460 kernel/workqueue.c:2417
> > kthread+0x4b5/0x4f0 kernel/kthread.c:254
> > ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:355
> >
> > Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> > ---
> > drivers/net/usb/asix_devices.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/usb/asix_devices.c
> > b/drivers/net/usb/asix_devices.c
> > index c9bc96310ed4..f514d19316b1 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -230,6 +230,7 @@ static int ax88172_bind(struct usbnet *dev,
> > struct usb_interface *intf)
> > int i;
> > unsigned long gpio_bits = dev->driver_info->data;
> >
> > + memset(buf, 0, sizeof(buf));
>
> For array variables defined in the function itself, isn't this usually
> done with:
>
> int ret = 0;
> - u8 buf[ETH_ALEN];
> + u8 buf[ETH_ALEN] = {0};
> int i;
> unsigned long gpio_bits = dev->driver_info->data;
>
> eg make the compiler do it (though maybe it's smart enough to elide the
> memset, I don't know). See drivers/net/ethernet/intel/igb/e1000_mac.c
> for an example.
>
Thank suggestion, applied in v2 without using memset().
Phong
^ permalink raw reply
* [PATCH v5 bpf-next 5/9] libbpf: add tracepoint attach API
From: Andrii Nakryiko @ 2019-07-01 23:58 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
Cc: Andrii Nakryiko
In-Reply-To: <20190701235903.660141-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>
Acked-by: Song Liu <songliubraving@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 79 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 4 ++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 84 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 7b6142408b15..d8c1743efc4a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4190,6 +4190,85 @@ 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, "%d\n");
+}
+
+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 event 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 v5 bpf-next 6/9] libbpf: add raw tracepoint attach API
From: Andrii Nakryiko @ 2019-07-01 23:59 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
Cc: Andrii Nakryiko
In-Reply-To: <20190701235903.660141-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>
Acked-by: Song Liu <songliubraving@fb.com>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 39 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 3 +++
tools/lib/bpf/libbpf.map | 1 +
3 files changed, 43 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d8c1743efc4a..72ee3d8d20a8 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4269,6 +4269,45 @@ 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 prog_fd, pfd;
+
+ prog_fd = bpf_program__fd(prog);
+ if (prog_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_fd;
+
+ pfd = bpf_raw_tracepoint_open(tp_name, prog_fd);
+ if (pfd < 0) {
+ pfd = -errno;
+ 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 v5 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-07-01 23:58 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
Cc: Andrii Nakryiko
In-Reply-To: <20190701235903.660141-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>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
---
tools/lib/bpf/libbpf.c | 169 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 ++
tools/lib/bpf/libbpf.map | 2 +
3 files changed, 178 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index bcaa294f819d..7b6142408b15 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -4021,6 +4021,175 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
return (struct bpf_link *)link;
}
+/*
+ * this function is expected to parse integer in the range of [0, 2^31-1] from
+ * given file using scanf format string fmt. If actual parsed value is
+ * negative, the result might be indistinguishable from error
+ */
+static int parse_uint_from_file(const char *file, const char *fmt)
+{
+ char buf[STRERR_BUFSIZE];
+ int err, ret;
+ FILE *f;
+
+ f = fopen(file, "r");
+ if (!f) {
+ err = -errno;
+ pr_debug("failed to open '%s': %s\n", file,
+ libbpf_strerror_r(err, buf, sizeof(buf)));
+ return err;
+ }
+ err = fscanf(f, fmt, &ret);
+ if (err != 1) {
+ err = err == EOF ? -EIO : -errno;
+ pr_debug("failed to parse '%s': %s\n", file,
+ libbpf_strerror_r(err, buf, sizeof(buf)));
+ fclose(f);
+ return err;
+ }
+ fclose(f);
+ return ret;
+}
+
+static int determine_kprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/kprobe/type";
+
+ return parse_uint_from_file(file, "%d\n");
+}
+
+static int determine_uprobe_perf_type(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/type";
+
+ return parse_uint_from_file(file, "%d\n");
+}
+
+static int determine_kprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
+
+ return parse_uint_from_file(file, "config:%d\n");
+}
+
+static int determine_uprobe_retprobe_bit(void)
+{
+ const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
+
+ return parse_uint_from_file(file, "config:%d\n");
+}
+
+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 v5 bpf-next 0/9] libbpf: add bpf_link and tracing attach APIs
From: Andrii Nakryiko @ 2019-07-01 23:58 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
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.
v4->v5:
- typo and small nits (Yonghong);
- validate pfd in attach_perf_event (Yonghong);
- parse_uint_from_file fixes (Yonghong);
- check for malloc failure in attach_raw_tracepoint (Yonghong);
- attach_probes selftests clean up fixes (Yonghong);
v3->v4:
- proper errno handling (Stanislav);
- bpf_fd -> prog_fd (Stanislav);
- switch to fprintf (Song);
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 | 367 ++++++++++++++++++
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 | 166 ++++++++
.../bpf/prog_tests/stacktrace_build_id.c | 55 +--
.../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, 664 insertions(+), 99 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
* [PATCH v5 bpf-next 9/9] selftests/bpf: convert existing tracepoint tests to new APIs
From: Andrii Nakryiko @ 2019-07-01 23:59 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
Cc: Andrii Nakryiko
In-Reply-To: <20190701235903.660141-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>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../bpf/prog_tests/stacktrace_build_id.c | 55 ++++---------------
.../selftests/bpf/prog_tests/stacktrace_map.c | 43 +++------------
.../bpf/prog_tests/stacktrace_map_raw_tp.c | 15 ++++-
3 files changed, 32 insertions(+), 81 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..ac44fda84833 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];
@@ -18,44 +20,16 @@ void test_stacktrace_build_id(void)
retry:
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
- goto out;
+ return;
- /* 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,14 +125,8 @@ 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);
-
-out:
- return;
}
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 v5 bpf-next 8/9] selftests/bpf: add kprobe/uprobe selftests
From: Andrii Nakryiko @ 2019-07-01 23:59 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, kernel-team, yhs
Cc: Andrii Nakryiko
In-Reply-To: <20190701235903.660141-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>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../selftests/bpf/prog_tests/attach_probe.c | 166 ++++++++++++++++++
.../selftests/bpf/progs/test_attach_probe.c | 55 ++++++
2 files changed, 221 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/attach_probe.c
create mode 100644 tools/testing/selftests/bpf/progs/test_attach_probe.c
diff --git a/tools/testing/selftests/bpf/prog_tests/attach_probe.c b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
new file mode 100644
index 000000000000..a4686395522c
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/attach_probe.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+
+ssize_t get_base_addr() {
+ size_t start;
+ char buf[256];
+ FILE *f;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!f)
+ return -errno;
+
+ while (fscanf(f, "%zx-%*x %s %*s\n", &start, buf) == 2) {
+ if (strcmp(buf, "r-xp") == 0) {
+ fclose(f);
+ return start;
+ }
+ }
+
+ fclose(f);
+ return -EINVAL;
+}
+
+#ifdef __x86_64__
+#define SYS_KPROBE_NAME "__x64_sys_nanosleep"
+#else
+#define SYS_KPROBE_NAME "sys_nanosleep"
+#endif
+
+void test_attach_probe(void)
+{
+ const char *kprobe_name = "kprobe/sys_nanosleep";
+ const char *kretprobe_name = "kretprobe/sys_nanosleep";
+ const char *uprobe_name = "uprobe/trigger_func";
+ const char *uretprobe_name = "uretprobe/trigger_func";
+ const int kprobe_idx = 0, kretprobe_idx = 1;
+ const int uprobe_idx = 2, uretprobe_idx = 3;
+ const char *file = "./test_attach_probe.o";
+ struct bpf_program *kprobe_prog, *kretprobe_prog;
+ struct bpf_program *uprobe_prog, *uretprobe_prog;
+ struct bpf_object *obj;
+ int err, prog_fd, duration = 0, res;
+ struct bpf_link *kprobe_link = NULL;
+ struct bpf_link *kretprobe_link = NULL;
+ struct bpf_link *uprobe_link = NULL;
+ struct bpf_link *uretprobe_link = NULL;
+ int results_map_fd;
+ size_t uprobe_offset;
+ ssize_t base_addr;
+
+ base_addr = get_base_addr();
+ if (CHECK(base_addr < 0, "get_base_addr",
+ "failed to find base addr: %zd", base_addr))
+ return;
+ uprobe_offset = (size_t)&get_base_addr - base_addr;
+
+ /* load programs */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ kprobe_prog = bpf_object__find_program_by_title(obj, kprobe_name);
+ if (CHECK(!kprobe_prog, "find_probe",
+ "prog '%s' not found\n", kprobe_name))
+ goto cleanup;
+ kretprobe_prog = bpf_object__find_program_by_title(obj, kretprobe_name);
+ if (CHECK(!kretprobe_prog, "find_probe",
+ "prog '%s' not found\n", kretprobe_name))
+ goto cleanup;
+ uprobe_prog = bpf_object__find_program_by_title(obj, uprobe_name);
+ if (CHECK(!uprobe_prog, "find_probe",
+ "prog '%s' not found\n", uprobe_name))
+ goto cleanup;
+ uretprobe_prog = bpf_object__find_program_by_title(obj, uretprobe_name);
+ if (CHECK(!uretprobe_prog, "find_probe",
+ "prog '%s' not found\n", uretprobe_name))
+ goto cleanup;
+
+ /* load maps */
+ results_map_fd = bpf_find_map(__func__, obj, "results_map");
+ if (CHECK(results_map_fd < 0, "find_results_map",
+ "err %d\n", results_map_fd))
+ goto cleanup;
+
+ kprobe_link = bpf_program__attach_kprobe(kprobe_prog,
+ false /* retprobe */,
+ SYS_KPROBE_NAME);
+ if (CHECK(IS_ERR(kprobe_link), "attach_kprobe",
+ "err %ld\n", PTR_ERR(kprobe_link))) {
+ kprobe_link = NULL;
+ goto cleanup;
+ }
+ kretprobe_link = bpf_program__attach_kprobe(kretprobe_prog,
+ true /* retprobe */,
+ SYS_KPROBE_NAME);
+ if (CHECK(IS_ERR(kretprobe_link), "attach_kretprobe",
+ "err %ld\n", PTR_ERR(kretprobe_link))) {
+ kretprobe_link = NULL;
+ goto cleanup;
+ }
+ uprobe_link = bpf_program__attach_uprobe(uprobe_prog,
+ false /* retprobe */,
+ 0 /* self pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uprobe_link), "attach_uprobe",
+ "err %ld\n", PTR_ERR(uprobe_link))) {
+ uprobe_link = NULL;
+ goto cleanup;
+ }
+ uretprobe_link = bpf_program__attach_uprobe(uretprobe_prog,
+ true /* retprobe */,
+ -1 /* any pid */,
+ "/proc/self/exe",
+ uprobe_offset);
+ if (CHECK(IS_ERR(uretprobe_link), "attach_uretprobe",
+ "err %ld\n", PTR_ERR(uretprobe_link))) {
+ uretprobe_link = NULL;
+ goto cleanup;
+ }
+
+ /* trigger & validate kprobe && kretprobe */
+ usleep(1);
+
+ err = bpf_map_lookup_elem(results_map_fd, &kprobe_idx, &res);
+ if (CHECK(err, "get_kprobe_res",
+ "failed to get kprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kprobe_idx + 1, "check_kprobe_res",
+ "wrong kprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &kretprobe_idx, &res);
+ if (CHECK(err, "get_kretprobe_res",
+ "failed to get kretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != kretprobe_idx + 1, "check_kretprobe_res",
+ "wrong kretprobe res: %d\n", res))
+ goto cleanup;
+
+ /* trigger & validate uprobe & uretprobe */
+ get_base_addr();
+
+ err = bpf_map_lookup_elem(results_map_fd, &uprobe_idx, &res);
+ if (CHECK(err, "get_uprobe_res",
+ "failed to get uprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uprobe_idx + 1, "check_uprobe_res",
+ "wrong uprobe res: %d\n", res))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(results_map_fd, &uretprobe_idx, &res);
+ if (CHECK(err, "get_uretprobe_res",
+ "failed to get uretprobe res: %d\n", err))
+ goto cleanup;
+ if (CHECK(res != uretprobe_idx + 1, "check_uretprobe_res",
+ "wrong uretprobe res: %d\n", res))
+ goto cleanup;
+
+cleanup:
+ bpf_link__destroy(kprobe_link);
+ bpf_link__destroy(kretprobe_link);
+ bpf_link__destroy(uprobe_link);
+ bpf_link__destroy(uretprobe_link);
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_attach_probe.c b/tools/testing/selftests/bpf/progs/test_attach_probe.c
new file mode 100644
index 000000000000..7a7c5cd728c8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_attach_probe.c
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ int type;
+ int max_entries;
+ int *key;
+ int *value;
+} results_map SEC(".maps") = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .max_entries = 4,
+};
+
+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
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