* [PATCH bpf-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-06-26 8:33 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
When an AF_XDP application received X packets, it does not mean X
frames can be stuffed into the producer ring. To make it easier for
AF_XDP applications this API allows them to check how many frames can
be added into the ring.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v1 -> v2
- Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
- Add caching so it will only touch global state when needed
tools/lib/bpf/xsk.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..6acb81102346 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
return &descs[idx & rx->mask];
}
-static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
+static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
{
__u32 free_entries = r->cached_cons - r->cached_prod;
- if (free_entries >= nb)
+ if (free_entries >= nb && nb != 0)
return free_entries;
/* Refresh the local tail pointer.
@@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
size_t nb, __u32 *idx)
{
- if (xsk_prod_nb_free(prod, nb) < nb)
+ if (xsk_prod__nb_free(prod, nb) < nb)
return 0;
*idx = prod->cached_prod;
--
2.20.1
^ permalink raw reply related
* [PATCH net] sctp: not bind the socket in sctp_connect
From: Xin Long @ 2019-06-26 8:31 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, syzkaller-bugs
Now when sctp_connect() is called with a wrong sa_family, it binds
to a port but doesn't set bp->port, then sctp_get_af_specific will
return NULL and sctp_connect() returns -EINVAL.
Then if sctp_bind() is called to bind to another port, the last
port it has bound will leak due to bp->port is NULL by then.
sctp_connect() doesn't need to bind ports, as later __sctp_connect
will do it if bp->port is NULL. So remove it from sctp_connect().
While at it, remove the unnecessary sockaddr.sa_family len check
as it's already done in sctp_inet_connect.
Fixes: 644fbdeacf1d ("sctp: fix the issue that flags are ignored when using kernel_connect")
Reported-by: syzbot+079bf326b38072f849d9@syzkaller.appspotmail.com
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 39ea0a3..f33aa9e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4816,35 +4816,17 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
static int sctp_connect(struct sock *sk, struct sockaddr *addr,
int addr_len, int flags)
{
- struct inet_sock *inet = inet_sk(sk);
struct sctp_af *af;
- int err = 0;
+ int err = -EINVAL;
lock_sock(sk);
-
pr_debug("%s: sk:%p, sockaddr:%p, addr_len:%d\n", __func__, sk,
addr, addr_len);
- /* We may need to bind the socket. */
- if (!inet->inet_num) {
- if (sk->sk_prot->get_port(sk, 0)) {
- release_sock(sk);
- return -EAGAIN;
- }
- inet->inet_sport = htons(inet->inet_num);
- }
-
/* Validate addr_len before calling common connect/connectx routine. */
- af = addr_len < offsetofend(struct sockaddr, sa_family) ? NULL :
- sctp_get_af_specific(addr->sa_family);
- if (!af || addr_len < af->sockaddr_len) {
- err = -EINVAL;
- } else {
- /* Pass correct addr len to common routine (so it knows there
- * is only one address being passed.
- */
+ af = sctp_get_af_specific(addr->sa_family);
+ if (af && addr_len >= af->sockaddr_len)
err = __sctp_connect(sk, addr, af->sockaddr_len, flags, NULL);
- }
release_sock(sk);
return err;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-06-26 8:30 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
In-Reply-To: <d4692ea57ba7a3fe33549fc6222fb8aea5a4225e.1561537432.git.echaudro@redhat.com>
Please ignore, this was supposed to be bpf-next not net-next :(
On 26 Jun 2019, at 10:27, Eelco Chaudron wrote:
> When an AF_XDP application received X packets, it does not mean X
> frames can be stuffed into the producer ring. To make it easier for
> AF_XDP applications this API allows them to check how many frames can
> be added into the ring.
>
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>
> v1 -> v2
> - Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
> - Add caching so it will only touch global state when needed
>
> tools/lib/bpf/xsk.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 82ea71a0f3ec..6acb81102346 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons
> *rx, __u32 idx)
> return &descs[idx & rx->mask];
> }
>
> -static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32
> nb)
> +static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32
> nb)
> {
> __u32 free_entries = r->cached_cons - r->cached_prod;
>
> - if (free_entries >= nb)
> + if (free_entries >= nb && nb != 0)
> return free_entries;
>
> /* Refresh the local tail pointer.
> @@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct
> xsk_ring_cons *r, __u32 nb)
> static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod
> *prod,
> size_t nb, __u32 *idx)
> {
> - if (xsk_prod_nb_free(prod, nb) < nb)
> + if (xsk_prod__nb_free(prod, nb) < nb)
> return 0;
>
> *idx = prod->cached_prod;
> --
> 2.20.1
^ permalink raw reply
* [PATCH net-next v2] libbpf: add xsk_ring_prod__nb_free() function
From: Eelco Chaudron @ 2019-06-26 8:27 UTC (permalink / raw)
To: netdev
Cc: ast, daniel, kafai, songliubraving, yhs, andrii.nakryiko,
magnus.karlsson
When an AF_XDP application received X packets, it does not mean X
frames can be stuffed into the producer ring. To make it easier for
AF_XDP applications this API allows them to check how many frames can
be added into the ring.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
v1 -> v2
- Renamed xsk_ring_prod__free() to xsk_ring_prod__nb_free()
- Add caching so it will only touch global state when needed
tools/lib/bpf/xsk.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..6acb81102346 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -76,11 +76,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
return &descs[idx & rx->mask];
}
-static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
+static inline __u32 xsk_prod__nb_free(struct xsk_ring_prod *r, __u32 nb)
{
__u32 free_entries = r->cached_cons - r->cached_prod;
- if (free_entries >= nb)
+ if (free_entries >= nb && nb != 0)
return free_entries;
/* Refresh the local tail pointer.
@@ -110,7 +110,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
static inline size_t xsk_ring_prod__reserve(struct xsk_ring_prod *prod,
size_t nb, __u32 *idx)
{
- if (xsk_prod_nb_free(prod, nb) < nb)
+ if (xsk_prod__nb_free(prod, nb) < nb)
return 0;
*idx = prod->cached_prod;
--
2.20.1
^ permalink raw reply related
* RE: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled indefinitely
From: Sudarsana Reddy Kalluru @ 2019-06-26 8:25 UTC (permalink / raw)
To: Guilherme Piccoli, jay.vosburgh@canonical.com
Cc: GR-everest-linux-l2, netdev@vger.kernel.org, Ariel Elior
In-Reply-To: <CAHD1Q_y5wWqOkPaC+JsuGMfBHbwPHbQF93Y-+06Nck=HKrif2g@mail.gmail.com>
> -----Original Message-----
> From: Guilherme Piccoli <gpiccoli@canonical.com>
> Sent: Wednesday, June 26, 2019 1:56 AM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>;
> jay.vosburgh@canonical.com
> Cc: GR-everest-linux-l2 <GR-everest-linux-l2@marvell.com>;
> netdev@vger.kernel.org; Ariel Elior <aelior@marvell.com>
> Subject: Re: [EXT] [PATCH V2] bnx2x: Prevent ptp_task to be rescheduled
> indefinitely
>
> Sudarsana, let me ask you something: why does the register is reading value
> 0x0 always in the TX timestamp routine if the RX filter is set to None? This is
> the main cause of the thread reschedule thing.
The register value of zero indicates there is no pending Tx timestamp to be read by the driver.
FW writes/latches the Tx timestamp for PTP event packet in this register. And it does the latching only if the register is free.
In this case user/app look to be requesting the Timestamp (via skb->tx_flags) for non-ptp Tx packet. In the Tx path, driver schedules a thread for reading the Tx timestamp,
bnx2x_start_xmit()
{
if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
schedule_work(&bp->ptp_task);
}
FW seem to be not timestamping the packet at all and driver is indefinitely waiting for it.
>
> Of course this thread thing is important to fix, but I was discussing with my
> leader here and we are curious on the reasoning the register is getting 0x0.
>
> Thanks in advance,
>
>
> Guilherme
^ permalink raw reply
* Re: [PATCH net-next] net: ipvlan: forward ingress packet to slave's l2 in l3s mode
From: Paolo Abeni @ 2019-06-26 8:16 UTC (permalink / raw)
To: Zhiyuan Hou, davem, idosch, daniel, petrm, jiri, tglx, linmiaohe
Cc: zhabin, caspar, netdev, linux-kernel
In-Reply-To: <20190625064208.2256-1-zhiyuan2048@linux.alibaba.com>
Hi,
On Tue, 2019-06-25 at 14:42 +0800, Zhiyuan Hou wrote:
> In ipvlan l3s mode, ingress packet is switched to slave interface and
> delivers to l4 stack. This may cause two problems:
>
> 1. When slave is in an ns different from master, the behavior of stack
> in slave ns may cause confusion for users. For example, iptables, tc,
> and other l2/l3 functions are not available for ingress packet.
>
> 2. l3s mode is not used for tap device, and cannot support ipvtap. But
> in VM or container based VM cases, tap device is a very common device.
>
> In l3s mode's input nf_hook, this patch calles the skb_forward_dev() to
> forward ingress packet to slave and uses nf_conntrack_confirm() to make
> conntrack work with new mode.
>
> Signed-off-by: Zha Bin <zhabin@linux.alibaba.com>
> Signed-off-by: Zhiyuan Hou <zhiyuan2048@linux.alibaba.com>
> ---
> drivers/net/ipvlan/ipvlan.h | 9 ++++++++-
> drivers/net/ipvlan/ipvlan_l3s.c | 16 ++++++++++++++--
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
> index 3837c897832e..48c814e24c3f 100644
> --- a/drivers/net/ipvlan/ipvlan.h
> +++ b/drivers/net/ipvlan/ipvlan.h
> @@ -172,6 +172,14 @@ void ipvlan_link_delete(struct net_device *dev, struct list_head *head);
> void ipvlan_link_setup(struct net_device *dev);
> int ipvlan_link_register(struct rtnl_link_ops *ops);
> #ifdef CONFIG_IPVLAN_L3S
> +
> +#include <net/netfilter/nf_conntrack_core.h>
> +
> +static inline int ipvlan_confirm_conntrack(struct sk_buff *skb)
> +{
> + return nf_conntrack_confirm(skb);
> +}
> +
> int ipvlan_l3s_register(struct ipvl_port *port);
> void ipvlan_l3s_unregister(struct ipvl_port *port);
> void ipvlan_migrate_l3s_hook(struct net *oldnet, struct net *newnet);
> @@ -206,5 +214,4 @@ static inline bool netif_is_ipvlan_port(const struct net_device *dev)
> {
> return rcu_access_pointer(dev->rx_handler) == ipvlan_handle_frame;
> }
> -
> #endif /* __IPVLAN_H */
> diff --git a/drivers/net/ipvlan/ipvlan_l3s.c b/drivers/net/ipvlan/ipvlan_l3s.c
> index 943d26cbf39f..ed210002f593 100644
> --- a/drivers/net/ipvlan/ipvlan_l3s.c
> +++ b/drivers/net/ipvlan/ipvlan_l3s.c
> @@ -95,14 +95,26 @@ static unsigned int ipvlan_nf_input(void *priv, struct sk_buff *skb,
> {
> struct ipvl_addr *addr;
> unsigned int len;
> + int ret = NF_ACCEPT;
> + bool success;
>
> addr = ipvlan_skb_to_addr(skb, skb->dev);
> if (!addr)
> goto out;
>
> - skb->dev = addr->master->dev;
> len = skb->len + ETH_HLEN;
> - ipvlan_count_rx(addr->master, len, true, false);
> +
> + ret = ipvlan_confirm_conntrack(skb);
> + if (ret != NF_ACCEPT) {
> + ipvlan_count_rx(addr->master, len, false, false);
> + goto out;
> + }
> +
> + skb_push_rcsum(skb, ETH_HLEN);
> + success = dev_forward_skb(addr->master->dev, skb) == NET_RX_SUCCESS;
This looks weird to me: if I read the code correctly, the skb will
traverse twice NF_INET_LOCAL_IN, once due to the l3s hooking and
another one due to dev_forward_skb().
Also, tc ingreess, etc will run after the first traversing of
NF_INET_LOCAL_IN.
All in all I think that if full l2 processing is required, a different
mode or a different virtual device should be used.
Cheers,
Paolo
^ permalink raw reply
* RE: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
From: Idan Burstein @ 2019-06-26 7:56 UTC (permalink / raw)
To: Sagi Grimberg, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, Tal Gilboa, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Yamin Friedman, Max Gurtovoy
In-Reply-To: <adb3687a-6db3-b1a4-cd32-8b4889550c81@grimberg.me>
" Please don't. This is a bad choice to opt it in by default."
I disagree here. I'd prefer Linux to have good out of the box experience (e.g. reach 100G in 4K NVMeOF on Intel servers) with the default parameters. Especially since Yamin have shown it is beneficial / not hurting in terms of performance for variety of use cases. The whole concept of DIM is that it adapts to the workload requirements in terms of bandwidth and latency.
Moreover, net-dim is enabled by default, I don't see why RDMA is different.
-----Original Message-----
From: linux-rdma-owner@vger.kernel.org <linux-rdma-owner@vger.kernel.org> On Behalf Of Sagi Grimberg
Sent: Wednesday, June 26, 2019 12:14 AM
To: Saeed Mahameed <saeedm@mellanox.com>; David S. Miller <davem@davemloft.net>; Doug Ledford <dledford@redhat.com>; Jason Gunthorpe <jgg@mellanox.com>
Cc: Leon Romanovsky <leonro@mellanox.com>; Or Gerlitz <ogerlitz@mellanox.com>; Tal Gilboa <talgi@mellanox.com>; netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Yamin Friedman <yaminf@mellanox.com>; Max Gurtovoy <maxg@mellanox.com>
Subject: Re: [for-next V2 10/10] RDMA/core: Provide RDMA DIM support for ULPs
> +static int ib_poll_dim_handler(struct irq_poll *iop, int budget) {
> + struct ib_cq *cq = container_of(iop, struct ib_cq, iop);
> + struct dim *dim = cq->dim;
> + int completed;
> +
> + completed = __ib_process_cq(cq, budget, cq->wc, IB_POLL_BATCH);
> + if (completed < budget) {
> + irq_poll_complete(&cq->iop);
> + if (ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0)
> + irq_poll_sched(&cq->iop);
> + }
> +
> + rdma_dim(dim, completed);
Why duplicate the entire thing for a one-liner?
> +
> + return completed;
> +}
> +
> static void ib_cq_completion_softirq(struct ib_cq *cq, void *private)
> {
> irq_poll_sched(&cq->iop);
> @@ -105,14 +157,18 @@ static void ib_cq_completion_softirq(struct
> ib_cq *cq, void *private)
>
> static void ib_cq_poll_work(struct work_struct *work)
> {
> - struct ib_cq *cq = container_of(work, struct ib_cq, work);
> + struct ib_cq *cq = container_of(work, struct ib_cq,
> + work);
Why was that changed?
> int completed;
>
> completed = __ib_process_cq(cq, IB_POLL_BUDGET_WORKQUEUE, cq->wc,
> IB_POLL_BATCH);
> +
newline?
> if (completed >= IB_POLL_BUDGET_WORKQUEUE ||
> ib_req_notify_cq(cq, IB_POLL_FLAGS) > 0)
> queue_work(cq->comp_wq, &cq->work);
> + else if (cq->dim)
> + rdma_dim(cq->dim, completed);
> }
>
> static void ib_cq_completion_workqueue(struct ib_cq *cq, void
> *private) @@ -166,6 +222,8 @@ struct ib_cq *__ib_alloc_cq_user(struct ib_device *dev, void *private,
> rdma_restrack_set_task(&cq->res, caller);
> rdma_restrack_kadd(&cq->res);
>
> + rdma_dim_init(cq);
> +
> switch (cq->poll_ctx) {
> case IB_POLL_DIRECT:
> cq->comp_handler = ib_cq_completion_direct; @@ -173,7 +231,13 @@
> struct ib_cq *__ib_alloc_cq_user(struct ib_device *dev, void *private,
> case IB_POLL_SOFTIRQ:
> cq->comp_handler = ib_cq_completion_softirq;
>
> - irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ, ib_poll_handler);
> + if (cq->dim) {
> + irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ,
> + ib_poll_dim_handler);
> + } else
> + irq_poll_init(&cq->iop, IB_POLL_BUDGET_IRQ,
> + ib_poll_handler);
> +
> ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
> break;
> case IB_POLL_WORKQUEUE:
> @@ -226,6 +290,9 @@ void ib_free_cq_user(struct ib_cq *cq, struct ib_udata *udata)
> WARN_ON_ONCE(1);
> }
>
> + if (cq->dim)
> + cancel_work_sync(&cq->dim->work);
> + kfree(cq->dim);
> kfree(cq->wc);
> rdma_restrack_del(&cq->res);
> ret = cq->device->ops.destroy_cq(cq, udata); diff --git
> a/drivers/infiniband/hw/mlx5/main.c
> b/drivers/infiniband/hw/mlx5/main.c
> index abac70ad5c7c..b1b45dbe24a5 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6305,6 +6305,8 @@ static int mlx5_ib_stage_caps_init(struct mlx5_ib_dev *dev)
> MLX5_CAP_GEN(dev->mdev, disable_local_lb_mc)))
> mutex_init(&dev->lb.mutex);
>
> + dev->ib_dev.use_cq_dim = true;
> +
Please don't. This is a bad choice to opt it in by default.
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: hns3: some code optimizations & bugfixes
From: tanhuazhong @ 2019-06-26 7:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel, salil.mehta, yisen.zhuang, linuxarm
In-Reply-To: <20190622.095323.1495992426494142587.davem@davemloft.net>
On 2019/6/22 21:53, David Miller wrote:
> From: Huazhong Tan <tanhuazhong@huawei.com>
> Date: Thu, 20 Jun 2019 16:52:34 +0800
>
>> This patch-set includes code optimizations and bugfixes for
>> the HNS3 ethernet controller driver.
>>
>> [patch 1/11] fixes a selftest issue when doing autoneg.
>>
>> [patch 2/11 - 3-11] adds two code optimizations about VLAN issue.
>>
>> [patch 4/11] restores the MAC autoneg state after reset.
>>
>> [patch 5/11 - 8/11] adds some code optimizations and bugfixes about
>> HW errors handling.
>>
>> [patch 9/11 - 11/11] fixes some issues related to driver loading and
>> unloading.
>
> Series applied, thanks.
>
Hi, david, has this patchset merged into net-next, why I cannot see it
after pulling net-next? Or is there some problem about this patchset I
have missed?
> .
>
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Russell King - ARM Linux admin @ 2019-06-26 7:41 UTC (permalink / raw)
To: Vladimir Oltean
Cc: René van Dorst, sean.wang, Florian Fainelli, David S. Miller,
matthias.bgg, Andrew Lunn, Vivien Didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <CA+h21hq_w8-96ehKYxcziSq1TjOjoKduZ+pB3umBfjODaKWd+A@mail.gmail.com>
On Wed, Jun 26, 2019 at 02:10:27AM +0300, Vladimir Oltean wrote:
> On Wed, 26 Jun 2019 at 01:58, Russell King - ARM Linux admin
> <linux@armlinux.org.uk> wrote:
> >
> > On Wed, Jun 26, 2019 at 01:14:59AM +0300, Vladimir Oltean wrote:
> > > On Wed, 26 Jun 2019 at 00:53, Russell King - ARM Linux admin
> > > <linux@armlinux.org.uk> wrote:
> > > >
> > > > On Tue, Jun 25, 2019 at 11:24:01PM +0300, Vladimir Oltean wrote:
> > > > > Hi Russell,
> > > > >
> > > > > On 6/24/19 6:39 PM, Russell King - ARM Linux admin wrote:
> > > > > > This should be removed - state->link is not for use in mac_config.
> > > > > > Even in fixed mode, the link can be brought up/down by means of a
> > > > > > gpio, and this should be dealt with via the mac_link_* functions.
> > > > > >
> > > > >
> > > > > What do you mean exactly that state->link is not for use, is that true in
> > > > > general?
> > > >
> > > > Yes. mac_config() should not touch it; it is not always in a defined
> > > > state. For example, if you set modes via ethtool (the
> > > > ethtool_ksettings_set API) then state->link will probably contain
> > > > zero irrespective of the true link state.
> > > >
> > >
> > > Experimentally, state->link is zero at the same time as state->speed
> > > is -1, so just ignoring !state->link made sense. This is not in-band
> > > AN. What is your suggestion? Should I proceed to try and configure the
> > > MAC for SPEED_UNKNOWN?
> >
> > What would you have done with a PHY when the link is down, what speed
> > would you have configured in the phylib adjust_link callback? phylib
> > also sets SPEED_UNKNOWN/DUPLEX_UNKNOWN when the link is down.
> >
>
> With phylib, I'd make the driver ignore the speed and do nothing.
> With phylink, I'd make the core not call mac_config.
> But what happened is I saw phylink call mac_config anyway, said
> 'weird' and proceeded to ignore it as I would have for phylib.
> I'm just not understanding your position - it seems like you're
> implying there's a bug in phylink and the function call with
> MLO_AN_FIXED, state->link=0 and state->speed=-1 should not have taken
> place, which is what I wanted to confirm.
It is not a bug. It is a request to configure the MAC, and what it's
saying is "we don't know what speed and/or duplex".
Take for instance when the network adapter is brought up initially.
The link is most likely down, but we should configure the initial MAC
operating parameters (such as the PHY interface). Phylink makes a
mac_config() call with the speed and duplex set to UNKNOWN.
Using your theory, we shouldn't be making that call. In which case,
MAC drivers aren't going to initially configure their interface
settings.
_That_ would be a bug.
> > It's unlikely that it would switch between SGMII and USXGMII
> > dynamically, as USXGMII supports speeds from 10G down to 10M.
> >
> > Where interface mode switching tends to be used is with modes such
> > as 10GBASE-R, which doesn't support anything except 10G. In order
> > for the PHY to operate at slower speeds, it has a few options:
> >
> > 1) perform rate adaption.
> > 2) dynamically switch interface type to an interface type that
> > supports the desired speed.
> > 3) just not support slower speeds.
> >
>
> So am I reading this correctly - it kind of makes sense for gigabit
> MAC drivers to not check for the MII interface changing protocol?
Again, that's incorrect in the general case. Gigabit includes SGMII
and 802.3z PHY protocols which need to be switched between for SFPs.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* Re: [PATCH bpf-next v9 02/10] bpf: Add eBPF program subtype and is_valid_subtype() verifier
From: Mickaël Salaün @ 2019-06-26 7:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: LKML, Aleksa Sarai, Alexander Viro, Alexei Starovoitov,
Andrew Morton, Andy Lutomirski, Arnaldo Carvalho de Melo,
Casey Schaufler, Daniel Borkmann, David Drysdale,
David S . Miller, Eric W . Biederman, James Morris, Jann Horn,
John Johansen, Jonathan Corbet, Kees Cook, Michael Kerrisk,
Mickaël Salaün, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Shuah Khan, Stephen Smalley, Tejun Heo,
Tetsuo Handa, Thomas Graf, Tycho Andersen, Will Drewry,
Kernel Hardening, Linux API, Linux-Fsdevel, LSM List,
Network Development
In-Reply-To: <CAADnVQ+Twio22VSi21RR5TY1Zm-1xRTGmREcXLSs5Jv-KWGTiw@mail.gmail.com>
On 26/06/2019 01:02, Alexei Starovoitov wrote:
> On Tue, Jun 25, 2019 at 3:04 PM Mickaël Salaün <mic@digikod.net> wrote:
>>
>> The goal of the program subtype is to be able to have different static
>> fine-grained verifications for a unique program type.
>>
>> The struct bpf_verifier_ops gets a new optional function:
>> is_valid_subtype(). This new verifier is called at the beginning of the
>> eBPF program verification to check if the (optional) program subtype is
>> valid.
>>
>> The new helper bpf_load_program_xattr() enables to verify a program with
>> subtypes.
>>
>> For now, only Landlock eBPF programs are using a program subtype (see
>> next commits) but this could be used by other program types in the
>> future.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: David S. Miller <davem@davemloft.net>
>> Link: https://lkml.kernel.org/r/20160827205559.GA43880@ast-mbp.thefacebook.com
>> ---
>>
>> Changes since v8:
>> * use bpf_load_program_xattr() instead of bpf_load_program() and add
>> bpf_verify_program_xattr() to deal with subtypes
>> * remove put_extra() since there is no more "previous" field (for now)
>>
>> Changes since v7:
>> * rename LANDLOCK_SUBTYPE_* to LANDLOCK_*
>> * move subtype in bpf_prog_aux and use only one bit for has_subtype
>> (suggested by Alexei Starovoitov)
>
> sorry to say, but I don't think the landlock will ever land,
> since posting huge patches once a year is missing a lot of development
> that is happening during that time.
You're right that it's been a while since the last patch set, but the
main reasons behind this was a lack of feedback (probably because of the
size of the patch set, which is now reduce to a consistent minimum), the
rework needed to address everyone's concern (Landlock modify kernel
components from different maintainers), and above all, the LSM stacking
infrastructure which was quite beefy and then took some time to land:
https://lore.kernel.org/lkml/50db058a-7dde-441b-a7f9-f6837fe8b69f@schaufler-ca.com/
This stacking infrastructure was required to have a useful version of
Landlock (which is used as a use case example), and it was released with
Linux v5.1 (last month). Now, I think everything is finally ready to
move forward.
> This 2/10 patch is an example.
> subtype concept was useful 2 years ago when v6 was posted.
> Since then bpf developers faced very similar problem in other parts
> and it was solved with 'expected_attach_type' field.
> See commit 5e43f899b03a ("bpf: Check attach type at prog load time")
> dated March 2018.
I saw this nice feature but I wasn't sure if it was the right field to
use. Indeed, I need more than a "type", but also some values (triggers)
as shown by this patch. What do you suggest?
^ permalink raw reply
* RE: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for configuration attributes.
From: Sudarsana Reddy Kalluru @ 2019-06-26 6:46 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jakub Kicinski, davem@davemloft.net, netdev@vger.kernel.org,
Michal Kalderon, Ariel Elior
In-Reply-To: <20190620133748.GD2504@nanopsycho>
> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, June 20, 2019 7:08 PM
> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>; davem@davemloft.net;
> netdev@vger.kernel.org; Michal Kalderon <mkalderon@marvell.com>; Ariel
> Elior <aelior@marvell.com>
> Subject: Re: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> configuration attributes.
>
> Thu, Jun 20, 2019 at 02:09:29PM CEST, skalluru@marvell.com wrote:
> >> -----Original Message-----
> >> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> >> Sent: Tuesday, June 18, 2019 4:24 AM
> >> To: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> Cc: davem@davemloft.net; netdev@vger.kernel.org; Michal Kalderon
> >> <mkalderon@marvell.com>; Ariel Elior <aelior@marvell.com>; Jiri Pirko
> >> <jiri@resnulli.us>
> >> Subject: [EXT] Re: [PATCH net-next 4/4] qed: Add devlink support for
> >> configuration attributes.
> >>
> >> External Email
> >>
> >> ---------------------------------------------------------------------
> >> - On Mon, 17 Jun 2019 04:45:28 -0700, Sudarsana Reddy Kalluru wrote:
> >> > This patch adds implementation for devlink callbacks for reading/
> >> > configuring the device attributes.
> >> >
> >> > Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
> >> > Signed-off-by: Ariel Elior <aelior@marvell.com>
> >>
> >> You need to provide documentation for your parameters, plus some of
> >> them look like they should potentially be port params, not device params.
> >
> >Thanks a lot for your review. Will add the required documentation. In case
> of Marvell adapter, any of the device/adapter/port parameters can be
> read/configurable via any PF (ethdev) on the port. Hence adding the
> commands at device level. Hope this is fine.
>
> No it is not. Port param should be port param.
>
> Also please be careful not to add any generic param as driver specific.
>
> Thanks!
Hi,
Could you please with my query on the devlink-port-params implementation. [had sent the same query earlier to jiri@mellanox.com (based on the copyright info)].
Kernel seem to be invoking the driver devlink callbacks (registered via DEVLINK_PARAM_DRIVER) only when the associated parameter is published via devlink_params_publish(). callnback invocation path,
devlink_nl_param_fill()
{
if (!param_item->published)
continue;
ctx.cmode = i;
err = devlink_param_get(devlink, param, &ctx);
}
The API devlink_params_publish() publishes only the devlink-dev parameters (i.e., registered via devlink_params_register()), not the devlink-port params which are registered via devlink_port_params_register(). I couldn't find any other interface for publishing the devlink-port-params.
I have manually verified setting the published flag for port-params (as in below) and, observed that kernel correctly invokes the callbacks of devlink-port-params.
list_for_each_entry(param_item, &dl_port.param_list, list) {
param_item->published = true;
}
Please let me know if I'm missing something here or, it's a missing functionality in the kernel.
Thanks,
Sudarsana
^ permalink raw reply
* Re: [for-next V2 05/10] linux/dim: Rename externally used net_dim members
From: Tal Gilboa @ 2019-06-26 6:38 UTC (permalink / raw)
To: Sagi Grimberg, Saeed Mahameed, David S. Miller, Doug Ledford,
Jason Gunthorpe
Cc: Leon Romanovsky, Or Gerlitz, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
In-Reply-To: <c97bbab4-13a9-b9e1-69f2-d4aba43e1c06@grimberg.me>
On 6/26/2019 12:57 AM, Sagi Grimberg wrote:
> Question, do any other nics use or plan to use this?
Yes, see the changed files list under drivers/net for existing usage.
>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
^ permalink raw reply
* Re: [PATCH] can: mcp251x: add error check when wq alloc failed
From: Sean Nyekjaer @ 2019-06-26 6:26 UTC (permalink / raw)
To: Willem de Bruijn, Weitao Hou
Cc: Wolfgang Grandegger, Marc Kleine-Budde, David Miller,
Greg Kroah-Hartman, allison, tglx, linux-can, Network Development,
linux-kernel
In-Reply-To: <CA+FuTSegsUvPSWX+CZuafSD32Sx+xJmYPiQ92geDNqAe8_JGrQ@mail.gmail.com>
On 25/06/2019 16.03, Willem de Bruijn wrote:
> On Tue, Jun 25, 2019 at 8:51 AM Weitao Hou <houweitaoo@gmail.com> wrote:
>>
>> add error check when workqueue alloc failed, and remove
>> redundant code to make it clear
>>
>> Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
>
> Acked-by: Willem de Bruijn <willemb@google.com>
>
Tested-by: Sean Nyekjaer <sean@geanix.com>
^ permalink raw reply
* [PATCH net v2] ipv4: reset rt_iif for recirculated mcast/bcast out pkts
From: Stephen Suryaputra @ 2019-06-26 6:21 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Stephen Suryaputra
Multicast or broadcast egress packets have rt_iif set to the oif. These
packets might be recirculated back as input and lookup to the raw
sockets may fail because they are bound to the incoming interface
(skb_iif). If rt_iif is not zero, during the lookup, inet_iif() function
returns rt_iif instead of skb_iif. Hence, the lookup fails.
v2: Make it non vrf specific (David Ahern). Reword the changelog to
reflect it.
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
include/net/route.h | 1 +
net/ipv4/ip_output.c | 12 ++++++++++++
net/ipv4/route.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 46 insertions(+)
diff --git a/include/net/route.h b/include/net/route.h
index 065b47754f05..55ff71ffb796 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -221,6 +221,7 @@ void ip_rt_get_source(u8 *src, struct sk_buff *skb, struct rtable *rt);
struct rtable *rt_dst_alloc(struct net_device *dev,
unsigned int flags, u16 type,
bool nopolicy, bool noxfrm, bool will_cache);
+struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt);
struct in_ifaddr;
void fib_add_ifaddr(struct in_ifaddr *);
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 16f9159234a2..8c2ec35b6512 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -318,6 +318,7 @@ static int ip_finish_output(struct net *net, struct sock *sk, struct sk_buff *sk
static int ip_mc_finish_output(struct net *net, struct sock *sk,
struct sk_buff *skb)
{
+ struct rtable *new_rt;
int ret;
ret = BPF_CGROUP_RUN_PROG_INET_EGRESS(sk, skb);
@@ -326,6 +327,17 @@ static int ip_mc_finish_output(struct net *net, struct sock *sk,
return ret;
}
+ /* Reset rt_iif so that inet_iif() will return skb->skb_iif. Setting
+ * this to non-zero causes ipi_ifindex in in_pktinfo to be overwritten,
+ * see ipv4_pktinfo_prepare().
+ */
+ new_rt = rt_dst_clone(net->loopback_dev, skb_rtable(skb));
+ if (new_rt) {
+ new_rt->rt_iif = 0;
+ skb_dst_drop(skb);
+ skb_dst_set(skb, &new_rt->dst);
+ }
+
return dev_loopback_xmit(net, sk, skb);
}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 6cb7cff22db9..8ea0735a6754 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1647,6 +1647,39 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
}
EXPORT_SYMBOL(rt_dst_alloc);
+struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt)
+{
+ struct rtable *new_rt;
+
+ new_rt = dst_alloc(&ipv4_dst_ops, dev, 1, DST_OBSOLETE_FORCE_CHK,
+ rt->dst.flags);
+
+ if (new_rt) {
+ new_rt->rt_genid = rt_genid_ipv4(dev_net(dev));
+ new_rt->rt_flags = rt->rt_flags;
+ new_rt->rt_type = rt->rt_type;
+ new_rt->rt_is_input = rt->rt_is_input;
+ new_rt->rt_iif = rt->rt_iif;
+ new_rt->rt_pmtu = rt->rt_pmtu;
+ new_rt->rt_mtu_locked = rt->rt_mtu_locked;
+ new_rt->rt_gw_family = rt->rt_gw_family;
+ if (rt->rt_gw_family == AF_INET)
+ new_rt->rt_gw4 = rt->rt_gw4;
+ else if (rt->rt_gw_family == AF_INET6)
+ new_rt->rt_gw6 = rt->rt_gw6;
+ INIT_LIST_HEAD(&new_rt->rt_uncached);
+
+ new_rt->dst.flags |= DST_HOST;
+ new_rt->dst.input = rt->dst.input;
+ new_rt->dst.output = rt->dst.output;
+ new_rt->dst.error = rt->dst.error;
+ new_rt->dst.lastuse = jiffies;
+ new_rt->dst.lwtstate = lwtstate_get(rt->dst.lwtstate);
+ }
+ return new_rt;
+}
+EXPORT_SYMBOL(rt_dst_clone);
+
/* called in rcu_read_lock() section */
int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
u8 tos, struct net_device *dev,
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 3/3] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-06-26 6:12 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190626061235.602633-1-andriin@fb.com>
Add test verifying perf buffer API functionality.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Song Liu <songliubraving@fb.com>
---
.../selftests/bpf/prog_tests/perf_buffer.c | 86 +++++++++++++++++++
.../selftests/bpf/progs/test_perf_buffer.c | 29 +++++++
2 files changed, 115 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
new file mode 100644
index 000000000000..3ba3e26141ac
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <pthread.h>
+#include <sched.h>
+#include <sys/socket.h>
+#include <test_progs.h>
+
+static void on_sample(void *ctx, void *data, __u32 size)
+{
+ cpu_set_t *cpu_seen = ctx;
+ int cpu = *(int *)data;
+
+ CPU_SET(cpu, cpu_seen);
+}
+
+void test_perf_buffer(void)
+{
+ int err, prog_fd, prog_pfd, nr_cpus, i, duration = 0;
+ const char *prog_name = "kprobe/sys_nanosleep";
+ const char *file = "./test_perf_buffer.o";
+ struct bpf_map *perf_buf_map;
+ cpu_set_t cpu_set, cpu_seen;
+ struct bpf_program *prog;
+ struct bpf_object *obj;
+ struct perf_buffer *pb;
+
+ nr_cpus = libbpf_num_possible_cpus();
+ if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
+ return;
+
+ /* load program */
+ err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
+ if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
+ return;
+
+ prog = bpf_object__find_program_by_title(obj, prog_name);
+ if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
+ goto out_close;
+
+ /* load map */
+ perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
+ if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
+ goto out_close;
+
+ /* attach kprobe */
+ prog_pfd = bpf_program__attach_kprobe(prog, false /* retprobe */,
+ "sys_nanosleep");
+ if (CHECK(prog_pfd < 0, "attach_kprobe", "err %d\n", prog_pfd))
+ goto out_close;
+
+ /* set up perf buffer */
+ pb = perf_buffer__new(perf_buf_map, 1, on_sample, NULL, &cpu_seen);
+ if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
+ goto out_detach;
+
+ /* trigger kprobe on every CPU */
+ CPU_ZERO(&cpu_seen);
+ for (i = 0; i < nr_cpus; i++) {
+ CPU_ZERO(&cpu_set);
+ CPU_SET(i, &cpu_set);
+
+ err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
+ &cpu_set);
+ if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
+ i, err))
+ goto out_detach;
+
+ usleep(1);
+ }
+
+ /* read perf buffer */
+ err = perf_buffer__poll(pb, 100);
+ if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
+ goto out_free_pb;
+
+ if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
+ "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
+ goto out_free_pb;
+
+out_free_pb:
+ perf_buffer__free(pb);
+out_detach:
+ libbpf_perf_event_disable_and_close(prog_pfd);
+out_close:
+ bpf_object__close(obj);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
new file mode 100644
index 000000000000..8609f0031bc0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019 Facebook
+
+#include <linux/ptrace.h>
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct {
+ int type;
+ int key_size;
+ int value_size;
+} perf_buf_map SEC(".maps") = {
+ .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+};
+
+SEC("kprobe/sys_nanosleep")
+int handle_sys_nanosleep_entry(struct pt_regs *ctx)
+{
+ int cpu = bpf_get_smp_processor_id();
+
+ bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
+ &cpu, sizeof(cpu));
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 2/3] libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
From: Andrii Nakryiko @ 2019-06-26 6:12 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190626061235.602633-1-andriin@fb.com>
For BPF_MAP_TYPE_PERF_EVENT_ARRAY typically correct size is number of
possible CPUs. This is impossible to specify at compilation time. This
change adds automatic setting of PERF_EVENT_ARRAY size to number of
system CPUs, unless non-zero size is specified explicitly. This allows
to adjust size for advanced specific cases, while providing convenient
and logical defaults.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index c74cc535902a..8f2b8a081ba7 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2114,6 +2114,7 @@ static int
bpf_object__create_maps(struct bpf_object *obj)
{
struct bpf_create_map_attr create_attr = {};
+ int nr_cpus = 0;
unsigned int i;
int err;
@@ -2136,7 +2137,21 @@ bpf_object__create_maps(struct bpf_object *obj)
create_attr.map_flags = def->map_flags;
create_attr.key_size = def->key_size;
create_attr.value_size = def->value_size;
- create_attr.max_entries = def->max_entries;
+ if (def->type == BPF_MAP_TYPE_PERF_EVENT_ARRAY &&
+ !def->max_entries) {
+ if (!nr_cpus)
+ nr_cpus = libbpf_num_possible_cpus();
+ if (nr_cpus < 0) {
+ pr_warning("failed to determine number of system CPUs: %d\n",
+ nr_cpus);
+ return nr_cpus;
+ }
+ pr_debug("map '%s': setting size to %d\n",
+ map->name, nr_cpus);
+ create_attr.max_entries = nr_cpus;
+ } else {
+ create_attr.max_entries = def->max_entries;
+ }
create_attr.btf_fd = 0;
create_attr.btf_key_type_id = 0;
create_attr.btf_value_type_id = 0;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 1/3] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-26 6:12 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
In-Reply-To: <20190626061235.602633-1-andriin@fb.com>
BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
to user space for additional processing. libbpf already has very low-level API
to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
use and requires a lot of code to set everything up. This patch adds
perf_buffer abstraction on top of it, abstracting setting up and polling
per-CPU logic into simple and convenient API, similar to what BCC provides.
perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
map entries. It accepts two user-provided callbacks: one for handling raw
samples and one for get notifications of lost samples due to buffer overflow.
perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
utilizing epoll instance.
perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
All APIs are not thread-safe. User should ensure proper locking/coordination if
used in multi-threaded set up.
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/libbpf.c | 282 +++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 12 ++
tools/lib/bpf/libbpf.map | 5 +-
3 files changed, 298 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9a4199b51300..c74cc535902a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -32,7 +32,9 @@
#include <linux/limits.h>
#include <linux/perf_event.h>
#include <linux/ring_buffer.h>
+#include <sys/epoll.h>
#include <sys/ioctl.h>
+#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
@@ -4322,6 +4324,286 @@ bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
return ret;
}
+struct perf_cpu_buf {
+ int fd;
+ void *base; /* mmap()'ed memory */
+ void *buf; /* for reconstructing segmented data */
+ size_t buf_size;
+};
+
+struct perf_buffer {
+ perf_buffer_sample_fn sample_cb;
+ perf_buffer_lost_fn lost_cb;
+ void *ctx; /* passed into callbacks */
+
+ size_t page_size;
+ size_t mmap_size;
+ struct perf_cpu_buf **cpu_bufs;
+ struct epoll_event *events;
+ int cpu_cnt;
+ int epfd; /* perf event FD */
+ int mapfd; /* BPF_MAP_TYPE_PERF_EVENT_ARRAY BPF map FD */
+};
+
+static void perf_buffer__free_cpu_buf(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf, int cpu)
+{
+ if (!cpu_buf)
+ return;
+ if (cpu_buf->base &&
+ munmap(cpu_buf->base, pb->mmap_size + pb->page_size))
+ pr_warning("failed to munmap cpu_buf #%d\n", cpu);
+ if (cpu_buf->fd >= 0) {
+ ioctl(cpu_buf->fd, PERF_EVENT_IOC_DISABLE, 0);
+ close(cpu_buf->fd);
+ }
+ free(cpu_buf->buf);
+ free(cpu_buf);
+}
+
+void perf_buffer__free(struct perf_buffer *pb)
+{
+ int i;
+
+ if (!pb)
+ return;
+ if (pb->cpu_bufs) {
+ for (i = 0; i < pb->cpu_cnt && pb->cpu_bufs[i]; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->cpu_bufs[i];
+
+ bpf_map_delete_elem(pb->mapfd, &i);
+ perf_buffer__free_cpu_buf(pb, cpu_buf, i);
+ }
+ free(pb->cpu_bufs);
+ }
+ if (pb->epfd >= 0)
+ close(pb->epfd);
+ free(pb->events);
+ free(pb);
+}
+
+static struct perf_cpu_buf *perf_buffer__open_cpu_buf(struct perf_buffer *pb,
+ int cpu)
+{
+ struct perf_event_attr attr = {};
+ struct perf_cpu_buf *cpu_buf;
+ char msg[STRERR_BUFSIZE];
+ int err;
+
+ cpu_buf = calloc(1, sizeof(*cpu_buf));
+ if (!cpu_buf)
+ return ERR_PTR(-ENOMEM);
+
+ attr.config = PERF_COUNT_SW_BPF_OUTPUT;
+ attr.type = PERF_TYPE_SOFTWARE;
+ attr.sample_type = PERF_SAMPLE_RAW;
+ attr.sample_period = 1;
+ attr.wakeup_events = 1;
+ cpu_buf->fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, cpu,
+ -1, PERF_FLAG_FD_CLOEXEC);
+ if (cpu_buf->fd < 0) {
+ err = -errno;
+ pr_warning("failed to open perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ cpu_buf->base = mmap(NULL, pb->mmap_size + pb->page_size,
+ PROT_READ | PROT_WRITE, MAP_SHARED,
+ cpu_buf->fd, 0);
+ if (cpu_buf->base == MAP_FAILED) {
+ cpu_buf->base = NULL;
+ err = -errno;
+ pr_warning("failed to mmap perf buffer on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ if (ioctl(cpu_buf->fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ pr_warning("failed to enable perf buffer event on cpu #%d: %s\n",
+ cpu, libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ return cpu_buf;
+
+error:
+ perf_buffer__free_cpu_buf(pb, cpu_buf, cpu);
+ return (struct perf_cpu_buf *)ERR_PTR(err);
+}
+
+struct perf_buffer *perf_buffer__new(struct bpf_map *map, size_t page_cnt,
+ perf_buffer_sample_fn sample_cb,
+ perf_buffer_lost_fn lost_cb, void *ctx)
+{
+ char msg[STRERR_BUFSIZE];
+ struct perf_buffer *pb;
+ int err, cpu;
+
+ if (bpf_map__def(map)->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
+ pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
+ bpf_map__name(map));
+ return ERR_PTR(-EINVAL);
+ }
+ if (bpf_map__fd(map) < 0) {
+ pr_warning("map '%s' doesn't have associated FD\n",
+ bpf_map__name(map));
+ return ERR_PTR(-EINVAL);
+ }
+ if (page_cnt & (page_cnt - 1)) {
+ pr_warning("page count should be power of two, but is %zu\n",
+ page_cnt);
+ return ERR_PTR(-EINVAL);
+ }
+
+ pb = calloc(1, sizeof(*pb));
+ if (!pb)
+ return ERR_PTR(-ENOMEM);
+
+ pb->sample_cb = sample_cb;
+ pb->lost_cb = lost_cb;
+ pb->ctx = ctx;
+ pb->page_size = getpagesize();
+ pb->mmap_size = pb->page_size * page_cnt;
+ pb->mapfd = bpf_map__fd(map);
+
+ pb->epfd = epoll_create1(EPOLL_CLOEXEC);
+ if (pb->epfd < 0) {
+ err = -errno;
+ pr_warning("failed to create epoll instance: %s\n",
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ pb->cpu_cnt = libbpf_num_possible_cpus();
+ if (pb->cpu_cnt < 0) {
+ err = pb->cpu_cnt;
+ goto error;
+ }
+ pb->events = calloc(pb->cpu_cnt, sizeof(*pb->events));
+ if (!pb->events) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate events: out of memory\n");
+ goto error;
+ }
+ pb->cpu_bufs = calloc(pb->cpu_cnt, sizeof(*pb->cpu_bufs));
+ if (!pb->cpu_bufs) {
+ err = -ENOMEM;
+ pr_warning("failed to allocate buffers: out of memory\n");
+ goto error;
+ }
+
+ for (cpu = 0; cpu < pb->cpu_cnt; cpu++) {
+ struct perf_cpu_buf *cpu_buf;
+
+ cpu_buf = perf_buffer__open_cpu_buf(pb, cpu);
+ if (IS_ERR(cpu_buf)) {
+ err = PTR_ERR(cpu_buf);
+ goto error;
+ }
+
+ pb->cpu_bufs[cpu] = cpu_buf;
+
+ err = bpf_map_update_elem(pb->mapfd, &cpu, &cpu_buf->fd, 0);
+ if (err) {
+ pr_warning("failed to set cpu #%d perf FD %d: %s\n",
+ cpu, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+
+ pb->events[cpu].events = EPOLLIN;
+ pb->events[cpu].data.ptr = cpu_buf;
+ if (epoll_ctl(pb->epfd, EPOLL_CTL_ADD, cpu_buf->fd,
+ &pb->events[cpu]) < 0) {
+ err = -errno;
+ pr_warning("failed to epoll_ctl cpu #%d perf FD %d: %s\n",
+ cpu, cpu_buf->fd,
+ libbpf_strerror_r(err, msg, sizeof(msg)));
+ goto error;
+ }
+ }
+
+ return pb;
+
+error:
+ if (pb)
+ perf_buffer__free(pb);
+ return ERR_PTR(err);
+}
+
+struct perf_sample_raw {
+ struct perf_event_header header;
+ uint32_t size;
+ char data[0];
+};
+
+struct perf_sample_lost {
+ struct perf_event_header header;
+ uint64_t id;
+ uint64_t lost;
+ uint64_t sample_id;
+};
+
+static enum bpf_perf_event_ret
+perf_buffer__process_record(struct perf_event_header *e, void *ctx)
+{
+ struct perf_buffer *pb = ctx;
+ void *data = e;
+
+ switch (e->type) {
+ case PERF_RECORD_SAMPLE: {
+ struct perf_sample_raw *s = data;
+
+ pb->sample_cb(pb->ctx, s->data, s->size);
+ break;
+ }
+ case PERF_RECORD_LOST: {
+ struct perf_sample_lost *s = data;
+
+ if (pb->lost_cb)
+ pb->lost_cb(pb->ctx, s->lost);
+ break;
+ }
+ default:
+ pr_warning("unknown perf sample type %d\n", e->type);
+ return LIBBPF_PERF_EVENT_ERROR;
+ }
+ return LIBBPF_PERF_EVENT_CONT;
+}
+
+static int perf_buffer__process_records(struct perf_buffer *pb,
+ struct perf_cpu_buf *cpu_buf)
+{
+ enum bpf_perf_event_ret ret;
+
+ ret = bpf_perf_event_read_simple(cpu_buf->base, pb->mmap_size,
+ pb->page_size, &cpu_buf->buf,
+ &cpu_buf->buf_size,
+ perf_buffer__process_record, pb);
+ if (ret != LIBBPF_PERF_EVENT_CONT)
+ return ret;
+ return 0;
+}
+
+int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms)
+{
+ int cnt, err;
+
+ cnt = epoll_wait(pb->epfd, pb->events, pb->cpu_cnt, timeout_ms);
+ for (int i = 0; i < cnt; i++) {
+ struct perf_cpu_buf *cpu_buf = pb->events[i].data.ptr;
+
+ err = perf_buffer__process_records(pb, cpu_buf);
+ if (err) {
+ pr_warning("error while processing records: %d\n", err);
+ return err;
+ }
+ }
+ return cnt < 0 ? -errno : cnt;
+}
+
struct bpf_prog_info_array_desc {
int array_offset; /* e.g. offset of jited_prog_insns */
int count_offset; /* e.g. offset of jited_prog_len */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bf7020a565c6..3bfde1a475ce 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -354,6 +354,18 @@ LIBBPF_API int bpf_prog_load(const char *file, enum bpf_prog_type type,
LIBBPF_API int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
LIBBPF_API int bpf_get_link_xdp_id(int ifindex, __u32 *prog_id, __u32 flags);
+struct perf_buffer;
+typedef void (*perf_buffer_sample_fn)(void *ctx, void *data, __u32 size);
+typedef void (*perf_buffer_lost_fn)(void *ctx, __u64 cnt);
+
+LIBBPF_API struct perf_buffer *perf_buffer__new(struct bpf_map *map,
+ size_t page_cnt,
+ perf_buffer_sample_fn sample_cb,
+ perf_buffer_lost_fn lost_cb,
+ void *ctx);
+LIBBPF_API void perf_buffer__free(struct perf_buffer *pb);
+LIBBPF_API int perf_buffer__poll(struct perf_buffer *pb, int timeout_ms);
+
enum bpf_perf_event_ret {
LIBBPF_PERF_EVENT_DONE = 0,
LIBBPF_PERF_EVENT_ERROR = -1,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2382fbda4cbb..10f48103110a 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
btf_dump__dump_type;
btf_dump__free;
btf_dump__new;
- btf__parse_elf;
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__parse_elf;
libbpf_num_possible_cpus;
libbpf_perf_event_disable_and_close;
+ perf_buffer__free;
+ perf_buffer__new;
+ perf_buffer__poll;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 0/3] libbpf: add perf buffer abstraction and API
From: Andrii Nakryiko @ 2019-06-26 6:12 UTC (permalink / raw)
To: andrii.nakryiko, ast, daniel, bpf, netdev, kernel-team; +Cc: Andrii Nakryiko
This patchset adds a high-level API for setting up and polling perf buffers
associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
described in corresponding commit.
Patch #1 adds a set of APIs to set up and work with perf buffer.
Patch #2 enhances libbpf to supprot auto-setting PERF_EVENT_ARRAY map size.
Patch #3 adds test.
Andrii Nakryiko (3):
libbpf: add perf buffer API
libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
selftests/bpf: test perf buffer API
tools/lib/bpf/libbpf.c | 299 +++++++++++++++++-
tools/lib/bpf/libbpf.h | 12 +
tools/lib/bpf/libbpf.map | 5 +-
.../selftests/bpf/prog_tests/perf_buffer.c | 86 +++++
.../selftests/bpf/progs/test_perf_buffer.c | 29 ++
5 files changed, 429 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
--
2.17.1
^ permalink raw reply
* Re: samples/bpf compilation failures - 5.2.0
From: Srinivas Ramana @ 2019-06-26 5:41 UTC (permalink / raw)
To: Alexei Starovoitov, Joel Fernandes, Daniel Borkmann,
Martin KaFai Lau, Song Liu, Yonghong Song, netdev, bpf
In-Reply-To: <faaf8b1c-9552-a0ae-3088-2f4255dff857@codeaurora.org>
+ Joel if he has seen this issue.
On 5/28/2019 2:27 PM, Srinivas Ramana wrote:
> Hello,
>
> I am trying to build samples/bpf in kernel(5.2.0-rc1) but unsuccessful
> with below errors. Can you help to point what i am missing or if there
> is some known issue?
>
> ==============================8<===================================
> $ make samples/bpf/
> LLC=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> CLANG=/local/mnt/workspace/tools/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> V=1
> make -C /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel -f
> /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/Makefile
> samples/bpf/
> ................
> ................
> ................
> make KBUILD_MODULES=1 -f ./scripts/Makefile.build obj=samples/bpf
> (cat /dev/null; ) > samples/bpf/modules.order
> make -C
> /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/
> RM='rm -rf' LDFLAGS=
> srctree=/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../
> O=
>
> Auto-detecting system features:
> ... libelf: [ on ]
> ... bpf: [ on ]
>
> make -C
> /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build
> CFLAGS= LDFLAGS= fixdep
> make -f
> /local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../..//tools/build/Makefile.build
> dir=. obj=fixdep
> ld -r -o fixdep-in.o fixdep.o
> ld: fixdep.o: Relocations in generic ELF (EM: 183)
> ld: fixdep.o: Relocations in generic ELF (EM: 183)
> fixdep.o: error adding symbols: File in wrong format
> make[5]: *** [fixdep-in.o] Error 1
> make[4]: *** [fixdep-in.o] Error 2
> make[3]: *** [fixdep] Error 2
> make[2]: ***
> [/local/mnt/workspace/sramana/kdev_torvalds/kdev/kernel/samples/bpf/../../tools/lib/bpf/libbpf.a]
> Error 2
> make[1]: *** [samples/bpf/] Error 2
> make: *** [sub-make] Error 2
> ==============================>8=======================================
>
>
> I am using the below commands to build:
> ========================================================
> export ARCH=arm64
> export CROSS_COMPILE=<path>linaro-toolchain/5.1/bin/aarch64-linux-gnu-
> export CLANG_TRIPLE=arm64-linux-gnu-
>
> make
> CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> defconfig
>
> make
> CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> -j8
>
> make
> CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> headers_install INSTALL_HDR_PATH=./usr
>
> make samples/bpf/
> LLC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/llc
> CLANG=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
> V=1
> CC=<path>/clang_ubuntu/clang/clang+llvm-8.0.0-x86_64-linux-gnu-ubuntu-14.04/bin/clang
>
> ========================================================
>
> Thanks,
> -- Srinivas R
>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation
Center, Inc., is a member of Code Aurora Forum, a Linux Foundation
Collaborative Project
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] selftests/bpf: test perf buffer API
From: Andrii Nakryiko @ 2019-06-26 5:11 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Networking, Kernel Team
In-Reply-To: <20190625232601.3227055-3-andriin@fb.com>
On Tue, Jun 25, 2019 at 4:26 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add test verifying perf buffer API functionality.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> .../selftests/bpf/prog_tests/perf_buffer.c | 86 +++++++++++++++++++
> .../selftests/bpf/progs/test_perf_buffer.c | 31 +++++++
> 2 files changed, 117 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_buffer.c b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> new file mode 100644
> index 000000000000..3ba3e26141ac
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <pthread.h>
> +#include <sched.h>
> +#include <sys/socket.h>
> +#include <test_progs.h>
> +
> +static void on_sample(void *ctx, void *data, __u32 size)
> +{
> + cpu_set_t *cpu_seen = ctx;
> + int cpu = *(int *)data;
> +
> + CPU_SET(cpu, cpu_seen);
> +}
> +
> +void test_perf_buffer(void)
> +{
> + int err, prog_fd, prog_pfd, nr_cpus, i, duration = 0;
> + const char *prog_name = "kprobe/sys_nanosleep";
> + const char *file = "./test_perf_buffer.o";
> + struct bpf_map *perf_buf_map;
> + cpu_set_t cpu_set, cpu_seen;
> + struct bpf_program *prog;
> + struct bpf_object *obj;
> + struct perf_buffer *pb;
> +
> + nr_cpus = libbpf_num_possible_cpus();
> + if (CHECK(nr_cpus < 0, "nr_cpus", "err %d\n", nr_cpus))
> + return;
> +
> + /* load program */
> + err = bpf_prog_load(file, BPF_PROG_TYPE_KPROBE, &obj, &prog_fd);
> + if (CHECK(err, "obj_load", "err %d errno %d\n", err, errno))
> + return;
> +
> + prog = bpf_object__find_program_by_title(obj, prog_name);
> + if (CHECK(!prog, "find_probe", "prog '%s' not found\n", prog_name))
> + goto out_close;
> +
> + /* load map */
> + perf_buf_map = bpf_object__find_map_by_name(obj, "perf_buf_map");
> + if (CHECK(!perf_buf_map, "find_perf_buf_map", "not found\n"))
> + goto out_close;
> +
> + /* attach kprobe */
> + prog_pfd = bpf_program__attach_kprobe(prog, false /* retprobe */,
> + "sys_nanosleep");
> + if (CHECK(prog_pfd < 0, "attach_kprobe", "err %d\n", prog_pfd))
> + goto out_close;
> +
> + /* set up perf buffer */
> + pb = perf_buffer__new(perf_buf_map, 1, on_sample, NULL, &cpu_seen);
> + if (CHECK(IS_ERR(pb), "perf_buf__new", "err %ld\n", PTR_ERR(pb)))
> + goto out_detach;
> +
> + /* trigger kprobe on every CPU */
> + CPU_ZERO(&cpu_seen);
> + for (i = 0; i < nr_cpus; i++) {
> + CPU_ZERO(&cpu_set);
> + CPU_SET(i, &cpu_set);
> +
> + err = pthread_setaffinity_np(pthread_self(), sizeof(cpu_set),
> + &cpu_set);
> + if (err && CHECK(err, "set_affinity", "cpu #%d, err %d\n",
> + i, err))
> + goto out_detach;
> +
> + usleep(1);
> + }
> +
> + /* read perf buffer */
> + err = perf_buffer__poll(pb, 100);
> + if (CHECK(err < 0, "perf_buffer__poll", "err %d\n", err))
> + goto out_free_pb;
> +
> + if (CHECK(CPU_COUNT(&cpu_seen) != nr_cpus, "seen_cpu_cnt",
> + "expect %d, seen %d\n", nr_cpus, CPU_COUNT(&cpu_seen)))
> + goto out_free_pb;
> +
> +out_free_pb:
> + perf_buffer__free(pb);
> +out_detach:
> + libbpf_perf_event_disable_and_close(prog_pfd);
> +out_close:
> + bpf_object__close(obj);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_perf_buffer.c b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> new file mode 100644
> index 000000000000..ba961f608fd5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_perf_buffer.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Facebook
> +
> +#include <linux/ptrace.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct {
> + int type;
> + int key_size;
> + int value_size;
> + int max_entries;
> +} perf_buf_map SEC(".maps") = {
> + .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(int),
> + .max_entries = 56,
Oh, this is not right, "works for me" only :). I've been meaning to
actually have another change to handle not specified max_entries (or
equivalently, max_entries == 0) to mean "all possible CPUs" for
BPF_MAP_TYPE_PERF_EVENT_ARRAY. Will produce v2 with that change.
> +};
> +
> +SEC("kprobe/sys_nanosleep")
> +int handle_sys_nanosleep_entry(struct pt_regs *ctx)
> +{
> + int cpu = bpf_get_smp_processor_id();
> +
> + bpf_perf_event_output(ctx, &perf_buf_map, BPF_F_CURRENT_CPU,
> + &cpu, sizeof(cpu));
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net] vxlan: do not destroy fdb if register_netdevice() is failed
From: Roopa Prabhu @ 2019-06-26 5:03 UTC (permalink / raw)
To: Taehee Yoo; +Cc: David Miller, Netdev
In-Reply-To: <CAMArcTWG-KLsmzrtQRGGmnUN31yz4UMqJ9FLyv3xNNPoXY_6=Q@mail.gmail.com>
On Tue, Jun 25, 2019 at 9:08 AM Taehee Yoo <ap420073@gmail.com> wrote:
>
> On Tue, 25 Jun 2019 at 13:12, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> >
>
> Hi Roopa,
>
> Thank you for the review!
>
> > On Sun, Jun 23, 2019 at 7:18 PM Taehee Yoo <ap420073@gmail.com> wrote:
> > >
> > > On Mon, 24 Jun 2019 at 03:07, David Miller <davem@davemloft.net> wrote:
> > > >
> > >
> > > Hi David,
> > >
> > > Thank you for the review!
> > >
> > > > From: Taehee Yoo <ap420073@gmail.com>
> > > > Date: Thu, 20 Jun 2019 20:51:08 +0900
> > > >
> > > > > __vxlan_dev_create() destroys FDB using specific pointer which indicates
> > > > > a fdb when error occurs.
> > > > > But that pointer should not be used when register_netdevice() fails because
> > > > > register_netdevice() internally destroys fdb when error occurs.
> > > > >
> > > > > In order to avoid un-registered dev's notification, fdb destroying routine
> > > > > checks dev's register status before notification.
> > > >
> > > > Simply pass do_notify as false in this failure code path of __vxlan_dev_create(),
> > > > thank you.
> > >
> > > Failure path of __vxlan_dev_create() can't handle do_notify in that case
> > > because if register_netdevice() fails it internally calls
> > > ->ndo_uninit() which is
> > > vxlan_uninit().
> > > vxlan_uninit() internally calls vxlan_fdb_delete_default() and it callls
> > > vxlan_fdb_destroy().
> > > do_notify of vxlan_fdb_destroy() in vxlan_fdb_delete_default() is always true.
> > > So, failure path of __vxlan_dev_create() doesn't have any opportunity to
> > > handle do_notify.
> >
> >
> > I don't see register_netdevice calling ndo_uninit in case of all
> > errors. In the case where it does not,
> > does your patch leak the fdb entry ?.
> >
> > Wondering if we should just use vxlan_fdb_delete_default with a notify
> > flag to delete the entry if exists.
> > Will that help ?
> >
> > There is another commit that touched this code path:
> > commit 6db9246871394b3a136cd52001a0763676563840
> >
> > Author: Petr Machata <petrm@mellanox.com>
> > Date: Tue Dec 18 13:16:00 2018 +0000
> > vxlan: Fix error path in __vxlan_dev_create()
>
> I have checked up failure path of register_netdevice().
> Yes, this patch leaks fdb entry.
> There are 3 failure cases in the register_netdevice().
> A. error occurs before calling ->ndo_init().
> it doesn't call ->ndo_uninit().
> B. error occurs after calling ->ndo_init().
> it calls ->ndo_uninit() and dev->reg_state is NETREG_UNINITIALIZED.
> C. error occurs after registering netdev. it calls rollback_registered().
> rollback_registered() internally calls ->ndo_uninit()
> and dev->reg_state is NETREG_UNREGISTERING.
>
> A panic due to these problem could be fixed by using
> vxlan_fdb_delete_default() with notify flag.
> But notification problem could not be fixed clearly
> because of the case C.
yes, you are right. The notification issue still remains.
>
> I don't have clear solution for the case C.
> Please let me know, if you have any good idea for fixing the case C.
One option is a variant of fdb create. alloc the fdb but don't assign
it to the vxlan dev.
__vxlan_dev_create
create fdb entry
register_netdevice
rtnl_configure_link
link fdb to vxlan
fdb notify
Yet another option is moving fdb create after register_netdevice
__vxlan_dev_create
register_netdevice
rtnl_configure_link
create fdb entry
fdb notify
But if fdb create fails, user-space will see , NEWLINK + DELLINK when
creating a vxlan device and that seems weird.
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: fix max() type mismatch for 32bit
From: Andrii Nakryiko @ 2019-06-26 4:58 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: Alexei Starovoitov, Networking, Daniel Borkmann, bpf, open list
In-Reply-To: <20190625202700.28030-1-ivan.khoronzhuk@linaro.org>
On Tue, Jun 25, 2019 at 1:28 PM Ivan Khoronzhuk
<ivan.khoronzhuk@linaro.org> wrote:
>
> It fixes build error for 32bit caused by type mismatch
> size_t/unsigned long.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> ---
Sorry, forgot to mention, this should probably have
Fixes: bf82927125dd ("libbpf: refactor map initialization")
With that:
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/lib/bpf/libbpf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 68f45a96769f..5186b7710430 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -778,7 +778,7 @@ static struct bpf_map *bpf_object__add_map(struct bpf_object *obj)
> if (obj->nr_maps < obj->maps_cap)
> return &obj->maps[obj->nr_maps++];
>
> - new_cap = max(4ul, obj->maps_cap * 3 / 2);
> + new_cap = max((size_t)4, obj->maps_cap * 3 / 2);
> new_maps = realloc(obj->maps, new_cap * sizeof(*obj->maps));
> if (!new_maps) {
> pr_warning("alloc maps for object failed\n");
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net] net: make skb_dst_force return false when dst was cleared
From: Eric Dumazet @ 2019-06-26 4:50 UTC (permalink / raw)
To: Florian Westphal, Eric Dumazet; +Cc: netdev
In-Reply-To: <20190625195943.44jvck5syvnzxb55@breakpoint.cc>
On 6/25/19 12:59 PM, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> -static inline void skb_dst_force(struct sk_buff *skb)
>>> +static inline bool skb_dst_force(struct sk_buff *skb)
>>> {
>>> if (skb_dst_is_noref(skb)) {
>>> struct dst_entry *dst = skb_dst(skb);
>>> @@ -313,7 +314,10 @@ static inline void skb_dst_force(struct sk_buff *skb)
>>> dst = NULL;
>>>
>>> skb->_skb_refdst = (unsigned long)dst;
>>> + return dst != NULL;
>>> }
>>> +
>>> + return true;
>>
>> This will return true, even if skb has a NULL dst.
>
> Yes, that was intentional -- it should return false to
> let caller know that no reference could be obtained and
> that the dst was invalidated as a result.
Problem is that some callers ignore skb_dst_force() return value.
>
>> Say if we have two skb_dst_force() calls for some reason
>> on the same skb, only the first one will return false.
>
> What would you suggest instead?
>
> Alternative is something like
>
> if (skb_dst(skb)) {
> skb_dst_force(skb);
> if (!skb_dst(skb)) {
> kfree_skb(skb);
> goto err;
> }
> }
Simply change
return true;
by
return skb->_skb_refdst != 0UL;
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] libbpf: add perf buffer reading API
From: Andrii Nakryiko @ 2019-06-26 4:44 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf,
Networking, Kernel Team
In-Reply-To: <CAPhsuW6FeBHHNgT3OA6x6i9kVsKutnVR46DFdkeG0cggaKbTnQ@mail.gmail.com>
On Tue, Jun 25, 2019 at 7:19 PM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Tue, Jun 25, 2019 at 4:28 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> > to user space for additional processing. libbpf already has very low-level API
> > to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> > use and requires a lot of code to set everything up. This patch adds
> > perf_buffer abstraction on top of it, abstracting setting up and polling
> > per-CPU logic into simple and convenient API, similar to what BCC provides.
> >
> > perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> > map entries. It accepts two user-provided callbacks: one for handling raw
> > samples and one for get notifications of lost samples due to buffer overflow.
> >
> > perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> > utilizing epoll instance.
> >
> > perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
> >
> > All APIs are not thread-safe. User should ensure proper locking/coordination if
> > used in multi-threaded set up.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Overall looks good. Some nit below.
Thanks for review!
>
> > ---
> > tools/lib/bpf/libbpf.c | 282 +++++++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 12 ++
> > tools/lib/bpf/libbpf.map | 5 +-
> > 3 files changed, 298 insertions(+), 1 deletion(-)
>
> [...]
>
> > +struct perf_buffer *perf_buffer__new(struct bpf_map *map, size_t page_cnt,
> > + perf_buffer_sample_fn sample_cb,
> > + perf_buffer_lost_fn lost_cb, void *ctx)
> > +{
> > + char msg[STRERR_BUFSIZE];
> > + struct perf_buffer *pb;
> > + int err, cpu;
> > +
> > + if (bpf_map__def(map)->type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) {
> > + pr_warning("map '%s' should be BPF_MAP_TYPE_PERF_EVENT_ARRAY\n",
> > + bpf_map__name(map));
> > + return ERR_PTR(-EINVAL);
> > + }
> > + if (bpf_map__fd(map) < 0) {
> > + pr_warning("map '%s' doesn't have associated FD\n",
> > + bpf_map__name(map));
> > + return ERR_PTR(-EINVAL);
> > + }
> > + if (page_cnt & (page_cnt - 1)) {
> > + pr_warning("page count should be power of two, but is %zu\n",
> > + page_cnt);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + pb = calloc(1, sizeof(*pb));
> > + if (!pb)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + pb->sample_cb = sample_cb;
> > + pb->lost_cb = lost_cb;
>
> I think we need to check sample_cb != NULL && lost_cb != NULL.
I was thinking about making them all either optional or required, but
eventually decided on making sample_cb required and lost_cb optional,
as in practice rarely sample_cb wouldn't be provided, while not every
application would care about handling lost samples (as there is little
you can do about that, except for bumping some counter).
As for checking for NULL. I feel like that's overkill. If someone
provided NULL for sample_cb, they will get SIGSEGV with stack trace
immediately showing that's it's sample_cb being NULL. Unlike Java, C
libraries tend not to double-check every pointer for NULL. Checking
for things like whether map has FD or is of correct type is valuable,
because if you don't check it early, then you'll just eventually get
-EINVAL from kernel and will start a guessing game of what's wrong.
Checking for callback to be non-null feels unnecessary, as it will be
immediately obvious (and it's quite unlikely this will happen in
practice).
>
> > + pb->ctx = ctx;
> > + pb->page_size = getpagesize();
> > + pb->mmap_size = pb->page_size * page_cnt;
> > + pb->mapfd = bpf_map__fd(map);
> > +
> > + pb->epfd = epoll_create1(EPOLL_CLOEXEC);
> [...]
> > +perf_buffer__process_record(struct perf_event_header *e, void *ctx)
> > +{
> > + struct perf_buffer *pb = ctx;
> > + void *data = e;
> > +
> > + switch (e->type) {
> > + case PERF_RECORD_SAMPLE: {
> > + struct perf_sample_raw *s = data;
> > +
> > + pb->sample_cb(pb->ctx, s->data, s->size);
> > + break;
> > + }
> > + case PERF_RECORD_LOST: {
> > + struct perf_sample_lost *s = data;
> > +
> > + if (pb->lost_cb)
> > + pb->lost_cb(pb->ctx, s->lost);
>
> OK, we test lost_cb here, so not necessary at init time.
>
> [...]
> > bpf_program__attach_perf_event;
> > bpf_program__attach_raw_tracepoint;
> > bpf_program__attach_tracepoint;
> > bpf_program__attach_uprobe;
> > + btf__parse_elf;
>
> Why move btf__parse_elf ?
I realized that I haven't put it in correct alphabetical order,
decided to fix it here, as it's just a single line change.
>
> Thanks,
> Song
^ permalink raw reply
* Re: kernel panic: stack is corrupted in validate_chain
From: syzbot @ 2019-06-26 4:41 UTC (permalink / raw)
To: ast, daniel, john.fastabend, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <000000000000c7a272058c2cde21@google.com>
syzbot has bisected this bug to:
commit e9db4ef6bf4ca9894bb324c76e01b8f1a16b2650
Author: John Fastabend <john.fastabend@gmail.com>
Date: Sat Jun 30 13:17:47 2018 +0000
bpf: sockhash fix omitted bucket lock in sock_close
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11d4e129a00000
start commit: 249155c2 Merge branch 'parisc-5.2-4' of git://git.kernel.o..
git tree: upstream
final crash: https://syzkaller.appspot.com/x/report.txt?x=13d4e129a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=15d4e129a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=9a31528e58cc12e2
dashboard link: https://syzkaller.appspot.com/bug?extid=6ba34346b252f2d497c7
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=135e34eea00000
Reported-by: syzbot+6ba34346b252f2d497c7@syzkaller.appspotmail.com
Fixes: e9db4ef6bf4c ("bpf: sockhash fix omitted bucket lock in sock_close")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
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