* [PATCH net-next 5/5] ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv4 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv4/af_inet.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8421e2f5bbb3..9a2f17d0c5f5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -797,6 +797,8 @@ int inet_send_prepare(struct sock *sk)
}
EXPORT_SYMBOL_GPL(inet_send_prepare);
+INDIRECT_CALLABLE_DECLARE(int udp_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -804,7 +806,8 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udp_sendmsg,
+ sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -822,6 +825,8 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
}
EXPORT_SYMBOL(inet_sendpage);
+INDIRECT_CALLABLE_DECLARE(int udp_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -832,8 +837,9 @@ int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udp_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 4/5] ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This avoids an indirect call per syscall for common ipv6 transports
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/af_inet6.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 4628681eca88..d5e98ee9fc79 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,8 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+INDIRECT_CALLABLE_DECLARE(int udpv6_sendmsg(struct sock *, struct msghdr *,
+ size_t));
int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
{
struct sock *sk = sock->sk;
@@ -571,9 +573,12 @@ int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
- return sk->sk_prot->sendmsg(sk, msg, size);
+ return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udpv6_sendmsg,
+ sk, msg, size);
}
+INDIRECT_CALLABLE_DECLARE(int udpv6_recvmsg(struct sock *, struct msghdr *,
+ size_t, int, int, int *));
int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
int flags)
{
@@ -584,8 +589,9 @@ int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
if (likely(!(flags & MSG_ERRQUEUE)))
sock_rps_record_flow(sk);
- err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
- flags & ~MSG_DONTWAIT, &addr_len);
+ err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udpv6_recvmsg,
+ sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
if (err >= 0)
msg->msg_namelen = addr_len;
return err;
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/5] inet: factor out inet_send_prepare()
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
The same code is replicated verbatim in multiple places, and the next
patches will introduce an additional user for it. Factor out a
helper and use it where appropriate. No functional change intended.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/inet_common.h | 1 +
net/ipv4/af_inet.c | 21 +++++++++++++--------
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 975901a95c0f..ae2ba897675c 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -25,6 +25,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr,
int addr_len, int flags);
int inet_accept(struct socket *sock, struct socket *newsock, int flags,
bool kern);
+int inet_send_prepare(struct sock *sk);
int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 52bdb881a506..8421e2f5bbb3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -784,10 +784,8 @@ int inet_getname(struct socket *sock, struct sockaddr *uaddr,
}
EXPORT_SYMBOL(inet_getname);
-int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+int inet_send_prepare(struct sock *sk)
{
- struct sock *sk = sock->sk;
-
sock_rps_record_flow(sk);
/* We may need to bind the socket. */
@@ -795,6 +793,17 @@ int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
inet_autobind(sk))
return -EAGAIN;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(inet_send_prepare);
+
+int inet_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
return sk->sk_prot->sendmsg(sk, msg, size);
}
EXPORT_SYMBOL(inet_sendmsg);
@@ -804,11 +813,7 @@ ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
{
struct sock *sk = sock->sk;
- sock_rps_record_flow(sk);
-
- /* We may need to bind the socket. */
- if (!inet_sk(sk)->inet_num && !sk->sk_prot->no_autobind &&
- inet_autobind(sk))
+ if (unlikely(inet_send_prepare(sk)))
return -EAGAIN;
if (sk->sk_prot->sendpage)
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 3/5] net: adjust socket level ICW to cope with ipv6 variant of {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
After the previous patch we have ipv{6,4} variants for {recv,send}msg,
we should use the generic _INET ICW variant to call into the proper
build-in.
This also allows dropping the now unused and rather ugly _INET4 ICW macro
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/socket.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/socket.c b/net/socket.c
index 963df5dbdd54..f5e0e460012b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -103,13 +103,6 @@
#include <net/busy_poll.h>
#include <linux/errqueue.h>
-/* proto_ops for ipv4 and ipv6 use the same {recv,send}msg function */
-#if IS_ENABLED(CONFIG_INET)
-#define INDIRECT_CALL_INET4(f, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
-#else
-#define INDIRECT_CALL_INET4(f, f1, ...) f(__VA_ARGS__)
-#endif
-
#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sysctl_net_busy_read __read_mostly;
unsigned int sysctl_net_busy_poll __read_mostly;
@@ -643,8 +636,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_sendmsg(struct socket *, struct msghdr *,
size_t));
static inline int sock_sendmsg_nosec(struct socket *sock, struct msghdr *msg)
{
- int ret = INDIRECT_CALL_INET4(sock->ops->sendmsg, inet_sendmsg, sock,
- msg, msg_data_left(msg));
+ int ret = INDIRECT_CALL_INET(sock->ops->sendmsg, inet6_sendmsg,
+ inet_sendmsg, sock, msg,
+ msg_data_left(msg));
BUG_ON(ret == -EIOCBQUEUED);
return ret;
}
@@ -874,8 +868,9 @@ INDIRECT_CALLABLE_DECLARE(int inet_recvmsg(struct socket *, struct msghdr *,
static inline int sock_recvmsg_nosec(struct socket *sock, struct msghdr *msg,
int flags)
{
- return INDIRECT_CALL_INET4(sock->ops->recvmsg, inet_recvmsg, sock, msg,
- msg_data_left(msg), flags);
+ return INDIRECT_CALL_INET(sock->ops->recvmsg, inet6_recvmsg,
+ inet_recvmsg, sock, msg, msg_data_left(msg),
+ flags);
}
/**
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 2/5] ipv6: provide and use ipv6 specific version for {recv,send}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
In-Reply-To: <cover.1561999976.git.pabeni@redhat.com>
This will simplify indirect call wrapper invocation in the following
patch.
No functional change intended, any - out-of-tree - IPv6 user of
inet_{recv,send}msg can keep using the existing functions.
SCTP code still uses the existing version even for ipv6: as this series
will not add ICW for SCTP, moving to the new helper would not give
any benefit.
The only other in-kernel user of inet_{recv,send}msg is
pvcalls_conn_back_read(), but psvcalls explicitly creates only IPv4 socket,
so no need to update that code path, too.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/ipv6.h | 3 +++
net/ipv6/af_inet6.c | 35 +++++++++++++++++++++++++++++++----
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index b41f6a0fa903..aecc28dff8f8 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1089,6 +1089,9 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
int inet6_hash_connect(struct inet_timewait_death_row *death_row,
struct sock *sk);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size);
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags);
/*
* reassembly.c
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 7382a927d1eb..4628681eca88 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -564,6 +564,33 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
}
EXPORT_SYMBOL(inet6_ioctl);
+int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
+{
+ struct sock *sk = sock->sk;
+
+ if (unlikely(inet_send_prepare(sk)))
+ return -EAGAIN;
+
+ return sk->sk_prot->sendmsg(sk, msg, size);
+}
+
+int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
+ int flags)
+{
+ struct sock *sk = sock->sk;
+ int addr_len = 0;
+ int err;
+
+ if (likely(!(flags & MSG_ERRQUEUE)))
+ sock_rps_record_flow(sk);
+
+ err = sk->sk_prot->recvmsg(sk, msg, size, flags & MSG_DONTWAIT,
+ flags & ~MSG_DONTWAIT, &addr_len);
+ if (err >= 0)
+ msg->msg_namelen = addr_len;
+ return err;
+}
+
const struct proto_ops inet6_stream_ops = {
.family = PF_INET6,
.owner = THIS_MODULE,
@@ -580,8 +607,8 @@ const struct proto_ops inet6_stream_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
#ifdef CONFIG_MMU
.mmap = tcp_mmap,
#endif
@@ -614,8 +641,8 @@ const struct proto_ops inet6_dgram_ops = {
.shutdown = inet_shutdown, /* ok */
.setsockopt = sock_common_setsockopt, /* ok */
.getsockopt = sock_common_getsockopt, /* ok */
- .sendmsg = inet_sendmsg, /* ok */
- .recvmsg = inet_recvmsg, /* ok */
+ .sendmsg = inet6_sendmsg, /* retpoline's sake */
+ .recvmsg = inet6_recvmsg, /* retpoline's sake */
.mmap = sock_no_mmap,
.sendpage = sock_no_sendpage,
.set_peek_off = sk_set_peek_off,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/5] net: use ICW for sk_proto->{send,recv}msg
From: Paolo Abeni @ 2019-07-01 17:09 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
This series extends ICW usage to one of the few remaining spots in fast-path
still hitting per packet retpoline overhead, namely the sk_proto->{send,recv}msg
calls.
The first 3 patches in this series refactor the existing code so that applying
the ICW macros is straight-forward: we demux inet_{recv,send}msg in ipv4 and
ipv6 variants so that each of them can easily select the appropriate TCP or UDP
direct call. While at it, a new helper is created to avoid excessive code
duplication, and the current ICWs for inet_{recv,send}msg are adjusted
accordingly.
The last 2 patches really introduce the new ICW use-case, respectively for the
ipv6 and the ipv4 code path.
This gives up to 5% performance improvement under UDP flood, and smaller but
measurable gains for TCP RR workloads.
Paolo Abeni (5):
inet: factor out inet_send_prepare()
ipv6: provide and use ipv6 specific version for {recv,send}msg
net: adjust socket level ICW to cope with ipv6 variant of
{recv,send}msg
ipv6: use indirect call wrappers for {tcp,udpv6}_{recv,send}msg()
ipv4: use indirect call wrappers for {tcp,udp}_{recv,send}msg()
include/net/inet_common.h | 1 +
include/net/ipv6.h | 3 +++
net/ipv4/af_inet.c | 33 ++++++++++++++++++++-----------
net/ipv6/af_inet6.c | 41 +++++++++++++++++++++++++++++++++++----
net/socket.c | 17 ++++++----------
5 files changed, 69 insertions(+), 26 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Yonghong Song @ 2019-07-01 17:09 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-5-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 165 +++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 7 ++
> tools/lib/bpf/libbpf.map | 2 +
> 3 files changed, 174 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 98c155ec3bfa..2f79e9563db9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4019,6 +4019,171 @@ struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> return (struct bpf_link *)link;
> }
>
> +static int parse_value_from_file(const char *file, const char *fmt)
Here, the value from the file must be positive int values to avoid
confusion between valid value vs. error code.
Could you add a comment to state this fact for the current
uprobe/kprobe/tracepoint support?
> +{
> + char buf[STRERR_BUFSIZE];
> + int err, ret;
> + FILE *f;
> +
> + f = fopen(file, "r");
> + if (!f) {
> + err = -errno;
> + pr_debug("failed to open '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
fclose(f) is not needed. fopen has failed.
> + return err;
> + }
> + err = fscanf(f, fmt, &ret);
> + if (err != 1) {
> + err = err == EOF ? -EIO : -errno;
> + pr_debug("failed to parse '%s': %s\n", file,
> + libbpf_strerror_r(err, buf, sizeof(buf)));
> + fclose(f);
> + return err;
> + }
> + fclose(f);
> + return ret;
> +}
> +
> +static int determine_kprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_uprobe_perf_type(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/type";
> +
> + return parse_value_from_file(file, "%d\n");
> +}
> +
> +static int determine_kprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/kprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int determine_uprobe_retprobe_bit(void)
> +{
> + const char *file = "/sys/bus/event_source/devices/uprobe/format/retprobe";
> +
> + return parse_value_from_file(file, "config:%d\n");
> +}
> +
> +static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
> + uint64_t offset, int pid)
> +{
> + struct perf_event_attr attr = {};
> + char errmsg[STRERR_BUFSIZE];
> + int type, pfd, err;
> +
> + type = uprobe ? determine_uprobe_perf_type()
> + : determine_kprobe_perf_type();
> + if (type < 0) {
> + pr_warning("failed to determine %s perf type: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(type, errmsg, sizeof(errmsg)));
> + return type;
> + }
> + if (retprobe) {
> + int bit = uprobe ? determine_uprobe_retprobe_bit()
> + : determine_kprobe_retprobe_bit();
> +
> + if (bit < 0) {
> + pr_warning("failed to determine %s retprobe bit: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(bit, errmsg,
> + sizeof(errmsg)));
> + return bit;
> + }
> + attr.config |= 1 << bit;
> + }
> + attr.size = sizeof(attr);
> + attr.type = type;
> + attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> + attr.config2 = offset; /* kprobe_addr or probe_offset */
> +
> + /* pid filter is meaningful only for uprobes */
> + pfd = syscall(__NR_perf_event_open, &attr,
> + pid < 0 ? -1 : pid /* pid */,
> + pid == -1 ? 0 : -1 /* cpu */,
> + -1 /* group_fd */, PERF_FLAG_FD_CLOEXEC);
> + if (pfd < 0) {
> + err = -errno;
> + pr_warning("%s perf_event_open() failed: %s\n",
> + uprobe ? "uprobe" : "kprobe",
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return err;
> + }
> + return pfd;
> +}
> +
> +struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
> + bool retprobe,
> + const char *func_name)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
> + 0 /* offset */, -1 /* pid */);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "kretprobe" : "kprobe", func_name,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> +struct bpf_link *bpf_program__attach_uprobe(struct bpf_program *prog,
> + bool retprobe, pid_t pid,
> + const char *binary_path,
> + size_t func_offset)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int pfd, err;
> +
> + pfd = perf_event_open_probe(true /* uprobe */, retprobe,
> + binary_path, func_offset, pid);
> + if (pfd < 0) {
> + pr_warning("program '%s': failed to create %s '%s:0x%zx' perf event: %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(pfd);
> + }
> + link = bpf_program__attach_perf_event(prog, pfd);
> + if (IS_ERR(link)) {
> + close(pfd);
> + err = PTR_ERR(link);
> + pr_warning("program '%s': failed to attach to %s '%s:0x%zx': %s\n",
> + bpf_program__title(prog, false),
> + retprobe ? "uretprobe" : "uprobe",
> + binary_path, func_offset,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return link;
> + }
> + return link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 1bf66c4a9330..bd767cc11967 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -171,6 +171,13 @@ LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> LIBBPF_API struct bpf_link *
> bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
> + const char *func_name);
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
> + pid_t pid, const char *binary_path,
> + size_t func_offset);
>
> struct bpf_insn;
>
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 756f5aa802e9..57a40fb60718 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,7 +169,9 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_kprobe;
> bpf_program__attach_perf_event;
> + bpf_program__attach_uprobe;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH net-next 0/7] net/rds: RDMA fixes
From: santosh.shilimkar @ 2019-07-01 17:06 UTC (permalink / raw)
To: Gerd Rausch, netdev; +Cc: David Miller
In-Reply-To: <f1f5ca90-a98a-4c7a-c918-ef26f02f6ee7@oracle.com>
On 7/1/19 9:39 AM, Gerd Rausch wrote:
> A number of net/rds fixes necessary to make "rds_rdma.ko"
> pass some basic Oracle internal tests.
>
> Gerd Rausch (7):
> net/rds: Give fr_state a chance to transition to FRMR_IS_FREE
> net/rds: Get rid of "wait_clean_list_grace" and add locking
> net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after
> posting IB_WR_LOCAL_INV
> net/rds: Fix NULL/ERR_PTR inconsistency
> net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been
> successful
> net/rds: Keep track of and wait for FRWR segments in use upon shutdown
> net/rds: Initialize ic->i_fastreg_wrs upon allocation
>
Will apply these on top of earlier few fixes after going through them.
Thanks for posting them out.
Regards,
Santosh
^ permalink raw reply
* Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Yonghong Song @ 2019-07-01 17:03 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-4-andriin@fb.com>
On 6/28/19 8:49 PM, Andrii Nakryiko wrote:
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 61 ++++++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 3 ++
> tools/lib/bpf/libbpf.map | 1 +
> 3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
> #include <linux/limits.h>
> #include <linux/perf_event.h>
> #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
> return err;
> }
>
> +struct bpf_link_fd {
> + struct bpf_link link; /* has to be at the top of struct */
> + int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> + struct bpf_link_fd *l = (void *)link;
> + int err;
> +
> + if (l->fd < 0)
> + return 0;
> +
> + err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> + if (err)
> + err = -errno;
> +
> + close(l->fd);
> + return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> + int pfd)
> +{
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link_fd *link;
> + int prog_fd, err;
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warning("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
should we check validity of pfd here?
If pfd < 0, we just return ERR_PTR(-EINVAL)?
This way, in bpf_link__destroy_perf_event(), we do not need to check
l->fd < 0 since it will be always nonnegative.
> +
> + link = malloc(sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->link.destroy = &bpf_link__destroy_perf_event;
> + link->fd = pfd;
> +
> + if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> + err = -errno;
> + free(link);
> + pr_warning("program '%s': failed to enable pfd %d: %s\n",
> + bpf_program__title(prog, false), pfd,
> + libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> + return ERR_PTR(err);
> + }
> + return (struct bpf_link *)link;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
> LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
> global:
> bpf_link__destroy;
> bpf_object__load_xattr;
> + bpf_program__attach_perf_event;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
>
^ permalink raw reply
* Re: [PATCH v2 0/3] vsock/virtio: several fixes in the .probe() and .remove()
From: Stefano Garzarella @ 2019-07-01 17:03 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: netdev, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Stefan Hajnoczi, David S. Miller
In-Reply-To: <20190701151113.GE11900@stefanha-x1.localdomain>
On Mon, Jul 01, 2019 at 04:11:13PM +0100, Stefan Hajnoczi wrote:
> On Fri, Jun 28, 2019 at 02:36:56PM +0200, Stefano Garzarella wrote:
> > During the review of "[PATCH] vsock/virtio: Initialize core virtio vsock
> > before registering the driver", Stefan pointed out some possible issues
> > in the .probe() and .remove() callbacks of the virtio-vsock driver.
> >
> > This series tries to solve these issues:
> > - Patch 1 adds RCU critical sections to avoid use-after-free of
> > 'the_virtio_vsock' pointer.
> > - Patch 2 stops workers before to call vdev->config->reset(vdev) to
> > be sure that no one is accessing the device.
> > - Patch 3 moves the works flush at the end of the .remove() to avoid
> > use-after-free of 'vsock' object.
> >
> > v2:
> > - Patch 1: use RCU to protect 'the_virtio_vsock' pointer
> > - Patch 2: no changes
> > - Patch 3: flush works only at the end of .remove()
> > - Removed patch 4 because virtqueue_detach_unused_buf() returns all the buffers
> > allocated.
> >
> > v1: https://patchwork.kernel.org/cover/10964733/
>
> This looks good to me.
Thanks for the review!
>
> Did you run any stress tests? For example an SMP guest constantly
> connecting and sending packets together with a script that
> hotplug/unplugs vhost-vsock-pci from the host side.
Yes, I started an SMP guest (-smp 4 -monitor tcp:127.0.0.1:1234,server,nowait)
and I run these scripts to stress the .probe()/.remove() path:
- guest
while true; do
cat /dev/urandom | nc-vsock -l 4321 > /dev/null &
cat /dev/urandom | nc-vsock -l 5321 > /dev/null &
cat /dev/urandom | nc-vsock -l 6321 > /dev/null &
cat /dev/urandom | nc-vsock -l 7321 > /dev/null &
wait
done
- host
while true; do
cat /dev/urandom | nc-vsock 3 4321 > /dev/null &
cat /dev/urandom | nc-vsock 3 5321 > /dev/null &
cat /dev/urandom | nc-vsock 3 6321 > /dev/null &
cat /dev/urandom | nc-vsock 3 7321 > /dev/null &
sleep 2
echo "device_del v1" | nc 127.0.0.1 1234
sleep 1
echo "device_add vhost-vsock-pci,id=v1,guest-cid=3" | nc 127.0.0.1 1234
sleep 1
done
Do you think is enough or is better to have a test more accurate?
Thanks,
Stefano
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Nikolay Aleksandrov @ 2019-07-01 17:03 UTC (permalink / raw)
To: Martin Weinelt, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <21ab085f-0f7f-88bc-b661-af74dd9eeea2@linuxlounge.net>
Hi Martin,
On 01/07/2019 19:53, Martin Weinelt wrote:
> Hi Nik,
>
> more info below.
>
> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>> Hello,
>>>
>>> we've recently been experiencing memory leaks on our Linux-based
>>> routers,
>>> at least as far back as v4.19.16.
>>>
>>> After rebuilding with KASAN it found a use-after-free in
>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>>
>>> Please find the KASAN report below, I'm anot sure what else to provide
>>> so
>>> feel free to ask.
>>>
>>> Best,
>>> Martin
>>>
>>>
>>
>> Hi Martin,
>> I'll look into this, are there any specific steps to reproduce it?
>>
>> Thanks,
>> Nik
>>>
> Each server is a KVM Guest and has 18 bridges with the same master/slave
> relationships:
>
> bridge -> batman-adv -> {l2 tunnel, virtio device}
>
> Linus Lüssing from the batman-adv asked me to apply this patch to help
> debugging.
>
> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
> report, not sure if the additional information at the end is a result of
> the added patch though.
>
> Best,
> Martin
>
I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
I've had these on my "to fix" list for some time now, will prepare, test the fixes and
send them for review. In a few minutes I'll send a test patch for you.
That being said, I thought you said you've been experiencing memory leaks, but below
reports are for out-of-bounds accesses, could you please clarify if you were
speaking about these or is there another issue as well ?
If you're experiencing memory leaks, are you sure they're related to the bridge ?
You could try kmemleak for those.
Thank you,
Nik
> From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
> Date: Sat, 29 Jun 2019 20:24:23 +0200
> Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
> call traces
>
> ---
> net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 63 insertions(+), 7 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index de22c8fbbb15..224a43318955 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct net_bridge_port *port,
> const struct in6_addr *group,
> __u16 vid, const unsigned char *src);
> +
> +static noinline void br_ip6_multicast_leave_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +static noinline void br_ip6_multicast_leave_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
> +{
> + return ipv6_addr_is_ll_all_nodes(addr);
> +}
> #endif
>
> static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
> @@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
> {
> struct br_ip br_group;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_addgroup(group))
> return 0;
>
> memset(&br_group, 0, sizeof(br_group));
> @@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
>
> return br_multicast_add_group(br, port, &br_group, src);
> }
> +
> +static noinline int br_ip6_multicast_add_group_mld2report(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> +
> +static noinline int br_ip6_multicast_add_group_ipv6rcv(
> + struct net_bridge *br,
> + struct net_bridge_port *port,
> + const struct in6_addr *group,
> + __u16 vid,
> + const unsigned char *src)
> +{
> + return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> #endif
>
> static void br_multicast_router_expired(struct timer_list *t)
> @@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
> if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
> grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
> ntohs(*nsrcs) == 0) {
> - br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
> + br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
> vid, src);
> } else {
> - err = br_ip6_multicast_add_group(br, port,
> + err = br_ip6_multicast_add_group_mld2report(br, port,
> &grec->grec_mca, vid,
> src);
> if (err)
> @@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
> struct br_ip br_group;
> struct bridge_mcast_own_query *own_query;
>
> - if (ipv6_addr_is_ll_all_nodes(group))
> + if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
> return;
>
> own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
> @@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> err = ipv6_mc_check_mld(skb);
>
> if (err == -ENOMSG) {
> - if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
> + if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>
> if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
> @@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> case ICMPV6_MGM_REPORT:
> src = eth_hdr(skb)->h_source;
> BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> - err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
> + err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
> src);
> break;
> case ICMPV6_MLD2_REPORT:
> @@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
> break;
> case ICMPV6_MGM_REDUCTION:
> src = eth_hdr(skb)->h_source;
> - br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
> + br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
> break;
> }
>
>
^ permalink raw reply
* [PATCH net v2] ipv4: don't set IPv6 only flags to IPv4 addresses
From: Matteo Croce @ 2019-07-01 17:01 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI
Avoid the situation where an IPV6 only flag is applied to an IPv4 address:
# ip addr add 192.0.2.1/24 dev dummy0 nodad home mngtmpaddr noprefixroute
# ip -4 addr show dev dummy0
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
inet 192.0.2.1/24 scope global noprefixroute dummy0
valid_lft forever preferred_lft forever
Or worse, by sending a malicious netlink command:
# ip -4 addr show dev dummy0
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
inet 192.0.2.1/24 scope global nodad optimistic dadfailed home tentative mngtmpaddr noprefixroute stable-privacy dummy0
valid_lft forever preferred_lft forever
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
net/ipv4/devinet.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c6bd0f7a020a..c5ebfa199794 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -62,6 +62,11 @@
#include <net/net_namespace.h>
#include <net/addrconf.h>
+#define IPV6ONLY_FLAGS \
+ (IFA_F_NODAD | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | \
+ IFA_F_HOMEADDRESS | IFA_F_TENTATIVE | \
+ IFA_F_MANAGETEMPADDR | IFA_F_STABLE_PRIVACY)
+
static struct ipv4_devconf ipv4_devconf = {
.data = {
[IPV4_DEVCONF_ACCEPT_REDIRECTS - 1] = 1,
@@ -468,6 +473,9 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
ifa->ifa_flags &= ~IFA_F_SECONDARY;
last_primary = &in_dev->ifa_list;
+ /* Don't set IPv6 only flags to IPv4 addresses */
+ ifa->ifa_flags &= ~IPV6ONLY_FLAGS;
+
for (ifap = &in_dev->ifa_list; (ifa1 = *ifap) != NULL;
ifap = &ifa1->ifa_next) {
if (!(ifa1->ifa_flags & IFA_F_SECONDARY) &&
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v4 bpf-next 2/9] libbpf: introduce concept of bpf_link
From: Yonghong Song @ 2019-07-01 17:01 UTC (permalink / raw)
To: Andrii Nakryiko, andrii.nakryiko@gmail.com, bpf@vger.kernel.org,
netdev@vger.kernel.org, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, sdf@fomichev.me, Song Liu
In-Reply-To: <20190629034906.1209916-3-andriin@fb.com>
On 6/28/19 8:48 PM, Andrii Nakryiko wrote:
> bpf_link is and abstraction of an association of a BPF program and one
"is and" => "is an".
> of many possible BPF attachment points (hooks). This allows to have
> uniform interface for detaching BPF programs regardless of the nature of
> link and how it was created. Details of creation and setting up of
> a specific bpf_link is handled by corresponding attachment methods
> (bpf_program__attach_xxx) added in subsequent commits. Once successfully
> created, bpf_link has to be eventually destroyed with
> bpf_link__destroy(), at which point BPF program is disassociated from
> a hook and all the relevant resources are freed.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> tools/lib/bpf/libbpf.h | 4 ++++
> tools/lib/bpf/libbpf.map | 3 ++-
> 3 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6e6ebef11ba3..455795e6f8af 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -3941,6 +3941,23 @@ int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> return 0;
> }
>
> +struct bpf_link {
> + int (*destroy)(struct bpf_link *link);
> +};
> +
> +int bpf_link__destroy(struct bpf_link *link)
> +{
> + int err;
> +
> + if (!link)
> + return 0;
> +
> + err = link->destroy(link);
> + free(link);
> +
> + return err;
> +}
> +
> enum bpf_perf_event_ret
> bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
> void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index d639f47e3110..5082a5ebb0c2 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -165,6 +165,10 @@ LIBBPF_API int bpf_program__pin(struct bpf_program *prog, const char *path);
> LIBBPF_API int bpf_program__unpin(struct bpf_program *prog, const char *path);
> LIBBPF_API void bpf_program__unload(struct bpf_program *prog);
>
> +struct bpf_link;
> +
> +LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
> +
> struct bpf_insn;
>
> /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 2c6d835620d2..3cde850fc8da 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -167,10 +167,11 @@ LIBBPF_0.0.3 {
>
> LIBBPF_0.0.4 {
> global:
> + bpf_link__destroy;
> + bpf_object__load_xattr;
> btf_dump__dump_type;
> btf_dump__free;
> btf_dump__new;
> btf__parse_elf;
> - bpf_object__load_xattr;
> libbpf_num_possible_cpus;
> } LIBBPF_0.0.3;
>
^ permalink raw reply
* [PATCH net-next] tipc: remove ub->ubsock checks
From: Xin Long @ 2019-07-01 16:57 UTC (permalink / raw)
To: network dev; +Cc: Jon Maloy, Ying Xue, tipc-discussion, davem
Both tipc_udp_enable and tipc_udp_disable are called under rtnl_lock,
ub->ubsock could never be NULL in tipc_udp_disable and cleanup_bearer,
so remove the check.
Also remove the one in tipc_udp_enable by adding "free" label.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/udp_media.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 62b85db..287df687 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -759,7 +759,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
err = dst_cache_init(&ub->rcast.dst_cache, GFP_ATOMIC);
if (err)
- goto err;
+ goto free;
/**
* The bcast media address port is used for all peers and the ip
@@ -771,13 +771,14 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
else
err = tipc_udp_rcast_add(b, &remote);
if (err)
- goto err;
+ goto free;
return 0;
-err:
+
+free:
dst_cache_destroy(&ub->rcast.dst_cache);
- if (ub->ubsock)
- udp_tunnel_sock_release(ub->ubsock);
+ udp_tunnel_sock_release(ub->ubsock);
+err:
kfree(ub);
return err;
}
@@ -795,8 +796,7 @@ static void cleanup_bearer(struct work_struct *work)
}
dst_cache_destroy(&ub->rcast.dst_cache);
- if (ub->ubsock)
- udp_tunnel_sock_release(ub->ubsock);
+ udp_tunnel_sock_release(ub->ubsock);
synchronize_net();
kfree(ub);
}
@@ -811,8 +811,7 @@ static void tipc_udp_disable(struct tipc_bearer *b)
pr_err("UDP bearer instance not found\n");
return;
}
- if (ub->ubsock)
- sock_set_flag(ub->ubsock->sk, SOCK_DEAD);
+ sock_set_flag(ub->ubsock->sk, SOCK_DEAD);
RCU_INIT_POINTER(ub->bearer, NULL);
/* sock_release need to be done outside of rtnl lock */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 3/3] selftests/bpf: add verifier tests for wide stores
From: Andrii Nakryiko @ 2019-07-01 16:55 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song
In-Reply-To: <20190701163103.237550-4-sdf@google.com>
On Mon, Jul 1, 2019 at 9:54 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Make sure that wide stores are allowed at proper (aligned) addresses.
> Note that user_ip6 is naturally aligned on 8-byte boundary, so
> correct addresses are user_ip6[0] and user_ip6[2]. msg_src_ip6 is,
> however, aligned on a 4-byte bondary, so only msg_src_ip6[1]
> can be wide-stored.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> tools/testing/selftests/bpf/test_verifier.c | 17 +++++++--
> .../selftests/bpf/verifier/wide_store.c | 36 +++++++++++++++++++
> 2 files changed, 50 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/verifier/wide_store.c
>
<snip>
^ permalink raw reply
* [PATCH net-next] tipc: use rcu dereference functions properly
From: Xin Long @ 2019-07-01 16:54 UTC (permalink / raw)
To: network dev; +Cc: Jon Maloy, Ying Xue, tipc-discussion, davem
For these places are protected by rcu_read_lock, we change from
rcu_dereference_rtnl to rcu_dereference, as there is no need to
check if rtnl lock is held.
For these places are protected by rtnl_lock, we change from
rcu_dereference_rtnl to rtnl_dereference/rcu_dereference_protected,
as no extra memory barriers are needed under rtnl_lock() which also
protects tn->bearer_list[] and dev->tipc_ptr/b->media_ptr updating.
rcu_dereference_rtnl will be only used in the places where it could
be under rcu_read_lock or rtnl_lock.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/tipc/bearer.c | 14 +++++++-------
net/tipc/udp_media.c | 8 ++++----
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 2bed658..a809c0e 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -62,7 +62,7 @@ static struct tipc_bearer *bearer_get(struct net *net, int bearer_id)
{
struct tipc_net *tn = tipc_net(net);
- return rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ return rcu_dereference(tn->bearer_list[bearer_id]);
}
static void bearer_disable(struct net *net, struct tipc_bearer *b);
@@ -210,7 +210,7 @@ void tipc_bearer_add_dest(struct net *net, u32 bearer_id, u32 dest)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ b = rcu_dereference(tn->bearer_list[bearer_id]);
if (b)
tipc_disc_add_dest(b->disc);
rcu_read_unlock();
@@ -222,7 +222,7 @@ void tipc_bearer_remove_dest(struct net *net, u32 bearer_id, u32 dest)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tn->bearer_list[bearer_id]);
+ b = rcu_dereference(tn->bearer_list[bearer_id]);
if (b)
tipc_disc_remove_dest(b->disc);
rcu_read_unlock();
@@ -444,7 +444,7 @@ int tipc_l2_send_msg(struct net *net, struct sk_buff *skb,
struct net_device *dev;
int delta;
- dev = (struct net_device *)rcu_dereference_rtnl(b->media_ptr);
+ dev = (struct net_device *)rcu_dereference(b->media_ptr);
if (!dev)
return 0;
@@ -481,7 +481,7 @@ int tipc_bearer_mtu(struct net *net, u32 bearer_id)
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(tipc_net(net)->bearer_list[bearer_id]);
+ b = rcu_dereference(tipc_net(net)->bearer_list[bearer_id]);
if (b)
mtu = b->mtu;
rcu_read_unlock();
@@ -574,8 +574,8 @@ static int tipc_l2_rcv_msg(struct sk_buff *skb, struct net_device *dev,
struct tipc_bearer *b;
rcu_read_lock();
- b = rcu_dereference_rtnl(dev->tipc_ptr) ?:
- rcu_dereference_rtnl(orig_dev->tipc_ptr);
+ b = rcu_dereference(dev->tipc_ptr) ?:
+ rcu_dereference(orig_dev->tipc_ptr);
if (likely(b && test_bit(0, &b->up) &&
(skb->pkt_type <= PACKET_MULTICAST))) {
skb_mark_not_on_list(skb);
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index b8962df..62b85db 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -231,7 +231,7 @@ static int tipc_udp_send_msg(struct net *net, struct sk_buff *skb,
}
skb_set_inner_protocol(skb, htons(ETH_P_TIPC));
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rcu_dereference(b->media_ptr);
if (!ub) {
err = -ENODEV;
goto out;
@@ -490,7 +490,7 @@ int tipc_udp_nl_dump_remoteip(struct sk_buff *skb, struct netlink_callback *cb)
}
}
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub) {
rtnl_unlock();
return -EINVAL;
@@ -532,7 +532,7 @@ int tipc_udp_nl_add_bearer_data(struct tipc_nl_msg *msg, struct tipc_bearer *b)
struct udp_bearer *ub;
struct nlattr *nest;
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub)
return -ENODEV;
@@ -806,7 +806,7 @@ static void tipc_udp_disable(struct tipc_bearer *b)
{
struct udp_bearer *ub;
- ub = rcu_dereference_rtnl(b->media_ptr);
+ ub = rtnl_dereference(b->media_ptr);
if (!ub) {
pr_err("UDP bearer instance not found\n");
return;
--
2.1.0
^ permalink raw reply related
* Re: [PATCH bpf-next v2 1/3] bpf: allow wide (u64) aligned stores for some fields of bpf_sock_addr
From: Andrii Nakryiko @ 2019-07-01 16:54 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Yonghong Song,
kernel test robot
In-Reply-To: <20190701163103.237550-2-sdf@google.com>
On Mon, Jul 1, 2019 at 9:51 AM Stanislav Fomichev <sdf@google.com> wrote:
>
> Since commit cd17d7770578 ("bpf/tools: sync bpf.h") clang decided
> that it can do a single u64 store into user_ip6[2] instead of two
> separate u32 ones:
>
> # 17: (18) r2 = 0x100000000000000
> # ; ctx->user_ip6[2] = bpf_htonl(DST_REWRITE_IP6_2);
> # 19: (7b) *(u64 *)(r1 +16) = r2
> # invalid bpf_context access off=16 size=8
>
> From the compiler point of view it does look like a correct thing
> to do, so let's support it on the kernel side.
>
> Credit to Andrii Nakryiko for a proper implementation of
> bpf_ctx_wide_store_ok.
>
> Cc: Andrii Nakryiko <andriin@fb.com>
> Cc: Yonghong Song <yhs@fb.com>
> Fixes: cd17d7770578 ("bpf/tools: sync bpf.h")
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> include/linux/filter.h | 6 ++++++
> include/uapi/linux/bpf.h | 4 ++--
> net/core/filter.c | 22 ++++++++++++++--------
> 3 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 340f7d648974..3901007e36f1 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -746,6 +746,12 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
> return size <= size_default && (size & (size - 1)) == 0;
> }
>
> +#define bpf_ctx_wide_store_ok(off, size, type, field) \
> + (size == sizeof(__u64) && \
> + off >= offsetof(type, field) && \
> + off + sizeof(__u64) <= offsetofend(type, field) && \
> + off % sizeof(__u64) == 0)
> +
> #define bpf_classic_proglen(fprog) (fprog->len * sizeof(fprog->filter[0]))
>
> static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index a396b516a2b2..586867fe6102 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -3237,7 +3237,7 @@ struct bpf_sock_addr {
> __u32 user_ip4; /* Allows 1,2,4-byte read and 4-byte write.
> * Stored in network byte order.
> */
> - __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> + __u32 user_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
typo: an -> and
> * Stored in network byte order.
> */
> __u32 user_port; /* Allows 4-byte read and write.
> @@ -3249,7 +3249,7 @@ struct bpf_sock_addr {
> __u32 msg_src_ip4; /* Allows 1,2,4-byte read an 4-byte write.
same
> * Stored in network byte order.
> */
> - __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4-byte write.
> + __u32 msg_src_ip6[4]; /* Allows 1,2,4-byte read an 4,8-byte write.
the power of copy/paste! :)
> * Stored in network byte order.
> */
> __bpf_md_ptr(struct bpf_sock *, sk);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index dc8534be12fc..5d33f2146dab 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -6849,6 +6849,16 @@ static bool sock_addr_is_valid_access(int off, int size,
> if (!bpf_ctx_narrow_access_ok(off, size, size_default))
> return false;
> } else {
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + user_ip6))
> + return true;
> +
> + if (bpf_ctx_wide_store_ok(off, size,
> + struct bpf_sock_addr,
> + msg_src_ip6))
> + return true;
> +
> if (size != size_default)
> return false;
> }
> @@ -7689,9 +7699,6 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> /* SOCK_ADDR_STORE_NESTED_FIELD_OFF() has semantic similar to
> * SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF() but for store operation.
> *
> - * It doesn't support SIZE argument though since narrow stores are not
> - * supported for now.
> - *
> * In addition it uses Temporary Field TF (member of struct S) as the 3rd
> * "register" since two registers available in convert_ctx_access are not
> * enough: we can't override neither SRC, since it contains value to store, nor
> @@ -7699,7 +7706,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> * instructions. But we need a temporary place to save pointer to nested
> * structure whose field we want to store to.
> */
> -#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, TF) \
> +#define SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, OFF, TF) \
> do { \
> int tmp_reg = BPF_REG_9; \
> if (si->src_reg == tmp_reg || si->dst_reg == tmp_reg) \
> @@ -7710,8 +7717,7 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> offsetof(S, TF)); \
> *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(S, F), tmp_reg, \
> si->dst_reg, offsetof(S, F)); \
> - *insn++ = BPF_STX_MEM( \
> - BPF_FIELD_SIZEOF(NS, NF), tmp_reg, si->src_reg, \
> + *insn++ = BPF_STX_MEM(SIZE, tmp_reg, si->src_reg, \
> bpf_target_off(NS, NF, FIELD_SIZEOF(NS, NF), \
> target_size) \
> + OFF); \
> @@ -7723,8 +7729,8 @@ static u32 xdp_convert_ctx_access(enum bpf_access_type type,
> TF) \
> do { \
> if (type == BPF_WRITE) { \
> - SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, OFF, \
> - TF); \
> + SOCK_ADDR_STORE_NESTED_FIELD_OFF(S, NS, F, NF, SIZE, \
> + OFF, TF); \
> } else { \
> SOCK_ADDR_LOAD_NESTED_FIELD_SIZE_OFF( \
> S, NS, F, NF, SIZE, OFF); \
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* Re: Use-after-free in br_multicast_rcv
From: Martin Weinelt @ 2019-07-01 16:53 UTC (permalink / raw)
To: nikolay, bridge, Roopa Prabhu; +Cc: netdev
In-Reply-To: <E0170D52-C181-4F0F-B5F8-F1801C2A8F5A@cumulusnetworks.com>
Hi Nik,
more info below.
On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>> Hello,
>>
>> we've recently been experiencing memory leaks on our Linux-based
>> routers,
>> at least as far back as v4.19.16.
>>
>> After rebuilding with KASAN it found a use-after-free in
>> br_multicast_rcv which I could reproduce on v5.2.0-rc6.
>>
>> Please find the KASAN report below, I'm anot sure what else to provide
>> so
>> feel free to ask.
>>
>> Best,
>> Martin
>>
>>
>
> Hi Martin,
> I'll look into this, are there any specific steps to reproduce it?
>
> Thanks,
> Nik
>
Each server is a KVM Guest and has 18 bridges with the same master/slave
relationships:
bridge -> batman-adv -> {l2 tunnel, virtio device}
Linus Lüssing from the batman-adv asked me to apply this patch to help
debugging.
v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN
report, not sure if the additional information at the end is a result of
the added patch though.
Best,
Martin
From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
Date: Sat, 29 Jun 2019 20:24:23 +0200
Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
call traces
---
net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
1 file changed, 63 insertions(+), 7 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..224a43318955 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct net_bridge_port *port,
const struct in6_addr *group,
__u16 vid, const unsigned char *src);
+
+static noinline void br_ip6_multicast_leave_group_mld2report(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+static noinline void br_ip6_multicast_leave_group_ipv6rcv(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+
+static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
+{
+ return ipv6_addr_is_ll_all_nodes(addr);
+}
#endif
static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
@@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
{
struct br_ip br_group;
- if (ipv6_addr_is_ll_all_nodes(group))
+ if (ipv6_addr_is_ll_all_nodes_addgroup(group))
return 0;
memset(&br_group, 0, sizeof(br_group));
@@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
return br_multicast_add_group(br, port, &br_group, src);
}
+
+static noinline int br_ip6_multicast_add_group_mld2report(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
+
+static noinline int br_ip6_multicast_add_group_ipv6rcv(
+ struct net_bridge *br,
+ struct net_bridge_port *port,
+ const struct in6_addr *group,
+ __u16 vid,
+ const unsigned char *src)
+{
+ return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
#endif
static void br_multicast_router_expired(struct timer_list *t)
@@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
ntohs(*nsrcs) == 0) {
- br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
+ br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
vid, src);
} else {
- err = br_ip6_multicast_add_group(br, port,
+ err = br_ip6_multicast_add_group_mld2report(br, port,
&grec->grec_mca, vid,
src);
if (err)
@@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
struct br_ip br_group;
struct bridge_mcast_own_query *own_query;
- if (ipv6_addr_is_ll_all_nodes(group))
+ if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
return;
own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
@@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
err = ipv6_mc_check_mld(skb);
if (err == -ENOMSG) {
- if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
+ if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
@@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
case ICMPV6_MGM_REPORT:
src = eth_hdr(skb)->h_source;
BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
- err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
+ err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
src);
break;
case ICMPV6_MLD2_REPORT:
@@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
break;
case ICMPV6_MGM_REDUCTION:
src = eth_hdr(skb)->h_source;
- br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
+ br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
break;
}
--
2.20.1
[ 409.353973] ==================================================================
[ 409.356970] BUG: KASAN: out-of-bounds in br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.359360] Read of size 2 at addr ffff888046e980b8 by task swapper/2/0
[ 409.361668]
[ 409.362215] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G OE 5.2.0-rc6+ #1
[ 409.364503] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 409.367006] Call Trace:
[ 409.367699] <IRQ>
[ 409.368283] dump_stack+0x71/0xab
[ 409.369175] print_address_description+0x6a/0x280
[ 409.370350] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.371752] __kasan_report+0x152/0x1aa
[ 409.372703] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.373950] ? br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.375308] kasan_report+0xe/0x20
[ 409.376149] br_multicast_rcv+0x36e9/0x4080 [bridge]
[ 409.377460] ? br_multicast_disable_port+0x150/0x150 [bridge]
[ 409.379079] ? kvm_clock_get_cycles+0xd/0x10
[ 409.380291] ? __kasan_kmalloc.constprop.6+0xa6/0xf0
[ 409.381693] ? netif_receive_skb_internal+0x84/0x1a0
[ 409.383075] ? __netif_receive_skb+0x1b0/0x1b0
[ 409.384355] ? br_fdb_update+0x10e/0x6e0 [bridge]
[ 409.385700] ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[ 409.387283] br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[ 409.388807] ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
[ 409.390200] ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
[ 409.391685] br_handle_frame+0x731/0xd90 [bridge]
[ 409.393017] ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
[ 409.394591] ? __update_load_avg_se+0x592/0xa70
[ 409.395862] ? __update_load_avg_se+0x592/0xa70
[ 409.397065] __netif_receive_skb_core+0xced/0x2d70
[ 409.398208] ? napi_complete_done+0x10/0x360
[ 409.399249] ? virtqueue_get_buf_ctx+0x271/0x1130 [virtio_ring]
[ 409.400632] ? do_xdp_generic+0x20/0x20
[ 409.401565] ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
[ 409.404824] ? virtnet_poll+0x94d/0xc78 [virtio_net]
[ 409.407552] ? receive_buf+0x5120/0x5120 [virtio_net]
[ 409.410255] ? __netif_receive_skb_one_core+0x97/0x1d0
[ 409.413023] ? account_entity_enqueue+0x340/0x4c0
[ 409.415639] __netif_receive_skb_one_core+0x97/0x1d0
[ 409.418346] ? __netif_receive_skb_core+0x2d70/0x2d70
[ 409.421143] ? _raw_write_trylock+0x100/0x100
[ 409.423775] process_backlog+0x19c/0x650
[ 409.426383] net_rx_action+0x71e/0xbc0
[ 409.428960] ? napi_complete_done+0x360/0x360
[ 409.431693] ? handle_irq_event_percpu+0xeb/0x140
[ 409.434705] ? _raw_spin_lock+0x7a/0xd0
[ 409.439881] ? _raw_write_trylock+0x100/0x100
[ 409.444967] __do_softirq+0x1db/0x5f9
[ 409.447999] irq_exit+0x123/0x150
[ 409.451116] do_IRQ+0x71/0x160
[ 409.453709] common_interrupt+0xf/0xf
[ 409.456384] </IRQ>
[ 409.458708] RIP: 0010:native_safe_halt+0xe/0x10
[ 409.461587] Code: 09 f9 fe 48 8b 04 24 e9 12 ff ff ff e9 07 00 00 00 0f 00 2d d4 60 52 00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d c4 60 52 00 fb f4 <c3> 90 66 66 66 66 90 41 56 41 55 41 54 55 53 e8 7e 05 ba fe 65 44
[ 409.469813] RSP: 0018:ffff88805056fd98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd3
[ 409.474002] RAX: ffffffff97f1ad00 RBX: 0000000000000002 RCX: ffffffff96abbbd6
[ 409.477775] RDX: 1ffff1100a0a63d0 RSI: 0000000000000004 RDI: ffff8880510b3f38
[ 409.481830] RBP: ffffed100a0a63d0 R08: ffffed100a2167e8 R09: ffffed100a2167e7
[ 409.486087] R10: ffff8880510b3f3b R11: ffffed100a2167e8 R12: ffffffff98e60480
[ 409.490757] R13: 0000000000000002 R14: 0000000000000000 R15: ffff888050531e80
[ 409.495061] ? ldsem_down_write+0x590/0x590
[ 409.499258] ? rcu_idle_enter+0x106/0x150
[ 409.502755] ? tsc_verify_tsc_adjust+0x96/0x2a0
[ 409.506287] default_idle+0x1f/0x280
[ 409.509105] do_idle+0x2d8/0x3e0
[ 409.511741] ? arch_cpu_idle_exit+0x40/0x40
[ 409.514588] cpu_startup_entry+0x19/0x20
[ 409.517355] start_secondary+0x316/0x3f0
[ 409.520051] ? set_cpu_sibling_map+0x19c0/0x19c0
[ 409.522871] secondary_startup_64+0xa4/0xb0
[ 409.525603]
[ 409.527708] Allocated by task 0:
[ 409.530201] save_stack+0x19/0x70
[ 409.532757] __kasan_kmalloc.constprop.6+0xa6/0xf0
[ 409.535660] __kmalloc_node_track_caller+0xe3/0x2c0
[ 409.538641] __kmalloc_reserve.isra.45+0x2e/0xb0
[ 409.541426] pskb_expand_head+0x118/0xcb0
[ 409.544107] __pskb_pull_tail+0xb9/0x1980
[ 409.546749] validate_xmit_skb+0x556/0xab0
[ 409.549387] __dev_queue_xmit+0x11bc/0x2b70
[ 409.552145] ip_finish_output2+0xa7e/0x18c0
[ 409.554817] ip_output+0x1a1/0x2b0
[ 409.557335] ip_forward+0xe39/0x1a20
[ 409.559875] ip_rcv+0xb3/0x180
[ 409.562271] __netif_receive_skb_one_core+0x145/0x1d0
[ 409.565068] netif_receive_skb_internal+0x84/0x1a0
[ 409.567841] napi_gro_flush+0x1d7/0x380
[ 409.570492] napi_complete_done+0x172/0x360
[ 409.573147] virtqueue_napi_complete+0x2a/0x70 [virtio_net]
[ 409.576154] virtnet_poll+0x94d/0xc78 [virtio_net]
[ 409.578963] net_rx_action+0x71e/0xbc0
[ 409.581482] __do_softirq+0x1db/0x5f9
[ 409.584023]
[ 409.586052] Freed by task 12968:
[ 409.588489] save_stack+0x19/0x70
[ 409.591065] __kasan_slab_free+0x122/0x180
[ 409.593775] kfree+0xa6/0x1f0
[ 409.596228] consume_skb+0x69/0x160
[ 409.598803] tun_do_read+0x53c/0x1b30 [tun]
[ 409.601563] tun_chr_read_iter+0x14a/0x2d0 [tun]
[ 409.604477] new_sync_read+0x3e3/0x6b0
[ 409.606956] vfs_read+0xe9/0x2d0
[ 409.609283] ksys_read+0xe8/0x1c0
[ 409.611566] do_syscall_64+0xa0/0x2c0
[ 409.613927] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 409.616671]
[ 409.618573] The buggy address belongs to the object at ffff888046e98000
[ 409.618573] which belongs to the cache kmalloc-2k of size 2048
[ 409.624699] The buggy address is located 184 bytes inside of
[ 409.624699] 2048-byte region [ffff888046e98000, ffff888046e98800)
[ 409.630602] The buggy address belongs to the page:
[ 409.633445] page:ffffea00011ba600 refcount:1 mapcount:0 mapping:ffff888050c02a80 index:0x0 compound_mapcount: 0
[ 409.638069] flags: 0xffffc000010200(slab|head)
[ 409.641048] raw: 00ffffc000010200 dead000000000100 dead000000000200 ffff888050c02a80
[ 409.644876] raw: 0000000000000000 00000000000f000f 00000001ffffffff 0000000000000000
[ 409.648486] page dumped because: kasan: bad access detected
[ 409.651569]
[ 409.653718] Memory state around the buggy address:
[ 409.656635] ffff888046e97f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 409.660222] ffff888046e98000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.663775] >ffff888046e98080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.667332] ^
[ 409.670247] ffff888046e98100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.673815] ffff888046e98180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 409.677391] ==================================================================
[ 409.681002] Disabling lock debugging due to kernel taint
^ permalink raw reply related
* Re: [PATCH 3/4] net: dsa: vsc73xx: add support for parallel mode
From: Florian Fainelli @ 2019-07-01 16:45 UTC (permalink / raw)
To: Pawel Dembicki
Cc: linus.walleij, Andrew Lunn, Vivien Didelot, David S. Miller,
Rob Herring, Mark Rutland, netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-3-paweldembicki@gmail.com>
On 7/1/19 8:27 AM, Pawel Dembicki wrote:
> This patch add platform part of vsc73xx driver.
> It allows to use chip connected by PI interface.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
[snip]
> + struct vsc73xx_platform *vsc_platform = vsc->priv;
> + u32 offset;
> +
> + if (!vsc73xx_is_addr_valid(block, subblock))
> + return -EINVAL;
> +
> + offset = vsc73xx_make_addr(block, subblock, reg);
> +
> + mutex_lock(&vsc->lock);
> + iowrite32be(val, vsc_platform->base_addr + offset);
> + mutex_unlock(&vsc->lock);
Similar question from Andrew, why is the locking done in the platform
layer, should not that be done in the core I/O operation instead?
> +
> + return 0;
> +}
> +
> +static int vsc73xx_platform_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct vsc73xx_platform *vsc_platform;
> + struct resource *res = NULL;
> + int ret;
> +
> + vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL);
> + if (!vsc_platform)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, vsc_platform);
> + vsc_platform->pdev = pdev;
> + vsc_platform->vsc.dev = dev;
> + vsc_platform->vsc.priv = vsc_platform;
> + vsc_platform->vsc.ops = &vsc73xx_platform_ops;
> +
> + /* obtain I/O memory space */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(&pdev->dev, "cannot obtain I/O memory space\n");
> + ret = -ENXIO;
> + return ret;
> + }
> +
> + vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res);
devm_ioremap_resource takes care of checking that the resource pointer
is valid, no need to do that here.
> + if (!vsc_platform->base_addr) {
if (IS_ERR(vsc_platform->base_addr))
--
Florian
^ permalink raw reply
* Re: [PATCH 1/4] net: dsa: Change DT bindings for Vitesse VSC73xx switches
From: Florian Fainelli @ 2019-07-01 16:44 UTC (permalink / raw)
To: Pawel Dembicki
Cc: linus.walleij, Andrew Lunn, Vivien Didelot, David S. Miller,
Rob Herring, Mark Rutland, netdev, devicetree, linux-kernel
In-Reply-To: <20190701152723.624-1-paweldembicki@gmail.com>
On 7/1/19 8:27 AM, Pawel Dembicki wrote:
> This commit document changes after split vsc73xx driver into core and
> spi part. The change of DT bindings is required for support the same
> vsc73xx chip, which need PI bus to communicate with CPU. It also
> introduce how to use vsc73xx platform driver.
>
> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> ---
> .../bindings/net/dsa/vitesse,vsc73xx.txt | 74 ++++++++++++++++---
> 1 file changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> index ed4710c40641..c6a4cd85891c 100644
> --- a/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/vitesse,vsc73xx.txt
> @@ -2,8 +2,8 @@ Vitesse VSC73xx Switches
> ========================
>
> This defines device tree bindings for the Vitesse VSC73xx switch chips.
> -The Vitesse company has been acquired by Microsemi and Microsemi in turn
> -acquired by Microchip but retains this vendor branding.
> +The Vitesse company has been acquired by Microsemi and Microsemi has
> +been acquired Microchip but retains this vendor branding.
>
> The currently supported switch chips are:
> Vitesse VSC7385 SparX-G5 5+1-port Integrated Gigabit Ethernet Switch
> @@ -11,16 +11,26 @@ Vitesse VSC7388 SparX-G8 8-port Integrated Gigabit Ethernet Switch
> Vitesse VSC7395 SparX-G5e 5+1-port Integrated Gigabit Ethernet Switch
> Vitesse VSC7398 SparX-G8e 8-port Integrated Gigabit Ethernet Switch
>
> -The device tree node is an SPI device so it must reside inside a SPI bus
> -device tree node, see spi/spi-bus.txt
> +This switch could have two different management interface.
> +
> +If SPI interface is used, the device tree node is an SPI device so it must
> +reside inside a SPI bus device tree node, see spi/spi-bus.txt
> +
> +If Platform driver is used, the device tree node is an platform device so it
> +must reside inside a platform bus device tree node.
That should not be required because the device remains the same and how
it connects to the host is entirely described by the Device Tree topology.
Take b53 for instance which supports MDIO and SPI by default, and
optionally memory mapped and SRAB (indirect memory map) accesses, they
all have the same compatible strings. Whether the switches will appear
as spi_device, platform_device, or something else is entirely based on
how the Device Tree is laid out.
--
Florian
^ permalink raw reply
* [PATCH net-next 7/7] net/rds: Initialize ic->i_fastreg_wrs upon allocation
From: Gerd Rausch @ 2019-07-01 16:40 UTC (permalink / raw)
To: Santosh Shilimkar, netdev; +Cc: David Miller
Otherwise, if an IB connection is torn down before "rds_ib_setup_qp"
is called, the value of "ic->i_fastreg_wrs" is still at zero
(as it wasn't initialized by "rds_ib_setup_qp").
Consequently "rds_ib_conn_path_shutdown" will spin forever,
waiting for it to go back to "RDS_IB_DEFAULT_FR_WR",
which of course will never happen as there are no
outstanding work requests.
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
net/rds/ib_cm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 1b6fd6c8b12b..4de0214da63c 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -527,7 +527,6 @@ static int rds_ib_setup_qp(struct rds_connection *conn)
attr.qp_type = IB_QPT_RC;
attr.send_cq = ic->i_send_cq;
attr.recv_cq = ic->i_recv_cq;
- atomic_set(&ic->i_fastreg_wrs, RDS_IB_DEFAULT_FR_WR);
/*
* XXX this can fail if max_*_wr is too large? Are we supposed
@@ -1139,6 +1138,7 @@ int rds_ib_conn_alloc(struct rds_connection *conn, gfp_t gfp)
spin_lock_init(&ic->i_ack_lock);
#endif
atomic_set(&ic->i_signaled_sends, 0);
+ atomic_set(&ic->i_fastreg_wrs, RDS_IB_DEFAULT_FR_WR);
/*
* rds_ib_conn_shutdown() waits for these to be emptied so they
--
2.18.0
^ permalink raw reply related
* [PATCH net-next 6/7] net/rds: Keep track of and wait for FRWR segments in use upon shutdown
From: Gerd Rausch @ 2019-07-01 16:40 UTC (permalink / raw)
To: Santosh Shilimkar, netdev; +Cc: David Miller
Since "rds_ib_free_frmr" and "rds_ib_free_frmr_list" simply put
the FRMR memory segments on the "drop_list" or "free_list",
and it is the job of "rds_ib_flush_mr_pool" to reap those entries
by ultimately issuing a "IB_WR_LOCAL_INV" work-request,
we need to trigger and then wait for all those memory segments
attached to a particular connection to be fully released before
we can move on to release the QP, CQ, etc.
So we make "rds_ib_conn_path_shutdown" wait for one more
atomic_t called "i_fastreg_inuse_count" that keeps track of how
many FRWR memory segments are out there marked "FRMR_IS_INUSE"
(and also wake_up rds_ib_ring_empty_wait, as they go away).
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
net/rds/ib.h | 1 +
net/rds/ib_cm.c | 7 +++++++
net/rds/ib_frmr.c | 45 ++++++++++++++++++++++++++++++++++++++-------
3 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/net/rds/ib.h b/net/rds/ib.h
index 66c03c7665b2..303c6ee8bdb7 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -156,6 +156,7 @@ struct rds_ib_connection {
/* To control the number of wrs from fastreg */
atomic_t i_fastreg_wrs;
+ atomic_t i_fastreg_inuse_count;
/* interrupt handling */
struct tasklet_struct i_send_tasklet;
diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 8891822eba4f..1b6fd6c8b12b 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -40,6 +40,7 @@
#include "rds_single_path.h"
#include "rds.h"
#include "ib.h"
+#include "ib_mr.h"
/*
* Set the selected protocol version
@@ -993,6 +994,11 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp)
ic->i_cm_id, err);
}
+ /* kick off "flush_worker" for all pools in order to reap
+ * all FRMR registrations that are still marked "FRMR_IS_INUSE"
+ */
+ rds_ib_flush_mrs();
+
/*
* We want to wait for tx and rx completion to finish
* before we tear down the connection, but we have to be
@@ -1005,6 +1011,7 @@ void rds_ib_conn_path_shutdown(struct rds_conn_path *cp)
wait_event(rds_ib_ring_empty_wait,
rds_ib_ring_empty(&ic->i_recv_ring) &&
(atomic_read(&ic->i_signaled_sends) == 0) &&
+ (atomic_read(&ic->i_fastreg_inuse_count) == 0) &&
(atomic_read(&ic->i_fastreg_wrs) == RDS_IB_DEFAULT_FR_WR));
tasklet_kill(&ic->i_send_tasklet);
tasklet_kill(&ic->i_recv_tasklet);
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index a5d8f4128515..19c4cafb6952 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -32,6 +32,24 @@
#include "ib_mr.h"
+static inline void
+rds_transition_frwr_state(struct rds_ib_mr *ibmr,
+ enum rds_ib_fr_state old_state,
+ enum rds_ib_fr_state new_state)
+{
+ if (cmpxchg(&ibmr->u.frmr.fr_state,
+ old_state, new_state) == old_state &&
+ old_state == FRMR_IS_INUSE) {
+ /* enforce order of ibmr->u.frmr.fr_state update
+ * before decrementing i_fastreg_inuse_count
+ */
+ smp_mb__before_atomic();
+ atomic_dec(&ibmr->ic->i_fastreg_inuse_count);
+ if (waitqueue_active(&rds_ib_ring_empty_wait))
+ wake_up(&rds_ib_ring_empty_wait);
+ }
+}
+
static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
int npages)
{
@@ -118,13 +136,18 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
if (unlikely(ret != ibmr->sg_len))
return ret < 0 ? ret : -EINVAL;
+ if (cmpxchg(&frmr->fr_state,
+ FRMR_IS_FREE, FRMR_IS_INUSE) != FRMR_IS_FREE)
+ return -EBUSY;
+
+ atomic_inc(&ibmr->ic->i_fastreg_inuse_count);
+
/* Perform a WR for the fast_reg_mr. Each individual page
* in the sg list is added to the fast reg page list and placed
* inside the fast_reg_mr WR. The key used is a rolling 8bit
* counter, which should guarantee uniqueness.
*/
ib_update_fast_reg_key(frmr->mr, ibmr->remap_count++);
- frmr->fr_state = FRMR_IS_INUSE;
frmr->fr_reg = true;
memset(®_wr, 0, sizeof(reg_wr));
@@ -141,7 +164,8 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
ret = ib_post_send(ibmr->ic->i_cm_id->qp, ®_wr.wr, NULL);
if (unlikely(ret)) {
/* Failure here can be because of -ENOMEM as well */
- frmr->fr_state = FRMR_IS_STALE;
+ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
+
atomic_inc(&ibmr->ic->i_fastreg_wrs);
if (printk_ratelimit())
pr_warn("RDS/IB: %s returned error(%d)\n",
@@ -163,7 +187,7 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
if (frmr->fr_reg) {
pr_warn("RDS/IB: %s registration still incomplete after 100msec\n",
__func__);
- frmr->fr_state = FRMR_IS_STALE;
+ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
ret = -EBUSY;
}
@@ -280,8 +304,12 @@ static int rds_ib_post_inv(struct rds_ib_mr *ibmr)
ret = ib_post_send(i_cm_id->qp, s_wr, NULL);
if (unlikely(ret)) {
- frmr->fr_state = FRMR_IS_STALE;
+ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
frmr->fr_inv = false;
+ /* enforce order of frmr->fr_inv update
+ * before incrementing i_fastreg_wrs
+ */
+ smp_mb__before_atomic();
atomic_inc(&ibmr->ic->i_fastreg_wrs);
pr_err("RDS/IB: %s returned error(%d)\n", __func__, ret);
goto out;
@@ -316,7 +344,7 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
struct rds_ib_frmr *frmr = &ibmr->u.frmr;
if (wc->status != IB_WC_SUCCESS) {
- frmr->fr_state = FRMR_IS_STALE;
+ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_STALE);
if (rds_conn_up(ic->conn))
rds_ib_conn_error(ic->conn,
"frmr completion <%pI4,%pI4> status %u(%s), vendor_err 0x%x, disconnecting and reconnecting\n",
@@ -328,8 +356,7 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
}
if (frmr->fr_inv) {
- if (frmr->fr_state == FRMR_IS_INUSE)
- frmr->fr_state = FRMR_IS_FREE;
+ rds_transition_frwr_state(ibmr, FRMR_IS_INUSE, FRMR_IS_FREE);
frmr->fr_inv = false;
wake_up(&frmr->fr_inv_done);
}
@@ -339,6 +366,10 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
wake_up(&frmr->fr_reg_done);
}
+ /* enforce order of frmr->{fr_reg,fr_inv} update
+ * before incrementing i_fastreg_wrs
+ */
+ smp_mb__before_atomic();
atomic_inc(&ic->i_fastreg_wrs);
}
--
2.18.0
^ permalink raw reply related
* [PATCH net-next 5/7] net/rds: Set fr_state only to FRMR_IS_FREE if IB_WR_LOCAL_INV had been successful
From: Gerd Rausch @ 2019-07-01 16:40 UTC (permalink / raw)
To: Santosh Shilimkar, netdev; +Cc: David Miller
Fix a bug where fr_state first goes to FRMR_IS_STALE, because of a failure
of operation IB_WR_LOCAL_INV, but then gets set back to "FRMR_IS_FREE"
uncoditionally, even though the operation failed.
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
net/rds/ib_frmr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index 3c953034dca3..a5d8f4128515 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -328,7 +328,8 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
}
if (frmr->fr_inv) {
- frmr->fr_state = FRMR_IS_FREE;
+ if (frmr->fr_state == FRMR_IS_INUSE)
+ frmr->fr_state = FRMR_IS_FREE;
frmr->fr_inv = false;
wake_up(&frmr->fr_inv_done);
}
--
2.18.0
^ permalink raw reply related
* [PATCH net-next 4/7] net/rds: Fix NULL/ERR_PTR inconsistency
From: Gerd Rausch @ 2019-07-01 16:39 UTC (permalink / raw)
To: Santosh Shilimkar, netdev; +Cc: David Miller
Make function "rds_ib_try_reuse_ibmr" return NULL in case
memory region could not be allocated, since callers
simply check if the return value is not NULL.
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
net/rds/ib_rdma.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rds/ib_rdma.c b/net/rds/ib_rdma.c
index 6b047e63a769..c8c1e3ae8d84 100644
--- a/net/rds/ib_rdma.c
+++ b/net/rds/ib_rdma.c
@@ -450,7 +450,7 @@ struct rds_ib_mr *rds_ib_try_reuse_ibmr(struct rds_ib_mr_pool *pool)
rds_ib_stats_inc(s_ib_rdma_mr_8k_pool_depleted);
else
rds_ib_stats_inc(s_ib_rdma_mr_1m_pool_depleted);
- return ERR_PTR(-EAGAIN);
+ break;
}
/* We do have some empty MRs. Flush them out. */
@@ -464,7 +464,7 @@ struct rds_ib_mr *rds_ib_try_reuse_ibmr(struct rds_ib_mr_pool *pool)
return ibmr;
}
- return ibmr;
+ return NULL;
}
static void rds_ib_mr_pool_flush_worker(struct work_struct *work)
--
2.18.0
^ permalink raw reply related
* [PATCH net-next 3/7] net/rds: Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition after posting IB_WR_LOCAL_INV
From: Gerd Rausch @ 2019-07-01 16:39 UTC (permalink / raw)
To: Santosh Shilimkar, netdev; +Cc: David Miller
In order to:
1) avoid a silly bouncing between "clean_list" and "drop_list"
triggered by function "rds_ib_reg_frmr" as it is releases frmr
regions whose state is not "FRMR_IS_FREE" right away.
2) prevent an invalid access error in a race from a pending
"IB_WR_LOCAL_INV" operation with a teardown ("dma_unmap_sg", "put_page")
and de-registration ("ib_dereg_mr") of the corresponding
memory region.
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
---
net/rds/ib_frmr.c | 89 ++++++++++++++++++++++++++++++-----------------
net/rds/ib_mr.h | 2 ++
2 files changed, 59 insertions(+), 32 deletions(-)
diff --git a/net/rds/ib_frmr.c b/net/rds/ib_frmr.c
index 9f8aa310c27a..3c953034dca3 100644
--- a/net/rds/ib_frmr.c
+++ b/net/rds/ib_frmr.c
@@ -76,6 +76,7 @@ static struct rds_ib_mr *rds_ib_alloc_frmr(struct rds_ib_device *rds_ibdev,
frmr->fr_state = FRMR_IS_FREE;
init_waitqueue_head(&frmr->fr_inv_done);
+ init_waitqueue_head(&frmr->fr_reg_done);
return ibmr;
out_no_cigar:
@@ -124,6 +125,7 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
*/
ib_update_fast_reg_key(frmr->mr, ibmr->remap_count++);
frmr->fr_state = FRMR_IS_INUSE;
+ frmr->fr_reg = true;
memset(®_wr, 0, sizeof(reg_wr));
reg_wr.wr.wr_id = (unsigned long)(void *)ibmr;
@@ -144,7 +146,29 @@ static int rds_ib_post_reg_frmr(struct rds_ib_mr *ibmr)
if (printk_ratelimit())
pr_warn("RDS/IB: %s returned error(%d)\n",
__func__, ret);
+ goto out;
+ }
+
+ if (!frmr->fr_reg)
+ goto out;
+
+ /* Wait for the registration to complete in order to prevent an invalid
+ * access error resulting from a race between the memory region already
+ * being accessed while registration is still pending.
+ */
+ wait_event_timeout(frmr->fr_reg_done, !frmr->fr_reg,
+ msecs_to_jiffies(100));
+
+ /* Registration did not complete within one second, something's wrong */
+ if (frmr->fr_reg) {
+ pr_warn("RDS/IB: %s registration still incomplete after 100msec\n",
+ __func__);
+ frmr->fr_state = FRMR_IS_STALE;
+ ret = -EBUSY;
}
+
+out:
+
return ret;
}
@@ -262,6 +286,26 @@ static int rds_ib_post_inv(struct rds_ib_mr *ibmr)
pr_err("RDS/IB: %s returned error(%d)\n", __func__, ret);
goto out;
}
+
+ if (frmr->fr_state != FRMR_IS_INUSE)
+ goto out;
+
+ /* Wait for the FRMR_IS_FREE (or FRMR_IS_STALE) transition in order to
+ * 1) avoid a silly bouncing between "clean_list" and "drop_list"
+ * triggered by function "rds_ib_reg_frmr" as it is releases frmr
+ * regions whose state is not "FRMR_IS_FREE" right away.
+ * 2) prevents an invalid access error in a race
+ * from a pending "IB_WR_LOCAL_INV" operation
+ * with a teardown ("dma_unmap_sg", "put_page")
+ * and de-registration ("ib_dereg_mr") of the corresponding
+ * memory region.
+ */
+ wait_event_timeout(frmr->fr_inv_done, frmr->fr_state != FRMR_IS_INUSE,
+ msecs_to_jiffies(50));
+
+ if (frmr->fr_state == FRMR_IS_INUSE)
+ ret = -EBUSY;
+
out:
return ret;
}
@@ -289,6 +333,11 @@ void rds_ib_mr_cqe_handler(struct rds_ib_connection *ic, struct ib_wc *wc)
wake_up(&frmr->fr_inv_done);
}
+ if (frmr->fr_reg) {
+ frmr->fr_reg = false;
+ wake_up(&frmr->fr_reg_done);
+ }
+
atomic_inc(&ic->i_fastreg_wrs);
}
@@ -297,14 +346,18 @@ void rds_ib_unreg_frmr(struct list_head *list, unsigned int *nfreed,
{
struct rds_ib_mr *ibmr, *next;
struct rds_ib_frmr *frmr;
- int ret = 0;
+ int ret = 0, ret2;
unsigned int freed = *nfreed;
/* String all ib_mr's onto one list and hand them to ib_unmap_fmr */
list_for_each_entry(ibmr, list, unmap_list) {
- if (ibmr->sg_dma_len)
- ret |= rds_ib_post_inv(ibmr);
+ if (ibmr->sg_dma_len) {
+ ret2 = rds_ib_post_inv(ibmr);
+ if (ret2 && !ret)
+ ret = ret2;
+ }
}
+
if (ret)
pr_warn("RDS/IB: %s failed (err=%d)\n", __func__, ret);
@@ -347,36 +400,8 @@ struct rds_ib_mr *rds_ib_reg_frmr(struct rds_ib_device *rds_ibdev,
}
do {
- if (ibmr) {
- /* Memory regions make it onto the "clean_list" via
- * "rds_ib_flush_mr_pool", after the memory region has
- * been posted for invalidation via "rds_ib_post_inv".
- *
- * At that point in time, "fr_state" may still be
- * in state "FRMR_IS_INUSE", since the only place where
- * "fr_state" transitions to "FRMR_IS_FREE" is in
- * is in "rds_ib_mr_cqe_handler", which is
- * triggered by a tasklet.
- *
- * So in case we notice that
- * "fr_state != FRMR_IS_FREE" (see below), * we wait for
- * "fr_inv_done" to trigger with a maximum of 10msec.
- * Then we check again, and only put the memory region
- * onto the drop_list (via "rds_ib_free_frmr")
- * in case the situation remains unchanged.
- *
- * This avoids the problem of memory-regions bouncing
- * between "clean_list" and "drop_list" before they
- * even have a chance to be properly invalidated.
- */
- frmr = &ibmr->u.frmr;
- wait_event_timeout(frmr->fr_inv_done,
- frmr->fr_state == FRMR_IS_FREE,
- msecs_to_jiffies(10));
- if (frmr->fr_state == FRMR_IS_FREE)
- break;
+ if (ibmr)
rds_ib_free_frmr(ibmr, true);
- }
ibmr = rds_ib_alloc_frmr(rds_ibdev, nents);
if (IS_ERR(ibmr))
return ibmr;
diff --git a/net/rds/ib_mr.h b/net/rds/ib_mr.h
index ab26c20ed66f..9045a8c0edff 100644
--- a/net/rds/ib_mr.h
+++ b/net/rds/ib_mr.h
@@ -58,6 +58,8 @@ struct rds_ib_frmr {
enum rds_ib_fr_state fr_state;
bool fr_inv;
wait_queue_head_t fr_inv_done;
+ bool fr_reg;
+ wait_queue_head_t fr_reg_done;
struct ib_send_wr fr_wr;
unsigned int dma_npages;
unsigned int sg_byte_len;
--
2.18.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox