* Re: [PATCH net-next V4 0/5] vhost: accelerate metadata access through vmap()
From: Michael S. Tsirkin @ 2019-01-23 13:58 UTC (permalink / raw)
To: Jason Wang; +Cc: virtualization, netdev, linux-kernel, kvm
In-Reply-To: <20190123095557.30168-1-jasowang@redhat.com>
On Wed, Jan 23, 2019 at 05:55:52PM +0800, Jason Wang wrote:
> This series tries to access virtqueue metadata through kernel virtual
> address instead of copy_user() friends since they had too much
> overheads like checks, spec barriers or even hardware feature
> toggling.
>
> Test shows about 24% improvement on TX PPS. It should benefit other
> cases as well.
ok I think this addresses most comments but it's a big change and we
just started 1.1 review so to pls give me a week to review this ok?
> Changes from V3:
> - don't try to use vmap for file backed pages
> - rebase to master
> Changes from V2:
> - fix buggy range overlapping check
> - tear down MMU notifier during vhost ioctl to make sure invalidation
> request can read metadata userspace address and vq size without
> holding vq mutex.
> Changes from V1:
> - instead of pinning pages, use MMU notifier to invalidate vmaps and
> remap duing metadata prefetch
> - fix build warning on MIPS
>
> Jason Wang (5):
> vhost: generalize adding used elem
> vhost: fine grain userspace memory accessors
> vhost: rename vq_iotlb_prefetch() to vq_meta_prefetch()
> vhost: introduce helpers to get the size of metadata area
> vhost: access vq metadata through kernel virtual address
>
> drivers/vhost/net.c | 4 +-
> drivers/vhost/vhost.c | 441 +++++++++++++++++++++++++++++++++++++-----
> drivers/vhost/vhost.h | 15 +-
> mm/shmem.c | 1 +
> 4 files changed, 410 insertions(+), 51 deletions(-)
>
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: use the proper optlen when doing strncpy in bpf_getsockopt()
From: Daniel Borkmann @ 2019-01-23 13:53 UTC (permalink / raw)
To: Martin Lau, Yafang Shao, Lawrence Brakmo
Cc: ast@kernel.org, netdev@vger.kernel.org, shaoyafang@didiglobal.com
In-Reply-To: <20190122180457.goq4xgtbi5cuxzwz@kafai-mbp.dhcp.thefacebook.com>
On 01/22/2019 07:04 PM, Martin Lau wrote:
> On Sat, Jan 19, 2019 at 02:20:52PM +0800, Yafang Shao wrote:
>> As the last character of optval will be set with 0, so just copying
>> (optlen - 1) characters is enough.
> I am not sure this optimization is needed but I think it will
Yeah, it's totally fine as is.
^ permalink raw reply
* Re: [PATCH bpf-next v2 2/8] libbpf: Add a helper for retrieving a prog via index
From: Maciej Fijalkowski @ 2019-01-23 13:41 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev, jakub.kicinski, brouer
In-Reply-To: <91d162e0-3d15-c1d8-1e80-8d0a4f561540@iogearbox.net>
On Wed, 23 Jan 2019 11:41:11 +0100
Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 01/21/2019 10:10 AM, Maciej Fijalkowski wrote:
> > xdp_redirect_cpu has a 6 different XDP programs that can be attached to
> > network interface. This sample has a option --prognum that allows user
> > for specifying which particular program from a given set will be
> > attached to network interface.
> > In order to make it easier when converting the mentioned sample to
> > libbpf usage, add a function to libbpf that will return program's fd for
> > a given index.
> >
> > Note that there is already a bpf_object__find_prog_by_idx, which could
> > be exported and might be used for that purpose, but it operates on the
> > number of ELF section and here we need an index from a programs array
> > within the bpf_object.
>
> Series in general looks good to me. Few minor comments, mainly in relation
> to the need for libbpf extensions.
>
> Would it not be a better interface to the user to instead choose the prog
> based on section name and then retrieve it via bpf_object__find_program_by_title()
> instead of prognum (which feels less user friendly) at least?
>
I couldn't decide which one from:
* adding a libbpf helper
* changing the xdp_redirect_cpu behaviour
would be more invasive when I was converting this sample to libbpf support.
Your suggestion sounds good, but I'm wondering about the actual implementation.
I suppose that we would choose the program via command line. Some program
section names in this sample are a bit long and it might be irritating for
user to type in for example "xdp_cpu_map5_lb_hash_ip_pairs", no? Or maybe we
can live with this. In case of typo and program being not found it would be
good to print out the section names to choose from in usage().
> > Signed-off-by: Maciej Fijalkowski <maciejromanfijalkowski@gmail.com>
> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > tools/lib/bpf/libbpf.c | 8 ++++++++
> > tools/lib/bpf/libbpf.h | 3 +++
> > tools/lib/bpf/libbpf.map | 1 +
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index dc838bea403f..21c84d0f6128 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -935,6 +935,14 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
> > return err;
> > }
> >
> > +int
> > +bpf_object__get_prog_fd_by_num(struct bpf_object *obj, int idx)
> > +{
> > + if (idx >= 0 && idx < obj->nr_programs)
> > + return bpf_program__fd(&obj->programs[idx]);
> > + return -ENOENT;
> > +}
> > +
> > static struct bpf_program *
> > bpf_object__find_prog_by_idx(struct bpf_object *obj, int idx)
> > {
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 7f10d36abdde..ca1b381cb3ad 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -95,6 +95,9 @@ LIBBPF_API int bpf_object__btf_fd(const struct bpf_object *obj);
> > LIBBPF_API struct bpf_program *
> > bpf_object__find_program_by_title(struct bpf_object *obj, const char *title);
> >
> > +LIBBPF_API int
> > +bpf_object__get_prog_fd_by_num(struct bpf_object *obj, int idx);
> > +
> > LIBBPF_API struct bpf_object *bpf_object__next(struct bpf_object *prev);
> > #define bpf_object__for_each_safe(pos, tmp) \
> > for ((pos) = bpf_object__next(NULL), \
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 7c59e4f64082..871d2fc07150 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -127,4 +127,5 @@ LIBBPF_0.0.1 {
> > LIBBPF_0.0.2 {
> > global:
> > bpf_object__find_map_fd_by_name;
> > + bpf_object__get_prog_fd_by_num;
> > } LIBBPF_0.0.1;
> >
>
^ permalink raw reply
* Re: [PATCH 1/3] treewide: Lift switch variables out of switches
From: William Kucharski @ 2019-01-23 13:21 UTC (permalink / raw)
To: Jann Horn
Cc: Greg KH, Kees Cook, kernel list, Ard Biesheuvel, Laura Abbott,
Alexander Popov, xen-devel, dri-devel, intel-gfx, intel-wired-lan,
Network Development, linux-usb, linux-fsdevel, Linux-MM, dev,
linux-kbuild, linux-security-module, Kernel Hardening
In-Reply-To: <CAG48ez2vfXkr9dozJiGmze8k49VOXfs=K7M8bv0aQsDDpzrEFQ@mail.gmail.com>
> On Jan 23, 2019, at 5:09 AM, Jann Horn <jannh@google.com> wrote:
>
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .
It bothers me that we are going out of our way to deprecate valid C constructs
in favor of placing the declarations elsewhere.
As current compiler warnings would catch any reference before initialization
usage anyway, it seems like we are letting a compiler warning rather than the
language standard dictate syntax.
Certainly if we want to make it a best practice coding style issue we can, and
then an appropriate note explaining why should be added to
Documentation/process/coding-style.rst.
^ permalink raw reply
* Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF_XDP
From: Daniel Borkmann @ 2019-01-23 13:19 UTC (permalink / raw)
To: bjorn.topel, ast, netdev
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson
In-Reply-To: <20190118130305.11504-4-bjorn.topel@gmail.com>
On 01/18/2019 02:03 PM, bjorn.topel@gmail.com wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This patch adds the sock_diag interface for querying sockets from user
> space. Tools like iproute2 ss(8) can use this interface to list open
> AF_XDP sockets.
>
> The user-space ABI is defined in linux/xdp_diag.h and includes netlink
> request and response structs. The request can query sockets and the
> response contains socket information about the rings, umems, inode and
> more.
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Series looks good, few minor nits inline:
> ---
> include/uapi/linux/xdp_diag.h | 72 +++++++++++++
> net/xdp/Kconfig | 8 ++
> net/xdp/Makefile | 1 +
> net/xdp/xsk.c | 6 +-
> net/xdp/xsk.h | 12 +++
> net/xdp/xsk_diag.c | 192 ++++++++++++++++++++++++++++++++++
> 6 files changed, 286 insertions(+), 5 deletions(-)
> create mode 100644 include/uapi/linux/xdp_diag.h
> create mode 100644 net/xdp/xsk.h
> create mode 100644 net/xdp/xsk_diag.c
>
> diff --git a/include/uapi/linux/xdp_diag.h b/include/uapi/linux/xdp_diag.h
> new file mode 100644
> index 000000000000..efe8ce281dce
> --- /dev/null
> +++ b/include/uapi/linux/xdp_diag.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * xdp_diag: interface for query/monitor XDP sockets
> + * Copyright(c) 2019 Intel Corporation.
> + */
> +
> +#ifndef _LINUX_XDP_DIAG_H
> +#define _LINUX_XDP_DIAG_H
> +
> +#include <linux/types.h>
> +
> +struct xdp_diag_req {
> + __u8 sdiag_family;
> + __u8 sdiag_protocol;
> + __u16 pad;
Presumably this one is for future use? Maybe better as '__u16 :16;' to
avoid compile errors if someone tries to zero 'pad' member manually?
> + __u32 xdiag_ino;
> + __u32 xdiag_show;
> + __u32 xdiag_cookie[2];
> +};
> +
> +struct xdp_diag_msg {
> + __u8 xdiag_family;
> + __u8 xdiag_type;
> + __u32 xdiag_ino;
> + __u32 xdiag_cookie[2];
> +};
> +
> +#define XDP_SHOW_INFO (1 << 0) /* Basic information */
> +#define XDP_SHOW_RING_CFG (1 << 1)
> +#define XDP_SHOW_UMEM (1 << 2)
> +#define XDP_SHOW_MEMINFO (1 << 3)
> +
> +enum {
> + XDP_DIAG_NONE,
> + XDP_DIAG_INFO,
> + XDP_DIAG_UID,
> + XDP_DIAG_RX_RING,
> + XDP_DIAG_TX_RING,
> + XDP_DIAG_UMEM,
> + XDP_DIAG_UMEM_FILL_RING,
> + XDP_DIAG_UMEM_COMPLETION_RING,
> + XDP_DIAG_MEMINFO,
> + __XDP_DIAG_MAX,
> +};
> +
> +#define XDP_DIAG_MAX (__XDP_DIAG_MAX - 1)
> +
> +struct xdp_diag_info {
> + __u32 ifindex;
> + __u32 queue_id;
> +};
> +
> +struct xdp_diag_ring {
> + __u32 entries; /*num descs */
> +};
> +
> +#define XDP_DU_F_ZEROCOPY (1 << 0)
> +
> +struct xdp_diag_umem {
> + __u64 size;
> + __u32 id;
> + __u32 num_pages;
> + __u32 chunk_size;
> + __u32 headroom;
> + __u32 ifindex;
> + __u32 queue_id;
> + __u32 flags;
> + __u32 refs;
> +};
> +
> +
Nit: one newline too much
> +#endif /* _LINUX_XDP_DIAG_H */
> diff --git a/net/xdp/Kconfig b/net/xdp/Kconfig
> index 90e4a7152854..0255b33cff4b 100644
> --- a/net/xdp/Kconfig
> +++ b/net/xdp/Kconfig
> @@ -5,3 +5,11 @@ config XDP_SOCKETS
> help
> XDP sockets allows a channel between XDP programs and
> userspace applications.
> +
> +config XDP_SOCKETS_DIAG
> + tristate "XDP sockets: monitoring interface"
> + depends on XDP_SOCKETS
> + default n
> + help
> + Support for PF_XDP sockets monitoring interface used by the ss tool.
> + If unsure, say Y.
> diff --git a/net/xdp/Makefile b/net/xdp/Makefile
> index 04f073146256..59dbfdf93dca 100644
> --- a/net/xdp/Makefile
> +++ b/net/xdp/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_XDP_SOCKETS) += xsk.o xdp_umem.o xsk_queue.o
> +obj-$(CONFIG_XDP_SOCKETS_DIAG) += xsk_diag.o
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 80ca48cefc42..949d3bbccb2f 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -27,14 +27,10 @@
>
> #include "xsk_queue.h"
> #include "xdp_umem.h"
> +#include "xsk.h"
>
> #define TX_BATCH_SIZE 16
>
> -static struct xdp_sock *xdp_sk(struct sock *sk)
> -{
> - return (struct xdp_sock *)sk;
> -}
> -
> bool xsk_is_setup_for_bpf_map(struct xdp_sock *xs)
> {
> return READ_ONCE(xs->rx) && READ_ONCE(xs->umem) &&
> diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
> new file mode 100644
> index 000000000000..ba8120610426
> --- /dev/null
> +++ b/net/xdp/xsk.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2019 Intel Corporation. */
> +
> +#ifndef XSK_H_
> +#define XSK_H_
> +
> +static inline struct xdp_sock *xdp_sk(struct sock *sk)
> +{
> + return (struct xdp_sock *)sk;
> +}
> +
> +#endif /* XSK_H_ */
> diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
> new file mode 100644
> index 000000000000..2585d7a4ac2f
> --- /dev/null
> +++ b/net/xdp/xsk_diag.c
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* XDP sockets monitoring support
> + *
> + * Copyright(c) 2019 Intel Corporation.
> + *
> + * Author: Björn Töpel <bjorn.topel@intel.com>
> + */
> +
> +#include <linux/module.h>
> +#include <net/xdp_sock.h>
> +#include <linux/xdp_diag.h>
> +#include <linux/sock_diag.h>
> +
> +#include "xsk_queue.h"
> +#include "xsk.h"
> +
> +static int xsk_diag_put_info(const struct xdp_sock *xs, struct sk_buff *nlskb)
> +{
> + struct xdp_diag_info di;
Not a bug, but can we generally zero all the structs on stack such that once
we extend them nothing gets potentially leaked.
> + di.ifindex = xs->dev ? xs->dev->ifindex : 0;
> + di.queue_id = xs->queue_id;
> + return nla_put(nlskb, XDP_DIAG_INFO, sizeof(di), &di);
> +}
> +
> +static int xsk_diag_put_ring(const struct xsk_queue *queue, int nl_type,
> + struct sk_buff *nlskb)
> +{
> + struct xdp_diag_ring dr;
Ditto.
> + dr.entries = queue->nentries;
> + return nla_put(nlskb, nl_type, sizeof(dr), &dr);
> +}
> +
> +static int xsk_diag_put_rings_cfg(const struct xdp_sock *xs,
> + struct sk_buff *nlskb)
> +{
> + int err = 0;
> +
> + if (xs->rx)
> + err = xsk_diag_put_ring(xs->rx, XDP_DIAG_RX_RING, nlskb);
> + if (!err && xs->tx)
> + err = xsk_diag_put_ring(xs->tx, XDP_DIAG_TX_RING, nlskb);
> + return err;
> +}
> +
> +static int xsk_diag_put_umem(const struct xdp_sock *xs, struct sk_buff *nlskb)
> +{
> + struct xdp_umem *umem = xs->umem;
> + struct xdp_diag_umem du;
Ditto
> + int err;
> +
> + if (!umem)
> + return 0;
> +
> + du.id = umem->id;
> + du.size = umem->size;
> + du.num_pages = umem->npgs;
> + du.chunk_size = (__u32)(~umem->chunk_mask + 1);
> + du.headroom = umem->headroom;
> + du.ifindex = umem->dev ? umem->dev->ifindex : 0;
> + du.queue_id = umem->queue_id;
> + du.flags = 0;
> + if (umem->zc)
> + du.flags |= XDP_DU_F_ZEROCOPY;
> + du.refs = refcount_read(&umem->users);
> +
> + err = nla_put(nlskb, XDP_DIAG_UMEM, sizeof(du), &du);
> +
> + if (!err && umem->fq)
> + err = xsk_diag_put_ring(xs->tx, XDP_DIAG_UMEM_FILL_RING, nlskb);
> + if (!err && umem->cq) {
> + err = xsk_diag_put_ring(xs->tx, XDP_DIAG_UMEM_COMPLETION_RING,
> + nlskb);
> + }
> + return err;
> +}
> +
> +static int xsk_diag_fill(struct sock *sk, struct sk_buff *nlskb,
> + struct xdp_diag_req *req,
> + struct user_namespace *user_ns,
> + u32 portid, u32 seq, u32 flags, int sk_ino)
> +{
> + struct xdp_sock *xs = xdp_sk(sk);
> + struct xdp_diag_msg *msg;
> + struct nlmsghdr *nlh;
> +
> + nlh = nlmsg_put(nlskb, portid, seq, SOCK_DIAG_BY_FAMILY, sizeof(*msg),
> + flags);
> + if (!nlh)
> + return -EMSGSIZE;
> +
> + msg = nlmsg_data(nlh);
> + msg->xdiag_family = AF_XDP;
> + msg->xdiag_type = sk->sk_type;
> + msg->xdiag_ino = sk_ino;
> + sock_diag_save_cookie(sk, msg->xdiag_cookie);
Don't we have a hole in struct xdp_diag_msg after xdiag_type? Probably better
to memset everything in the beginning as well.
> + if ((req->xdiag_show & XDP_SHOW_INFO) && xsk_diag_put_info(xs, nlskb))
> + goto out_nlmsg_trim;
> +
> + if ((req->xdiag_show & XDP_SHOW_INFO) &&
> + nla_put_u32(nlskb, XDP_DIAG_UID,
> + from_kuid_munged(user_ns, sock_i_uid(sk))))
> + goto out_nlmsg_trim;
> +
> + if ((req->xdiag_show & XDP_SHOW_RING_CFG) &&
> + xsk_diag_put_rings_cfg(xs, nlskb))
> + goto out_nlmsg_trim;
> +
> + if ((req->xdiag_show & XDP_SHOW_UMEM) &&
> + xsk_diag_put_umem(xs, nlskb))
> + goto out_nlmsg_trim;
> +
> + if ((req->xdiag_show & XDP_SHOW_MEMINFO) &&
> + sock_diag_put_meminfo(sk, nlskb, XDP_DIAG_MEMINFO))
> + goto out_nlmsg_trim;
> +
> + nlmsg_end(nlskb, nlh);
> + return 0;
> +
> +out_nlmsg_trim:
> + nlmsg_cancel(nlskb, nlh);
> + return -EMSGSIZE;
> +}
> +
> +static int xsk_diag_dump(struct sk_buff *nlskb, struct netlink_callback *cb)
> +{
> + struct xdp_diag_req *req = nlmsg_data(cb->nlh);
> + struct net *net = sock_net(nlskb->sk);
> + int num = 0, s_num = cb->args[0];
> + struct sock *sk;
> +
> + mutex_lock(&net->xdp.lock);
> +
> + sk_for_each(sk, &net->xdp.list) {
> + if (!net_eq(sock_net(sk), net))
> + continue;
> + if (num++ < s_num)
> + continue;
> +
> + if (xsk_diag_fill(sk, nlskb, req,
> + sk_user_ns(NETLINK_CB(cb->skb).sk),
> + NETLINK_CB(cb->skb).portid,
> + cb->nlh->nlmsg_seq, NLM_F_MULTI,
> + sock_i_ino(sk)) < 0) {
> + num--;
> + break;
> + }
> + }
> +
> + mutex_unlock(&net->xdp.lock);
> + cb->args[0] = num;
> + return nlskb->len;
> +}
> +
> +static int xsk_diag_handler_dump(struct sk_buff *nlskb, struct nlmsghdr *hdr)
> +{
> + struct netlink_dump_control c = { .dump = xsk_diag_dump };
> + int hdrlen = sizeof(struct xdp_diag_req);
> + struct net *net = sock_net(nlskb->sk);
> + struct xdp_diag_req *req;
> +
> + if (nlmsg_len(hdr) < hdrlen)
> + return -EINVAL;
> +
> + if (!(hdr->nlmsg_flags & NLM_F_DUMP))
> + return -EOPNOTSUPP;
> +
> + req = nlmsg_data(hdr);
> + return netlink_dump_start(net->diag_nlsk, nlskb, hdr, &c);
> +}
> +
> +static const struct sock_diag_handler xsk_diag_handler = {
> + .family = AF_XDP,
> + .dump = xsk_diag_handler_dump,
> +};
> +
> +static int __init xsk_diag_init(void)
> +{
> + return sock_diag_register(&xsk_diag_handler);
> +}
> +
> +static void __exit xsk_diag_exit(void)
> +{
> + sock_diag_unregister(&xsk_diag_handler);
> +}
> +
> +module_init(xsk_diag_init);
> +module_exit(xsk_diag_exit);
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_SOCK_DIAG, AF_XDP);
>
^ permalink raw reply
* Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct
From: Andrew Lunn @ 2019-01-23 13:15 UTC (permalink / raw)
To: Igor Russkikh; +Cc: David S . Miller, netdev@vger.kernel.org, Nikita Danilov
In-Reply-To: <1bac2c2c-2c60-5043-3b74-0b20138a72c1@aquantia.com>
On Wed, Jan 23, 2019 at 09:49:25AM +0000, Igor Russkikh wrote:
>
> >
> > Hi Igor
> >
> > err = readx_poll_timeout(hw_atl_itr_res_irq_get, self, alt_itr_res,
> > alt_itr_res == 0, 10, 1000);
> >
> > The advantage of using readx_poll_timeout is that it is used by lots
> > of other drivers and works. It is much better to use core
> > infrastructure, then build your own.
>
> Hi Andrew, agreed, but driver have more incompatible places with constructs like
>
> AQ_HW_WAIT_FOR(orig_stats_val !=
> (aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
> BIT(CAPS_HI_STATISTICS)),
> 1U, 10000U);
You can define a little helper:
static u32 aq_hw_read_mpi_state2_addr(struct aq_hw_s *self)
{
return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
}
Then
readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self,
stats_val, orig_stats_val != stats_val & BIT(CAPS_HI_STATISTICS));
Given you have the comment:
/* Wait FW to report back */
You think the current code is not very readable. So you could actually
have:
static int sq_fw2_update_stats_fw_wait(aq_hw_s *self, u32 orig_stats_val)
{
u32 stats_val;
return readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self,
stats_val,
orig_stats_val != stats_val & BIT(CAPS_HI_STATISTICS),
1, 1000);
}
You see this sort of construct in quite a lot of drivers.
> Found duplicating readl_poll_timeout declaration here:
> https://elixir.bootlin.com/linux/latest/source/drivers/phy/qualcomm/phy-qcom-ufs-i.h#L27
> Not sure what's the reason, but may worth cleaning it up.
Thanks.
Andrew
^ permalink raw reply
* Re: [PATCH ipvs-next] ipvs: avoid indirect calls when calculating checksums
From: Simon Horman @ 2019-01-23 12:50 UTC (permalink / raw)
To: Julian Anastasov
Cc: Matteo Croce, lvs-devel, netdev, netfilter-devel, coreteam,
Wensong Zhang
In-Reply-To: <alpine.LFD.2.21.1901201410480.20940@ja.home.ssi.bg>
On Sun, Jan 20, 2019 at 02:11:31PM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Sat, 19 Jan 2019, Matteo Croce wrote:
>
> > The function pointer ip_vs_protocol->csum_check is only used in protocol
> > specific code, and never in the generic one.
> > Remove the function pointer from struct ip_vs_protocol and call the
> > checksum functions directly.
> > This reduces the performance impact of the Spectre mitigation, and
> > should give a small improvement even with RETPOLINES disabled.
> >
> > Signed-off-by: Matteo Croce <mcroce@redhat.com>
>
> Looks good to me, thanks!
>
> Acked-by: Julian Anastasov <ja@ssi.bg>
Likewise, Pablo could you consider applying this to nf-next?
Acked-by: Simon Horman <horms@verge.net.au>
>
> > ---
> > include/net/ip_vs.h | 3 ---
> > net/netfilter/ipvs/ip_vs_proto_ah_esp.c | 2 --
> > net/netfilter/ipvs/ip_vs_proto_sctp.c | 8 +++++---
> > net/netfilter/ipvs/ip_vs_proto_tcp.c | 12 +++++++-----
> > net/netfilter/ipvs/ip_vs_proto_udp.c | 12 +++++++-----
> > 5 files changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> > index a0d2e0bb9a94..047f9a5ccaad 100644
> > --- a/include/net/ip_vs.h
> > +++ b/include/net/ip_vs.h
> > @@ -453,9 +453,6 @@ struct ip_vs_protocol {
> > int (*dnat_handler)(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > struct ip_vs_conn *cp, struct ip_vs_iphdr *iph);
> >
> > - int (*csum_check)(int af, struct sk_buff *skb,
> > - struct ip_vs_protocol *pp);
> > -
> > const char *(*state_name)(int state);
> >
> > void (*state_transition)(struct ip_vs_conn *cp, int direction,
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_ah_esp.c b/net/netfilter/ipvs/ip_vs_proto_ah_esp.c
> > index 5320d39976e1..480598cb0f05 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_ah_esp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_ah_esp.c
> > @@ -129,7 +129,6 @@ struct ip_vs_protocol ip_vs_protocol_ah = {
> > .conn_out_get = ah_esp_conn_out_get,
> > .snat_handler = NULL,
> > .dnat_handler = NULL,
> > - .csum_check = NULL,
> > .state_transition = NULL,
> > .register_app = NULL,
> > .unregister_app = NULL,
> > @@ -152,7 +151,6 @@ struct ip_vs_protocol ip_vs_protocol_esp = {
> > .conn_out_get = ah_esp_conn_out_get,
> > .snat_handler = NULL,
> > .dnat_handler = NULL,
> > - .csum_check = NULL,
> > .state_transition = NULL,
> > .register_app = NULL,
> > .unregister_app = NULL,
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> > index b0cd7d08f2a7..bc3d1625ecc8 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> > @@ -9,6 +9,9 @@
> > #include <net/sctp/checksum.h>
> > #include <net/ip_vs.h>
> >
> > +static int
> > +sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp);
> > +
> > static int
> > sctp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
> > struct ip_vs_proto_data *pd,
> > @@ -105,7 +108,7 @@ sctp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!sctp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /* Call application helper if needed */
> > @@ -152,7 +155,7 @@ sctp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!sctp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /* Call application helper if needed */
> > @@ -587,7 +590,6 @@ struct ip_vs_protocol ip_vs_protocol_sctp = {
> > .conn_out_get = ip_vs_conn_out_get_proto,
> > .snat_handler = sctp_snat_handler,
> > .dnat_handler = sctp_dnat_handler,
> > - .csum_check = sctp_csum_check,
> > .state_name = sctp_state_name,
> > .state_transition = sctp_state_transition,
> > .app_conn_bind = sctp_app_conn_bind,
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > index 1770fc6ce960..6a275f989085 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > @@ -31,6 +31,9 @@
> >
> > #include <net/ip_vs.h>
> >
> > +static int
> > +tcp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp);
> > +
> > static int
> > tcp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
> > struct ip_vs_proto_data *pd,
> > @@ -166,7 +169,7 @@ tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!tcp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /* Call application helper if needed */
> > @@ -192,7 +195,7 @@ tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > tcp_fast_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
> > cp->dport, cp->vport);
> > if (skb->ip_summed == CHECKSUM_COMPLETE)
> > - skb->ip_summed = (cp->app && pp->csum_check) ?
> > + skb->ip_summed = cp->app ?
> > CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
> > } else {
> > /* full checksum calculation */
> > @@ -244,7 +247,7 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!tcp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /*
> > @@ -275,7 +278,7 @@ tcp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > tcp_fast_csum_update(cp->af, tcph, &cp->vaddr, &cp->daddr,
> > cp->vport, cp->dport);
> > if (skb->ip_summed == CHECKSUM_COMPLETE)
> > - skb->ip_summed = (cp->app && pp->csum_check) ?
> > + skb->ip_summed = cp->app ?
> > CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
> > } else {
> > /* full checksum calculation */
> > @@ -736,7 +739,6 @@ struct ip_vs_protocol ip_vs_protocol_tcp = {
> > .conn_out_get = ip_vs_conn_out_get_proto,
> > .snat_handler = tcp_snat_handler,
> > .dnat_handler = tcp_dnat_handler,
> > - .csum_check = tcp_csum_check,
> > .state_name = tcp_state_name,
> > .state_transition = tcp_state_transition,
> > .app_conn_bind = tcp_app_conn_bind,
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
> > index 0f53c49025f8..3285718264d5 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_udp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
> > @@ -28,6 +28,9 @@
> > #include <net/ip.h>
> > #include <net/ip6_checksum.h>
> >
> > +static int
> > +udp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp);
> > +
> > static int
> > udp_conn_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
> > struct ip_vs_proto_data *pd,
> > @@ -156,7 +159,7 @@ udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!udp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /*
> > @@ -186,7 +189,7 @@ udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > udp_fast_csum_update(cp->af, udph, &cp->daddr, &cp->vaddr,
> > cp->dport, cp->vport);
> > if (skb->ip_summed == CHECKSUM_COMPLETE)
> > - skb->ip_summed = (cp->app && pp->csum_check) ?
> > + skb->ip_summed = cp->app ?
> > CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
> > } else {
> > /* full checksum calculation */
> > @@ -239,7 +242,7 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > int ret;
> >
> > /* Some checks before mangling */
> > - if (pp->csum_check && !pp->csum_check(cp->af, skb, pp))
> > + if (!udp_csum_check(cp->af, skb, pp))
> > return 0;
> >
> > /*
> > @@ -270,7 +273,7 @@ udp_dnat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > udp_fast_csum_update(cp->af, udph, &cp->vaddr, &cp->daddr,
> > cp->vport, cp->dport);
> > if (skb->ip_summed == CHECKSUM_COMPLETE)
> > - skb->ip_summed = (cp->app && pp->csum_check) ?
> > + skb->ip_summed = cp->app ?
> > CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
> > } else {
> > /* full checksum calculation */
> > @@ -494,7 +497,6 @@ struct ip_vs_protocol ip_vs_protocol_udp = {
> > .conn_out_get = ip_vs_conn_out_get_proto,
> > .snat_handler = udp_snat_handler,
> > .dnat_handler = udp_dnat_handler,
> > - .csum_check = udp_csum_check,
> > .state_transition = udp_state_transition,
> > .state_name = udp_state_name,
> > .register_app = udp_register_app,
> > --
> > 2.20.1
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>
^ permalink raw reply
* Re: [PATCH ipvs-next] ipvs: use indirect call wrappers
From: Simon Horman @ 2019-01-23 12:47 UTC (permalink / raw)
To: Julian Anastasov, Pablo Neira Ayuso
Cc: Matteo Croce, lvs-devel, netdev, netfilter-devel, coreteam,
Wensong Zhang, Paolo Abeni
In-Reply-To: <alpine.LFD.2.21.1901201411400.20940@ja.home.ssi.bg>
On Sun, Jan 20, 2019 at 02:12:04PM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Sat, 19 Jan 2019, Matteo Croce wrote:
>
> > Use the new indirect call wrappers in IPVS when calling the TCP or UDP
> > protocol specific functions.
> > This avoids an indirect calls in IPVS, and reduces the performance
> > impact of the Spectre mitigation.
> >
> > Signed-off-by: Matteo Croce <mcroce@redhat.com>
>
> Looks good to me, thanks!
>
> Acked-by: Julian Anastasov <ja@ssi.bg>
Likewise, Pablo could you consider applying this to nf-next?
Acked-by: Simon Horman <horms@verge.net.au>
>
> > ---
> > net/netfilter/ipvs/ip_vs_core.c | 49 +++++++++++++++++++++++-----
> > net/netfilter/ipvs/ip_vs_proto_tcp.c | 3 +-
> > net/netfilter/ipvs/ip_vs_proto_udp.c | 3 +-
> > 3 files changed, 45 insertions(+), 10 deletions(-)
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > index fe9abf3cc10a..e969dad66991 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -53,6 +53,7 @@
> > #endif
> >
> > #include <net/ip_vs.h>
> > +#include <linux/indirect_call_wrapper.h>
> >
> >
> > EXPORT_SYMBOL(register_ip_vs_scheduler);
> > @@ -70,6 +71,29 @@ EXPORT_SYMBOL(ip_vs_get_debug_level);
> > #endif
> > EXPORT_SYMBOL(ip_vs_new_conn_out);
> >
> > +#ifdef CONFIG_IP_VS_PROTO_TCP
> > +INDIRECT_CALLABLE_DECLARE(int
> > + tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > + struct ip_vs_conn *cp, struct ip_vs_iphdr *iph));
> > +#endif
> > +
> > +#ifdef CONFIG_IP_VS_PROTO_UDP
> > +INDIRECT_CALLABLE_DECLARE(int
> > + udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > + struct ip_vs_conn *cp, struct ip_vs_iphdr *iph));
> > +#endif
> > +
> > +#if defined(CONFIG_IP_VS_PROTO_TCP) && defined(CONFIG_IP_VS_PROTO_UDP)
> > +#define SNAT_CALL(f, ...) \
> > + INDIRECT_CALL_2(f, tcp_snat_handler, udp_snat_handler, __VA_ARGS__)
> > +#elif defined(CONFIG_IP_VS_PROTO_TCP)
> > +#define SNAT_CALL(f, ...) INDIRECT_CALL_1(f, tcp_snat_handler, __VA_ARGS__)
> > +#elif defined(CONFIG_IP_VS_PROTO_UDP)
> > +#define SNAT_CALL(f, ...) INDIRECT_CALL_1(f, udp_snat_handler, __VA_ARGS__)
> > +#else
> > +#define SNAT_CALL(f, ...) f(__VA_ARGS__)
> > +#endif
> > +
> > static unsigned int ip_vs_net_id __read_mostly;
> > /* netns cnt used for uniqueness */
> > static atomic_t ipvs_netns_cnt = ATOMIC_INIT(0);
> > @@ -478,7 +502,9 @@ ip_vs_schedule(struct ip_vs_service *svc, struct sk_buff *skb,
> > */
> > if ((!skb->dev || skb->dev->flags & IFF_LOOPBACK)) {
> > iph->hdr_flags ^= IP_VS_HDR_INVERSE;
> > - cp = pp->conn_in_get(svc->ipvs, svc->af, skb, iph);
> > + cp = INDIRECT_CALL_1(pp->conn_in_get,
> > + ip_vs_conn_in_get_proto, svc->ipvs,
> > + svc->af, skb, iph);
> > iph->hdr_flags ^= IP_VS_HDR_INVERSE;
> >
> > if (cp) {
> > @@ -972,7 +998,8 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb,
> > ip_vs_fill_iph_skb_icmp(AF_INET, skb, offset, true, &ciph);
> >
> > /* The embedded headers contain source and dest in reverse order */
> > - cp = pp->conn_out_get(ipvs, AF_INET, skb, &ciph);
> > + cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto,
> > + ipvs, AF_INET, skb, &ciph);
> > if (!cp)
> > return NF_ACCEPT;
> >
> > @@ -1028,7 +1055,8 @@ static int ip_vs_out_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
> > return NF_ACCEPT;
> >
> > /* The embedded headers contain source and dest in reverse order */
> > - cp = pp->conn_out_get(ipvs, AF_INET6, skb, &ciph);
> > + cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto,
> > + ipvs, AF_INET6, skb, &ciph);
> > if (!cp)
> > return NF_ACCEPT;
> >
> > @@ -1263,7 +1291,8 @@ handle_response(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
> > goto drop;
> >
> > /* mangle the packet */
> > - if (pp->snat_handler && !pp->snat_handler(skb, pp, cp, iph))
> > + if (pp->snat_handler &&
> > + !SNAT_CALL(pp->snat_handler, skb, pp, cp, iph))
> > goto drop;
> >
> > #ifdef CONFIG_IP_VS_IPV6
> > @@ -1389,7 +1418,8 @@ ip_vs_out(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, in
> > /*
> > * Check if the packet belongs to an existing entry
> > */
> > - cp = pp->conn_out_get(ipvs, af, skb, &iph);
> > + cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto,
> > + ipvs, af, skb, &iph);
> >
> > if (likely(cp)) {
> > if (IP_VS_FWD_METHOD(cp) != IP_VS_CONN_F_MASQ)
> > @@ -1644,7 +1674,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
> > /* The embedded headers contain source and dest in reverse order.
> > * For IPIP this is error for request, not for reply.
> > */
> > - cp = pp->conn_in_get(ipvs, AF_INET, skb, &ciph);
> > + cp = INDIRECT_CALL_1(pp->conn_in_get, ip_vs_conn_in_get_proto,
> > + ipvs, AF_INET, skb, &ciph);
> >
> > if (!cp) {
> > int v;
> > @@ -1796,7 +1827,8 @@ static int ip_vs_in_icmp_v6(struct netns_ipvs *ipvs, struct sk_buff *skb,
> > /* The embedded headers contain source and dest in reverse order
> > * if not from localhost
> > */
> > - cp = pp->conn_in_get(ipvs, AF_INET6, skb, &ciph);
> > + cp = INDIRECT_CALL_1(pp->conn_in_get, ip_vs_conn_in_get_proto,
> > + ipvs, AF_INET6, skb, &ciph);
> >
> > if (!cp) {
> > int v;
> > @@ -1925,7 +1957,8 @@ ip_vs_in(struct netns_ipvs *ipvs, unsigned int hooknum, struct sk_buff *skb, int
> > /*
> > * Check if the packet belongs to an existing connection entry
> > */
> > - cp = pp->conn_in_get(ipvs, af, skb, &iph);
> > + cp = INDIRECT_CALL_1(pp->conn_in_get, ip_vs_conn_in_get_proto,
> > + ipvs, af, skb, &iph);
> >
> > conn_reuse_mode = sysctl_conn_reuse_mode(ipvs);
> > if (conn_reuse_mode && !iph.fragoffs && is_new_conn(skb, &iph) && cp) {
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > index 6a275f989085..479419759983 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> > @@ -28,6 +28,7 @@
> > #include <net/ip6_checksum.h>
> > #include <linux/netfilter.h>
> > #include <linux/netfilter_ipv4.h>
> > +#include <linux/indirect_call_wrapper.h>
> >
> > #include <net/ip_vs.h>
> >
> > @@ -146,7 +147,7 @@ tcp_partial_csum_update(int af, struct tcphdr *tcph,
> > }
> >
> >
> > -static int
> > +INDIRECT_CALLABLE_SCOPE int
> > tcp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > struct ip_vs_conn *cp, struct ip_vs_iphdr *iph)
> > {
> > diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
> > index 3285718264d5..646c384910fb 100644
> > --- a/net/netfilter/ipvs/ip_vs_proto_udp.c
> > +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
> > @@ -23,6 +23,7 @@
> > #include <linux/netfilter.h>
> > #include <linux/netfilter_ipv4.h>
> > #include <linux/udp.h>
> > +#include <linux/indirect_call_wrapper.h>
> >
> > #include <net/ip_vs.h>
> > #include <net/ip.h>
> > @@ -136,7 +137,7 @@ udp_partial_csum_update(int af, struct udphdr *uhdr,
> > }
> >
> >
> > -static int
> > +INDIRECT_CALLABLE_SCOPE int
> > udp_snat_handler(struct sk_buff *skb, struct ip_vs_protocol *pp,
> > struct ip_vs_conn *cp, struct ip_vs_iphdr *iph)
> > {
> > --
> > 2.20.1
>
> Regards
>
> --
> Julian Anastasov <ja@ssi.bg>
>
^ permalink raw reply
* Re: [PATCH net-next 0/7] Devlink health updates
From: Eran Ben Elisha @ 2019-01-23 12:34 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev@vger.kernel.org, Jiri Pirko, David S. Miller,
Saeed Mahameed, Moshe Shemesh
In-Reply-To: <20190123114452.GE2191@nanopsycho>
On 1/23/2019 1:44 PM, Jiri Pirko wrote:
> Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote:
>> This patchset fixes some comments that were received for the devlink
>> health series, mostly around the devlink health buffers API.
>>
>> It offers a new devlink<->driver API for passing health dump and diagnose info.
>> As part of this patchset, the new API is developed and integrated into the
>> devlink health and mlx5e TX reporter.
>> Also, added some helpers together with the new API, which reduce the code
>> required by the driver to fill dump and diagnose significantly.
>>
>> Eventually, it also deletes the old API.
>>
>> In addition, it includes some small fixes in the devlink and mlx5e TX reporter.
>
> We are (re-)defining UAPI here. You need to present some examples
> of devlink tool output, both in json and stdout.
Actually we don't really redefining the section, but only having naming
change, DEVLINK_ATTR_HEALTH_BUFFER_OBJECT* to DEVLINK_ATTR_MSG_OBJECT*.
It is pretty much the same in UAPI perspective. All examples from
original patchset are still 100% true.
> Again, much more convenient to be done for the whole patchset, not this
> "fixup-one" :/
The core of the change you have asked for it fully implemented in patch
0001 (and will be kept as is if I would do the re-done procedure). Me
and Moshe had an internal review yesterday and it doesn't look like a
nightmare at all.
Please try to give it a real try as I worked a lot in order to meet the
DD set here, and if you still find it really hard, let me know.
>
>
>>
>>
>> Eran Ben Elisha (7):
>> devlink: Add devlink msg API
>> net/mlx5e: Move driver to use devlink msg API
>> devlink: move devlink health reporter to use devlink msg API
>> devlink: Delete depracated health buffers API
>> devlink: Remove spaces around "=" in the logger print
>> devlink: Fix use-after-free at reporter destroy
>> net/mlx5e: Add RTNL lock to TX recover flow
>>
>> .../mellanox/mlx5/core/en/reporter_tx.c | 124 +---
>> include/net/devlink.h | 79 +--
>> include/trace/events/devlink.h | 2 +-
>> include/uapi/linux/devlink.h | 14 +-
>> net/core/devlink.c | 633 ++++++++----------
>> 5 files changed, 342 insertions(+), 510 deletions(-)
>>
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH net-next] net/mlx4: Mark expected switch fall-through
From: Tariq Toukan @ 2019-01-23 12:29 UTC (permalink / raw)
To: Gustavo A. R. Silva, Tariq Toukan, David S. Miller
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20190123080511.GA32528@embeddedor>
On 1/23/2019 10:05 AM, Gustavo A. R. Silva wrote:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> This patch fixes the following warning:
>
> drivers/net/ethernet/mellanox/mlx4/eq.c: In function ‘mlx4_eq_int’:
> drivers/net/ethernet/mellanox/mlx4/mlx4.h:219:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
> if (mlx4_debug_level) \
> ^
> drivers/net/ethernet/mellanox/mlx4/eq.c:558:4: note: in expansion of macro ‘mlx4_dbg’
> mlx4_dbg(dev, "%s: MLX4_EVENT_TYPE_SRQ_LIMIT. srq_no=0x%x, eq 0x%x\n",
> ^~~~~~~~
> drivers/net/ethernet/mellanox/mlx4/eq.c:561:3: note: here
> case MLX4_EVENT_TYPE_SRQ_CATAS_ERROR:
> ^~~~
>
> Warning level 3 was used: -Wimplicit-fallthrough=3
>
> This patch is part of the ongoing efforts to enabling
> -Wimplicit-fallthrough.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/eq.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/eq.c b/drivers/net/ethernet/mellanox/mlx4/eq.c
> index 4953c852c247..2f4201023836 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/eq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/eq.c
> @@ -558,6 +558,7 @@ static int mlx4_eq_int(struct mlx4_dev *dev, struct mlx4_eq *eq)
> mlx4_dbg(dev, "%s: MLX4_EVENT_TYPE_SRQ_LIMIT. srq_no=0x%x, eq 0x%x\n",
> __func__, be32_to_cpu(eqe->event.srq.srqn),
> eq->eqn);
> + /* fall through */
> case MLX4_EVENT_TYPE_SRQ_CATAS_ERROR:
> if (mlx4_is_master(dev)) {
> /* forward only to slave owning the SRQ */
>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Thanks for your patch.
^ permalink raw reply
* RE: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
From: Fabrizio Castro @ 2019-01-23 12:18 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring,
Mark Rutland, Michael Turquette, Stephen Boyd, David S. Miller,
Magnus Damm, Geert Uytterhoeven, Chris Paterson, Biju Das,
linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-clk@vger.kernel.org
In-Reply-To: <CAMuHMdVkFJ7Cy-+tNHsOcYaxDj82WjMynuFuPRXa4k=DH2fGCQ@mail.gmail.com>
Hello Geert,
Thank you for your feedback!
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 23 January 2019 12:08
> Subject: Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
>
> Hi Fabrizio,
>
> On Wed, Jan 23, 2019 at 1:01 PM Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> > > From: Simon Horman <horms@verge.net.au>
> > > Sent: 23 January 2019 11:38
> > > Subject: Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
> > >
> > > On Wed, Jan 23, 2019 at 10:51:23AM +0100, Simon Horman wrote:
> > > > On Fri, Jan 18, 2019 at 01:13:53PM +0100, Simon Horman wrote:
> > > > > On Thu, Jan 17, 2019 at 02:54:15PM +0000, Fabrizio Castro wrote:
> > > > > > According to the latest information, clkp2 is available on RZ/G2.
> > > > > > Modify CAN0 and CAN1 nodes accordingly.
> > > > > >
> > > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com>
>
> > > > Thanks again, applied for v5.1.
> > >
> > > Sorry, I was a little hasty there.
> > >
> > > This patch depends on the presence of R8A774A1_CLK_CANFD which
> > > (rightly) is added in a different patch in this series which is to
> > > go upstream via a different tree.
> > >
> > > I have dropped this patch for now. I think there are two solution to this
> > > problem.
> > >
> > > 1. Provide a version of this patch which uses a numeric index instead of
> > > R8A774A1_CLK_CANFD. And then, once R8A774A1_CLK_CANFD is present in an
> > > RC release provide a patch to switch to using R8A774A1_CLK_CANFD.
> > >
> > > 2. Defer this patch until R8A774A1_CLK_CANFD is present in an RC release.
> >
> > Yeah, my personal preference is solution 2.
>
> Note that solution 2 will probably defer the DT patch to v5.2.
Yeah, my understanding is that even if we chose solution 1 we would still need to be
waiting for v5.2 for the patch to switch to using R8A774A1_CLK_CANFD to appear in
a rc, therefore I think solution 2 is fine.
Thanks,
Fab
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* Re: [PATCH 1/3] treewide: Lift switch variables out of switches
From: Ard Biesheuvel @ 2019-01-23 12:12 UTC (permalink / raw)
To: Jann Horn
Cc: Greg KH, Kees Cook, kernel list, Laura Abbott, Alexander Popov,
xen-devel, dri-devel, intel-gfx, intel-wired-lan,
Network Development, linux-usb, linux-fsdevel, Linux-MM, dev,
Linux Kbuild mailing list, linux-security-module,
Kernel Hardening
In-Reply-To: <CAG48ez2vfXkr9dozJiGmze8k49VOXfs=K7M8bv0aQsDDpzrEFQ@mail.gmail.com>
On Wed, 23 Jan 2019 at 13:09, Jann Horn <jannh@google.com> wrote:
>
> On Wed, Jan 23, 2019 at 1:04 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > > Variables declared in a switch statement before any case statements
> > > cannot be initialized, so move all instances out of the switches.
> > > After this, future always-initialized stack variables will work
> > > and not throw warnings like this:
> > >
> > > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > > fs/fcntl.c:738:13: warning: statement will never be executed [-Wswitch-unreachable]
> > > siginfo_t si;
> > > ^~
> >
> > That's a pain, so this means we can't have any new variables in { }
> > scope except for at the top of a function?
>
> AFAICS this only applies to switch statements (because they jump to a
> case and don't execute stuff at the start of the block), not blocks
> after if/while/... .
>
I guess that means it may apply to other cases where you do a 'goto'
into the middle of a for() loop, for instance (at the first
iteration), which is also a valid pattern.
Is there any way to tag these assignments so the diagnostic disregards them?
^ permalink raw reply
* Re: [PATCH 1/3] treewide: Lift switch variables out of switches
From: Jann Horn @ 2019-01-23 12:09 UTC (permalink / raw)
To: Greg KH
Cc: Kees Cook, kernel list, Ard Biesheuvel, Laura Abbott,
Alexander Popov, xen-devel, dri-devel, intel-gfx, intel-wired-lan,
Network Development, linux-usb, linux-fsdevel, Linux-MM, dev,
linux-kbuild, linux-security-module, Kernel Hardening
In-Reply-To: <20190123115829.GA31385@kroah.com>
On Wed, Jan 23, 2019 at 1:04 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> > Variables declared in a switch statement before any case statements
> > cannot be initialized, so move all instances out of the switches.
> > After this, future always-initialized stack variables will work
> > and not throw warnings like this:
> >
> > fs/fcntl.c: In function ‘send_sigio_to_task’:
> > fs/fcntl.c:738:13: warning: statement will never be executed [-Wswitch-unreachable]
> > siginfo_t si;
> > ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?
AFAICS this only applies to switch statements (because they jump to a
case and don't execute stuff at the start of the block), not blocks
after if/while/... .
> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(
^ permalink raw reply
* Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
From: Geert Uytterhoeven @ 2019-01-23 12:07 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Simon Horman, Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring,
Mark Rutland, Michael Turquette, Stephen Boyd, David S. Miller,
Magnus Damm, Geert Uytterhoeven, Chris Paterson, Biju Das,
linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-clk@vger.kernel.org
In-Reply-To: <TYXPR01MB1775ABDCFD939F91F1053534C0990@TYXPR01MB1775.jpnprd01.prod.outlook.com>
Hi Fabrizio,
On Wed, Jan 23, 2019 at 1:01 PM Fabrizio Castro
<fabrizio.castro@bp.renesas.com> wrote:
> > From: Simon Horman <horms@verge.net.au>
> > Sent: 23 January 2019 11:38
> > Subject: Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
> >
> > On Wed, Jan 23, 2019 at 10:51:23AM +0100, Simon Horman wrote:
> > > On Fri, Jan 18, 2019 at 01:13:53PM +0100, Simon Horman wrote:
> > > > On Thu, Jan 17, 2019 at 02:54:15PM +0000, Fabrizio Castro wrote:
> > > > > According to the latest information, clkp2 is available on RZ/G2.
> > > > > Modify CAN0 and CAN1 nodes accordingly.
> > > > >
> > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com>
> > > Thanks again, applied for v5.1.
> >
> > Sorry, I was a little hasty there.
> >
> > This patch depends on the presence of R8A774A1_CLK_CANFD which
> > (rightly) is added in a different patch in this series which is to
> > go upstream via a different tree.
> >
> > I have dropped this patch for now. I think there are two solution to this
> > problem.
> >
> > 1. Provide a version of this patch which uses a numeric index instead of
> > R8A774A1_CLK_CANFD. And then, once R8A774A1_CLK_CANFD is present in an
> > RC release provide a patch to switch to using R8A774A1_CLK_CANFD.
> >
> > 2. Defer this patch until R8A774A1_CLK_CANFD is present in an RC release.
>
> Yeah, my personal preference is solution 2.
Note that solution 2 will probably defer the DT patch to v5.2.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* RE: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
From: Fabrizio Castro @ 2019-01-23 12:00 UTC (permalink / raw)
To: Simon Horman
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, David S. Miller, Magnus Damm,
Geert Uytterhoeven, Chris Paterson, Biju Das,
linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-clk@vger.kernel.org
In-Reply-To: <20190123113812.e3nojtxjxywmg4gn@verge.net.au>
Hello Simon,
Thank you for your feedback!
> From: Simon Horman <horms@verge.net.au>
> Sent: 23 January 2019 11:38
> Subject: Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
>
> On Wed, Jan 23, 2019 at 10:51:23AM +0100, Simon Horman wrote:
> > On Fri, Jan 18, 2019 at 01:13:53PM +0100, Simon Horman wrote:
> > > On Thu, Jan 17, 2019 at 02:54:15PM +0000, Fabrizio Castro wrote:
> > > > According to the latest information, clkp2 is available on RZ/G2.
> > > > Modify CAN0 and CAN1 nodes accordingly.
> > > >
> > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com>
> > >
> > > Thanks,
> > >
> > > This looks fine to me but I will wait to see if there are other reviews
> > > before applying.
> > >
> > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> >
> > Thanks again, applied for v5.1.
>
> Sorry, I was a little hasty there.
>
> This patch depends on the presence of R8A774A1_CLK_CANFD which
> (rightly) is added in a different patch in this series which is to
> go upstream via a different tree.
>
> I have dropped this patch for now. I think there are two solution to this
> problem.
>
> 1. Provide a version of this patch which uses a numeric index instead of
> R8A774A1_CLK_CANFD. And then, once R8A774A1_CLK_CANFD is present in an
> RC release provide a patch to switch to using R8A774A1_CLK_CANFD.
>
> 2. Defer this patch until R8A774A1_CLK_CANFD is present in an RC release.
>
Yeah, my personal preference is solution 2.
Thanks,
Fab
Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
^ permalink raw reply
* Re: [PATCH 1/3] treewide: Lift switch variables out of switches
From: Greg KH @ 2019-01-23 11:58 UTC (permalink / raw)
To: Kees Cook
Cc: linux-kernel, Ard Biesheuvel, Laura Abbott, Alexander Popov,
xen-devel, dri-devel, intel-gfx, intel-wired-lan, netdev,
linux-usb, linux-fsdevel, linux-mm, dev, linux-kbuild,
linux-security-module, kernel-hardening
In-Reply-To: <20190123110349.35882-2-keescook@chromium.org>
On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
>
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-Wswitch-unreachable]
> siginfo_t si;
> ^~
That's a pain, so this means we can't have any new variables in { }
scope except for at the top of a function?
That's going to be a hard thing to keep from happening over time, as
this is valid C :(
greg k-h
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: don't hardcode iptables/nc path in test_tcpnotify_user
From: Daniel Borkmann @ 2019-01-23 11:57 UTC (permalink / raw)
To: Stanislav Fomichev, netdev; +Cc: davem, ast
In-Reply-To: <20190117195612.157663-1-sdf@google.com>
On 01/17/2019 08:56 PM, Stanislav Fomichev wrote:
> system() is calling shell which should find the appropriate full path
> via $PATH. On some systems, full path to iptables and/or nc might be
> different that we one we have hardcoded.
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf: allow BPF programs access skb_shared_info->gso_segs field
From: Daniel Borkmann @ 2019-01-23 11:55 UTC (permalink / raw)
To: Martin Lau, Eric Dumazet
Cc: Alexei Starovoitov, netdev, Eric Dumazet, Eddie Hao,
Vlad Dumitrescu, Xiaotian Pei, Yuchung Cheng
In-Reply-To: <20190118184218.wckefzgzepbciurq@kafai-mbp.dhcp.thefacebook.com>
On 01/18/2019 07:42 PM, Martin Lau wrote:
> On Thu, Jan 17, 2019 at 03:31:57PM -0800, Eric Dumazet wrote:
>> This adds the ability to read gso_segs from a BPF program.
>>
>> v2: refined Eddie Hao patch to address Alexei feedback.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: Eddie Hao <eddieh@google.com>
>> Cc: Vlad Dumitrescu <vladum@google.com>
>> Cc: Xiaotian Pei <xiaotian@google.com>
>> Cc: Yuchung Cheng <ycheng@google.com>
>> ---
>> include/uapi/linux/bpf.h | 1 +
>> net/core/filter.c | 21 ++++++++++++
>> tools/include/uapi/linux/bpf.h | 1 +
>> tools/testing/selftests/bpf/test_verifier.c | 36 +++++++++++++++++++++
>> 4 files changed, 59 insertions(+)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index 91c43884f295f60a85268ddf0020bf8aa47f8329..2940a9854f6d8e493518ca894e0c9c630ae4ab7a 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -2540,6 +2540,7 @@ struct __sk_buff {
>> __bpf_md_ptr(struct bpf_flow_keys *, flow_keys);
>> __u64 tstamp;
>> __u32 wire_len;
>> + __u32 gso_segs;
>> };
>>
>> struct bpf_tunnel_key {
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index 2b3b436ef5457bf44c99780d6dec0b5f403f005c..a6ff5d9a04cf06926ee75cbc523456d12baf25ae 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -6700,6 +6700,27 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>> target_size));
>> break;
>>
>> + case offsetof(struct __sk_buff, gso_segs):
>> + /* si->dst_reg = skb_shinfo(SKB); */
>> +#ifdef NET_SKBUFF_DATA_USES_OFFSET
>> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff, head),
>> + si->dst_reg, si->src_reg,
>> + offsetof(struct sk_buff, head));
>> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff, end),
>> + BPF_REG_TMP, si->src_reg,
>> + offsetof(struct sk_buff, end));
> I am not sure BPF_REG_TMP can be used for non-classic BPF.
> The earlier insn could be using BPF_REG_TMP (which is BPF_REG_2) and
> R2 would become loss after this BPF_LDX_MEM.
Yes, this will indeed corrupt R2 register. BPF_REG_TMP can only be used for
reg mapping out of classic BPF.
> Daniel, can BPF_REG_AX be used here as a tmp?
BPF_REG_AX would work in this case, yes. Neither of the above insns are used
in blinding nor would they collide with current verifier rewrites.
>> + *insn++ = BPF_ALU64_REG(BPF_ADD, si->dst_reg, BPF_REG_TMP);
>> +#else
>> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff, end),
>> + si->dst_reg, si->src_reg,
>> + offsetof(struct sk_buff, end));
>> +#endif
>> + *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct skb_shared_info, gso_segs),
>> + si->dst_reg, si->dst_reg,
>> + bpf_target_off(struct skb_shared_info,
>> + gso_segs, 2,
>> + target_size));
>> + break;
>> case offsetof(struct __sk_buff, wire_len):
>> BUILD_BUG_ON(FIELD_SIZEOF(struct qdisc_skb_cb, pkt_len) != 4);
>>
>> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
>> index 91c43884f295f60a85268ddf0020bf8aa47f8329..2940a9854f6d8e493518ca894e0c9c630ae4ab7a 100644
>> --- a/tools/include/uapi/linux/bpf.h
>> +++ b/tools/include/uapi/linux/bpf.h
>> @@ -2540,6 +2540,7 @@ struct __sk_buff {
>> __bpf_md_ptr(struct bpf_flow_keys *, flow_keys);
>> __u64 tstamp;
>> __u32 wire_len;
>> + __u32 gso_segs;
>> };
>>
>> struct bpf_tunnel_key {
>> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
>> index 2fd90d4568926d13542783c870507d43a6d6bb64..2c46531044bdf9ec1e4fa47e2c94c9edb0ac3d08 100644
>> --- a/tools/testing/selftests/bpf/test_verifier.c
>> +++ b/tools/testing/selftests/bpf/test_verifier.c
>> @@ -5663,6 +5663,42 @@ static struct bpf_test tests[] = {
>> .result = ACCEPT,
>> .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
>> },
>> + {
>> + "read gso_segs from CGROUP_SKB",
>> + .insns = {
>> + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
>> + offsetof(struct __sk_buff, gso_segs)),
>> + BPF_MOV64_IMM(BPF_REG_0, 0),
>> + BPF_EXIT_INSN(),
>> + },
>> + .result = ACCEPT,
>> + .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
>> + },
>> + {
>> + "write gso_segs from CGROUP_SKB",
>> + .insns = {
>> + BPF_MOV64_IMM(BPF_REG_0, 0),
>> + BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
>> + offsetof(struct __sk_buff, gso_segs)),
>> + BPF_MOV64_IMM(BPF_REG_0, 0),
>> + BPF_EXIT_INSN(),
>> + },
>> + .result = REJECT,
>> + .result_unpriv = REJECT,
>> + .errstr = "invalid bpf_context access off=164 size=4",
>> + .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
>> + },
>> + {
>> + "read gso_segs from CLS",
>> + .insns = {
>> + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
>> + offsetof(struct __sk_buff, gso_segs)),
>> + BPF_MOV64_IMM(BPF_REG_0, 0),
>> + BPF_EXIT_INSN(),
>> + },
>> + .result = ACCEPT,
>> + .prog_type = BPF_PROG_TYPE_SCHED_CLS,
>> + },
>> {
>> "multiple registers share map_lookup_elem result",
>> .insns = {
>> --
>> 2.20.1.321.g9e740568ce-goog
>>
^ permalink raw reply
* Re: [PATCH net-next 0/7] Devlink health updates
From: Jiri Pirko @ 2019-01-23 11:44 UTC (permalink / raw)
To: Eran Ben Elisha
Cc: netdev, Jiri Pirko, David S. Miller, Saeed Mahameed,
Moshe Shemesh
In-Reply-To: <1548172644-30862-1-git-send-email-eranbe@mellanox.com>
Tue, Jan 22, 2019 at 04:57:17PM CET, eranbe@mellanox.com wrote:
>This patchset fixes some comments that were received for the devlink
>health series, mostly around the devlink health buffers API.
>
>It offers a new devlink<->driver API for passing health dump and diagnose info.
>As part of this patchset, the new API is developed and integrated into the
>devlink health and mlx5e TX reporter.
>Also, added some helpers together with the new API, which reduce the code
>required by the driver to fill dump and diagnose significantly.
>
>Eventually, it also deletes the old API.
>
>In addition, it includes some small fixes in the devlink and mlx5e TX reporter.
We are (re-)defining UAPI here. You need to present some examples
of devlink tool output, both in json and stdout.
Again, much more convenient to be done for the whole patchset, not this
"fixup-one" :/
>
>
>Eran Ben Elisha (7):
> devlink: Add devlink msg API
> net/mlx5e: Move driver to use devlink msg API
> devlink: move devlink health reporter to use devlink msg API
> devlink: Delete depracated health buffers API
> devlink: Remove spaces around "=" in the logger print
> devlink: Fix use-after-free at reporter destroy
> net/mlx5e: Add RTNL lock to TX recover flow
>
> .../mellanox/mlx5/core/en/reporter_tx.c | 124 +---
> include/net/devlink.h | 79 +--
> include/trace/events/devlink.h | 2 +-
> include/uapi/linux/devlink.h | 14 +-
> net/core/devlink.c | 633 ++++++++----------
> 5 files changed, 342 insertions(+), 510 deletions(-)
>
>--
>2.17.1
>
^ permalink raw reply
* [PATCH] netfilter: ipt_CLUSTERIP: fix warning unused variable cn
From: Anders Roxell @ 2019-01-23 11:48 UTC (permalink / raw)
To: pablo, kadlec, fw, davem, kuznet, yoshfuji
Cc: netfilter-devel, coreteam, netdev, linux-kernel, Anders Roxell
When CONFIG_PROC_FS isn't set the variable cn isn't used.
net/ipv4/netfilter/ipt_CLUSTERIP.c: In function ‘clusterip_net_exit’:
net/ipv4/netfilter/ipt_CLUSTERIP.c:849:24: warning: unused variable ‘cn’ [-Wunused-variable]
struct clusterip_net *cn = clusterip_pernet(net);
^~
Rework so the variable 'cn' is declared inside "#ifdef CONFIG_PROC_FS".
Fixes: b12f7bad5ad3 ("netfilter: ipt_CLUSTERIP: remove wrong WARN_ON_ONCE in netns exit routine")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
net/ipv4/netfilter/ipt_CLUSTERIP.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index b61977db9b7f..2a909e5f9ba0 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -846,9 +846,9 @@ static int clusterip_net_init(struct net *net)
static void clusterip_net_exit(struct net *net)
{
+#ifdef CONFIG_PROC_FS
struct clusterip_net *cn = clusterip_pernet(net);
-#ifdef CONFIG_PROC_FS
mutex_lock(&cn->mutex);
proc_remove(cn->procdir);
cn->procdir = NULL;
--
2.19.2
^ permalink raw reply related
* Re: [PATCH 2/3] arm64: dts: renesas: r8a774a1: Add clkp2 clock to CAN nodes
From: Simon Horman @ 2019-01-23 11:38 UTC (permalink / raw)
To: Fabrizio Castro
Cc: Wolfgang Grandegger, Marc Kleine-Budde, Rob Herring, Mark Rutland,
Michael Turquette, Stephen Boyd, David S. Miller, Magnus Damm,
Geert Uytterhoeven, Chris Paterson, Biju Das, linux-can, netdev,
devicetree, linux-renesas-soc, linux-clk
In-Reply-To: <20190123095122.l3engeuwx6qr5lzc@verge.net.au>
On Wed, Jan 23, 2019 at 10:51:23AM +0100, Simon Horman wrote:
> On Fri, Jan 18, 2019 at 01:13:53PM +0100, Simon Horman wrote:
> > On Thu, Jan 17, 2019 at 02:54:15PM +0000, Fabrizio Castro wrote:
> > > According to the latest information, clkp2 is available on RZ/G2.
> > > Modify CAN0 and CAN1 nodes accordingly.
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> > > Reviewed-by: Chris Paterson <Chris.Paterson2@renesas.com>
> >
> > Thanks,
> >
> > This looks fine to me but I will wait to see if there are other reviews
> > before applying.
> >
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
>
> Thanks again, applied for v5.1.
Sorry, I was a little hasty there.
This patch depends on the presence of R8A774A1_CLK_CANFD which
(rightly) is added in a different patch in this series which is to
go upstream via a different tree.
I have dropped this patch for now. I think there are two solution to this
problem.
1. Provide a version of this patch which uses a numeric index instead of
R8A774A1_CLK_CANFD. And then, once R8A774A1_CLK_CANFD is present in an
RC release provide a patch to switch to using R8A774A1_CLK_CANFD.
2. Defer this patch until R8A774A1_CLK_CANFD is present in an RC release.
^ permalink raw reply
* Re: [PATCH net-next v7 4/8] devlink: Add support for driverinit get value for devlink_port
From: Vasundhara Volam @ 2019-01-23 11:36 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Miller, michael.chan@broadcom.com, Jiri Pirko,
Jakub Kicinski, mkubecek, Netdev
In-Reply-To: <20190123110839.GD2191@nanopsycho>
On Wed, Jan 23, 2019 at 4:47 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Jan 18, 2019 at 08:09:41AM CET, vasundhara-v.volam@broadcom.com wrote:
> >Add support for "driverinit" configuration mode value for devlink_port
> >configuration parameters. Add devlink_port_param_driverinit_value_get()
> >function to help the driver get the value from devlink_port.
> >
> >Also, move the common code to __devlink_param_driverinit_value_get()
> >to be used by both device and port params.
> >
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >---
> > include/net/devlink.h | 8 ++++++
> > net/core/devlink.c | 67 ++++++++++++++++++++++++++++++++++++++-------------
> > 2 files changed, 58 insertions(+), 17 deletions(-)
> >
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index 98b8a66..09f3f43 100644
> >--- a/include/net/devlink.h
> >+++ b/include/net/devlink.h
> >@@ -838,6 +838,14 @@ static inline bool devlink_dpipe_table_counter_enabled(struct devlink *devlink,
> > {
> > }
> >
> >+static inline int
> >+devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
> >+ u32 param_id,
> >+ union devlink_param_value *init_val)
> >+{
> >+ return -EOPNOTSUPP;
> >+}
>
>
> You are missing function declaration in case IS_ENABLED(CONFIG_NET_DEVLINK
>
> Otherwise this looks fine.
Thank you for pointing. It is a mistake. I will fix it in next version.
>
>
> >+
> > static inline struct devlink_region *
> > devlink_region_create(struct devlink *devlink,
> > const char *region_name,
>
> [...]
>
^ permalink raw reply
* Re: [PATCH bpf-next v4] libbpf: Show supported ELF section names on when failed to guess a prog/attach type
From: Daniel Borkmann @ 2019-01-23 11:33 UTC (permalink / raw)
To: Taeung Song, Alexei Starovoitov
Cc: netdev, Quentin Monnet, Jakub Kicinski, Andrey Ignatov
In-Reply-To: <20190121130638.7554-1-treeze.taeung@gmail.com>
On 01/21/2019 02:06 PM, Taeung Song wrote:
> We need to let users check their wrong ELF section name
> with proper ELF section names when failed to get a prog/attach type from it.
> Because users can't realize libbpf guess prog/attach types from
> given ELF section names.
> For example, when a 'cgroup' section name of a BPF program is used,
> show available ELF section names(types).
>
> Before:
>
> $ bpftool prog load bpf-prog.o /sys/fs/bpf/prog1
> Error: failed to guess program type based on ELF section name cgroup
>
> After:
>
> libbpf: failed to guess program type based on ELF section name 'cgroup'
> libbpf: supported section(type) names are: socket kprobe/ kretprobe/ classifier action tracepoint/ raw_tracepoint/ xdp perf_event lwt_in lwt_out lwt_xmit lwt_seg6local cgroup_skb/ingress cgroup_skb/egress cgroup/skb cgroup/sock cgroup/post_bind4 cgroup/post_bind6 cgroup/dev sockops sk_skb/stream_parser sk_skb/stream_verdict sk_skb sk_msg lirc_mode2 flow_dissector cgroup/bind4 cgroup/bind6 cgroup/connect4 cgroup/connect6 cgroup/sendmsg4 cgroup/sendmsg6
>
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Andrey Ignatov <rdna@fb.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 bpf] bpf: sock recvbuff must be limited by rmem_max in bpf_setsockopt()
From: Daniel Borkmann @ 2019-01-23 11:33 UTC (permalink / raw)
To: Yafang Shao, kafai, brakmo, ast; +Cc: netdev, shaoyafang
In-Reply-To: <1548218239-31223-1-git-send-email-laoar.shao@gmail.com>
On 01/23/2019 05:37 AM, Yafang Shao wrote:
> When sock recvbuff is set by bpf_setsockopt(), the value must by limited
> by rmem_max.
> It is the same with sendbuff.
>
> Fixes: 8c4b4c7e9ff0 ("bpf: Add setsockopt helper function to bpf")
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Lawrence Brakmo <brakmo@fb.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Applied, thanks!
^ permalink raw reply
* [RFC] Connection Tracking Offload netdev RFC v1.0, part 1/2: command line + implementation
From: Guy Shattah @ 2019-01-23 11:29 UTC (permalink / raw)
To: Guy Shattah, Marcelo Leitner, Aaron Conole, John Hurley,
Simon Horman, Justin Pettit, Gregory Rose, Eelco Chaudron,
Flavio Leitner, Florian Westphal, Jiri Pirko, Rashid Khan,
Sushil Kulkarni, Andy Gospodarek, Roi Dayan, Yossi Kuperman,
Or Gerlitz, Rony Efraim, davem@davemloft.net
Cc: netdev@vger.kernel.org
--------------------------------------------------------------------------
Connection Tracking Offload netdev RFC v1.0 Part 1/2 - TC with Connection Tracking - command line + implementation
--------------------------------------------------------------------------
OVS recirculation ID is to be translated to TC chain, as described in https://www.netdevconf.org/2.2/papers/efraim-extendtctoct-talk.pdf
------------------------------------------------------------------------------------
CT Matches:
------------------------------------------------------------------------------------
The ct match acts on ct_state bits or ct variables which were modified as a result from a connection tracking action.
Some of the information can be extacted directly from struct nf_conn and the rest of the information could be taken by using
conntrack_mt...() [/net/netfilter/xt_conntrack.c]
1. ct_state - a new variable
The ct_state match is used to test result of connection tracking.
The bits are set or unset according to the results of the connection tracking module.
The following Match able ct_state items are supported:
* ±trk - Tracked - Been through the connection tracker
* ±new – a new connection
* ±est - Established connection
* ±dnat - Packet’s source address/port was mangled by NAT.
* ±snat - Packet’s destination address/port was mangled by NAT.
* ±inv - Invalid packet
* ±rel – Related to an existing connection
* ±rpl - Reply: Connection must be established
Example #1: "tc filter add dev eth5 protocol ip parent ffff: chain 100 flower ct_state
+trk +est -dnat action mirred egress redirect dev eth6"
2. three additional integer variables.
These variables, which can be set from within the ct_action, are introduced:
ct_zone - to commit the connection in (u16) Logically separate connection tracking
table/Multi-tenancy
ct_mark - Attach metadata to particular connections (u32)
ct_label – similar to mark (128 bits)
Example #2: "tc filter add dev eth5 protocol ip parent ffff: chain 100 flower
ct_state +trk +est ct_label 10 ct_zone 9 action drop "
Complete list of the flags and their description can be found at:
http://www.openvswitch.org/support/dist-docs/ovs-fields.7.txt
------------------------------------------------------------------------------------
CT actions:
------------------------------------------------------------------------------------
The ct_action action sends packet to ConnTrack ( nf_conntrack_in() method) and then updates ct_state bits according to the result from connection tracking.
[1] CT Action has the following possible arguments:
1. commit: Commit the connection to the connection tracking module which will be
stored beyond the lifetime of packet in the pipeline.
2. force: The force flag may be used in addition to commit flag to effectively terminate
the existing connection and start a new one in the current direction.
3. chain = K (chain is similar to ct 'table' in OVS syntax) : Clone packet to send to
connection tracker. When the connection tracker is finished, resume processing
in chain K for that packet. The original packet continues right after the ct(...) action.
4. Set variable: ct_zone, ct_mark, ct_label (see description above)
Example #3:: "tc .... action ct ct_zone 7 commit ct_label 0x0123456789ABCDEF0000111222"
5. NAT: Specifies the address and port translation for the connection being tracked.
Example #4:
"ct_action nat src 10.0.0.1 10.1.1.0" rewrite source ip+port from the list.
Example #5: "tc ... action ct nat src 10.0.0.1 10.1.1.0" rewrite source ip+port
from the list.
Example #6: "tc ... action ct nat auto" rewrite packets automatically from
saved kernel NAT list
-----
[2] CT action also has 3 new parameters
Three new variables which can be set from within the ct_action.
1. ct_zone: 16 bit
2. ct_mark: 32bit
3. ct_label: 128bit
Example #7: tc..... action ct ct_zone 7 commit ct_label x0123456789ABCDEF0000111222
------
[3] NAT action.
Supporting
(1) specific NAT for source
(2) specific NAT for destination
(3) automatic.
TC, when instructed when and how to do so, will do a NAT translation by using the kernel NAT module.
Resulting in a modified skb returning to the following TC chain for further processing
Example #8: "tc filter add dev eth5 protocol ip parent ffff: chain 100 flower action
ct commit nat src 10.0.0.0 10.0.0.255"
Commit a new connection to Conntrack and replace NAT the source ip address
Example #9: "tc filter add dev eth5 protocol ip parent ffff: chain 100 flower action ct
commit nat auto"
Commit a new connection to Conntrack and replace NAT the source ip address
Additional examples can be found at OVS NAT patch comments:
https://lwn.net/Articles/674868/
[3] match on newly added variables ( ct_zone, ct_mark, ct_label) Example #10: "tc ct_zone 3 ct_mark 0x333 ...."
----------------------------------------
Connection-Tracking action:
----------------------------
TC data path calls Connection Tracking nf_conntrack_in() method with skb which returns connTrack result inside skb->_nfct which is of type struct nf_conn.
Connection-Tracking Match:
----------------------------
connection tracking match can be done using conntrack_mt...() [/net/netfilter/xt_conntrack.c] calls which can be used to match connection tracking information.
Connection-Tracking NAT:
-------------------------------
NAT implementation details are the same as in OVS. As described in:
* https://lwn.net/Articles/674868/
* https://lwn.net/Articles/671459/
* http://www.openvswitch.org/support/ovscon2014/17/1030-conntrack_nat.pdf
Required OVS changes
-------------------------------
1. OVS has to be modified to send Connection-tracking datapath messages to TC 2. OVS datapath has to be enhanced to support enforcement of window-validation
^ 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