* Re: [PATCH net-next v4 1/4] net/sched: Introduce action ct
From: Florian Westphal @ 2019-07-08 14:02 UTC (permalink / raw)
To: Paul Blakey
Cc: Florian Westphal, Jiri Pirko, Roi Dayan, Yossi Kuperman,
Oz Shlomo, Marcelo Ricardo Leitner, netdev@vger.kernel.org,
David Miller, Aaron Conole, Zhike Wang, Rony Efraim,
nst-kernel@redhat.com, John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <55a5a05f-b2c0-dda0-e961-75a7b4821dc1@mellanox.com>
Paul Blakey <paulb@mellanox.com> wrote:
> Like if conntrack has just timed it out (or conntrack flushed), and skb
> holds the last ref?
Yes, thats very unlikely but its possible.
^ permalink raw reply
* Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
From: Jason Gunthorpe @ 2019-07-08 14:13 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: Leon Romanovsky, Kirsher, Jeffrey T, dledford@redhat.com,
davem@davemloft.net, Ismail, Mustafa, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, nhorman@redhat.com, sassmann@redhat.com,
poswald@suse.com, Ertman, David M
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A68512AA@fmsmsx124.amr.corp.intel.com>
On Sat, Jul 06, 2019 at 04:15:20PM +0000, Saleem, Shiraz wrote:
> > Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> >
> > On Fri, Jul 05, 2019 at 04:42:19PM +0000, Saleem, Shiraz wrote:
> > > > Subject: Re: [rdma 14/16] RDMA/irdma: Add ABI definitions
> > > >
> > > > On Thu, Jul 04, 2019 at 10:40:21AM +0300, Leon Romanovsky wrote:
> > > > > On Wed, Jul 03, 2019 at 07:12:57PM -0700, Jeff Kirsher wrote:
> > > > > > From: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > > >
> > > > > > Add ABI definitions for irdma.
> > > > > >
> > > > > > Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> > > > > > Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> > > > > > include/uapi/rdma/irdma-abi.h | 130
> > > > > > ++++++++++++++++++++++++++++++++++
> > > > > > 1 file changed, 130 insertions(+) create mode 100644
> > > > > > include/uapi/rdma/irdma-abi.h
> > > > > >
> > > > > > diff --git a/include/uapi/rdma/irdma-abi.h
> > > > > > b/include/uapi/rdma/irdma-abi.h new file mode 100644 index
> > > > > > 000000000000..bdfbda4c829e
> > > > > > +++ b/include/uapi/rdma/irdma-abi.h
> > > > > > @@ -0,0 +1,130 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> > > > > > +/* Copyright (c) 2006 - 2019 Intel Corporation. All rights reserved.
> > > > > > + * Copyright (c) 2005 Topspin Communications. All rights reserved.
> > > > > > + * Copyright (c) 2005 Cisco Systems. All rights reserved.
> > > > > > + * Copyright (c) 2005 Open Grid Computing, Inc. All rights reserved.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef IRDMA_ABI_H
> > > > > > +#define IRDMA_ABI_H
> > > > > > +
> > > > > > +#include <linux/types.h>
> > > > > > +
> > > > > > +/* irdma must support legacy GEN_1 i40iw kernel
> > > > > > + * and user-space whose last ABI ver is 5 */ #define
> > > > > > +IRDMA_ABI_VER
> > > > > > +6
> > > > >
> > > > > Can you please elaborate about it more?
> > > > > There is no irdma code in RDMA yet, so it makes me wonder why new
> > > > > define shouldn't start from 1.
> > > >
> > > > It is because they are ABI compatible with the current user space,
> > > > which raises the question why we even have this confusing header file..
> > >
> > > It is because we need to support current providers/i40iw user-space.
> > > Our user-space patch series will introduce a new provider (irdma)
> > > whose ABI ver. is also 6 (capable of supporting X722 and which will
> > > work with i40iw driver on older kernels) and removes providers/i40iw from rdma-
> > core.
> >
> > Why on earth would we do that?
> >
> A unified library providers/irdma to go in hand with the driver irdma and uses the ABI header.
> It can support the new network device e810 and existing x722 iWARP device. It obsoletes
> providers/i40iw and extends its ABI. So why keep providers/i40iw around in rdma-core?
Why rewrite a perfectly good userspace that is compatible with the
future and past kernels?
Is there something so wrong with the userspace provider to need this?
Jason
^ permalink raw reply
* Re: [PATCH nf-next 1/3] netfilter: nf_nat_proto: add nf_nat_bridge_ops support
From: Florian Westphal @ 2019-07-08 14:17 UTC (permalink / raw)
To: wenxu; +Cc: pablo, fw, netfilter-devel, netdev
In-Reply-To: <1562574567-8293-1-git-send-email-wenxu@ucloud.cn>
wenxu@ucloud.cn <wenxu@ucloud.cn> wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> Add nf_nat_bridge_ops to do nat in the bridge family
Whats the use case for this?
The reason I'm asking is that a bridge doesn't know about IP,
Bridge netfilter (the call-iptables thing) has a lot of glue code
to detect dnat rewrites and updates target mac address, including
support for redirect (suddently packet has to be pushed up the stack)
or changes in the oif to non-bridge ports (it even checks forward sysctl
state ..) and so on.
Thats something that I don't want to support in nftables.
For NAT on bridge, it should be possible already to push such packets
up the stack by
bridge input meta iif eth0 ip saddr 192.168.0.0/16 \
meta pkttype set unicast ether daddr set 00:11:22:33:44:55
then normal ip processing handles this and nat should "just work".
If above doesn't work for you I'd like to understand why.
^ permalink raw reply
* Re: [PATCH bpf] xdp: fix potential deadlock on socket mutex
From: Jonathan Lemon @ 2019-07-08 14:21 UTC (permalink / raw)
To: Ilya Maximets
Cc: netdev, linux-kernel, bpf, xdp-newbies, David S. Miller,
Björn Töpel, Magnus Karlsson, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190708110344.23278-1-i.maximets@samsung.com>
On 8 Jul 2019, at 4:03, Ilya Maximets wrote:
> There are 2 call chains:
>
> a) xsk_bind --> xdp_umem_assign_dev
> b) unregister_netdevice_queue --> xsk_notifier
>
> with the following locking order:
>
> a) xs->mutex --> rtnl_lock
> b) rtnl_lock --> xdp.lock --> xs->mutex
>
> Different order of taking 'xs->mutex' and 'rtnl_lock' could produce a
> deadlock here. Fix that by moving the 'rtnl_lock' before 'xs->lock' in
> the bind call chain (a).
>
> Reported-by: syzbot+bf64ec93de836d7f4c2c@syzkaller.appspotmail.com
> Fixes: 455302d1c9ae ("xdp: fix hang while unregistering device bound
> to xdp socket")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Thanks, Ilya!
I think in the long run the locking needs to be revisited,
but this should fix the deadlock for now.
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>
> This patch is a fix for patch that is not yet in mainline, but
> already in 'net' tree. I'm not sure what is the correct process
> for applying such fixes.
>
> net/xdp/xdp_umem.c | 16 ++++++----------
> net/xdp/xsk.c | 2 ++
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 20c91f02d3d8..83de74ca729a 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -87,21 +87,20 @@ int xdp_umem_assign_dev(struct xdp_umem *umem,
> struct net_device *dev,
> struct netdev_bpf bpf;
> int err = 0;
>
> + ASSERT_RTNL();
> +
> force_zc = flags & XDP_ZEROCOPY;
> force_copy = flags & XDP_COPY;
>
> if (force_zc && force_copy)
> return -EINVAL;
>
> - rtnl_lock();
> - if (xdp_get_umem_from_qid(dev, queue_id)) {
> - err = -EBUSY;
> - goto out_rtnl_unlock;
> - }
> + if (xdp_get_umem_from_qid(dev, queue_id))
> + return -EBUSY;
>
> err = xdp_reg_umem_at_qid(dev, umem, queue_id);
> if (err)
> - goto out_rtnl_unlock;
> + return err;
>
> umem->dev = dev;
> umem->queue_id = queue_id;
> @@ -110,7 +109,7 @@ int xdp_umem_assign_dev(struct xdp_umem *umem,
> struct net_device *dev,
>
> if (force_copy)
> /* For copy-mode, we are done. */
> - goto out_rtnl_unlock;
> + return 0;
>
> if (!dev->netdev_ops->ndo_bpf ||
> !dev->netdev_ops->ndo_xsk_async_xmit) {
> @@ -125,7 +124,6 @@ int xdp_umem_assign_dev(struct xdp_umem *umem,
> struct net_device *dev,
> err = dev->netdev_ops->ndo_bpf(dev, &bpf);
> if (err)
> goto err_unreg_umem;
> - rtnl_unlock();
>
> umem->zc = true;
> return 0;
> @@ -135,8 +133,6 @@ int xdp_umem_assign_dev(struct xdp_umem *umem,
> struct net_device *dev,
> err = 0; /* fallback to copy mode */
> if (err)
> xdp_clear_umem_at_qid(dev, queue_id);
> -out_rtnl_unlock:
> - rtnl_unlock();
> return err;
> }
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 703cf5ea448b..2aa6072a3e55 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -416,6 +416,7 @@ static int xsk_bind(struct socket *sock, struct
> sockaddr *addr, int addr_len)
> if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY))
> return -EINVAL;
>
> + rtnl_lock();
> mutex_lock(&xs->mutex);
> if (xs->state != XSK_READY) {
> err = -EBUSY;
> @@ -501,6 +502,7 @@ static int xsk_bind(struct socket *sock, struct
> sockaddr *addr, int addr_len)
> xs->state = XSK_BOUND;
> out_release:
> mutex_unlock(&xs->mutex);
> + rtnl_unlock();
> return err;
> }
>
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH net-next 11/16] net/mlx5e: Add support to rx reporter diagnose
From: Jiri Pirko @ 2019-07-08 14:22 UTC (permalink / raw)
To: Tariq Toukan
Cc: David S. Miller, netdev, Eran Ben Elisha, ayal, jiri,
Saeed Mahameed, moshe
In-Reply-To: <1562500388-16847-12-git-send-email-tariqt@mellanox.com>
Sun, Jul 07, 2019 at 01:53:03PM CEST, tariqt@mellanox.com wrote:
>From: Aya Levin <ayal@mellanox.com>
>
>Add rx reporter, which supports diagnose call-back. Diagnostics output
>include: information common to all RQs: RQ type, RQ size, RQ stride
>size, CQ size and CQ stride size. In addition advertise information per
>RQ and its related icosq and attached CQ.
>
>$ devlink health diagnose pci/0000:00:0b.0 reporter rx
>Common config:
> RQ: type: 2 stride size: 2048 size: 8
> CQ: stride size: 64 size: 1024
> RQs:
> channel ix: 0 rqn: 4284 HW state: 1 SW state: 3 posted WQEs: 7 cc: 7 ICOSQ HW state: 1 CQ: cqn: 1032 HW status: 0
> channel ix: 1 rqn: 4289 HW state: 1 SW state: 3 posted WQEs: 7 cc: 7 ICOSQ HW state: 1 CQ: cqn: 1036 HW status: 0
> channel ix: 2 rqn: 4294 HW state: 1 SW state: 3 posted WQEs: 7 cc: 7 ICOSQ HW state: 1 CQ: cqn: 1040 HW status: 0
> channel ix: 3 rqn: 4299 HW state: 1 SW state: 3 posted WQEs: 7 cc: 7 ICOSQ HW state: 1 CQ: cqn: 1044 HW status: 0
>
>$ devlink health diagnose pci/0000:00:0b.0 reporter rx -jp
>{
> "Common config": [
> "RQ": {
> "type": 2,
> "stride size": 2048,
> "size": 8
> },
> "CQ": {
> "stride size": 64,
> "size": 1024
> } ],
> "RQs": [ {
> "channel ix": 0,
> "rqn": 4284,
> "HW state": 1,
> "SW state": 3,
> "posted WQEs": 7,
> "cc": 7,
> "ICOSQ HW state": 1,
> "CQ": {
> "cqn": 1032,
> "HW status": 0
> }
> },{
> "channel ix": 1,
> "rqn": 4289,
> "HW state": 1,
> "SW state": 3,
> "posted WQEs": 7,
> "cc": 7,
> "ICOSQ HW state": 1,
> "CQ": {
> "cqn": 1036,
> "HW status": 0
> }
> },{
> "channel ix": 2,
> "rqn": 4294,
> "HW state": 1,
> "SW state": 3,
> "posted WQEs": 7,
> "cc": 7,
> "ICOSQ HW state": 1,
> "CQ": {
> "cqn": 1040,
> "HW status": 0
> }
> },{
> "channel ix": 3,
> "rqn": 4299,
> "HW state": 1,
> "SW state": 3,
> "posted WQEs": 7,
> "cc": 7,
> "ICOSQ HW state": 1,
> "CQ": {
> "cqn": 1044,
> "HW status": 0
> }
> } ]
>}
>
>Signed-off-by: Aya Levin <ayal@mellanox.com>
>Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
>---
> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 4 +-
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 21 +++
> .../net/ethernet/mellanox/mlx5/core/en/health.c | 31 ++++
> .../net/ethernet/mellanox/mlx5/core/en/health.h | 7 +
> .../ethernet/mellanox/mlx5/core/en/reporter_rx.c | 194 +++++++++++++++++++++
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 29 +--
> 6 files changed, 258 insertions(+), 28 deletions(-)
> create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>index 23d566a45a30..a3b9659649a8 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
>@@ -24,8 +24,8 @@ mlx5_core-y := main.o cmd.o debugfs.o fw.o eq.o uar.o pagealloc.o \
> mlx5_core-$(CONFIG_MLX5_CORE_EN) += en_main.o en_common.o en_fs.o en_ethtool.o \
> en_tx.o en_rx.o en_dim.o en_txrx.o en/xdp.o en_stats.o \
> en_selftest.o en/port.o en/monitor_stats.o en/health.o \
>- en/reporter_tx.o en/params.o en/xsk/umem.o en/xsk/setup.o \
>- en/xsk/rx.o en/xsk/tx.o
>+ en/reporter_tx.o en/reporter_rx.o en/params.o en/xsk/umem.o \
>+ en/xsk/setup.o en/xsk/rx.o en/xsk/tx.o
>
> #
> # Netdev extra
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>index 263558875f20..f7c5cf7a7064 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>@@ -848,6 +848,7 @@ struct mlx5e_priv {
> struct mlx5e_tls *tls;
> #endif
> struct devlink_health_reporter *tx_reporter;
>+ struct devlink_health_reporter *rx_reporter;
> struct mlx5e_xsk xsk;
> };
>
>@@ -888,6 +889,26 @@ netdev_tx_t mlx5e_sq_xmit(struct mlx5e_txqsq *sq, struct sk_buff *skb,
> int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget);
> void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq);
>
>+static inline u32 mlx5e_rqwq_get_size(struct mlx5e_rq *rq)
>+{
>+ switch (rq->wq_type) {
>+ case MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ:
>+ return mlx5_wq_ll_get_size(&rq->mpwqe.wq);
>+ default:
>+ return mlx5_wq_cyc_get_size(&rq->wqe.wq);
>+ }
>+}
>+
>+static inline u32 mlx5e_rqwq_get_cur_sz(struct mlx5e_rq *rq)
>+{
>+ switch (rq->wq_type) {
>+ case MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ:
>+ return rq->mpwqe.wq.cur_sz;
>+ default:
>+ return rq->wqe.wq.cur_sz;
>+ }
>+}
>+
> bool mlx5e_check_fragmented_striding_rq_cap(struct mlx5_core_dev *mdev);
> bool mlx5e_striding_rq_possible(struct mlx5_core_dev *mdev,
> struct mlx5e_params *params);
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/health.c b/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
>index a266717d41e5..a0579de8e2e0 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/health.c
>@@ -96,6 +96,37 @@ int mlx5e_reporter_cq_common_diagnose(struct mlx5e_cq *cq, struct devlink_fmsg *
> return 0;
> }
>
>+int mlx5e_health_create_reporters(struct mlx5e_priv *priv)
>+{
>+ int err;
>+
>+ err = mlx5e_reporter_tx_create(priv);
>+ if (err)
>+ return err;
>+
>+ err = mlx5e_reporter_rx_create(priv);
>+ if (err)
>+ return err;
>+
>+ return 0;
>+}
>+
>+void mlx5e_health_destroy_reporters(struct mlx5e_priv *priv)
>+{
>+ mlx5e_reporter_rx_destroy(priv);
>+ mlx5e_reporter_tx_destroy(priv);
>+}
>+
>+void mlx5e_health_channels_update(struct mlx5e_priv *priv)
>+{
>+ if (priv->tx_reporter)
>+ devlink_health_reporter_state_update(priv->tx_reporter,
>+ DEVLINK_HEALTH_REPORTER_STATE_HEALTHY);
>+ if (priv->rx_reporter)
>+ devlink_health_reporter_state_update(priv->rx_reporter,
>+ DEVLINK_HEALTH_REPORTER_STATE_HEALTHY);
>+}
Could you do this introduction of mlx5e_health_create_reporters(),
mlx5e_health_destroy_reporters(), and mlx5e_health_channels_update()
in a separate patch before this one?
>+
> int mlx5e_health_sq_to_ready(struct mlx5e_channel *channel, u32 sqn)
> {
> struct mlx5_core_dev *mdev = channel->mdev;
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/health.h b/drivers/net/ethernet/mellanox/mlx5/core/en/health.h
>index b0c8bda3d25f..7829ea229914 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en/health.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/health.h
>@@ -16,6 +16,9 @@
> int mlx5e_reporter_named_obj_nest_start(struct devlink_fmsg *fmsg, char *name);
> int mlx5e_reporter_named_obj_nest_end(struct devlink_fmsg *fmsg);
>
>+int mlx5e_reporter_rx_create(struct mlx5e_priv *priv);
>+void mlx5e_reporter_rx_destroy(struct mlx5e_priv *priv);
>+
> #define MLX5E_REPORTER_PER_Q_MAX_LEN 256
>
> struct mlx5e_err_ctx {
>@@ -29,5 +32,9 @@ struct mlx5e_err_ctx {
> int mlx5e_health_report(struct mlx5e_priv *priv,
> struct devlink_health_reporter *reporter, char *err_str,
> struct mlx5e_err_ctx *err_ctx);
>+int mlx5e_health_create_reporters(struct mlx5e_priv *priv);
>+void mlx5e_health_destroy_reporters(struct mlx5e_priv *priv);
>+void mlx5e_health_channels_update(struct mlx5e_priv *priv);
>+
>
> #endif
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
>new file mode 100644
>index 000000000000..b24bdff473b0
>--- /dev/null
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
>@@ -0,0 +1,194 @@
>+// SPDX-License-Identifier: GPL-2.0
>+// Copyright (c) 2019 Mellanox Technologies.
>+
>+#include "health.h"
>+#include "params.h"
>+
>+static int mlx5e_query_rq_state(struct mlx5_core_dev *dev, u32 rqn, u8 *state)
>+{
>+ int outlen = MLX5_ST_SZ_BYTES(query_rq_out);
>+ void *out;
>+ void *rqc;
>+ int err;
>+
>+ out = kvzalloc(outlen, GFP_KERNEL);
>+ if (!out)
>+ return -ENOMEM;
>+
>+ err = mlx5_core_query_rq(dev, rqn, out);
>+ if (err)
>+ goto out;
>+
>+ rqc = MLX5_ADDR_OF(query_rq_out, out, rq_context);
>+ *state = MLX5_GET(rqc, rqc, state);
>+
>+out:
>+ kvfree(out);
>+ return err;
>+}
>+
>+static int mlx5e_rx_reporter_build_diagnose_output(struct mlx5e_rq *rq,
>+ struct devlink_fmsg *fmsg)
>+{
>+ struct mlx5e_priv *priv = rq->channel->priv;
>+ struct mlx5e_params *params = &priv->channels.params;
>+ struct mlx5e_icosq *icosq = &rq->channel->icosq;
>+ u8 icosq_hw_state;
>+ int wqes_sz;
>+ u8 hw_state;
>+ u16 wq_head;
>+ int err;
>+
>+ err = mlx5e_query_rq_state(priv->mdev, rq->rqn, &hw_state);
>+ if (err)
>+ return err;
>+
>+ err = mlx5_core_query_sq_state(priv->mdev, icosq->sqn, &icosq_hw_state);
>+ if (err)
>+ return err;
>+
>+ wqes_sz = mlx5e_rqwq_get_cur_sz(rq);
>+ wq_head = params->rq_wq_type == MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ ?
>+ rq->mpwqe.wq.head : mlx5_wq_cyc_get_head(&rq->wqe.wq);
>+
>+ err = devlink_fmsg_obj_nest_start(fmsg);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u32_pair_put(fmsg, "channel ix", rq->channel->ix);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u32_pair_put(fmsg, "rqn", rq->rqn);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u8_pair_put(fmsg, "HW state", hw_state);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u8_pair_put(fmsg, "SW state", rq->state);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u32_pair_put(fmsg, "posted WQEs", wqes_sz);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u32_pair_put(fmsg, "cc", wq_head);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_u8_pair_put(fmsg, "ICOSQ HW state", icosq_hw_state);
>+ if (err)
>+ return err;
>+
>+ err = mlx5e_reporter_cq_diagnose(&rq->cq, fmsg);
>+ if (err)
>+ return err;
>+
>+ err = devlink_fmsg_obj_nest_end(fmsg);
>+ if (err)
>+ return err;
>+
>+ return 0;
>+}
>+
>+static int mlx5e_rx_reporter_diagnose(struct devlink_health_reporter *reporter,
>+ struct devlink_fmsg *fmsg)
>+{
>+ struct mlx5e_priv *priv = devlink_health_reporter_priv(reporter);
>+ struct mlx5e_params *params = &priv->channels.params;
>+ struct mlx5e_rq *generic_rq;
>+ u32 rq_stride, rq_sz;
>+ int i, err = 0;
>+
>+ mutex_lock(&priv->state_lock);
>+
>+ if (!test_bit(MLX5E_STATE_OPENED, &priv->state))
>+ goto unlock;
>+
>+ generic_rq = &priv->channels.c[0]->rq;
>+ rq_sz = mlx5e_rqwq_get_size(generic_rq);
>+ rq_stride = BIT(mlx5e_mpwqe_get_log_stride_size(priv->mdev, params, NULL));
>+
>+ err = devlink_fmsg_arr_pair_nest_start(fmsg, "Common config");
>+ if (err)
>+ goto unlock;
>+
>+ err = mlx5e_reporter_named_obj_nest_start(fmsg, "RQ");
>+ if (err)
>+ goto unlock;
>+
>+ err = devlink_fmsg_u8_pair_put(fmsg, "type", params->rq_wq_type);
>+ if (err)
>+ goto unlock;
>+
>+ err = devlink_fmsg_u64_pair_put(fmsg, "stride size", rq_stride);
>+ if (err)
>+ goto unlock;
>+
>+ err = devlink_fmsg_u32_pair_put(fmsg, "size", rq_sz);
>+ if (err)
>+ goto unlock;
>+
>+ err = devlink_fmsg_arr_pair_nest_end(fmsg);
This is odd. I think that you should have
mlx5e_reporter_named_obj_nest_end() called here and
devlink_fmsg_arr_pair_nest_end() after
mlx5e_reporter_cq_common_diagnose()
Misuse of this seems to be a pattern. We need some checker for this
apparently.
>+ if (err)
>+ goto unlock;
>+
>+ err = mlx5e_reporter_cq_common_diagnose(&generic_rq->cq, fmsg);
>+ if (err)
>+ goto unlock;
>+
>+ err = mlx5e_reporter_named_obj_nest_end(fmsg);
>+ if (err)
>+ goto unlock;
>+
>+ err = devlink_fmsg_arr_pair_nest_start(fmsg, "RQs");
>+ if (err)
>+ goto unlock;
>+
>+ for (i = 0; i < priv->channels.num; i++) {
>+ struct mlx5e_rq *rq = &priv->channels.c[i]->rq;
>+
>+ err = mlx5e_rx_reporter_build_diagnose_output(rq, fmsg);
>+ if (err)
>+ goto unlock;
>+ }
>+ err = devlink_fmsg_arr_pair_nest_end(fmsg);
>+ if (err)
>+ goto unlock;
>+unlock:
>+ mutex_unlock(&priv->state_lock);
>+ return err;
>+}
>+
>+static const struct devlink_health_reporter_ops mlx5_rx_reporter_ops = {
>+ .name = "rx",
>+ .diagnose = mlx5e_rx_reporter_diagnose,
>+};
>+
>+int mlx5e_reporter_rx_create(struct mlx5e_priv *priv)
>+{
>+ struct devlink_health_reporter *reporter;
>+ struct mlx5_core_dev *mdev = priv->mdev;
>+ struct devlink *devlink = priv_to_devlink(mdev);
>+
>+ reporter =
>+ devlink_health_reporter_create(devlink, &mlx5_rx_reporter_ops,
>+ 0, false, priv);
Rather align like this:
reporter = devlink_health_reporter_create(devlink,
&mlx5_rx_reporter_ops,
0, false, priv);
>+ if (IS_ERR(reporter))
>+ netdev_warn(priv->netdev, "Failed to create rx reporter, err = %ld\n",
>+ PTR_ERR(reporter));
>+ else
>+ priv->tx_reporter = reporter;
>+ return PTR_ERR_OR_ZERO(reporter);
Change the flow to:
if (IS_ERR(reporter)) {
netdev_warn(priv->netdev, "Failed to create rx reporter, err = %ld\n",
PTR_ERR(reporter));
return PTR_ERR(reporter);
}
priv->tx_reporter = reporter;
return 0;
>+}
>+
>+void mlx5e_reporter_rx_destroy(struct mlx5e_priv *priv)
>+{
>+ if (!priv->rx_reporter)
>+ return;
>+
>+ devlink_health_reporter_destroy(priv->rx_reporter);
>+}
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>index 98c925e72706..3922905e909f 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>@@ -247,26 +247,6 @@ static inline void mlx5e_build_umr_wqe(struct mlx5e_rq *rq,
> ucseg->mkey_mask = cpu_to_be64(MLX5_MKEY_MASK_FREE);
> }
>
>-static u32 mlx5e_rqwq_get_size(struct mlx5e_rq *rq)
>-{
>- switch (rq->wq_type) {
>- case MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ:
>- return mlx5_wq_ll_get_size(&rq->mpwqe.wq);
>- default:
>- return mlx5_wq_cyc_get_size(&rq->wqe.wq);
>- }
>-}
>-
>-static u32 mlx5e_rqwq_get_cur_sz(struct mlx5e_rq *rq)
>-{
>- switch (rq->wq_type) {
>- case MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ:
>- return rq->mpwqe.wq.cur_sz;
>- default:
>- return rq->wqe.wq.cur_sz;
>- }
>-}
>-
> static int mlx5e_rq_alloc_mpwqe_info(struct mlx5e_rq *rq,
> struct mlx5e_channel *c)
> {
>@@ -2320,10 +2300,7 @@ int mlx5e_open_channels(struct mlx5e_priv *priv,
> goto err_close_channels;
> }
>
>- if (priv->tx_reporter)
>- devlink_health_reporter_state_update(priv->tx_reporter,
>- DEVLINK_HEALTH_REPORTER_STATE_HEALTHY);
>-
>+ mlx5e_health_channels_update(priv);
> kvfree(cparam);
> return 0;
>
>@@ -3201,7 +3178,6 @@ static void mlx5e_cleanup_nic_tx(struct mlx5e_priv *priv)
> {
> int tc;
>
>- mlx5e_reporter_tx_destroy(priv);
> for (tc = 0; tc < priv->profile->max_tc; tc++)
> mlx5e_destroy_tis(priv->mdev, priv->tisn[tc]);
> }
>@@ -4985,12 +4961,14 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
> mlx5_core_err(mdev, "TLS initialization failed, %d\n", err);
> mlx5e_build_nic_netdev(netdev);
> mlx5e_build_tc2txq_maps(priv);
>+ mlx5e_health_create_reporters(priv);
>
> return 0;
> }
>
> static void mlx5e_nic_cleanup(struct mlx5e_priv *priv)
> {
>+ mlx5e_health_destroy_reporters(priv);
> mlx5e_tls_cleanup(priv);
> mlx5e_ipsec_cleanup(priv);
> mlx5e_netdev_cleanup(priv->netdev, priv);
>@@ -5093,7 +5071,6 @@ static int mlx5e_init_nic_tx(struct mlx5e_priv *priv)
> #ifdef CONFIG_MLX5_CORE_EN_DCB
> mlx5e_dcbnl_initialize(priv);
> #endif
>- mlx5e_reporter_tx_create(priv);
> return 0;
> }
>
>--
>1.8.3.1
>
^ permalink raw reply
* Re: [PATCH net-next 3/4] bnxt_en: optimized XDP_REDIRECT support
From: Andy Gospodarek @ 2019-07-08 14:26 UTC (permalink / raw)
To: Ilias Apalodimas; +Cc: Michael Chan, davem, netdev, hawk, ast, ivan.khoronzhuk
In-Reply-To: <20190708082803.GA28592@apalos>
On Mon, Jul 08, 2019 at 11:28:03AM +0300, Ilias Apalodimas wrote:
> Thanks Andy, Michael
>
> > + if (event & BNXT_REDIRECT_EVENT)
> > + xdp_do_flush_map();
> > +
> > if (event & BNXT_TX_EVENT) {
> > struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
> > u16 prod = txr->tx_prod;
> > @@ -2254,9 +2257,23 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
> >
> > for (j = 0; j < max_idx;) {
> > struct bnxt_sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
> > - struct sk_buff *skb = tx_buf->skb;
> > + struct sk_buff *skb;
> > int k, last;
> >
> > + if (i < bp->tx_nr_rings_xdp &&
> > + tx_buf->action == XDP_REDIRECT) {
> > + dma_unmap_single(&pdev->dev,
> > + dma_unmap_addr(tx_buf, mapping),
> > + dma_unmap_len(tx_buf, len),
> > + PCI_DMA_TODEVICE);
> > + xdp_return_frame(tx_buf->xdpf);
> > + tx_buf->action = 0;
> > + tx_buf->xdpf = NULL;
> > + j++;
> > + continue;
> > + }
> > +
>
> Can't see the whole file here and maybe i am missing something, but since you
> optimize for that and start using page_pool, XDP_TX will be a re-synced (and
> not remapped) buffer that can be returned to the pool and resynced for
> device usage.
> Is that happening later on the tx clean function?
Take a look at the way we treat the buffers in bnxt_rx_xdp() when we
receive them and then in bnxt_tx_int_xdp() when the transmits have
completed (for XDP_TX and XDP_REDIRECT). I think we are doing what is
proper with respect to mapping vs sync for both cases, but I would be
fine to be corrected.
>
> > + skb = tx_buf->skb;
> > if (!skb) {
> > j++;
> > continue;
> > @@ -2517,6 +2534,13 @@ static int bnxt_alloc_rx_rings(struct bnxt *bp)
> > if (rc < 0)
> > return rc;
> >
> > + rc = xdp_rxq_info_reg_mem_model(&rxr->xdp_rxq,
> > + MEM_TYPE_PAGE_SHARED, NULL);
> > + if (rc) {
> > + xdp_rxq_info_unreg(&rxr->xdp_rxq);
>
> I think you can use page_pool_free directly here (and pge_pool_destroy once
> Ivan's patchset gets nerged), that's what mlx5 does iirc. Can we keep that
> common please?
That's an easy change, I can do that.
>
> If Ivan's patch get merged please note you'll have to explicitly
> page_pool_destroy, after calling xdp_rxq_info_unreg() in the general unregister
> case (not the error habdling here). Sorry for the confusion this might bring!
Funny enough the driver was basically doing that until page_pool_destroy
was removed (these patches are not new). I saw last week there was
discussion to add it back, but I did not want to wait to get this on the
list before that was resolved.
This path works as expected with the code in the tree today so it seemed
like the correct approach to post something that is working, right? :-)
>
> > + return rc;
> > + }
> > +
> > rc = bnxt_alloc_ring(bp, &ring->ring_mem);
> > if (rc)
> > return rc;
> > @@ -10233,6 +10257,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
> [...]
>
> Thanks!
> /Ilias
^ permalink raw reply
* Re: [PATCH net-next v6 05/15] ethtool: helper functions for netlink interface
From: Jiri Pirko @ 2019-07-08 14:40 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190708122251.GB24474@unicorn.suse.cz>
Mon, Jul 08, 2019 at 02:22:51PM CEST, mkubecek@suse.cz wrote:
>On Wed, Jul 03, 2019 at 12:04:35PM +0200, Jiri Pirko wrote:
>> Tue, Jul 02, 2019 at 06:34:37PM CEST, mkubecek@suse.cz wrote:
>> >On Tue, Jul 02, 2019 at 03:05:15PM +0200, Jiri Pirko wrote:
>> >> Tue, Jul 02, 2019 at 01:50:04PM CEST, mkubecek@suse.cz wrote:
>> >> >+/**
>> >> >+ * ethnl_is_privileged() - check if request has sufficient privileges
>> >> >+ * @skb: skb with client request
>> >> >+ *
>> >> >+ * Checks if client request has CAP_NET_ADMIN in its netns. Unlike the flags
>> >> >+ * in genl_ops, this allows finer access control, e.g. allowing or denying
>> >> >+ * the request based on its contents or witholding only part of the data
>> >> >+ * from unprivileged users.
>> >> >+ *
>> >> >+ * Return: true if request is privileged, false if not
>> >> >+ */
>> >> >+static inline bool ethnl_is_privileged(struct sk_buff *skb)
>> >>
>> >> I wonder why you need this helper. Genetlink uses
>> >> ops->flags & GENL_ADMIN_PERM for this.
>> >
>> >It's explained in the function description. Sometimes we need finer
>> >control than by request message type. An example is the WoL password:
>> >ETHTOOL_GWOL is privileged because of it but I believe there si no
>> >reason why unprivileged user couldn't see enabled WoL modes, we can
>> >simply omit the password for him. (Also, it allows to combine query for
>> >WoL settings with other unprivileged settings.)
>>
>> Why can't we have rather:
>> ETHTOOL_WOL_GET for all
>> ETHTOOL_WOL_PASSWORD_GET with GENL_ADMIN_PERM
>> ?
>> Better to stick with what we have in gennetlink rather then to bend the
>> implementation from the very beginning I think.
>
>We can. But it would also mean two separate SET requests (or breaking
>the rule that _GET_REPLY, _SET and _NTF share the layout). That would be
>unfortunate as ethtool_ops callback does not actually allow setting only
>the modes so that the ETHTOOL_MSG_WOL_SET request (which would have to
>go first as many drivers ignore .sopass if WAKE_MAGICSECURE is not set)
>would have to pass a different password (most likely just leaving what
>->get_wol() put there) and that password would be actually set until the
>second request arrives. There goes the idea of getting rid of ioctl
>interface raciness...
I understand. That is my concern, not to bring baggage from ioclt :/
>
>I would rather see returning to WoL modes not being visible to
>unprivileged users than that (even if there is no actual reason for it).
>Anyway, shortening the series left WoL settings out if the first part so
>that I can split this out for now and leave the discussion for when we
>get to WoL one day.
Fine.
>
>> >> >+/**
>> >> >+ * ethnl_reply_header_size() - total size of reply header
>> >> >+ *
>> >> >+ * This is an upper estimate so that we do not need to hold RTNL lock longer
>> >> >+ * than necessary (to prevent rename between size estimate and composing the
>> >>
>> >> I guess this description is not relevant anymore. I don't see why to
>> >> hold rtnl mutex for this function...
>> >
>> >You don't need it for this function, it's the other way around: unless
>> >you hold RTNL lock for the whole time covering both checking needed
>> >message size and filling the message - and we don't - the device could
>> >be renamed in between. Thus if we returned size based on current device
>> >name, it might not be sufficient at the time the header is filled.
>> >That's why this function returns maximum possible size (which is
>> >actually a constant).
>>
>> I suggest to avoid the description. It is misleading. Perhaps something
>> to have in a patch description but not here in code.
>
>The reason I put the comment there was to prevent someone "optimizing"
>the helper by using strlen() later. Maybe something shorter and more to
>the point, e.g.
>
> Using IFNAMSIZ is faster and prevents a race if the device is renamed
> before we fill the name into skb.
>
>?
Sounds good, thanks!
>
>Michal
^ permalink raw reply
* [PATCH net-next] net: dsa: vsc73xx: Fix Kconfig warning and build errors
From: YueHaibing @ 2019-07-08 14:42 UTC (permalink / raw)
To: andrew, vivien.didelot, f.fainelli, davem, paweldembicki
Cc: linux-kernel, netdev, YueHaibing
Fix Kconfig dependency warning and subsequent build errors
caused by OF is not set:
WARNING: unmet direct dependencies detected for NET_DSA_VITESSE_VSC73XX
Depends on [n]: NETDEVICES [=y] && HAVE_NET_DSA [=y] && OF [=n] && NET_DSA [=m]
Selected by [m]:
- NET_DSA_VITESSE_VSC73XX_PLATFORM [=m] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && HAS_IOMEM [=y]
Move OF and NET_DSA dependencies to NET_DSA_VITESSE_VSC73XX/
NET_DSA_VITESSE_VSC73XX_PLATFORM to fix this.
Reported-by: Hulk Robot <hulkci@huawei.com>
Fixes: 631e83bf7c0e ("net: dsa: vsc73xx: add support for parallel mode")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/dsa/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/Kconfig b/drivers/net/dsa/Kconfig
index cf9dbd1..e28c209 100644
--- a/drivers/net/dsa/Kconfig
+++ b/drivers/net/dsa/Kconfig
@@ -100,8 +100,6 @@ config NET_DSA_SMSC_LAN9303_MDIO
config NET_DSA_VITESSE_VSC73XX
tristate
- depends on OF
- depends on NET_DSA
select FIXED_PHY
select VITESSE_PHY
select GPIOLIB
@@ -112,6 +110,7 @@ config NET_DSA_VITESSE_VSC73XX
config NET_DSA_VITESSE_VSC73XX_SPI
tristate "Vitesse VSC7385/7388/7395/7398 SPI mode support"
depends on SPI
+ depends on OF && NET_DSA
select NET_DSA_VITESSE_VSC73XX
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
@@ -120,6 +119,7 @@ config NET_DSA_VITESSE_VSC73XX_SPI
config NET_DSA_VITESSE_VSC73XX_PLATFORM
tristate "Vitesse VSC7385/7388/7395/7398 Platform mode support"
depends on HAS_IOMEM
+ depends on OF && NET_DSA
select NET_DSA_VITESSE_VSC73XX
---help---
This enables support for the Vitesse VSC7385, VSC7388, VSC7395
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v5 1/5] devlink: Refactor physical port attributes
From: Jiri Pirko @ 2019-07-08 14:44 UTC (permalink / raw)
To: Parav Pandit; +Cc: netdev, jiri, saeedm, jakub.kicinski
In-Reply-To: <20190708041549.56601-2-parav@mellanox.com>
Mon, Jul 08, 2019 at 06:15:45AM CEST, parav@mellanox.com wrote:
>To support additional devlink port flavours and to support few common
>and few different port attributes, move physical port attributes to a
>different structure.
>
>Signed-off-by: Parav Pandit <parav@mellanox.com>
>---
>Changelog:
>v4->v5:
> - Addressed comments from Jiri.
> - Moved check for physical port flavours check to separate patch.
>v3->v4:
> - Addressed comments from Jiri.
> - Renamed phys_port to physical to be consistent with pci_pf.
> - Removed port_number from __devlink_port_attrs_set and moved
> assigment to caller function.
> - Used capital letter while moving old comment to new structure.
> - Removed helper function is_devlink_phy_port_num_supported().
>v2->v3:
> - Address comments from Jakub.
> - Made port_number and split_port_number applicable only to
> physical port flavours by having in union.
>v1->v2:
> - Limited port_num attribute to physical ports
> - Updated PCI PF attribute set API to not have port_number
>---
> include/net/devlink.h | 13 ++++++++--
> net/core/devlink.c | 59 ++++++++++++++++++++++++++++---------------
> 2 files changed, 50 insertions(+), 22 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 6625ea068d5e..c79a1370867a 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -38,14 +38,23 @@ struct devlink {
> char priv[0] __aligned(NETDEV_ALIGN);
> };
>
>+struct devlink_port_phys_attrs {
>+ u32 port_number; /* Same value as "split group".
>+ * A physical port which is visible to the user
>+ * for a given port flavour.
>+ */
>+ u32 split_subport_number;
>+};
>+
> struct devlink_port_attrs {
> u8 set:1,
> split:1,
> switch_port:1;
> enum devlink_port_flavour flavour;
>- u32 port_number; /* same value as "split group" */
>- u32 split_subport_number;
> struct netdev_phys_item_id switch_id;
>+ union {
>+ struct devlink_port_phys_attrs physical;
You can shorten this to just "phys". Would be better.
With that
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH net-next v5 2/5] devlink: Return physical port fields only for applicable port flavours
From: Jiri Pirko @ 2019-07-08 14:44 UTC (permalink / raw)
To: Parav Pandit; +Cc: netdev, jiri, saeedm, jakub.kicinski
In-Reply-To: <20190708041549.56601-3-parav@mellanox.com>
Mon, Jul 08, 2019 at 06:15:46AM CEST, parav@mellanox.com wrote:
>Physical port number and split group fields are applicable only to
>physical port flavours such as PHYSICAL, CPU and DSA.
>Hence limit returning those values in netlink response to such port
>flavours.
>
>Signed-off-by: Parav Pandit <parav@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Nathan Chancellor @ 2019-07-08 14:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Brian Norris, Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu,
Sriram R, ath10k, linux-wireless, netdev, linux-kernel,
clang-built-linux
In-Reply-To: <20190708125050.3689133-1-arnd@arndb.de>
On Mon, Jul 08, 2019 at 02:50:06PM +0200, Arnd Bergmann wrote:
> As clang points out, the vht_pfr is assigned to a struct member
> without being initialized in one case:
>
> drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
> arvif->vht_pfr = vht_pfr;
> ^~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
> u8 vht_pfr;
>
> Add an explicit but probably incorrect initialization here.
> I suspect we want a better fix here, but chose this approach to
> illustrate the issue.
Yup, I reached out to the maintainers when this issue first cropped up,
should have taken your approach though.
https://lore.kernel.org/lkml/20190702181837.GA118849@archlinux-epyc/
Initializing to zero is better than uninitialized.
>
> Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next 3/4] bnxt_en: optimized XDP_REDIRECT support
From: Ilias Apalodimas @ 2019-07-08 14:51 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Michael Chan, davem, netdev, hawk, ast, ivan.khoronzhuk
In-Reply-To: <20190708142606.GF87269@C02RW35GFVH8.dhcp.broadcom.net>
Hi Andy,
> On Mon, Jul 08, 2019 at 11:28:03AM +0300, Ilias Apalodimas wrote:
> > Thanks Andy, Michael
> >
> > > + if (event & BNXT_REDIRECT_EVENT)
> > > + xdp_do_flush_map();
> > > +
> > > if (event & BNXT_TX_EVENT) {
> > > struct bnxt_tx_ring_info *txr = bnapi->tx_ring;
> > > u16 prod = txr->tx_prod;
> > > @@ -2254,9 +2257,23 @@ static void bnxt_free_tx_skbs(struct bnxt *bp)
> > >
> > > for (j = 0; j < max_idx;) {
> > > struct bnxt_sw_tx_bd *tx_buf = &txr->tx_buf_ring[j];
> > > - struct sk_buff *skb = tx_buf->skb;
> > > + struct sk_buff *skb;
> > > int k, last;
> > >
> > > + if (i < bp->tx_nr_rings_xdp &&
> > > + tx_buf->action == XDP_REDIRECT) {
> > > + dma_unmap_single(&pdev->dev,
> > > + dma_unmap_addr(tx_buf, mapping),
> > > + dma_unmap_len(tx_buf, len),
> > > + PCI_DMA_TODEVICE);
> > > + xdp_return_frame(tx_buf->xdpf);
> > > + tx_buf->action = 0;
> > > + tx_buf->xdpf = NULL;
> > > + j++;
> > > + continue;
> > > + }
> > > +
> >
> > Can't see the whole file here and maybe i am missing something, but since you
> > optimize for that and start using page_pool, XDP_TX will be a re-synced (and
> > not remapped) buffer that can be returned to the pool and resynced for
> > device usage.
> > Is that happening later on the tx clean function?
>
> Take a look at the way we treat the buffers in bnxt_rx_xdp() when we
> receive them and then in bnxt_tx_int_xdp() when the transmits have
> completed (for XDP_TX and XDP_REDIRECT). I think we are doing what is
> proper with respect to mapping vs sync for both cases, but I would be
> fine to be corrected.
>
Yea seems to be doing the right thing,
XDP_TX syncs correctly and reuses with bnxt_reuse_rx_data() right?
This might be a bit confusing for someone reading the driver on the first time,
probably because you'll end up with 2 ways of recycling buffers.
Once a buffers get freed on the XDP path it's either fed back to the pool, so
the next requested buffer get served from the pools cache (ndo_xdp_xmit case in
the patch). If the buffer is used for XDP_TX is's synced correctly but recycled
via bnxt_reuse_rx_data() right? Since you are moving to page pool please
consider having a common approach towards the recycling path. I understand that
means tracking buffers types and make sure you do the right thing on 'tx clean'.
I've done something similar on the netsec driver and i do think this might be a
good thing to add on page_pool API
Again this isn't a blocker at least for me but you already have the buffer type
(via tx_buf->action)
> >
> > > + skb = tx_buf->skb;
> > > if (!skb) {
> > > j++;
> > > continue;
> > > @@ -2517,6 +2534,13 @@ static int bnxt_alloc_rx_rings(struct bnxt *bp)
> > > if (rc < 0)
> > > return rc;
> > >
> > > + rc = xdp_rxq_info_reg_mem_model(&rxr->xdp_rxq,
> > > + MEM_TYPE_PAGE_SHARED, NULL);
> > > + if (rc) {
> > > + xdp_rxq_info_unreg(&rxr->xdp_rxq);
> >
> > I think you can use page_pool_free directly here (and pge_pool_destroy once
> > Ivan's patchset gets nerged), that's what mlx5 does iirc. Can we keep that
> > common please?
>
> That's an easy change, I can do that.
>
> >
> > If Ivan's patch get merged please note you'll have to explicitly
> > page_pool_destroy, after calling xdp_rxq_info_unreg() in the general unregister
> > case (not the error habdling here). Sorry for the confusion this might bring!
>
> Funny enough the driver was basically doing that until page_pool_destroy
> was removed (these patches are not new). I saw last week there was
> discussion to add it back, but I did not want to wait to get this on the
> list before that was resolved.
Fair enough
>
> This path works as expected with the code in the tree today so it seemed
> like the correct approach to post something that is working, right? :-)
Yes.
It will continue to work even if you dont change the call in the future.
This is more a 'let's not spread the code' attempt, but removing and re-adding
page_pool_destroy() was/is our mess. We might as well live with the
consequences!
>
> >
> > > + return rc;
> > > + }
> > > +
> > > rc = bnxt_alloc_ring(bp, &ring->ring_mem);
> > > if (rc)
> > > return rc;
> > > @@ -10233,6 +10257,7 @@ static const struct net_device_ops bnxt_netdev_ops = {
> > [...]
> >
Thanks!
/Ilias
^ permalink raw reply
* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Andrew Lunn @ 2019-07-08 14:57 UTC (permalink / raw)
To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <5e35a41c-be0e-efd4-cb69-cf5c860b872e@eks-engel.de>
> Hi Andrew,
> I got it working a little bit better. When I'm fast enough I can read
> the registers I want but it isn't a solution.
Why do you need to read registers?
What you actually might be interested in is the debugfs patches in
Viviens github tree.
> Here is an output of the tracing even with my custom accesses.
> mii -i 2 0 0x9b60; mii -i 2 1
> phyid:2, reg:0x01 -> 0xc801
>
> Do you know how to delete EEInt bit? It is always one. And now all
> accesses coming from the kworker thread. Maybe this is your polling
> function?
EEInt should clear on read for older chips. For 6390, it might be
necessary to read global 2, register 0x13, index 03.
> I view the INT pin on an oscilloscope but it never changed. So maybe
> this is the problem. We just soldered a pull-up to that pin but it
> still never changend. Maybe you have an idea?
The EEInt bit is probably masked. So it will not generate in
interrupt.
> So what I think is, because of the EEInt bit is never set back to one
> i will poll it as fast as possible.
Is it forever looping in mv88e6xxx_g1_irq_thread_work? Or is it the
polling code, mv88e6xxx_irq_poll() firing every 100ms?
Andrew
^ permalink raw reply
* [PATCH net,v2] nfc: fix potential illegal memory access
From: Yang Wei @ 2019-07-08 14:57 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: davem, tglx, albin_yang
The frags_q is not properly initialized, it may result in illegal memory
access when conn_info is NULL.
The "goto free_exit" should be replaced by "goto exit".
Signed-off-by: Yang Wei <albin_yang@163.com>
---
net/nfc/nci/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/nfc/nci/data.c b/net/nfc/nci/data.c
index 0a0c265..ce3382b 100644
--- a/net/nfc/nci/data.c
+++ b/net/nfc/nci/data.c
@@ -107,7 +107,7 @@ static int nci_queue_tx_data_frags(struct nci_dev *ndev,
conn_info = nci_get_conn_info_by_conn_id(ndev, conn_id);
if (!conn_info) {
rc = -EPROTO;
- goto free_exit;
+ goto exit;
}
__skb_queue_head_init(&frags_q);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH bpf-next v2 4/6] xdp: Add devmap_hash map type for looking up devices by hashed index
From: Y Song @ 2019-07-08 15:01 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <874l3w26lb.fsf@toke.dk>
On Mon, Jul 8, 2019 at 2:55 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Y Song <ys114321@gmail.com> writes:
>
> > On Sat, Jul 6, 2019 at 1:48 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> From: Toke Høiland-Jørgensen <toke@redhat.com>
> >>
> >> A common pattern when using xdp_redirect_map() is to create a device map
> >> where the lookup key is simply ifindex. Because device maps are arrays,
> >> this leaves holes in the map, and the map has to be sized to fit the
> >> largest ifindex, regardless of how many devices actually are actually
> >> needed in the map.
> >>
> >> This patch adds a second type of device map where the key is looked up
> >> using a hashmap, instead of being used as an array index. This allows maps
> >> to be densely packed, so they can be smaller.
> >>
> >> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> >> ---
> >> include/linux/bpf.h | 7 ++
> >> include/linux/bpf_types.h | 1
> >> include/trace/events/xdp.h | 3 -
> >> kernel/bpf/devmap.c | 192 ++++++++++++++++++++++++++++++++++++++++++++
> >> kernel/bpf/verifier.c | 2
> >> net/core/filter.c | 9 ++
> >> 6 files changed, 211 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >> index bfdb54dd2ad1..f9a506147c8a 100644
> >> --- a/include/linux/bpf.h
> >> +++ b/include/linux/bpf.h
> >> @@ -713,6 +713,7 @@ struct xdp_buff;
> >> struct sk_buff;
> >>
> >> struct bpf_dtab_netdev *__dev_map_lookup_elem(struct bpf_map *map, u32 key);
> >> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key);
> >> void __dev_map_flush(struct bpf_map *map);
> >> int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
> >> struct net_device *dev_rx);
> >> @@ -799,6 +800,12 @@ static inline struct net_device *__dev_map_lookup_elem(struct bpf_map *map,
> >> return NULL;
> >> }
> >>
> >> +static inline struct net_device *__dev_map_hash_lookup_elem(struct bpf_map *map,
> >> + u32 key)
> >> +{
> >> + return NULL;
> >> +}
> >> +
> >> static inline void __dev_map_flush(struct bpf_map *map)
> >> {
> >> }
> >> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> >> index eec5aeeeaf92..36a9c2325176 100644
> >> --- a/include/linux/bpf_types.h
> >> +++ b/include/linux/bpf_types.h
> >> @@ -62,6 +62,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY_OF_MAPS, array_of_maps_map_ops)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_HASH_OF_MAPS, htab_of_maps_map_ops)
> >> #ifdef CONFIG_NET
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
> >> +BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP_HASH, dev_map_hash_ops)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_SK_STORAGE, sk_storage_map_ops)
> >> #if defined(CONFIG_BPF_STREAM_PARSER)
> >> BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
> >> diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> >> index 68899fdc985b..8c8420230a10 100644
> >> --- a/include/trace/events/xdp.h
> >> +++ b/include/trace/events/xdp.h
> >> @@ -175,7 +175,8 @@ struct _bpf_dtab_netdev {
> >> #endif /* __DEVMAP_OBJ_TYPE */
> >>
> >> #define devmap_ifindex(fwd, map) \
> >> - ((map->map_type == BPF_MAP_TYPE_DEVMAP) ? \
> >> + ((map->map_type == BPF_MAP_TYPE_DEVMAP || \
> >> + map->map_type == BPF_MAP_TYPE_DEVMAP_HASH) ? \
> >> ((struct _bpf_dtab_netdev *)fwd)->dev->ifindex : 0)
> >>
> >> #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
> >> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> >> index a2fe16362129..341af02f049d 100644
> >> --- a/kernel/bpf/devmap.c
> >> +++ b/kernel/bpf/devmap.c
> >> @@ -37,6 +37,12 @@
> >> * notifier hook walks the map we know that new dev references can not be
> >> * added by the user because core infrastructure ensures dev_get_by_index()
> >> * calls will fail at this point.
> >> + *
> >> + * The devmap_hash type is a map type which interprets keys as ifindexes and
> >> + * indexes these using a hashmap. This allows maps that use ifindex as key to be
> >> + * densely packed instead of having holes in the lookup array for unused
> >> + * ifindexes. The setup and packet enqueue/send code is shared between the two
> >> + * types of devmap; only the lookup and insertion is different.
> >> */
> >> #include <linux/bpf.h>
> >> #include <net/xdp.h>
> >> @@ -59,6 +65,7 @@ struct xdp_bulk_queue {
> >>
> >> struct bpf_dtab_netdev {
> >> struct net_device *dev; /* must be first member, due to tracepoint */
> >> + struct hlist_node index_hlist;
> >> struct bpf_dtab *dtab;
> >> unsigned int idx; /* keep track of map index for tracepoint */
> >> struct xdp_bulk_queue __percpu *bulkq;
> >> @@ -70,11 +77,29 @@ struct bpf_dtab {
> >> struct bpf_dtab_netdev **netdev_map;
> >> struct list_head __percpu *flush_list;
> >> struct list_head list;
> >> +
> >> + /* these are only used for DEVMAP_HASH type maps */
> >> + unsigned int items;
> >> + struct hlist_head *dev_index_head;
> >> + spinlock_t index_lock;
> >> };
> >>
> >> static DEFINE_SPINLOCK(dev_map_lock);
> >> static LIST_HEAD(dev_map_list);
> >>
> >> +static struct hlist_head *dev_map_create_hash(void)
> >> +{
> >> + int i;
> >> + struct hlist_head *hash;
> >> +
> >> + hash = kmalloc_array(NETDEV_HASHENTRIES, sizeof(*hash), GFP_KERNEL);
> >> + if (hash != NULL)
> >> + for (i = 0; i < NETDEV_HASHENTRIES; i++)
> >> + INIT_HLIST_HEAD(&hash[i]);
> >> +
> >> + return hash;
> >> +}
> >> +
> >> static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> bool check_memlock)
> >> {
> >> @@ -98,6 +123,9 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> cost = (u64) dtab->map.max_entries * sizeof(struct bpf_dtab_netdev *);
> >> cost += sizeof(struct list_head) * num_possible_cpus();
> >>
> >> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH)
> >> + cost += sizeof(struct hlist_head) * NETDEV_HASHENTRIES;
> >> +
> >> /* if map size is larger than memlock limit, reject it */
> >> err = bpf_map_charge_init(&dtab->map.memory, cost);
> >> if (err)
> >> @@ -116,8 +144,18 @@ static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr *attr,
> >> if (!dtab->netdev_map)
> >> goto free_percpu;
> >>
> >> + if (attr->map_type == BPF_MAP_TYPE_DEVMAP_HASH) {
> >> + dtab->dev_index_head = dev_map_create_hash();
> >> + if (!dtab->dev_index_head)
> >> + goto free_map_area;
> >> +
> >> + spin_lock_init(&dtab->index_lock);
> >> + }
> >> +
> >> return 0;
> >>
> >> +free_map_area:
> >> + bpf_map_area_free(dtab->netdev_map);
> >> free_percpu:
> >> free_percpu(dtab->flush_list);
> >> free_charge:
> >> @@ -199,6 +237,7 @@ static void dev_map_free(struct bpf_map *map)
> >>
> >> free_percpu(dtab->flush_list);
> >> bpf_map_area_free(dtab->netdev_map);
> >> + kfree(dtab->dev_index_head);
> >> kfree(dtab);
> >> }
> >>
> >> @@ -219,6 +258,70 @@ static int dev_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
> >> return 0;
> >> }
> >>
> >> +static inline struct hlist_head *dev_map_index_hash(struct bpf_dtab *dtab,
> >> + int idx)
> >> +{
> >> + return &dtab->dev_index_head[idx & (NETDEV_HASHENTRIES - 1)];
> >> +}
> >> +
> >> +struct bpf_dtab_netdev *__dev_map_hash_lookup_elem(struct bpf_map *map, u32 key)
> >> +{
> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> >> + struct hlist_head *head = dev_map_index_hash(dtab, key);
> >> + struct bpf_dtab_netdev *dev;
> >> +
> >> + hlist_for_each_entry_rcu(dev, head, index_hlist)
> >> + if (dev->idx == key)
> >> + return dev;
> >> +
> >> + return NULL;
> >> +}
> >> +
> >> +static int dev_map_hash_get_next_key(struct bpf_map *map, void *key,
> >> + void *next_key)
> >> +{
> >> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> >> + u32 idx, *next = next_key;
> >> + struct bpf_dtab_netdev *dev, *next_dev;
> >> + struct hlist_head *head;
> >> + int i = 0;
> >> +
> >> + if (!key)
> >> + goto find_first;
> >> +
> >> + idx = *(u32 *)key;
> >> +
> >> + dev = __dev_map_hash_lookup_elem(map, idx);
> >> + if (!dev)
> >> + goto find_first;
> >> +
> >> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(&dev->index_hlist)),
> >> + struct bpf_dtab_netdev, index_hlist);
> >
> > Just want to get a better understanding. Why do you want
> > hlist_entry_safe instead of hlist_entry?
>
> Erm, because the entry might not exist? The _safe variant just checks
> the list pointer before casting to the containing struct.
hlist_entry_safe is certainly correct. Just want to better understand when
the entry might not exist. By looking at hashtab.c implementation, it has
the same possible-null-entry assumption, so we should be ok here.
>
> > Also, maybe rcu_dereference instead of rcu_dereference_raw?
>
> Well, the _raw() variant comes from the equivalent function in
> hashtab.c, where I originally nicked most of this function, so think it
> makes more sense to keep them the same.
Okay.
>
> > dev_map_hash_get_next_key() is called in syscall.c within rcu_read_lock region.
> >
> >> +
> >> + if (next_dev) {
> >> + *next = next_dev->idx;
> >> + return 0;
> >> + }
> >> +
> >> + i = idx & (NETDEV_HASHENTRIES - 1);
> >> + i++;
> >> +
> >> + find_first:
> >> + for (; i < NETDEV_HASHENTRIES; i++) {
> >> + head = dev_map_index_hash(dtab, i);
> >> +
> >> + next_dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
> >> + struct bpf_dtab_netdev,
> >> + index_hlist);
> >
> > ditto. The same question as the above.
> >
> >> + if (next_dev) {
> >> + *next = next_dev->idx;
> >> + return 0;
> >> + }
> >> + }
> >> +
> >> + return -ENOENT;
> >> +}
> >> +
> >> static int bq_xmit_all(struct xdp_bulk_queue *bq, u32 flags,
> >> bool in_napi_ctx)
> >> {
> >> @@ -374,6 +477,15 @@ static void *dev_map_lookup_elem(struct bpf_map *map, void *key)
> >> return dev ? &dev->ifindex : NULL;
> >> }
> >>
> >> +static void *dev_map_hash_lookup_elem(struct bpf_map *map, void *key)
> >> +{
> >> + struct bpf_dtab_netdev *obj = __dev_map_hash_lookup_elem(map,
> >> + *(u32 *)key);
> >> + struct net_device *dev = obj ? obj->dev : NULL;
> >
> > When obj->dev could be NULL?
>
> Never. This could just as well be written as
>
> return obj ? obj->dev->ifindex : NULL;
>
> but writing it this way keeps it consistent with the existing
> dev_map_lookup_elem() function.
Okay then in order to be consistent with the existing code base.
Thanks for all the explanation!
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] xdp: Refactor devmap allocation code for reuse
From: Jonathan Lemon @ 2019-07-08 15:06 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156234940841.2378.6629890565300526702.stgit@alrua-x1>
On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> The subsequent patch to add a new devmap sub-type can re-use much of
> the
> initialisation and allocation code, so refactor it into separate
> functions.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> kernel/bpf/devmap.c | 137
> +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 84 insertions(+), 53 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index d83cf8ccc872..a2fe16362129 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -60,7 +60,7 @@ struct xdp_bulk_queue {
> struct bpf_dtab_netdev {
> struct net_device *dev; /* must be first member, due to tracepoint
> */
> struct bpf_dtab *dtab;
> - unsigned int bit;
> + unsigned int idx; /* keep track of map index for tracepoint */
> struct xdp_bulk_queue __percpu *bulkq;
> struct rcu_head rcu;
> };
> @@ -75,28 +75,22 @@ struct bpf_dtab {
> static DEFINE_SPINLOCK(dev_map_lock);
> static LIST_HEAD(dev_map_list);
>
> -static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> +static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr
> *attr,
> + bool check_memlock)
This check_memlock parameter appears to be unused.
--
Jonathan
> {
> - struct bpf_dtab *dtab;
> int err, cpu;
> u64 cost;
>
> - if (!capable(CAP_NET_ADMIN))
> - return ERR_PTR(-EPERM);
> -
> /* check sanity of attributes */
> if (attr->max_entries == 0 || attr->key_size != 4 ||
> attr->value_size != 4 || attr->map_flags &
> ~DEV_CREATE_FLAG_MASK)
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> /* Lookup returns a pointer straight to dev->ifindex, so make sure
> the
> * verifier prevents writes from the BPF side
> */
> attr->map_flags |= BPF_F_RDONLY_PROG;
>
> - dtab = kzalloc(sizeof(*dtab), GFP_USER);
> - if (!dtab)
> - return ERR_PTR(-ENOMEM);
>
> bpf_map_init_from_attr(&dtab->map, attr);
>
> @@ -107,9 +101,7 @@ static struct bpf_map *dev_map_alloc(union
> bpf_attr *attr)
> /* if map size is larger than memlock limit, reject it */
> err = bpf_map_charge_init(&dtab->map.memory, cost);
> if (err)
> - goto free_dtab;
> -
> - err = -ENOMEM;
> + return -EINVAL;
>
> dtab->flush_list = alloc_percpu(struct list_head);
> if (!dtab->flush_list)
> @@ -124,19 +116,38 @@ static struct bpf_map *dev_map_alloc(union
> bpf_attr *attr)
> if (!dtab->netdev_map)
> goto free_percpu;
>
> - spin_lock(&dev_map_lock);
> - list_add_tail_rcu(&dtab->list, &dev_map_list);
> - spin_unlock(&dev_map_lock);
> -
> - return &dtab->map;
> + return 0;
>
> free_percpu:
> free_percpu(dtab->flush_list);
> free_charge:
> bpf_map_charge_finish(&dtab->map.memory);
> -free_dtab:
> - kfree(dtab);
> - return ERR_PTR(err);
> + return -ENOMEM;
> +}
> +
> +static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
> +{
> + struct bpf_dtab *dtab;
> + int err;
> +
> + if (!capable(CAP_NET_ADMIN))
> + return ERR_PTR(-EPERM);
> +
> + dtab = kzalloc(sizeof(*dtab), GFP_USER);
> + if (!dtab)
> + return ERR_PTR(-ENOMEM);
> +
> + err = dev_map_init_map(dtab, attr, true);
> + if (err) {
> + kfree(dtab);
> + return ERR_PTR(err);
> + }
> +
> + spin_lock(&dev_map_lock);
> + list_add_tail_rcu(&dtab->list, &dev_map_list);
> + spin_unlock(&dev_map_lock);
> +
> + return &dtab->map;
> }
>
> static void dev_map_free(struct bpf_map *map)
> @@ -235,7 +246,7 @@ static int bq_xmit_all(struct xdp_bulk_queue *bq,
> u32 flags,
> out:
> bq->count = 0;
>
> - trace_xdp_devmap_xmit(&obj->dtab->map, obj->bit,
> + trace_xdp_devmap_xmit(&obj->dtab->map, obj->idx,
> sent, drops, bq->dev_rx, dev, err);
> bq->dev_rx = NULL;
> __list_del_clearprev(&bq->flush_node);
> @@ -412,17 +423,52 @@ static int dev_map_delete_elem(struct bpf_map
> *map, void *key)
> return 0;
> }
>
> -static int dev_map_update_elem(struct bpf_map *map, void *key, void
> *value,
> - u64 map_flags)
> +static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
> + struct bpf_dtab *dtab,
> + u32 ifindex,
> + unsigned int idx)
> {
> - struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> - struct net *net = current->nsproxy->net_ns;
> gfp_t gfp = GFP_ATOMIC | __GFP_NOWARN;
> + struct bpf_dtab_netdev *dev;
> + struct xdp_bulk_queue *bq;
> + int cpu;
> +
> + dev = kmalloc_node(sizeof(*dev), gfp, dtab->map.numa_node);
> + if (!dev)
> + return ERR_PTR(-ENOMEM);
> +
> + dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
> + sizeof(void *), gfp);
> + if (!dev->bulkq) {
> + kfree(dev);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + for_each_possible_cpu(cpu) {
> + bq = per_cpu_ptr(dev->bulkq, cpu);
> + bq->obj = dev;
> + }
> +
> + dev->dev = dev_get_by_index(net, ifindex);
> + if (!dev->dev) {
> + free_percpu(dev->bulkq);
> + kfree(dev);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + dev->idx = idx;
> + dev->dtab = dtab;
> +
> + return dev;
> +}
> +
> +static int __dev_map_update_elem(struct net *net, struct bpf_map
> *map,
> + void *key, void *value, u64 map_flags)
> +{
> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> struct bpf_dtab_netdev *dev, *old_dev;
> u32 ifindex = *(u32 *)value;
> - struct xdp_bulk_queue *bq;
> u32 i = *(u32 *)key;
> - int cpu;
>
> if (unlikely(map_flags > BPF_EXIST))
> return -EINVAL;
> @@ -434,31 +480,9 @@ static int dev_map_update_elem(struct bpf_map
> *map, void *key, void *value,
> if (!ifindex) {
> dev = NULL;
> } else {
> - dev = kmalloc_node(sizeof(*dev), gfp, map->numa_node);
> - if (!dev)
> - return -ENOMEM;
> -
> - dev->bulkq = __alloc_percpu_gfp(sizeof(*dev->bulkq),
> - sizeof(void *), gfp);
> - if (!dev->bulkq) {
> - kfree(dev);
> - return -ENOMEM;
> - }
> -
> - for_each_possible_cpu(cpu) {
> - bq = per_cpu_ptr(dev->bulkq, cpu);
> - bq->obj = dev;
> - }
> -
> - dev->dev = dev_get_by_index(net, ifindex);
> - if (!dev->dev) {
> - free_percpu(dev->bulkq);
> - kfree(dev);
> - return -EINVAL;
> - }
> -
> - dev->bit = i;
> - dev->dtab = dtab;
> + dev = __dev_map_alloc_node(net, dtab, ifindex, i);
> + if (IS_ERR(dev))
> + return PTR_ERR(dev);
> }
>
> /* Use call_rcu() here to ensure rcu critical sections have
> completed
> @@ -472,6 +496,13 @@ static int dev_map_update_elem(struct bpf_map
> *map, void *key, void *value,
> return 0;
> }
>
> +static int dev_map_update_elem(struct bpf_map *map, void *key, void
> *value,
> + u64 map_flags)
> +{
> + return __dev_map_update_elem(current->nsproxy->net_ns,
> + map, key, value, map_flags);
> +}
> +
> const struct bpf_map_ops dev_map_ops = {
> .map_alloc = dev_map_alloc,
> .map_free = dev_map_free,
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type
From: Jonathan Lemon @ 2019-07-08 15:10 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156234940798.2378.9008707939063611210.stgit@alrua-x1>
On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
> This series adds a new map type, devmap_hash, that works like the
> existing
> devmap type, but using a hash-based indexing scheme. This is useful
> for the use
> case where a devmap is indexed by ifindex (for instance for use with
> the routing
> table lookup helper). For this use case, the regular devmap needs to
> be sized
> after the maximum ifindex number, not the number of devices in it. A
> hash-based
> indexing scheme makes it possible to size the map after the number of
> devices it
> should contain instead.
This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
this actually
smaller than an array? What ifindex values are you seeing?
--
Jonathan
>
> This was previously part of my patch series that also turned the
> regular
> bpf_redirect() helper into a map-based one; for this series I just
> pulled out
> the patches that introduced the new map type.
>
> Changelog:
>
> Changes to these patches since the previous series:
>
> - Rebase on top of the other devmap changes (makes this one simpler!)
> - Don't enforce key==val, but allow arbitrary indexes.
> - Rename the type to devmap_hash to reflect the fact that it's just a
> hashmap now.
>
> ---
>
> Toke Høiland-Jørgensen (3):
> include/bpf.h: Remove map_insert_ctx() stubs
> xdp: Refactor devmap allocation code for reuse
> xdp: Add devmap_hash map type for looking up devices by hashed
> index
>
>
> include/linux/bpf.h | 11 -
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3
> include/uapi/linux/bpf.h | 7 -
> kernel/bpf/devmap.c | 325
> ++++++++++++++++++++++++++-----
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 +
> tools/bpf/bpftool/map.c | 1
> tools/include/uapi/linux/bpf.h | 7 -
> tools/lib/bpf/libbpf_probes.c | 1
> tools/testing/selftests/bpf/test_maps.c | 16 ++
> 11 files changed, 316 insertions(+), 67 deletions(-)
^ permalink raw reply
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Matt Hart @ 2019-07-08 15:11 UTC (permalink / raw)
To: andriin; +Cc: andrii.nakryiko, ast, bpf, daniel, kernel-team, netdev, sdf
Hi all,
I bisected a perf build error on ARMv7 to this patch:
libbpf.c: In function ‘perf_event_open_probe’:
libbpf.c:4112:17: error: cast from pointer to integer of different
size [-Werror=pointer-to-int-cast]
attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
^
Is this a known issue?
^ permalink raw reply
* Re: [PATCH net-next 1/2] bpf: skip sockopt hooks without CONFIG_NET
From: Yonghong Song @ 2019-07-08 15:06 UTC (permalink / raw)
To: Arnd Bergmann, Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, Martin Lau, Stanislav Fomichev, Song Liu,
Mauricio Vasquez B, Roman Gushchin, Matt Mullins,
Willem de Bruijn, Andrey Ignatov, netdev@vger.kernel.org,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190708125733.3944836-1-arnd@arndb.de>
On 7/8/19 5:57 AM, Arnd Bergmann wrote:
> When CONFIG_NET is disabled, we get a link error:
>
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_setsockopt':
> cgroup.c:(.text+0x3010): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3258): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `__cgroup_bpf_run_filter_getsockopt':
> cgroup.c:(.text+0x3568): undefined reference to `lock_sock_nested'
> cgroup.c:(.text+0x3870): undefined reference to `release_sock'
> kernel/bpf/cgroup.o: In function `cg_sockopt_func_proto':
> cgroup.c:(.text+0x41d8): undefined reference to `bpf_sk_storage_delete_proto'
>
> None of this code is useful in this configuration anyway, so we can
> simply hide it in an appropriate #ifdef.
>
> Fixes: 0d01da6afc54 ("bpf: implement getsockopt and setsockopt hooks")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
FYI.
There is already a patch to fix the same issue,
https://lore.kernel.org/bpf/e9e489fe-feec-a211-82aa-5df0c6a308d1@huawei.com/T/#t
which has been acked and not merged yet.
> ---
> include/linux/bpf_types.h | 2 ++
> kernel/bpf/cgroup.c | 6 ++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index eec5aeeeaf92..3c7222b2db96 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -30,8 +30,10 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE, raw_tracepoint_writable)
> #ifdef CONFIG_CGROUP_BPF
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SYSCTL, cg_sysctl)
> +#ifdef CONFIG_NET
> BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_SOCKOPT, cg_sockopt)
> #endif
> +#endif
> #ifdef CONFIG_BPF_LIRC_MODE2
> BPF_PROG_TYPE(BPF_PROG_TYPE_LIRC_MODE2, lirc_mode2)
> #endif
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 76fa0076f20d..7be44460bd93 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -590,6 +590,7 @@ int cgroup_bpf_prog_query(const union bpf_attr *attr,
> return ret;
> }
>
> +#ifdef CONFIG_NET
> /**
> * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering
> * @sk: The socket sending or receiving traffic
> @@ -750,6 +751,7 @@ int __cgroup_bpf_run_filter_sock_ops(struct sock *sk,
> return ret == 1 ? 0 : -EPERM;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sock_ops);
> +#endif
>
> int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
> short access, enum bpf_attach_type type)
> @@ -939,6 +941,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_sysctl);
>
> +#ifdef CONFIG_NET
> static bool __cgroup_bpf_prog_array_is_empty(struct cgroup *cgrp,
> enum bpf_attach_type attach_type)
> {
> @@ -1120,6 +1123,7 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
> return ret;
> }
> EXPORT_SYMBOL(__cgroup_bpf_run_filter_getsockopt);
> +#endif
>
> static ssize_t sysctl_cpy_dir(const struct ctl_dir *dir, char **bufp,
> size_t *lenp)
> @@ -1382,6 +1386,7 @@ const struct bpf_verifier_ops cg_sysctl_verifier_ops = {
> const struct bpf_prog_ops cg_sysctl_prog_ops = {
> };
>
> +#ifdef CONFIG_NET
> static const struct bpf_func_proto *
> cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> @@ -1531,3 +1536,4 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
>
> const struct bpf_prog_ops cg_sockopt_prog_ops = {
> };
> +#endif
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
From: Anton Protopopov @ 2019-07-08 15:11 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
netdev, bpf, linux-kernel, andriin
In-Reply-To: <734dd45a-95b0-a7fd-9e1d-0535ef4d3e12@iogearbox.net>
пт, 5 июл. 2019 г. в 17:44, Daniel Borkmann <daniel@iogearbox.net>:
>
> On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > Add a new API bpf_object__reuse_maps() which can be used to replace all maps in
> > an object by maps pinned to a directory provided in the path argument. Namely,
> > each map M in the object will be replaced by a map pinned to path/M.name.
> >
> > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > ---
> > tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++++++
> > tools/lib/bpf/libbpf.h | 2 ++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 37 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 4907997289e9..84c9e8f7bfd3 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
> > return 0;
> > }
> >
> > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
> > +{
> > + struct bpf_map *map;
> > +
> > + if (!obj)
> > + return -ENOENT;
> > +
> > + if (!path)
> > + return -EINVAL;
> > +
> > + bpf_object__for_each_map(map, obj) {
> > + int len, err;
> > + int pinned_map_fd;
> > + char buf[PATH_MAX];
>
> We'd need to skip the case of bpf_map__is_internal(map) since they are always
> recreated for the given object.
>
> > + len = snprintf(buf, PATH_MAX, "%s/%s", path, bpf_map__name(map));
> > + if (len < 0) {
> > + return -EINVAL;
> > + } else if (len >= PATH_MAX) {
> > + return -ENAMETOOLONG;
> > + }
> > +
> > + pinned_map_fd = bpf_obj_get(buf);
> > + if (pinned_map_fd < 0)
> > + return pinned_map_fd;
>
> Should we rather have a new map definition attribute that tells to reuse
> the map if it's pinned in bpf fs, and if not, we create it and later on
> pin it? This is what iproute2 is doing and which we're making use of heavily.
What do you think about adding a new generic field, say load_flags,
to the bpf_map_def structure and a particular flag, say LOAD_F_STICKY
for this purpose? And it will be cleared for internal maps, so we will skip
them as well.
> In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> too limiting for a generic API as new version of an object file may contain
> new maps which are not yet present in bpf fs at that point.
How permissive should it be? Is it ok to just print a warning on any
bpf_obj_get()
failure? Or does it make sense to skip some specific error (ENOENT) and reject
on other errors?
>
> > + err = bpf_map__reuse_fd(map, pinned_map_fd);
> > + if (err)
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
> > {
> > struct bpf_program *prog;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index d639f47e3110..7fe465a1be76 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
> > LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
> > LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
> > const char *path);
> > +LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
> > + const char *path);
> > LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
> > const char *path);
> > LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 2c6d835620d2..66a30be6696c 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
> > btf_dump__new;
> > btf__parse_elf;
> > bpf_object__load_xattr;
> > + bpf_object__reuse_maps;
> > libbpf_num_possible_cpus;
> > } LIBBPF_0.0.3;
> >
>
^ permalink raw reply
* Re: [PATCH iproute2] ip-route: fix json formatting for metrics
From: Andrea Claudi @ 2019-07-08 15:14 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, David Ahern
In-Reply-To: <f1535e547aa6da8216ca2a0da7c06b645a132929.1562578533.git.aclaudi@redhat.com>
On Mon, Jul 8, 2019 at 11:38 AM Andrea Claudi <aclaudi@redhat.com> wrote:
>
> Setting metrics for routes currently lead to non-parsable
> json output. For example:
>
> $ ip link add type dummy
> $ ip route add 192.168.2.0 dev dummy0 metric 100 mtu 1000 rto_min 3
> $ ip -j route | jq
> parse error: ':' not as part of an object at line 1, column 319
>
> Fixing this opening a json object in the metrics array and using
> print_string() instead of fprintf().
>
> This is the output for the above commands applying this patch:
>
> $ ip -j route | jq
> [
> {
> "dst": "192.168.2.0",
> "dev": "dummy0",
> "scope": "link",
> "metric": 100,
> "flags": [],
> "metrics": [
> {
> "mtu": 1000,
> "rto_min": 3
> }
> ]
> }
> ]
>
> Fixes: 663c3cb23103f ("iproute: implement JSON and color output")
> Fixes: 968272e791710 ("iproute: refactor metrics print")
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> ---
> ip/iproute.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 1669e0138259e..2f9b612b0b506 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -578,6 +578,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> int i;
>
> open_json_array(PRINT_JSON, "metrics");
> + open_json_object(NULL);
>
> parse_rtattr(mxrta, RTAX_MAX, RTA_DATA(rta), RTA_PAYLOAD(rta));
>
> @@ -611,7 +612,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> print_rtax_features(fp, val);
> break;
> default:
> - fprintf(fp, "%u ", val);
> + print_uint(PRINT_ANY, mx_names[i], "%u ", val);
> break;
>
> case RTAX_RTT:
> @@ -639,6 +640,7 @@ static void print_rta_metrics(FILE *fp, const struct rtattr *rta)
> }
> }
>
> + close_json_object();
> close_json_array(PRINT_JSON, NULL);
> }
>
> --
> 2.20.1
>
Sorry, I forgot to add:
Reported-by: Frank Hofmann <fhofmann@cloudflare.com>
Regards,
Andrea
^ permalink raw reply
* Re: [PATCH iproute2 1/2] tc: added mask parameter in skbedit action
From: Stephen Hemminger @ 2019-07-08 15:15 UTC (permalink / raw)
To: Roman Mashak; +Cc: netdev, kernel, jhs, xiyou.wangcong, jiri
In-Reply-To: <1562195132-9829-1-git-send-email-mrv@mojatatu.com>
On Wed, 3 Jul 2019 19:05:31 -0400
Roman Mashak <mrv@mojatatu.com> wrote:
> + if (tb[TCA_SKBEDIT_MASK]) {
> + print_uint(PRINT_ANY, "mask", "/0x%x",
> + rta_getattr_u32(tb[TCA_SKBEDIT_MASK]));
Why not print in hex with.
print_hex(PRINT_ANY, "mask", "/%#x",
rta_getattr_u32(tb[TCA_SKBEDIT_MASK]));
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] xdp: Refactor devmap allocation code for reuse
From: Toke Høiland-Jørgensen @ 2019-07-08 15:19 UTC (permalink / raw)
To: Jonathan Lemon
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <A2ABED10-8475-4878-93DF-F16D106FC33D@flugsvamp.com>
"Jonathan Lemon" <jlemon@flugsvamp.com> writes:
> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>>
>> The subsequent patch to add a new devmap sub-type can re-use much of
>> the
>> initialisation and allocation code, so refactor it into separate
>> functions.
>>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>> kernel/bpf/devmap.c | 137
>> +++++++++++++++++++++++++++++++--------------------
>> 1 file changed, 84 insertions(+), 53 deletions(-)
>>
>> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
>> index d83cf8ccc872..a2fe16362129 100644
>> --- a/kernel/bpf/devmap.c
>> +++ b/kernel/bpf/devmap.c
>> @@ -60,7 +60,7 @@ struct xdp_bulk_queue {
>> struct bpf_dtab_netdev {
>> struct net_device *dev; /* must be first member, due to tracepoint
>> */
>> struct bpf_dtab *dtab;
>> - unsigned int bit;
>> + unsigned int idx; /* keep track of map index for tracepoint */
>> struct xdp_bulk_queue __percpu *bulkq;
>> struct rcu_head rcu;
>> };
>> @@ -75,28 +75,22 @@ struct bpf_dtab {
>> static DEFINE_SPINLOCK(dev_map_lock);
>> static LIST_HEAD(dev_map_list);
>>
>> -static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
>> +static int dev_map_init_map(struct bpf_dtab *dtab, union bpf_attr
>> *attr,
>> + bool check_memlock)
>
> This check_memlock parameter appears to be unused.
Ah yes, good catch! That was left over from when the patch set also
contained the "default map" stuff. Will fix.
-Toke
^ permalink raw reply
* Re: [PATCH v7 bpf-next 0/5] libbpf: add perf buffer abstraction and API
From: Daniel Borkmann @ 2019-07-08 15:23 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko, bpf, netdev, ast, kernel-team
In-Reply-To: <20190706180628.3919653-1-andriin@fb.com>
On 07/06/2019 08:06 PM, Andrii Nakryiko wrote:
> This patchset adds a high-level API for setting up and polling perf buffers
> associated with BPF_MAP_TYPE_PERF_EVENT_ARRAY map. Details of APIs are
> described in corresponding commit.
>
> Patch #1 adds a set of APIs to set up and work with perf buffer.
> Patch #2 enhances libbpf to support auto-setting PERF_EVENT_ARRAY map size.
> Patch #3 adds test.
> Patch #4 converts bpftool map event_pipe to new API.
> Patch #5 updates README to mention perf_buffer_ prefix.
>
> v6->v7:
> - __x64_ syscall prefix (Yonghong);
> v5->v6:
> - fix C99 for loop variable initialization usage (Yonghong);
> v4->v5:
> - initialize perf_buffer_raw_opts in bpftool map event_pipe (Jakub);
> - add perf_buffer_ to README;
> v3->v4:
> - fixed bpftool event_pipe cmd error handling (Jakub);
> v2->v3:
> - added perf_buffer__new_raw for more low-level control;
> - converted bpftool map event_pipe to new API (Daniel);
> - fixed bug with error handling in create_maps (Song);
> v1->v2:
> - add auto-sizing of PERF_EVENT_ARRAY maps;
>
> Andrii Nakryiko (5):
> libbpf: add perf buffer API
> libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
> selftests/bpf: test perf buffer API
> tools/bpftool: switch map event_pipe to libbpf's perf_buffer
> libbpf: add perf_buffer_ prefix to README
>
> tools/bpf/bpftool/map_perf_ring.c | 201 +++------
> tools/lib/bpf/README.rst | 3 +-
> tools/lib/bpf/libbpf.c | 397 +++++++++++++++++-
> tools/lib/bpf/libbpf.h | 49 +++
> tools/lib/bpf/libbpf.map | 4 +
> .../selftests/bpf/prog_tests/perf_buffer.c | 100 +++++
> .../selftests/bpf/progs/test_perf_buffer.c | 25 ++
> 7 files changed, 634 insertions(+), 145 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/perf_buffer.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_perf_buffer.c
>
Applied, thanks!
^ permalink raw reply
* [ANNOUNCE] iproute2 5.2
From: Stephen Hemminger @ 2019-07-08 15:26 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
The 5.2 kernel has been released, and therefore time for another
update to iproute2.
Not a lot of big new features in this release. Just the usual array of
small fixes across the board.
Download:
https://www.kernel.org/pub/linux/utils/net/iproute2/iproute2-5.2.0.tar.gz
Repository for upcoming release:
git://git.kernel.org/pub/scm/network/iproute2/iproute2.git
And future release (net-next):
git://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git
Thanks for all the contributions.
Report problems (or enhancements) to the netdev@vger.kernel.org mailing list.
---
Andrea Claudi (6):
Makefile: use make -C
ip address: do not set nodad option for IPv4 addresses
ip address: do not set home option for IPv4 addresses
ip address: do not set mngtmpaddr option for IPv4 addresses
man: tc-netem.8: fix URL for netem page
tc: netem: fix r parameter in Bernoulli loss model
Baruch Siach (2):
devlink: fix format string warning for 32bit targets
devlink: fix libc and kernel headers collision
David Ahern (5):
Update kernel headers
Update kernel headers
Update kernel headers
uapi: wrap SIOCGSTAMP and SIOCGSTAMPNS in ifndef
Update kernel headers
Davide Caratti (2):
man: tc-skbedit.8: document 'inheritdsfield'
tc: simple: don't hardcode the control action
Eyal Birger (1):
tc: adjust xtables_match and xtables_target to changes in recent iptables
Gal Pressman (1):
rdma: Update node type strings
Hangbin Liu (1):
ip/iptoken: fix dump error when ipv6 disabled
Hoang Le (3):
tipc: add link broadcast set method and ratio
tipc: add link broadcast get
tipc: add link broadcast man page
Ido Schimmel (2):
ipneigh: Print neighbour offload indication
devlink: Increase column size for larger shared buffers
Josh Hunt (1):
ss: add option to print socket information on one line
Kristian Evensen (1):
ip fou: Support binding FOU ports
Lucas Siba 2019-04-20 11:40 UTC (1):
Update tc-bpf.8 man page examples
Lukasz Czapnik (1):
tc: flower: fix port value truncation
Mahesh Bandewar (1):
ip6tunnel: fix 'ip -6 {show|change} dev <name>' cmds
Matteo Croce (4):
ip: reset netns after each command in batch mode
netns: switch netns in the child when executing commands
ip vrf: use hook to change VRF in the child
netns: make netns_{save,restore} static
Michael Forney (1):
ipmroute: Prevent overlapping storage of `filter` global
Mike Manning (1):
iplink_vlan: add support for VLAN bridge binding flag
Moshe Shemesh (1):
devlink: mnlg: Catch returned error value of dumpit commands
Nicolas Dichtel (3):
lib: suppress error msg when filling the cache
iplink: don't try to get ll addr len when creating an iface
ip monitor: display interfaces from all groups
Nikolay Aleksandrov (2):
iplink: bridge: add support for vlan_stats_per_port
bridge: mdb: restore text output format
Paolo Abeni (2):
tc: add support for plug qdisc
m_mirred: don't bail if the control action is missing
Parav Pandit (1):
devlink: Increase bus, device buffer size to 64 bytes
Pete Morici (1):
Add support for configuring MACsec gcm-aes-256 cipher type.
Roman Mashak (1):
tc: Fix binding of gact action by index.
Stefano Brivio (1):
iproute: Set flags and attributes on dump to get IPv6 cached routes to be flushed
Stephen Hemminger (14):
uapi: update to elf-em header
uapi: add include/linux/net.h
uapi: update headers to import asm-generic/sockios.h
mailmap: add myself
mailmap: map David's mail address
uapi: add sockios.h
uapi: merge bpf.h from 5.2
rdma: update uapi headers
man: fix macaddr section of ip-link
uapi: minor upstream btf.h header change
testsuite: intent if/else in Makefile
uapi: update headers and add if_link.h and if_infiniband.h
devlink: replace print macros with functions
vv5.2.0
Steve Wise (4):
Add .mailmap file
rdma: add helper rd_sendrecv_msg()
rdma: add 'link add/delete' commands
rdma: man page update for link add/delete
Tomasz Torcz (1):
ss: in --numeric mode, print raw numbers for data rates
Vinicius Costa Gomes (2):
taprio: Add support for changing schedules
taprio: Add support for cycle_time and cycle_time_extension
^ 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