* Re: [PATCH bpf-next V7 8/8] bpf/selftests: activating bpf_check_mtu BPF-helper
From: Andrii Nakryiko @ 2020-11-21 7:41 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: bpf, Networking, Daniel Borkmann, Alexei Starovoitov,
Maciej Żenczykowski, Lorenz Bauer, shaun, Lorenzo Bianconi,
Marek Majkowski, John Fastabend, Jakub Kicinski, eyal.birger,
colrack
In-Reply-To: <160588912738.2817268.9380466634324530673.stgit@firesoul>
On Fri, Nov 20, 2020 at 8:21 AM Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
>
> Adding selftest for BPF-helper bpf_check_mtu(). Making sure
> it can be used from both XDP and TC.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> tools/testing/selftests/bpf/prog_tests/check_mtu.c | 37 ++++++++++++++++++++
> tools/testing/selftests/bpf/progs/test_check_mtu.c | 33 ++++++++++++++++++
> 2 files changed, 70 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/check_mtu.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_check_mtu.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/check_mtu.c b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
> new file mode 100644
> index 000000000000..09b8f986a17b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/check_mtu.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 Red Hat */
> +#include <uapi/linux/bpf.h>
> +#include <linux/if_link.h>
> +#include <test_progs.h>
> +
> +#include "test_check_mtu.skel.h"
> +#define IFINDEX_LO 1
> +
> +void test_check_mtu_xdp(struct test_check_mtu *skel)
this should be static func, otherwise it's treated as an independent selftest.
> +{
> + int err = 0;
> + int fd;
> +
> + fd = bpf_program__fd(skel->progs.xdp_use_helper);
> + err = bpf_set_link_xdp_fd(IFINDEX_LO, fd, XDP_FLAGS_SKB_MODE);
> + if (CHECK_FAIL(err))
please use CHECK() or one of ASSERT_xxx() helpers. CHECK_FAIL() should
be used for high-volume unlikely to fail test (i.e., very rarely).
> + return;
> +
> + bpf_set_link_xdp_fd(IFINDEX_LO, -1, 0);
> +}
> +
> +void test_check_mtu(void)
> +{
> + struct test_check_mtu *skel;
> +
> + skel = test_check_mtu__open_and_load();
> + if (CHECK_FAIL(!skel)) {
> + perror("test_check_mtu__open_and_load");
> + return;
> + }
> +
> + if (test__start_subtest("bpf_check_mtu XDP-attach"))
> + test_check_mtu_xdp(skel);
> +
> + test_check_mtu__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_check_mtu.c b/tools/testing/selftests/bpf/progs/test_check_mtu.c
> new file mode 100644
> index 000000000000..ab97ec925a32
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_check_mtu.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2020 Red Hat */
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +#include <stddef.h>
> +#include <stdint.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("xdp")
> +int xdp_use_helper(struct xdp_md *ctx)
> +{
> + uint32_t mtu_len = 0;
> + int delta = 20;
> +
> + if (bpf_check_mtu(ctx, 0, &mtu_len, delta, 0)) {
> + return XDP_ABORTED;
> + }
nit: unnecessary {} for single-line if; same below
> + return XDP_PASS;
> +}
> +
> +SEC("classifier")
> +int tc_use_helper(struct __sk_buff *ctx)
> +{
> + uint32_t mtu_len = 0;
> + int delta = -20;
> +
> + if (bpf_check_mtu(ctx, 0, &mtu_len, delta, 0)) {
> + return BPF_DROP;
> + }
> + return BPF_OK;
> +}
>
>
^ permalink raw reply
* Re: [RFC PATCH] bpf: preload: Fix build error when O= is set
From: Andrii Nakryiko @ 2020-11-21 7:37 UTC (permalink / raw)
To: David Gow
Cc: Alexei Starovoitov, Daniel Borkmann,
Toke Høiland-Jørgensen, Networking, bpf, open list,
Brendan Higgins, Masami Hiramatsu
In-Reply-To: <20201119085022.3606135-1-davidgow@google.com>
On Thu, Nov 19, 2020 at 12:51 AM David Gow <davidgow@google.com> wrote:
>
> If BPF_PRELOAD is enabled, and an out-of-tree build is requested with
> make O=<path>, compilation seems to fail with:
>
> tools/scripts/Makefile.include:4: *** O=.kunit does not exist. Stop.
> make[4]: *** [../kernel/bpf/preload/Makefile:8: kernel/bpf/preload/libbpf.a] Error 2
> make[3]: *** [../scripts/Makefile.build:500: kernel/bpf/preload] Error 2
> make[2]: *** [../scripts/Makefile.build:500: kernel/bpf] Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [.../Makefile:1799: kernel] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:185: __sub-make] Error 2
>
> By the looks of things, this is because the (relative path) O= passed on
> the command line is being passed to the libbpf Makefile, which then
> can't find the directory. Given OUTPUT= is being passed anyway, we can
> work around this by explicitly setting an empty O=, which will be
> ignored in favour of OUTPUT= in tools/scripts/Makefile.include.
Strange, but I can't repro it. I use make O=<absolute path> all the
time with no issues. I just tried specifically with a make O=.build,
where .build is inside Linux repo, and it still worked fine. See also
be40920fbf10 ("tools: Let O= makes handle a relative path with -C
option") which was supposed to address such an issue. So I'm wondering
what exactly is causing this problem.
>
> Signed-off-by: David Gow <davidgow@google.com>
> ---
>
> Hi all,
>
> I'm not 100% sure this is the correct fix here -- it seems to work for
> me, and makes some sense, but let me know if there's a better way.
>
> One other thing worth noting is that I've been hitting this with
> make allyesconfig on ARCH=um, but there's a comment in the Kconfig
> suggesting that, because BPF_PRELOAD depends on !COMPILE_TEST, that
> maybe it shouldn't be being built at all. I figured that it was worth
> trying to fix this anyway.
>
> Cheers,
> -- David
>
>
> kernel/bpf/preload/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile
> index 23ee310b6eb4..39848d296097 100644
> --- a/kernel/bpf/preload/Makefile
> +++ b/kernel/bpf/preload/Makefile
> @@ -5,7 +5,7 @@ LIBBPF_A = $(obj)/libbpf.a
> LIBBPF_OUT = $(abspath $(obj))
>
> $(LIBBPF_A):
> - $(Q)$(MAKE) -C $(LIBBPF_SRCS) OUTPUT=$(LIBBPF_OUT)/ $(LIBBPF_OUT)/libbpf.a
> + $(Q)$(MAKE) -C $(LIBBPF_SRCS) O= OUTPUT=$(LIBBPF_OUT)/ $(LIBBPF_OUT)/libbpf.a
>
> userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \
> -I $(srctree)/tools/lib/ -Wno-unused-result
> --
> 2.29.2.454.gaff20da3a2-goog
>
^ permalink raw reply
* [PATCH net,v2] net/packet: fix packet receive on L3 devices without visible hard header
From: Eyal Birger @ 2020-11-21 6:34 UTC (permalink / raw)
To: willemdebruijn.kernel, davem, kuba
Cc: Jason, netdev, xie.he.0141, Eyal Birger
In the patchset merged by commit b9fcf0a0d826
("Merge branch 'support-AF_PACKET-for-layer-3-devices'") L3 devices which
did not have header_ops were given one for the purpose of protocol parsing
on af_packet transmit path.
That change made af_packet receive path regard these devices as having a
visible L3 header and therefore aligned incoming skb->data to point to the
skb's mac_header. Some devices, such as ipip, xfrmi, and others, do not
reset their mac_header prior to ingress and therefore their incoming
packets became malformed.
Ideally these devices would reset their mac headers, or af_packet would be
able to rely on dev->hard_header_len being 0 for such cases, but it seems
this is not the case.
Fix by changing af_packet RX ll visibility criteria to include the
existence of a '.create()' header operation, which is used when creating
a device hard header - via dev_hard_header() - by upper layers, and does
not exist in these L3 devices.
As this predicate may be useful in other situations, add it as a common
dev_has_header() helper in netdevice.h.
Fixes: b9fcf0a0d826 ("Merge branch 'support-AF_PACKET-for-layer-3-devices'")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
v2:
- add common dev_has_header() helper as suggested by Willem de Bruijn
---
include/linux/netdevice.h | 5 +++++
net/packet/af_packet.c | 18 +++++++++---------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 964b494b0e8d..fa275a054f46 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3137,6 +3137,11 @@ static inline bool dev_validate_header(const struct net_device *dev,
return false;
}
+static inline bool dev_has_header(const struct net_device *dev)
+{
+ return dev->header_ops && dev->header_ops->create;
+}
+
typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr,
int len, int size);
int register_gifconf(unsigned int family, gifconf_func_t *gifconf);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index cefbd50c1090..7a18ffff8551 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -93,8 +93,8 @@
/*
Assumptions:
- - If the device has no dev->header_ops, there is no LL header visible
- above the device. In this case, its hard_header_len should be 0.
+ - If the device has no dev->header_ops->create, there is no LL header
+ visible above the device. In this case, its hard_header_len should be 0.
The device may prepend its own header internally. In this case, its
needed_headroom should be set to the space needed for it to add its
internal header.
@@ -108,26 +108,26 @@
On receive:
-----------
-Incoming, dev->header_ops != NULL
+Incoming, dev_has_header(dev) == true
mac_header -> ll header
data -> data
-Outgoing, dev->header_ops != NULL
+Outgoing, dev_has_header(dev) == true
mac_header -> ll header
data -> ll header
-Incoming, dev->header_ops == NULL
+Incoming, dev_has_header(dev) == false
mac_header -> data
However drivers often make it point to the ll header.
This is incorrect because the ll header should be invisible to us.
data -> data
-Outgoing, dev->header_ops == NULL
+Outgoing, dev_has_header(dev) == false
mac_header -> data. ll header is invisible to us.
data -> data
Resume
- If dev->header_ops == NULL we are unable to restore the ll header,
+ If dev_has_header(dev) == false we are unable to restore the ll header,
because it is invisible to us.
@@ -2069,7 +2069,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
skb->dev = dev;
- if (dev->header_ops) {
+ if (dev_has_header(dev)) {
/* The device has an explicit notion of ll header,
* exported to higher levels.
*
@@ -2198,7 +2198,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
if (!net_eq(dev_net(dev), sock_net(sk)))
goto drop;
- if (dev->header_ops) {
+ if (dev_has_header(dev)) {
if (sk->sk_type != SOCK_DGRAM)
skb_push(skb, skb->data - skb_mac_header(skb));
else if (skb->pkt_type == PACKET_OUTGOING) {
--
2.25.1
^ permalink raw reply related
* [net,v2] net/packet: fix packet receive on L3 devices without visible hard header
From: Eyal Birger @ 2020-11-21 6:28 UTC (permalink / raw)
To: willemdebruijn.kernel, davem, kuba
Cc: Jason, netdev, xie.he.0141, Eyal Birger
In the patchset merged by commit b9fcf0a0d826
("Merge branch 'support-AF_PACKET-for-layer-3-devices'") L3 devices which
did not have header_ops were given one for the purpose of protocol parsing
on af_packet transmit path.
That change made af_packet receive path regard these devices as having a
visible L3 header and therefore aligned incoming skb->data to point to the
skb's mac_header. Some devices, such as ipip, xfrmi, and others, do not
reset their mac_header prior to ingress and therefore their incoming
packets became malformed.
Ideally these devices would reset their mac headers, or af_packet would be
able to rely on dev->hard_header_len being 0 for such cases, but it seems
this is not the case.
Fix by changing af_packet RX ll visibility criteria to include the
existence of a '.create()' header operation, which is used when creating
a device hard header - via dev_hard_header() - by upper layers, and does
not exist in these L3 devices.
As this predicate may be useful in other situations, add it as a common
dev_has_header() helper in netdevice.h.
Fixes: b9fcf0a0d826 ("Merge branch 'support-AF_PACKET-for-layer-3-devices'")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
---
v2:
- add common dev_has_header() helper as suggested by Willem de Bruijn
---
include/linux/netdevice.h | 5 +++++
net/packet/af_packet.c | 18 +++++++++---------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 964b494b0e8d..fa275a054f46 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3137,6 +3137,11 @@ static inline bool dev_validate_header(const struct net_device *dev,
return false;
}
+static inline bool dev_has_header(const struct net_device *dev)
+{
+ return dev->header_ops && dev->header_ops->create;
+}
+
typedef int gifconf_func_t(struct net_device * dev, char __user * bufptr,
int len, int size);
int register_gifconf(unsigned int family, gifconf_func_t *gifconf);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index cefbd50c1090..7a18ffff8551 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -93,8 +93,8 @@
/*
Assumptions:
- - If the device has no dev->header_ops, there is no LL header visible
- above the device. In this case, its hard_header_len should be 0.
+ - If the device has no dev->header_ops->create, there is no LL header
+ visible above the device. In this case, its hard_header_len should be 0.
The device may prepend its own header internally. In this case, its
needed_headroom should be set to the space needed for it to add its
internal header.
@@ -108,26 +108,26 @@
On receive:
-----------
-Incoming, dev->header_ops != NULL
+Incoming, dev_has_header(dev) == true
mac_header -> ll header
data -> data
-Outgoing, dev->header_ops != NULL
+Outgoing, dev_has_header(dev) == true
mac_header -> ll header
data -> ll header
-Incoming, dev->header_ops == NULL
+Incoming, dev_has_header(dev) == false
mac_header -> data
However drivers often make it point to the ll header.
This is incorrect because the ll header should be invisible to us.
data -> data
-Outgoing, dev->header_ops == NULL
+Outgoing, dev_has_header(dev) == false
mac_header -> data. ll header is invisible to us.
data -> data
Resume
- If dev->header_ops == NULL we are unable to restore the ll header,
+ If dev_has_header(dev) == false we are unable to restore the ll header,
because it is invisible to us.
@@ -2069,7 +2069,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
skb->dev = dev;
- if (dev->header_ops) {
+ if (dev_has_header(dev)) {
/* The device has an explicit notion of ll header,
* exported to higher levels.
*
@@ -2198,7 +2198,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
if (!net_eq(dev_net(dev), sock_net(sk)))
goto drop;
- if (dev->header_ops) {
+ if (dev_has_header(dev)) {
if (sk->sk_type != SOCK_DGRAM)
skb_push(skb, skb->data - skb_mac_header(skb));
else if (skb->pkt_type == PACKET_OUTGOING) {
--
2.25.1
^ permalink raw reply related
* Re: [kuba@kernel.org: Re: [PATCH net-next v2 0/3] net: introduce rps_default_mask]
From: Marcel Apfelbaum @ 2020-11-21 5:39 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Marcel Apfelbaum, Saeed Mahameed, Jakub Kicinski, netdev,
Paolo Abeni, Jonathan Corbet, David S. Miller, Shuah Khan,
Marcelo Tosatti
In-Reply-To: <6e43ea1e-b166-f60e-9dd1-e907108a3b12@iogearbox.net>
Hi Daniel,
On Fri, Nov 20, 2020 at 11:56 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 11/20/20 6:39 PM, Marcel Apfelbaum wrote:
> > +netdev
> > [Sorry for the second email, I failed to set the text-only mode]
> > On Fri, Nov 20, 2020 at 7:30 PM Marcel Apfelbaum <mapfelba@redhat.com> wrote:
> [...]
> >>> ---------- Forwarded message ----------
> >>> From: Jakub Kicinski <kuba@kernel.org>
> >>> To: Paolo Abeni <pabeni@redhat.com>
> >>> Cc: Saeed Mahameed <saeed@kernel.org>, netdev@vger.kernel.org, Jonathan Corbet <corbet@lwn.net>, "David S. Miller" <davem@davemloft.net>, Shuah Khan <shuah@kernel.org>, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, Marcelo Tosatti <mtosatti@redhat.com>, Daniel Borkmann <daniel@iogearbox.net>
> >>> Bcc:
> >>> Date: Wed, 4 Nov 2020 11:42:26 -0800
> >>> Subject: Re: [PATCH net-next v2 0/3] net: introduce rps_default_mask
> >>> On Wed, 04 Nov 2020 18:36:08 +0100 Paolo Abeni wrote:
> >>>> On Tue, 2020-11-03 at 08:52 -0800, Jakub Kicinski wrote:
> >>>>> On Tue, 03 Nov 2020 16:22:07 +0100 Paolo Abeni wrote:
> >>>>>> The relevant use case is an host running containers (with the related
> >>>>>> orchestration tools) in a RT environment. Virtual devices (veths, ovs
> >>>>>> ports, etc.) are created by the orchestration tools at run-time.
> >>>>>> Critical processes are allowed to send packets/generate outgoing, it gets a network-interface
> >>>> upstart job just as it does on a real host.
> >>>>
> >>>>>> network traffic - but any interrupt is moved away from the related
> >>>>>> cores, so that usual incoming network traffic processing does not
> >>>>>> happen there.
> >>>>>>
> >>>>>> Still an xmit operation on a virtual devices may be transmitted via ovs
> >>>>>> or veth, with the relevant forwarding operation happening in a softirq
> >>>>>> on the same CPU originating the packet.
> >>>>>>
> >>>>>> RPS is configured (even) on such virtual devices to move away the
> >>>>>> forwarding from the relevant CPUs.
> >>>>>>
> >>>>>> As Saeed noted, such configuration could be possibly performed via some
> >>>>>> user-space daemon monitoring network devices and network namespaces
> >>>>>> creation. That will be anyway prone to some race: the orchestation tool
> >>>>>> may create and enable the netns and virtual devices before the daemon
> >>>>>> has properly set the RPS mask.
> >>>>>>
> >>>>>> In the latter scenario some packet forwarding could still slip in the
> >>>>>> relevant CPU, causing measurable latency. In all non RT scenarios the
> >>>>>> above will be likely irrelevant, but in the RT context that is not
> >>>>>> acceptable - e.g. it causes in real environments latency above the
> >>>>>> defined limits, while the proposed patches avoid the issue.
> >>>>>>
> >>>>>> Do you see any other simple way to avoid the above race?
> >>>>>>
> >>>>>> Please let me know if the above answers your doubts,
> >>>>>
> >>>>> Thanks, that makes it clearer now.
> >>>>>
> >>>>> Depending on how RT-aware your container management is it may or may not
> >>>>> be the right place to configure this, as it creates the veth interface.
> >>>>> Presumably it's the container management which does the placement of
> >>>>> the tasks to cores, why is it not setting other attributes, like RPS?
> >>
> >> The CPU isolation is done statically at system boot by setting Linux kernel parameters,
> >> So the container management component, in this case the Machine Configuration Operator (for Openshift)
> >> or the K8s counterpart can't really help. (actually they would help if a global RPS mask would exist)
> >>
> >> I tried to tweak the rps_cpus mask using the container management stack, but there
> >> is no sane way to do it, please let me get a little into the details.
> >>
> >> The k8s orchestration component that deals with injecting the network device(s) into the
> >> container is CNI, which is interface based and implemented by a lot of plugins, making it
> >> hardly feasible to go over all the existing plugins and change them. Also what about
> >> the 3rd party ones?
> >>
> >> Writing a new CNI plugin and chain it into the existing one is also not an option AFAIK,
> >> they work at the network level and do not have access to sysfs (they handle the network namespaces).
> >> Even if it would be possible (I don't have a deep CNI understanding), it will require a cluster global configuration
> >> that is actually needed only for some of the cluster nodes.
>
> CNI chaining would be ugly, agree, but in a typical setting you'd have the CNI plugin
> itself which is responsible to set up the Pod for communication to the outside world;
> part of it would be creation of devices and moving them into the target netns, and
> then you also typically have an agent running in kube-system namespace in the hostns
> to which the CNI plugin talks to via IPC e.g. to set up IPAM and other state. Such
> agent usually sets up all sort of knobs for the networking layer upon bootstrap.
The main issue is that CNI is networking related, but the way to set
the RPS is by writing to /sys which is not considered network namespace
related and is read only inside the containers.
> Assuming you have a cluster where only some of the nodes have RT kernel, these would
> likely have special node annotations in K8s so you could select them to run certain
> workloads on it.. couldn't such agent be taught to be RT-aware and set up all the
> needed knobs?
I do agree this part may be doable, sadly is by far not the biggest problem.
> Agree it's a bit ugly to change the relevant CNI plugins to be RT-aware,
> but what if you also need other settings in future aside from RPS mask for RT? At some
> point you'd likely end up having to extend these anyway, no?
>
All networking changes are fair play, however setting the RPS mask
is related to networking but not a networking operation per se - is a
cross-domain operation (network namespace/mount namespace).
Thank you for your response,
Marcel
[...]
^ permalink raw reply
* INFO: task hung in sync_inodes_sb (4)
From: syzbot @ 2020-11-21 4:55 UTC (permalink / raw)
To: a, b.a.t.m.a.n, davem, emmanuel.grumbach, johannes.berg,
linux-fsdevel, linux-kernel, luciano.coelho, mareklindner, netdev,
sw, syzkaller-bugs, viro
Hello,
syzbot found the following issue on:
HEAD commit: 03430750 Add linux-next specific files for 20201116
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17027fdc500000
kernel config: https://syzkaller.appspot.com/x/.config?x=a1c4c3f27041fdb8
dashboard link: https://syzkaller.appspot.com/bug?extid=7d50f1e54a12ba3aeae2
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=124a8841500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15a4fce2500000
The issue was bisected to:
commit c68df2e7be0c1238ea3c281fd744a204ef3b15a0
Author: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Date: Thu Sep 15 13:30:02 2016 +0000
mac80211: allow using AP_LINK_PS with mac80211-generated TIM IE
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1445e981500000
final oops: https://syzkaller.appspot.com/x/report.txt?x=1645e981500000
console output: https://syzkaller.appspot.com/x/log.txt?x=1245e981500000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+7d50f1e54a12ba3aeae2@syzkaller.appspotmail.com
Fixes: c68df2e7be0c ("mac80211: allow using AP_LINK_PS with mac80211-generated TIM IE")
INFO: task syz-executor017:8513 blocked for more than 143 seconds.
Not tainted 5.10.0-rc3-next-20201116-syzkaller #0
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
task:syz-executor017 state:D stack:27448 pid: 8513 ppid: 8507 flags:0x00004000
Call Trace:
context_switch kernel/sched/core.c:4269 [inline]
__schedule+0x890/0x2030 kernel/sched/core.c:5019
schedule+0xcf/0x270 kernel/sched/core.c:5098
wb_wait_for_completion+0x17b/0x230 fs/fs-writeback.c:209
sync_inodes_sb+0x1a6/0x9d0 fs/fs-writeback.c:2559
__sync_filesystem fs/sync.c:34 [inline]
sync_filesystem fs/sync.c:67 [inline]
sync_filesystem+0x15c/0x260 fs/sync.c:48
generic_shutdown_super+0x70/0x370 fs/super.c:448
kill_block_super+0x97/0xf0 fs/super.c:1446
deactivate_locked_super+0x94/0x160 fs/super.c:335
deactivate_super+0xad/0xd0 fs/super.c:366
cleanup_mnt+0x3a3/0x530 fs/namespace.c:1123
task_work_run+0xdd/0x190 kernel/task_work.c:140
tracehook_notify_resume include/linux/tracehook.h:188 [inline]
exit_to_user_mode_loop kernel/entry/common.c:172 [inline]
exit_to_user_mode_prepare+0x1f0/0x200 kernel/entry/common.c:199
syscall_exit_to_user_mode+0x38/0x260 kernel/entry/common.c:274
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x44e0e7
Code: Unable to access opcode bytes at RIP 0x44e0bd.
RSP: 002b:00007fff42061288 EFLAGS: 00000206 ORIG_RAX: 00000000000000a6
RAX: 0000000000000000 RBX: 00000000000cee4c RCX: 000000000044e0e7
RDX: 0000000000400be0 RSI: 0000000000000002 RDI: 00007fff42061330
RBP: 0000000000002142 R08: 0000000000000000 R09: 0000000000000009
R10: 0000000000000005 R11: 0000000000000206 R12: 00007fff420623e0
R13: 0000000001f67880 R14: 0000000000000000 R15: 0000000000000000
Showing all locks held in the system:
2 locks held by kworker/u4:5/225:
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: atomic64_set include/asm-generic/atomic-instrumented.h:856 [inline]
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: atomic_long_set include/asm-generic/atomic-long.h:41 [inline]
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: set_work_data kernel/workqueue.c:616 [inline]
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: set_work_pool_and_clear_pending kernel/workqueue.c:643 [inline]
#0: ffff8881413a4138 ((wq_completion)writeback){+.+.}-{0:0}, at: process_one_work+0x821/0x15a0 kernel/workqueue.c:2243
#1: ffffc9000191fda8 ((work_completion)(&(&wb->dwork)->work)){+.+.}-{0:0}, at: process_one_work+0x854/0x15a0 kernel/workqueue.c:2247
1 lock held by khungtaskd/1655:
#0: ffffffff8b339ce0 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x53/0x260 kernel/locking/lockdep.c:6252
1 lock held by in:imklog/8188:
#0: ffff888017c8f4f0 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0xe9/0x100 fs/file.c:932
2 locks held by syz-executor017/8513:
#0: ffff88801a8500e0 (&type->s_umount_key#49){+.+.}-{3:3}, at: deactivate_super+0xa5/0xd0 fs/super.c:365
#1: ffff888143f5e708 (&bdi->wb_switch_rwsem){+.+.}-{3:3}, at: bdi_down_write_wb_switch_rwsem fs/fs-writeback.c:344 [inline]
#1: ffff888143f5e708 (&bdi->wb_switch_rwsem){+.+.}-{3:3}, at: sync_inodes_sb+0x18c/0x9d0 fs/fs-writeback.c:2557
=============================================
NMI backtrace for cpu 0
CPU: 0 PID: 1655 Comm: khungtaskd Not tainted 5.10.0-rc3-next-20201116-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x107/0x163 lib/dump_stack.c:120
nmi_cpu_backtrace.cold+0x44/0xd7 lib/nmi_backtrace.c:105
nmi_trigger_cpumask_backtrace+0x1b3/0x230 lib/nmi_backtrace.c:62
trigger_all_cpu_backtrace include/linux/nmi.h:147 [inline]
check_hung_uninterruptible_tasks kernel/hung_task.c:253 [inline]
watchdog+0xd89/0xf30 kernel/hung_task.c:338
kthread+0x3af/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:296
Sending NMI from CPU 0 to CPUs 1:
NMI backtrace for cpu 1 skipped: idling at native_safe_halt arch/x86/include/asm/irqflags.h:60 [inline]
NMI backtrace for cpu 1 skipped: idling at arch_safe_halt arch/x86/include/asm/irqflags.h:103 [inline]
NMI backtrace for cpu 1 skipped: idling at acpi_safe_halt drivers/acpi/processor_idle.c:111 [inline]
NMI backtrace for cpu 1 skipped: idling at acpi_idle_do_entry+0x1c9/0x250 drivers/acpi/processor_idle.c:517
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCHv4 net-next 3/3] octeontx2-af: Add devlink health reporters for NIX
From: George Cherian @ 2020-11-21 4:02 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: kuba, davem, sgoutham, lcherian, gakula, masahiroy,
george.cherian, willemdebruijn.kernel, saeed
In-Reply-To: <20201121040201.3171542-1-george.cherian@marvell.com>
Add health reporters for RVU NIX block.
NIX Health reporter handle following HW event groups
- GENERAL events
- RAS events
- RVU event
An event counter per event is maintained in SW.
Output:
# ./devlink health
pci/0002:01:00.0:
reporter npa
state healthy error 0 recover 0
reporter nix
state healthy error 0 recover 0
# ./devlink health dump show pci/0002:01:00.0 reporter nix
NIX_AF_GENERAL:
Memory Fault on NIX_AQ_INST_S read: 0
Memory Fault on NIX_AQ_RES_S write: 0
AQ Doorbell error: 0
Rx on unmapped PF_FUNC: 0
Rx multicast replication error: 0
Memory fault on NIX_RX_MCE_S read: 0
Memory fault on multicast WQE read: 0
Memory fault on mirror WQE read: 0
Memory fault on mirror pkt write: 0
Memory fault on multicast pkt write: 0
NIX_AF_RAS:
Poisoned data on NIX_AQ_INST_S read: 0
Poisoned data on NIX_AQ_RES_S write: 0
Poisoned data on HW context read: 0
Poisoned data on packet read from mirror buffer: 0
Poisoned data on packet read from mcast buffer: 0
Poisoned data on WQE read from mirror buffer: 0
Poisoned data on WQE read from multicast buffer: 0
Poisoned data on NIX_RX_MCE_S read: 0
NIX_AF_RVU:
Unmap Slot Error: 0
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: George Cherian <george.cherian@marvell.com>
---
.../marvell/octeontx2/af/rvu_devlink.c | 414 +++++++++++++++++-
.../marvell/octeontx2/af/rvu_devlink.h | 31 ++
.../marvell/octeontx2/af/rvu_struct.h | 10 +
3 files changed, 453 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index b7f0691d86b0..c02d0f56ae7a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -35,6 +35,131 @@ static int rvu_report_pair_end(struct devlink_fmsg *fmsg)
return devlink_fmsg_pair_nest_end(fmsg);
}
+static irqreturn_t rvu_nix_af_rvu_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_ctx *nix_event_context;
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_context = rvu_dl->nix_event_ctx;
+ nix_event_count = &nix_event_context->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_RVU_INT);
+ nix_event_context->nix_af_rvu_int = intr;
+
+ if (intr & BIT_ULL(0))
+ nix_event_count->unmap_slot_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT, intr);
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_nix_health_reporter, "NIX_AF_RVU Error",
+ nix_event_context);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_nix_af_err_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_ctx *nix_event_context;
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_context = rvu_dl->nix_event_ctx;
+ nix_event_count = &nix_event_context->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_ERR_INT);
+ nix_event_context->nix_af_rvu_err = intr;
+
+ if (intr & BIT_ULL(14))
+ nix_event_count->aq_inst_count++;
+ if (intr & BIT_ULL(13))
+ nix_event_count->aq_res_count++;
+ if (intr & BIT_ULL(12))
+ nix_event_count->aq_db_count++;
+ if (intr & BIT_ULL(6))
+ nix_event_count->rx_on_unmap_pf_count++;
+ if (intr & BIT_ULL(5))
+ nix_event_count->rx_mcast_repl_count++;
+ if (intr & BIT_ULL(4))
+ nix_event_count->rx_mcast_memfault_count++;
+ if (intr & BIT_ULL(3))
+ nix_event_count->rx_mcast_wqe_memfault_count++;
+ if (intr & BIT_ULL(2))
+ nix_event_count->rx_mirror_wqe_memfault_count++;
+ if (intr & BIT_ULL(1))
+ nix_event_count->rx_mirror_pktw_memfault_count++;
+ if (intr & BIT_ULL(0))
+ nix_event_count->rx_mcast_pktw_memfault_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT, intr);
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_nix_health_reporter, "NIX_AF_ERR Error",
+ nix_event_context);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_nix_af_ras_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_nix_event_ctx *nix_event_context;
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ nix_event_context = rvu_dl->nix_event_ctx;
+ nix_event_count = &nix_event_context->nix_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NIX_AF_RAS);
+ nix_event_context->nix_af_rvu_ras = intr;
+
+ if (intr & BIT_ULL(34))
+ nix_event_count->poison_aq_inst_count++;
+ if (intr & BIT_ULL(33))
+ nix_event_count->poison_aq_res_count++;
+ if (intr & BIT_ULL(32))
+ nix_event_count->poison_aq_cxt_count++;
+ if (intr & BIT_ULL(4))
+ nix_event_count->rx_mirror_data_poison_count++;
+ if (intr & BIT_ULL(3))
+ nix_event_count->rx_mcast_data_poison_count++;
+ if (intr & BIT_ULL(2))
+ nix_event_count->rx_mirror_wqe_poison_count++;
+ if (intr & BIT_ULL(1))
+ nix_event_count->rx_mcast_wqe_poison_count++;
+ if (intr & BIT_ULL(0))
+ nix_event_count->rx_mce_poison_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS, intr);
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_nix_health_reporter, "NIX_AF_RAS Error",
+ nix_event_context);
+
+ return IRQ_HANDLED;
+}
+
static bool rvu_common_request_irq(struct rvu *rvu, int offset,
const char *name, irq_handler_t fn)
{
@@ -52,6 +177,285 @@ static bool rvu_common_request_irq(struct rvu *rvu, int offset,
return rvu->irq_allocated[offset];
}
+static void rvu_nix_blk_unregister_interrupts(struct rvu *rvu,
+ int blkaddr)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int offs, i;
+
+ offs = rvu_read64(rvu, blkaddr, NIX_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!offs)
+ return;
+
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1C, ~0ULL);
+
+ if (rvu->irq_allocated[offs + NIX_AF_INT_VEC_RVU]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + NIX_AF_INT_VEC_RVU),
+ rvu_dl);
+ rvu->irq_allocated[offs + NIX_AF_INT_VEC_RVU] = false;
+ }
+
+ for (i = NIX_AF_INT_VEC_AF_ERR; i < NIX_AF_INT_VEC_CNT; i++)
+ if (rvu->irq_allocated[offs + i]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + i), rvu_dl);
+ rvu->irq_allocated[offs + i] = false;
+ }
+}
+
+static void rvu_nix_unregister_interrupts(struct rvu *rvu)
+{
+ int blkaddr = 0;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return;
+
+ rvu_nix_blk_unregister_interrupts(rvu, blkaddr);
+}
+
+static int rvu_nix_blk_register_interrupts(struct rvu *rvu,
+ int blkaddr)
+{
+ int base;
+ bool rc;
+
+ /* Get NIX AF MSIX vectors offset. */
+ base = rvu_read64(rvu, blkaddr, NIX_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!base) {
+ dev_warn(rvu->dev,
+ "Failed to get NIX%d NIX_AF_INT vector offsets\n",
+ blkaddr - BLKADDR_NIX0);
+ return 0;
+ }
+ /* Register and enable NIX_AF_RVU_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_RVU,
+ "NIX_AF_RVU_INT",
+ rvu_nix_af_rvu_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NIX_AF_ERR_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_AF_ERR,
+ "NIX_AF_ERR_INT",
+ rvu_nix_af_err_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NIX_AF_RAS interrupt */
+ rc = rvu_common_request_irq(rvu, base + NIX_AF_INT_VEC_POISON,
+ "NIX_AF_RAS",
+ rvu_nix_af_ras_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1S, ~0ULL);
+
+ return 0;
+err:
+ rvu_nix_unregister_interrupts(rvu);
+ return -1;
+}
+
+static int rvu_nix_register_interrupts(struct rvu *rvu)
+{
+ int blkaddr = 0;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ rvu_nix_blk_register_interrupts(rvu, blkaddr);
+
+ return 0;
+}
+
+static int rvu_nix_report_show(struct devlink_fmsg *fmsg, struct rvu *rvu)
+{
+ struct rvu_nix_event_ctx *nix_event_context;
+ struct rvu_nix_event_cnt *nix_event_count;
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int err;
+
+ nix_event_context = rvu_dl->nix_event_ctx;
+ nix_event_count = &nix_event_context->nix_event_cnt;
+ err = rvu_report_pair_start(fmsg, "NIX_AF_GENERAL");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tMemory Fault on NIX_AQ_INST_S read",
+ nix_event_count->aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory Fault on NIX_AQ_RES_S write",
+ nix_event_count->aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAQ Doorbell error",
+ nix_event_count->aq_db_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tRx on unmapped PF_FUNC",
+ nix_event_count->rx_on_unmap_pf_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tRx multicast replication error",
+ nix_event_count->rx_mcast_repl_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on NIX_RX_MCE_S read",
+ nix_event_count->rx_mcast_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on multicast WQE read",
+ nix_event_count->rx_mcast_wqe_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on mirror WQE read",
+ nix_event_count->rx_mirror_wqe_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on mirror pkt write",
+ nix_event_count->rx_mirror_pktw_memfault_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory fault on multicast pkt write",
+ nix_event_count->rx_mcast_pktw_memfault_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NIX_AF_RAS");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tPoisoned data on NIX_AQ_INST_S read",
+ nix_event_count->poison_aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NIX_AQ_RES_S write",
+ nix_event_count->poison_aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on HW context read",
+ nix_event_count->poison_aq_cxt_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on packet read from mirror buffer",
+ nix_event_count->rx_mirror_data_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on packet read from mcast buffer",
+ nix_event_count->rx_mcast_data_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on WQE read from mirror buffer",
+ nix_event_count->rx_mirror_wqe_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on WQE read from multicast buffer",
+ nix_event_count->rx_mcast_wqe_poison_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NIX_RX_MCE_S read",
+ nix_event_count->rx_mce_poison_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NIX_AF_RVU");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap Slot Error",
+ nix_event_count->unmap_slot_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ return 0;
+}
+
+static int rvu_nix_reporter_dump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *ctx,
+ struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+
+ return rvu_nix_report_show(fmsg, rvu);
+}
+
+static int rvu_nix_reporter_recover(struct devlink_health_reporter *reporter,
+ void *ctx, struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+ struct rvu_nix_event_ctx *nix_event_ctx = ctx;
+ int blkaddr;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ if (nix_event_ctx->nix_af_rvu_int) {
+ rvu_write64(rvu, blkaddr, NIX_AF_RVU_INT_ENA_W1S, ~0ULL);
+ nix_event_ctx->nix_af_rvu_int = 0;
+ }
+ if (nix_event_ctx->nix_af_rvu_err) {
+ rvu_write64(rvu, blkaddr, NIX_AF_ERR_INT_ENA_W1S, ~0ULL);
+ nix_event_ctx->nix_af_rvu_err = 0;
+ }
+ if (nix_event_ctx->nix_af_rvu_ras) {
+ rvu_write64(rvu, blkaddr, NIX_AF_RAS_ENA_W1S, ~0ULL);
+ nix_event_ctx->nix_af_rvu_ras = 0;
+ }
+
+ return 0;
+}
+
+static const struct devlink_health_reporter_ops rvu_nix_fault_reporter_ops = {
+ .name = "hw_nix",
+ .dump = rvu_nix_reporter_dump,
+ .recover = rvu_nix_reporter_recover,
+};
+
+static int rvu_nix_health_reporters_create(struct rvu_devlink *rvu_dl)
+{
+ struct devlink_health_reporter *rvu_nix_health_reporter;
+ struct rvu_nix_event_ctx *nix_event_context;
+ struct rvu *rvu = rvu_dl->rvu;
+
+ nix_event_context = kzalloc(sizeof(*nix_event_context), GFP_KERNEL);
+ if (!nix_event_context)
+ return -ENOMEM;
+
+ rvu_dl->nix_event_ctx = nix_event_context;
+ rvu_nix_health_reporter = devlink_health_reporter_create(rvu_dl->dl,
+ &rvu_nix_fault_reporter_ops,
+ 0, rvu);
+ if (IS_ERR(rvu_nix_health_reporter)) {
+ dev_warn(rvu->dev, "Failed to create nix reporter, err = %ld\n",
+ PTR_ERR(rvu_nix_health_reporter));
+ return PTR_ERR(rvu_nix_health_reporter);
+ }
+
+ rvu_dl->rvu_nix_health_reporter = rvu_nix_health_reporter;
+ rvu_nix_register_interrupts(rvu);
+ return 0;
+}
+
+static void rvu_nix_health_reporters_destroy(struct rvu_devlink *rvu_dl)
+{
+ struct rvu *rvu = rvu_dl->rvu;
+
+ if (!rvu_dl->rvu_nix_health_reporter)
+ return;
+
+ devlink_health_reporter_destroy(rvu_dl->rvu_nix_health_reporter);
+ rvu_nix_unregister_interrupts(rvu);
+}
+
static irqreturn_t rvu_npa_af_rvu_intr_handler(int irq, void *rvu_irq)
{
struct rvu_npa_event_ctx *npa_event_context;
@@ -214,7 +618,7 @@ static irqreturn_t rvu_npa_af_ras_intr_handler(int irq, void *rvu_irq)
/* Clear interrupts */
rvu_write64(rvu, blkaddr, NPA_AF_RAS, intr);
rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1C, ~0ULL);
- devlink_health_report(rvu_dl->rvu_npa_health_reporter, "HW NPA_AF_RAS Error reported",
+ devlink_health_report(rvu_dl->rvu_npa_health_reporter, "NPA_AF_RAS Error",
npa_event_context);
return IRQ_HANDLED;
}
@@ -481,9 +885,14 @@ static void rvu_npa_health_reporters_destroy(struct rvu_devlink *rvu_dl)
static int rvu_health_reporters_create(struct rvu *rvu)
{
struct rvu_devlink *rvu_dl;
+ int err;
rvu_dl = rvu->rvu_dl;
- return rvu_npa_health_reporters_create(rvu_dl);
+ err = rvu_npa_health_reporters_create(rvu_dl);
+ if (err)
+ return err;
+
+ return rvu_nix_health_reporters_create(rvu_dl);
}
static void rvu_health_reporters_destroy(struct rvu *rvu)
@@ -495,6 +904,7 @@ static void rvu_health_reporters_destroy(struct rvu *rvu)
rvu_dl = rvu->rvu_dl;
rvu_npa_health_reporters_destroy(rvu_dl);
+ rvu_nix_health_reporters_destroy(rvu_dl);
}
static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
index e04603a9952c..cfc513d945a0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -37,11 +37,42 @@ struct rvu_npa_event_ctx {
u64 npa_af_rvu_ras;
};
+struct rvu_nix_event_cnt {
+ u64 unmap_slot_count;
+ u64 aq_inst_count;
+ u64 aq_res_count;
+ u64 aq_db_count;
+ u64 rx_on_unmap_pf_count;
+ u64 rx_mcast_repl_count;
+ u64 rx_mcast_memfault_count;
+ u64 rx_mcast_wqe_memfault_count;
+ u64 rx_mirror_wqe_memfault_count;
+ u64 rx_mirror_pktw_memfault_count;
+ u64 rx_mcast_pktw_memfault_count;
+ u64 poison_aq_inst_count;
+ u64 poison_aq_res_count;
+ u64 poison_aq_cxt_count;
+ u64 rx_mirror_data_poison_count;
+ u64 rx_mcast_data_poison_count;
+ u64 rx_mirror_wqe_poison_count;
+ u64 rx_mcast_wqe_poison_count;
+ u64 rx_mce_poison_count;
+};
+
+struct rvu_nix_event_ctx {
+ struct rvu_nix_event_cnt nix_event_cnt;
+ u64 nix_af_rvu_int;
+ u64 nix_af_rvu_err;
+ u64 nix_af_rvu_ras;
+};
+
struct rvu_devlink {
struct devlink *dl;
struct rvu *rvu;
struct devlink_health_reporter *rvu_npa_health_reporter;
struct rvu_npa_event_ctx *npa_event_ctx;
+ struct devlink_health_reporter *rvu_nix_health_reporter;
+ struct rvu_nix_event_ctx *nix_event_ctx;
};
/* Devlink APIs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
index e2153d47c373..5e15f4fc11e3 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
@@ -74,6 +74,16 @@ enum npa_af_int_vec_e {
NPA_AF_INT_VEC_CNT = 0x5,
};
+/* NIX Admin function Interrupt Vector Enumeration */
+enum nix_af_int_vec_e {
+ NIX_AF_INT_VEC_RVU = 0x0,
+ NIX_AF_INT_VEC_GEN = 0x1,
+ NIX_AF_INT_VEC_AQ_DONE = 0x2,
+ NIX_AF_INT_VEC_AF_ERR = 0x3,
+ NIX_AF_INT_VEC_POISON = 0x4,
+ NIX_AF_INT_VEC_CNT = 0x5,
+};
+
/**
* RVU PF Interrupt Vector Enumeration
*/
--
2.25.1
^ permalink raw reply related
* [PATCHv4 net-next 2/3] octeontx2-af: Add devlink health reporters for NPA
From: George Cherian @ 2020-11-21 4:02 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: kuba, davem, sgoutham, lcherian, gakula, masahiroy,
george.cherian, willemdebruijn.kernel, saeed
In-Reply-To: <20201121040201.3171542-1-george.cherian@marvell.com>
Add health reporters for RVU NPA block.
NPA Health reporters handle following HW event groups
- GENERAL events
- ERROR events
- RAS events
- RVU event
An event counter per event is maintained in SW.
Output:
# devlink health
pci/0002:01:00.0:
reporter npa
state healthy error 0 recover 0
# devlink health dump show pci/0002:01:00.0 reporter npa
NPA_AF_GENERAL:
Unmap PF Error: 0
Free Disabled for NIX0 RX: 0
Free Disabled for NIX0 TX: 0
Free Disabled for NIX1 RX: 0
Free Disabled for NIX1 TX: 0
Free Disabled for SSO: 0
Free Disabled for TIM: 0
Free Disabled for DPI: 0
Free Disabled for AURA: 0
Alloc Disabled for Resvd: 0
NPA_AF_ERR:
Memory Fault on NPA_AQ_INST_S read: 0
Memory Fault on NPA_AQ_RES_S write: 0
AQ Doorbell Error: 0
Poisoned data on NPA_AQ_INST_S read: 0
Poisoned data on NPA_AQ_RES_S write: 0
Poisoned data on HW context read: 0
NPA_AF_RVU:
Unmap Slot Error: 0
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: George Cherian <george.cherian@marvell.com>
---
.../marvell/octeontx2/af/rvu_devlink.c | 492 +++++++++++++++++-
.../marvell/octeontx2/af/rvu_devlink.h | 31 ++
.../marvell/octeontx2/af/rvu_struct.h | 23 +
3 files changed, 545 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 04ef945e7e75..b7f0691d86b0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -5,10 +5,498 @@
*
*/
+#include<linux/bitfield.h>
+
#include "rvu.h"
+#include "rvu_reg.h"
+#include "rvu_struct.h"
#define DRV_NAME "octeontx2-af"
+static int rvu_report_pair_start(struct devlink_fmsg *fmsg, const char *name)
+{
+ int err;
+
+ err = devlink_fmsg_pair_nest_start(fmsg, name);
+ if (err)
+ return err;
+
+ return devlink_fmsg_obj_nest_start(fmsg);
+}
+
+static int rvu_report_pair_end(struct devlink_fmsg *fmsg)
+{
+ int err;
+
+ err = devlink_fmsg_obj_nest_end(fmsg);
+ if (err)
+ return err;
+
+ return devlink_fmsg_pair_nest_end(fmsg);
+}
+
+static bool rvu_common_request_irq(struct rvu *rvu, int offset,
+ const char *name, irq_handler_t fn)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int rc;
+
+ sprintf(&rvu->irq_name[offset * NAME_SIZE], name);
+ rc = request_irq(pci_irq_vector(rvu->pdev, offset), fn, 0,
+ &rvu->irq_name[offset * NAME_SIZE], rvu_dl);
+ if (rc)
+ dev_warn(rvu->dev, "Failed to register %s irq\n", name);
+ else
+ rvu->irq_allocated[offset] = true;
+
+ return rvu->irq_allocated[offset];
+}
+
+static irqreturn_t rvu_npa_af_rvu_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_context = rvu_dl->npa_event_ctx;
+ npa_event_count = &npa_event_context->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_RVU_INT);
+ npa_event_context->npa_af_rvu_int = intr;
+
+ if (intr & BIT_ULL(0))
+ npa_event_count->unmap_slot_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT, intr);
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_npa_health_reporter, "NPA_AF_RVU Error",
+ npa_event_context);
+
+ return IRQ_HANDLED;
+}
+
+static int rvu_npa_inpq_to_cnt(u16 in,
+ struct rvu_npa_event_cnt *npa_event_count)
+{
+ switch (in) {
+ case 0:
+ return 0;
+ case BIT(NPA_INPQ_NIX0_RX):
+ return npa_event_count->free_dis_nix0_rx_count++;
+ case BIT(NPA_INPQ_NIX0_TX):
+ return npa_event_count->free_dis_nix0_tx_count++;
+ case BIT(NPA_INPQ_NIX1_RX):
+ return npa_event_count->free_dis_nix1_rx_count++;
+ case BIT(NPA_INPQ_NIX1_TX):
+ return npa_event_count->free_dis_nix1_tx_count++;
+ case BIT(NPA_INPQ_SSO):
+ return npa_event_count->free_dis_sso_count++;
+ case BIT(NPA_INPQ_TIM):
+ return npa_event_count->free_dis_tim_count++;
+ case BIT(NPA_INPQ_DPI):
+ return npa_event_count->free_dis_dpi_count++;
+ case BIT(NPA_INPQ_AURA_OP):
+ return npa_event_count->free_dis_aura_count++;
+ case BIT(NPA_INPQ_INTERNAL_RSV):
+ return npa_event_count->free_dis_rsvd_count++;
+ }
+
+ return npa_event_count->alloc_dis_rsvd_count++;
+}
+
+static irqreturn_t rvu_npa_af_gen_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr, val;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_context = rvu_dl->npa_event_ctx;
+ npa_event_count = &npa_event_context->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_GEN_INT);
+ npa_event_context->npa_af_rvu_gen = intr;
+
+ if (intr & BIT_ULL(32))
+ npa_event_count->unmap_pf_count++;
+
+ val = FIELD_GET(GENMASK(31, 16), intr);
+ rvu_npa_inpq_to_cnt(val, npa_event_count);
+
+ val = FIELD_GET(GENMASK(15, 0), intr);
+ rvu_npa_inpq_to_cnt(val, npa_event_count);
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT, intr);
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_npa_health_reporter, "NPA_AF_GEN Error",
+ npa_event_context);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_npa_af_err_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+ npa_event_context = rvu_dl->npa_event_ctx;
+ npa_event_count = &npa_event_context->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_ERR_INT);
+ npa_event_context->npa_af_rvu_err = intr;
+
+ if (intr & BIT_ULL(14))
+ npa_event_count->aq_inst_count++;
+
+ if (intr & BIT_ULL(13))
+ npa_event_count->aq_res_count++;
+
+ if (intr & BIT_ULL(12))
+ npa_event_count->aq_db_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT, intr);
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_npa_health_reporter, "NPA_AF_ERR Error",
+ npa_event_context);
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rvu_npa_af_ras_intr_handler(int irq, void *rvu_irq)
+{
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu_irq;
+ struct rvu *rvu;
+ int blkaddr;
+ u64 intr;
+
+ rvu = rvu_dl->rvu;
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return IRQ_NONE;
+
+ npa_event_context = rvu_dl->npa_event_ctx;
+ npa_event_count = &npa_event_context->npa_event_cnt;
+ intr = rvu_read64(rvu, blkaddr, NPA_AF_RAS);
+ npa_event_context->npa_af_rvu_ras = intr;
+
+ if (intr & BIT_ULL(34))
+ npa_event_count->poison_aq_inst_count++;
+
+ if (intr & BIT_ULL(33))
+ npa_event_count->poison_aq_res_count++;
+
+ if (intr & BIT_ULL(32))
+ npa_event_count->poison_aq_cxt_count++;
+
+ /* Clear interrupts */
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS, intr);
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1C, ~0ULL);
+ devlink_health_report(rvu_dl->rvu_npa_health_reporter, "HW NPA_AF_RAS Error reported",
+ npa_event_context);
+ return IRQ_HANDLED;
+}
+
+static void rvu_npa_unregister_interrupts(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int i, offs, blkaddr;
+ u64 reg;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return;
+
+ reg = rvu_read64(rvu, blkaddr, NPA_PRIV_AF_INT_CFG);
+ offs = reg & 0x3FF;
+
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1C, ~0ULL);
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1C, ~0ULL);
+
+ for (i = 0; i < NPA_AF_INT_VEC_CNT; i++)
+ if (rvu->irq_allocated[offs + i]) {
+ free_irq(pci_irq_vector(rvu->pdev, offs + i), rvu_dl);
+ rvu->irq_allocated[offs + i] = false;
+ }
+}
+
+static int rvu_npa_register_interrupts(struct rvu *rvu)
+{
+ int blkaddr, base;
+ bool rc;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ /* Get NPA AF MSIX vectors offset. */
+ base = rvu_read64(rvu, blkaddr, NPA_PRIV_AF_INT_CFG) & 0x3ff;
+ if (!base) {
+ dev_warn(rvu->dev,
+ "Failed to get NPA_AF_INT vector offsets\n");
+ return 0;
+ }
+
+ /* Register and enable NPA_AF_RVU_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_RVU,
+ "NPA_AF_RVU_INT",
+ rvu_npa_af_rvu_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_GEN_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_GEN,
+ "NPA_AF_RVU_GEN",
+ rvu_npa_af_gen_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_ERR_INT interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_AF_ERR,
+ "NPA_AF_ERR_INT",
+ rvu_npa_af_err_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1S, ~0ULL);
+
+ /* Register and enable NPA_AF_RAS interrupt */
+ rc = rvu_common_request_irq(rvu, base + NPA_AF_INT_VEC_POISON,
+ "NPA_AF_RAS",
+ rvu_npa_af_ras_intr_handler);
+ if (!rc)
+ goto err;
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1S, ~0ULL);
+
+ return 0;
+err:
+ rvu_npa_unregister_interrupts(rvu);
+ return rc;
+}
+
+static int rvu_npa_report_show(struct devlink_fmsg *fmsg, struct rvu *rvu)
+{
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu_npa_event_cnt *npa_event_count;
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ int err;
+
+ npa_event_context = rvu_dl->npa_event_ctx;
+ npa_event_count = &npa_event_context->npa_event_cnt;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_GENERAL");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap PF Error",
+ npa_event_count->unmap_pf_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX0 RX",
+ npa_event_count->free_dis_nix0_rx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX0 TX",
+ npa_event_count->free_dis_nix0_tx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX1 RX",
+ npa_event_count->free_dis_nix1_rx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for NIX1 TX",
+ npa_event_count->free_dis_nix1_tx_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for SSO",
+ npa_event_count->free_dis_sso_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for TIM",
+ npa_event_count->free_dis_tim_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for DPI",
+ npa_event_count->free_dis_dpi_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tFree Disabled for AURA",
+ npa_event_count->free_dis_aura_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAlloc Disabled for Resvd",
+ npa_event_count->alloc_dis_rsvd_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_ERR");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tMemory Fault on NPA_AQ_INST_S read",
+ npa_event_count->aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tMemory Fault on NPA_AQ_RES_S write",
+ npa_event_count->aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tAQ Doorbell Error",
+ npa_event_count->aq_db_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NPA_AQ_INST_S read",
+ npa_event_count->poison_aq_inst_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on NPA_AQ_RES_S write",
+ npa_event_count->poison_aq_res_count);
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\n\tPoisoned data on HW context read",
+ npa_event_count->poison_aq_cxt_count);
+ if (err)
+ return err;
+ err = rvu_report_pair_end(fmsg);
+ if (err)
+ return err;
+ err = rvu_report_pair_start(fmsg, "NPA_AF_RVU");
+ if (err)
+ return err;
+ err = devlink_fmsg_u64_pair_put(fmsg, "\tUnmap Slot Error",
+ npa_event_count->unmap_slot_count);
+ if (err)
+ return err;
+ return rvu_report_pair_end(fmsg);
+}
+
+static int rvu_npa_reporter_dump(struct devlink_health_reporter *reporter,
+ struct devlink_fmsg *fmsg, void *ctx,
+ struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+
+ return rvu_npa_report_show(fmsg, rvu);
+}
+
+static int rvu_npa_reporter_recover(struct devlink_health_reporter *reporter,
+ void *ctx, struct netlink_ext_ack *netlink_extack)
+{
+ struct rvu *rvu = devlink_health_reporter_priv(reporter);
+ struct rvu_npa_event_ctx *npa_event_ctx = ctx;
+ int blkaddr;
+
+ blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NPA, 0);
+ if (blkaddr < 0)
+ return blkaddr;
+
+ if (npa_event_ctx->npa_af_rvu_int) {
+ rvu_write64(rvu, blkaddr, NPA_AF_RVU_INT_ENA_W1S, ~0ULL);
+ npa_event_ctx->npa_af_rvu_int = 0;
+ }
+ if (npa_event_ctx->npa_af_rvu_gen) {
+ rvu_write64(rvu, blkaddr, NPA_AF_GEN_INT_ENA_W1S, ~0ULL);
+ npa_event_ctx->npa_af_rvu_gen = 0;
+ }
+ if (npa_event_ctx->npa_af_rvu_err) {
+ rvu_write64(rvu, blkaddr, NPA_AF_ERR_INT_ENA_W1S, ~0ULL);
+ npa_event_ctx->npa_af_rvu_err = 0;
+ }
+ if (npa_event_ctx->npa_af_rvu_ras) {
+ rvu_write64(rvu, blkaddr, NPA_AF_RAS_ENA_W1S, ~0ULL);
+ npa_event_ctx->npa_af_rvu_ras = 0;
+ }
+
+ return 0;
+}
+
+static const struct devlink_health_reporter_ops rvu_npa_hw_fault_reporter_ops = {
+ .name = "hw_npa",
+ .recover = rvu_npa_reporter_recover,
+ .dump = rvu_npa_reporter_dump,
+};
+
+static int rvu_npa_health_reporters_create(struct rvu_devlink *rvu_dl)
+{
+ struct devlink_health_reporter *rvu_npa_health_reporter;
+ struct rvu_npa_event_ctx *npa_event_context;
+ struct rvu *rvu = rvu_dl->rvu;
+
+ npa_event_context = kzalloc(sizeof(*npa_event_context), GFP_KERNEL);
+ if (!npa_event_context)
+ return -ENOMEM;
+
+ rvu_dl->npa_event_ctx = npa_event_context;
+ rvu_npa_health_reporter = devlink_health_reporter_create(rvu_dl->dl,
+ &rvu_npa_hw_fault_reporter_ops,
+ 0, rvu);
+ if (IS_ERR(rvu_npa_health_reporter)) {
+ dev_warn(rvu->dev, "Failed to create npa reporter, err =%ld\n",
+ PTR_ERR(rvu_npa_health_reporter));
+ return PTR_ERR(rvu_npa_health_reporter);
+ }
+
+ rvu_dl->rvu_npa_health_reporter = rvu_npa_health_reporter;
+ rvu_npa_register_interrupts(rvu);
+
+ return 0;
+}
+
+static void rvu_npa_health_reporters_destroy(struct rvu_devlink *rvu_dl)
+{
+ struct rvu *rvu = rvu_dl->rvu;
+
+ if (!rvu_dl->rvu_npa_health_reporter)
+ return;
+
+ devlink_health_reporter_destroy(rvu_dl->rvu_npa_health_reporter);
+ rvu_npa_unregister_interrupts(rvu);
+ kfree(rvu_dl->npa_event_ctx);
+}
+
+static int rvu_health_reporters_create(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+
+ rvu_dl = rvu->rvu_dl;
+ return rvu_npa_health_reporters_create(rvu_dl);
+}
+
+static void rvu_health_reporters_destroy(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+
+ if (!rvu->rvu_dl)
+ return;
+
+ rvu_dl = rvu->rvu_dl;
+ rvu_npa_health_reporters_destroy(rvu_dl);
+}
+
static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
struct netlink_ext_ack *extack)
{
@@ -55,7 +543,8 @@ int rvu_register_dl(struct rvu *rvu)
rvu_dl->dl = dl;
rvu_dl->rvu = rvu;
rvu->rvu_dl = rvu_dl;
- return 0;
+
+ return rvu_health_reporters_create(rvu);
}
void rvu_unregister_dl(struct rvu *rvu)
@@ -66,6 +555,7 @@ void rvu_unregister_dl(struct rvu *rvu)
if (!dl)
return;
+ rvu_health_reporters_destroy(rvu);
devlink_unregister(dl);
devlink_free(dl);
kfree(rvu_dl);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
index 1ed6dde79a4e..e04603a9952c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -8,9 +8,40 @@
#ifndef RVU_DEVLINK_H
#define RVU_DEVLINK_H
+struct rvu_npa_event_cnt {
+ u64 unmap_slot_count;
+ u64 unmap_pf_count;
+ u64 free_dis_nix0_rx_count;
+ u64 free_dis_nix0_tx_count;
+ u64 free_dis_nix1_rx_count;
+ u64 free_dis_nix1_tx_count;
+ u64 free_dis_sso_count;
+ u64 free_dis_tim_count;
+ u64 free_dis_dpi_count;
+ u64 free_dis_aura_count;
+ u64 free_dis_rsvd_count;
+ u64 alloc_dis_rsvd_count;
+ u64 aq_inst_count;
+ u64 aq_res_count;
+ u64 aq_db_count;
+ u64 poison_aq_inst_count;
+ u64 poison_aq_res_count;
+ u64 poison_aq_cxt_count;
+};
+
+struct rvu_npa_event_ctx {
+ struct rvu_npa_event_cnt npa_event_cnt;
+ u64 npa_af_rvu_int;
+ u64 npa_af_rvu_gen;
+ u64 npa_af_rvu_err;
+ u64 npa_af_rvu_ras;
+};
+
struct rvu_devlink {
struct devlink *dl;
struct rvu *rvu;
+ struct devlink_health_reporter *rvu_npa_health_reporter;
+ struct rvu_npa_event_ctx *npa_event_ctx;
};
/* Devlink APIs */
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
index 723643868589..e2153d47c373 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_struct.h
@@ -64,6 +64,16 @@ enum rvu_af_int_vec_e {
RVU_AF_INT_VEC_CNT = 0x5,
};
+/* NPA Admin function Interrupt Vector Enumeration */
+enum npa_af_int_vec_e {
+ NPA_AF_INT_VEC_RVU = 0x0,
+ NPA_AF_INT_VEC_GEN = 0x1,
+ NPA_AF_INT_VEC_AQ_DONE = 0x2,
+ NPA_AF_INT_VEC_AF_ERR = 0x3,
+ NPA_AF_INT_VEC_POISON = 0x4,
+ NPA_AF_INT_VEC_CNT = 0x5,
+};
+
/**
* RVU PF Interrupt Vector Enumeration
*/
@@ -104,6 +114,19 @@ enum npa_aq_instop {
NPA_AQ_INSTOP_UNLOCK = 0x5,
};
+/* ALLOC/FREE input queues Enumeration from coprocessors */
+enum npa_inpq {
+ NPA_INPQ_NIX0_RX = 0x0,
+ NPA_INPQ_NIX0_TX = 0x1,
+ NPA_INPQ_NIX1_RX = 0x2,
+ NPA_INPQ_NIX1_TX = 0x3,
+ NPA_INPQ_SSO = 0x4,
+ NPA_INPQ_TIM = 0x5,
+ NPA_INPQ_DPI = 0x6,
+ NPA_INPQ_AURA_OP = 0xe,
+ NPA_INPQ_INTERNAL_RSV = 0xf,
+};
+
/* NPA admin queue instruction structure */
struct npa_aq_inst_s {
#if defined(__BIG_ENDIAN_BITFIELD)
--
2.25.1
^ permalink raw reply related
* [PATCHv4 net-next 1/3] octeontx2-af: Add devlink suppoort to af driver
From: George Cherian @ 2020-11-21 4:01 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: kuba, davem, sgoutham, lcherian, gakula, masahiroy,
george.cherian, willemdebruijn.kernel, saeed
In-Reply-To: <20201121040201.3171542-1-george.cherian@marvell.com>
Add devlink support to AF driver. Basic devlink support is added.
Currently info_get is the only supported devlink ops.
devlink ouptput looks like this
# devlink dev
pci/0002:01:00.0
# devlink dev info
pci/0002:01:00.0:
driver octeontx2-af
versions:
fixed:
mbox version: 9
Signed-off-by: Sunil Kovvuri Goutham <sgoutham@marvell.com>
Signed-off-by: Jerin Jacob <jerinj@marvell.com>
Signed-off-by: George Cherian <george.cherian@marvell.com>
---
.../net/ethernet/marvell/octeontx2/Kconfig | 1 +
.../ethernet/marvell/octeontx2/af/Makefile | 2 +-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 9 ++-
.../net/ethernet/marvell/octeontx2/af/rvu.h | 4 ++
.../marvell/octeontx2/af/rvu_devlink.c | 72 +++++++++++++++++++
.../marvell/octeontx2/af/rvu_devlink.h | 20 ++++++
6 files changed, 106 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
diff --git a/drivers/net/ethernet/marvell/octeontx2/Kconfig b/drivers/net/ethernet/marvell/octeontx2/Kconfig
index 543a1d047567..16caa02095fe 100644
--- a/drivers/net/ethernet/marvell/octeontx2/Kconfig
+++ b/drivers/net/ethernet/marvell/octeontx2/Kconfig
@@ -9,6 +9,7 @@ config OCTEONTX2_MBOX
config OCTEONTX2_AF
tristate "Marvell OcteonTX2 RVU Admin Function driver"
select OCTEONTX2_MBOX
+ select NET_DEVLINK
depends on (64BIT && COMPILE_TEST) || ARM64
depends on PCI
help
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/Makefile b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
index 7100d1dd856e..eb535c98ca38 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/Makefile
+++ b/drivers/net/ethernet/marvell/octeontx2/af/Makefile
@@ -10,4 +10,4 @@ obj-$(CONFIG_OCTEONTX2_AF) += octeontx2_af.o
octeontx2_mbox-y := mbox.o rvu_trace.o
octeontx2_af-y := cgx.o rvu.o rvu_cgx.o rvu_npa.o rvu_nix.o \
rvu_reg.o rvu_npc.o rvu_debugfs.o ptp.o rvu_npc_fs.o \
- rvu_cpt.o
+ rvu_cpt.o rvu_devlink.o
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index 9f901c0edcbb..e8fd712860a1 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2826,17 +2826,23 @@ static int rvu_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (err)
goto err_flr;
+ err = rvu_register_dl(rvu);
+ if (err)
+ goto err_irq;
+
rvu_setup_rvum_blk_revid(rvu);
/* Enable AF's VFs (if any) */
err = rvu_enable_sriov(rvu);
if (err)
- goto err_irq;
+ goto err_dl;
/* Initialize debugfs */
rvu_dbg_init(rvu);
return 0;
+err_dl:
+ rvu_unregister_dl(rvu);
err_irq:
rvu_unregister_interrupts(rvu);
err_flr:
@@ -2868,6 +2874,7 @@ static void rvu_remove(struct pci_dev *pdev)
rvu_dbg_exit(rvu);
rvu_unregister_interrupts(rvu);
+ rvu_unregister_dl(rvu);
rvu_flr_wq_destroy(rvu);
rvu_cgx_exit(rvu);
rvu_fwdata_exit(rvu);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index b6c0977499ab..b1a6ecfd563e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -12,7 +12,10 @@
#define RVU_H
#include <linux/pci.h>
+#include <net/devlink.h>
+
#include "rvu_struct.h"
+#include "rvu_devlink.h"
#include "common.h"
#include "mbox.h"
#include "npc.h"
@@ -422,6 +425,7 @@ struct rvu {
#ifdef CONFIG_DEBUG_FS
struct rvu_debugfs rvu_dbg;
#endif
+ struct rvu_devlink *rvu_dl;
};
static inline void rvu_write64(struct rvu *rvu, u64 block, u64 offset, u64 val)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
new file mode 100644
index 000000000000..04ef945e7e75
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Marvell OcteonTx2 RVU Devlink
+ *
+ * Copyright (C) 2020 Marvell.
+ *
+ */
+
+#include "rvu.h"
+
+#define DRV_NAME "octeontx2-af"
+
+static int rvu_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ char buf[10];
+ int err;
+
+ err = devlink_info_driver_name_put(req, DRV_NAME);
+ if (err)
+ return err;
+
+ sprintf(buf, "%X", OTX2_MBOX_VERSION);
+ return devlink_info_version_fixed_put(req, "mbox version:", buf);
+}
+
+static const struct devlink_ops rvu_devlink_ops = {
+ .info_get = rvu_devlink_info_get,
+};
+
+int rvu_register_dl(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl;
+ struct devlink *dl;
+ int err;
+
+ rvu_dl = kzalloc(sizeof(*rvu_dl), GFP_KERNEL);
+ if (!rvu_dl)
+ return -ENOMEM;
+
+ dl = devlink_alloc(&rvu_devlink_ops, sizeof(struct rvu_devlink));
+ if (!dl) {
+ dev_warn(rvu->dev, "devlink_alloc failed\n");
+ kfree(rvu_dl);
+ return -ENOMEM;
+ }
+
+ err = devlink_register(dl, rvu->dev);
+ if (err) {
+ dev_err(rvu->dev, "devlink register failed with error %d\n", err);
+ devlink_free(dl);
+ kfree(rvu_dl);
+ return err;
+ }
+
+ rvu_dl->dl = dl;
+ rvu_dl->rvu = rvu;
+ rvu->rvu_dl = rvu_dl;
+ return 0;
+}
+
+void rvu_unregister_dl(struct rvu *rvu)
+{
+ struct rvu_devlink *rvu_dl = rvu->rvu_dl;
+ struct devlink *dl = rvu_dl->dl;
+
+ if (!dl)
+ return;
+
+ devlink_unregister(dl);
+ devlink_free(dl);
+ kfree(rvu_dl);
+}
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
new file mode 100644
index 000000000000..1ed6dde79a4e
--- /dev/null
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Marvell OcteonTx2 RVU Devlink
+ *
+ * Copyright (C) 2020 Marvell.
+ *
+ */
+
+#ifndef RVU_DEVLINK_H
+#define RVU_DEVLINK_H
+
+struct rvu_devlink {
+ struct devlink *dl;
+ struct rvu *rvu;
+};
+
+/* Devlink APIs */
+int rvu_register_dl(struct rvu *rvu);
+void rvu_unregister_dl(struct rvu *rvu);
+
+#endif /* RVU_DEVLINK_H */
--
2.25.1
^ permalink raw reply related
* [PATCHv3 net-next 0/3] Add devlink and devlink health reporters to
From: George Cherian @ 2020-11-21 4:01 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: kuba, davem, sgoutham, lcherian, gakula, masahiroy,
george.cherian, willemdebruijn.kernel, saeed
Add basic devlink and devlink health reporters.
Devlink health reporters are added for NPA and NIX blocks.
These reporters report the error count in respective blocks.
Address Jakub's comment to add devlink support for error reporting.
https://www.spinics.net/lists/netdev/msg670712.html
Change-log:
v4
- Rebase to net-next (no logic changes).
v3
- Address Saeed's comments on v2.
- Renamed the reporter name as hw_*.
- Call devlink_health_report() when an event is raised.
- Added recover op too.
v2
- Address Willem's comments on v1.
- Fixed the sparse issues, reported by Jakub.
George Cherian (3):
octeontx2-af: Add devlink suppoort to af driver
octeontx2-af: Add devlink health reporters for NPA
octeontx2-af: Add devlink health reporters for NIX
.../net/ethernet/marvell/octeontx2/Kconfig | 1 +
.../ethernet/marvell/octeontx2/af/Makefile | 2 +-
.../net/ethernet/marvell/octeontx2/af/rvu.c | 9 +-
.../net/ethernet/marvell/octeontx2/af/rvu.h | 4 +
.../marvell/octeontx2/af/rvu_devlink.c | 972 ++++++++++++++++++
.../marvell/octeontx2/af/rvu_devlink.h | 82 ++
.../marvell/octeontx2/af/rvu_struct.h | 33 +
7 files changed, 1101 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
create mode 100644 drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.h
--
2.25.1
^ permalink raw reply
* Re: [PATCH net-next v2 0/9] ibmvnic: Performance improvements and other updates
From: Jakub Kicinski @ 2020-11-21 3:52 UTC (permalink / raw)
To: Thomas Falcon
Cc: netdev, linuxppc-dev, cforno12, ljp, ricklind, dnbanerg, drt,
brking, sukadev
In-Reply-To: <1605748345-32062-1-git-send-email-tlfalcon@linux.ibm.com>
On Wed, 18 Nov 2020 19:12:16 -0600 Thomas Falcon wrote:
> The first three patches utilize a hypervisor call allowing multiple
> TX and RX buffer replenishment descriptors to be sent in one operation,
> which significantly reduces hypervisor call overhead. The xmit_more
> and Byte Queue Limit API's are leveraged to provide this support
> for TX descriptors.
>
> The subsequent two patches remove superfluous code and members in
> TX completion handling function and TX buffer structure, respectively,
> and remove unused routines.
>
> Finally, four patches which ensure that device queue memory is
> cache-line aligned, resolving slowdowns observed in PCI traces,
> as well as optimize the driver's NAPI polling function and
> to RX buffer replenishment are provided by Dwip Banerjee.
>
> This series provides significant performance improvements, allowing
> the driver to fully utilize 100Gb NIC's.
Applied, thanks!
^ permalink raw reply
* [net PATCH] tcp: Set ECT0 bit in tos/tclass for synack when BPF needs ECN
From: Alexander Duyck @ 2020-11-21 3:47 UTC (permalink / raw)
To: netdev; +Cc: bpf, daniel, kafai, kernel-team, edumazet, brakmo, alexanderduyck
From: Alexander Duyck <alexanderduyck@fb.com>
When a BPF program is used to select between a type of TCP congestion
control algorithm that uses either ECN or not there is a case where the
synack for the frame was coming up without the ECT0 bit set. A bit of
research found that this was due to the final socket being configured to
dctcp while the listener socket was staying in cubic.
To reproduce it all that is needed is to monitor TCP traffic while running
the sample bpf program "samples/bpf/tcp_cong_kern.c". What is observed,
assuming tcp_dctcp module is loaded or compiled in and the traffic matches
the rules in the sample file, is that for all frames with the exception of
the synack the ECT0 bit is set.
To address that it is necessary to make one additional call to
tcp_bpf_ca_needs_ecn using the request socket and then use the output of
that to set the ECT0 bit for the tos/tclass of the packet.
Fixes: 91b5b21c7c16 ("bpf: Add support for changing congestion control")
Signed-off-by: Alexander Duyck <alexanderduyck@fb.com>
---
net/ipv4/tcp_ipv4.c | 12 ++++++++----
net/ipv6/tcp_ipv6.c | 9 +++++++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c5f8b686aa82..d20d779b18f3 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -980,13 +980,17 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
skb = tcp_make_synack(sk, dst, req, foc, synack_type, syn_skb);
- tos = sock_net(sk)->ipv4.sysctl_tcp_reflect_tos ?
- tcp_rsk(req)->syn_tos & ~INET_ECN_MASK :
- inet_sk(sk)->tos;
-
if (skb) {
__tcp_v4_send_check(skb, ireq->ir_loc_addr, ireq->ir_rmt_addr);
+ tos = sock_net(sk)->ipv4.sysctl_tcp_reflect_tos ?
+ tcp_rsk(req)->syn_tos & ~INET_ECN_MASK :
+ inet_sk(sk)->tos;
+
+ if (!INET_ECN_is_capable(tos) &&
+ tcp_bpf_ca_needs_ecn((struct sock *)req))
+ tos |= INET_ECN_ECT_0;
+
rcu_read_lock();
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3d49e8d0afee..e2b360e9bec4 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -527,11 +527,16 @@ static int tcp_v6_send_synack(const struct sock *sk, struct dst_entry *dst,
if (np->repflow && ireq->pktopts)
fl6->flowlabel = ip6_flowlabel(ipv6_hdr(ireq->pktopts));
- rcu_read_lock();
- opt = ireq->ipv6_opt;
tclass = sock_net(sk)->ipv4.sysctl_tcp_reflect_tos ?
tcp_rsk(req)->syn_tos & ~INET_ECN_MASK :
np->tclass;
+
+ if (!INET_ECN_is_capable(tclass) &&
+ tcp_bpf_ca_needs_ecn((struct sock *)req))
+ tclass |= INET_ECN_ECT_0;
+
+ rcu_read_lock();
+ opt = ireq->ipv6_opt;
if (!opt)
opt = rcu_dereference(np->opt);
err = ip6_xmit(sk, skb, fl6, sk->sk_mark, opt,
^ permalink raw reply related
* Re: [PATCH net-next 4/6] net: ipa: support retries on generic GSI commands
From: Jakub Kicinski @ 2020-11-21 3:46 UTC (permalink / raw)
To: Alex Elder
Cc: davem, evgreen, subashab, cpratapa, bjorn.andersson, netdev,
linux-kernel
In-Reply-To: <88104bdd-f464-326a-264e-570a8e4a81c0@linaro.org>
On Fri, 20 Nov 2020 21:31:09 -0600 Alex Elder wrote:
> On 11/20/20 8:49 PM, Jakub Kicinski wrote:
> > On Thu, 19 Nov 2020 16:49:27 -0600 Alex Elder wrote:
> >> + do
> >> + ret = gsi_generic_command(gsi, channel_id,
> >> + GSI_GENERIC_HALT_CHANNEL);
> >> + while (ret == -EAGAIN && retries--);
> >
> > This may well be the first time I've seen someone write a do while loop
> > without the curly brackets!
>
> I had them at one time, then saw I could get away
> without them. I don't have a preference but I see
> you accepted it as-is.
It was just an offhand comment, I don't have anything against it :)
^ permalink raw reply
* Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel
From: Hemant Kumar @ 2020-11-21 3:44 UTC (permalink / raw)
To: Shuah Khan, manivannan.sadhasivam, gregkh
Cc: linux-arm-msm, linux-kernel, jhugo, bbhatt, loic.poulain, netdev
In-Reply-To: <f337319e-d542-67b3-f31e-e4366d822d76@linuxfoundation.org>
Hi Shuah,
On 11/20/20 7:26 AM, Shuah Khan wrote:
> On 11/16/20 3:46 PM, Hemant Kumar wrote:
>> Loopback test opens the MHI device file node and writes
>> a data buffer to it. MHI UCI kernel space driver copies
>> the data and sends it to MHI uplink (Tx) LOOPBACK channel.
>> MHI device loops back the same data to MHI downlink (Rx)
>> LOOPBACK channel. This data is read by test application
>> and compared against the data sent. Test passes if data
>> buffer matches between Tx and Rx. Test application performs
>> open(), poll(), write(), read() and close() file operations.
>>
>> Signed-off-by: Hemant Kumar <hemantk@codeaurora.org>
>> ---
>> Documentation/mhi/uci.rst | 32 +
>> tools/testing/selftests/Makefile | 1 +
>> tools/testing/selftests/drivers/.gitignore | 1 +
>> tools/testing/selftests/drivers/mhi/Makefile | 8 +
>> tools/testing/selftests/drivers/mhi/config | 2 +
>> .../testing/selftests/drivers/mhi/loopback_test.c | 802
>> +++++++++++++++++++++
>> 6 files changed, 846 insertions(+)
>> create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
>> create mode 100644 tools/testing/selftests/drivers/mhi/config
>> create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c
>>
>> diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
>> index ce8740e..0a04afe 100644
>> --- a/Documentation/mhi/uci.rst
>> +++ b/Documentation/mhi/uci.rst
>> @@ -79,6 +79,38 @@ MHI client driver performs read operation, same
>> data gets looped back to MHI
>> host using LOOPBACK channel 1. LOOPBACK channel is used to verify
>> data path
>> and data integrity between MHI Host and MHI device.
>
> Nice.
[..]
>> +
>> +enum debug_level {
>> + DBG_LVL_VERBOSE,
>> + DBG_LVL_INFO,
>> + DBG_LVL_ERROR,
>> +};
>> +
>> +enum test_status {
>> + TEST_STATUS_SUCCESS,
>> + TEST_STATUS_ERROR,
>> + TEST_STATUS_NO_DEV,
>> +};
>> +
>
> Since you are running this test as part of kselftest, please use
> ksft errors nd returns.
Are you suggesting to use following macros instead of test_status enum ?
#define KSFT_PASS 0
#define KSFT_FAIL 1
>
>> +struct lb_test_ctx {
>> + char dev_node[256];
>> + unsigned char *tx_buff;
>> + unsigned char *rx_buff;
>> + unsigned int rx_pkt_count;
>> + unsigned int tx_pkt_count;
>> + int iterations;
>> + bool iter_complete;
>> + bool comp_complete;
>> + bool test_complete;
>> + bool all_complete;
>> + unsigned long buff_size;
>> + long byte_recvd;
>> + long byte_sent;
>> +};
>> +
>> +bool force_exit;
>> +char write_data = 'a';
>> +int completed_iterations;
>> +
>> +struct lb_test_ctx test_ctxt;
>> +enum debug_level msg_lvl;
>> +struct pollfd read_fd;
>> +struct pollfd write_fd;
>> +enum test_status mhi_test_return_value;
>> +enum test_status tx_status;
>> +enum test_status rx_status;
>> +enum test_status cmp_rxtx_status;
>> +
>> +#define test_log(test_msg_lvl, format, ...) do { \
>> + if (test_msg_lvl >= msg_lvl) \
>> + fprintf(stderr, format, ##__VA_ARGS__); \
>> +} while (0)
>> +
>> +static void loopback_test_sleep_ms(int ms)
>> +{
>> + usleep(1000 * ms);
>> +}
>> +
>
> Have you run this as part of "make kselftest" run. How does this
> sleep work in that env.?
Looks like kselftest runs this test application by directly executing
the binary, but this test application requires a valid mhi device file
node as a required parameter. So considering that requirement, is this
test application qualifies to run using kselftest ? Without a valid
device file node test would fail. Is there an option to run this test as
standalone test which can only be run when a mhi device file node is
present ? Having said that i tested this driver by
directly executing it using the test binary which is compiled using
make loopback_test under mhi dir.
> Are there any cases where this test can't run and have to - those
> cases need to be skips.
Yes, as this test application can not run by itself it needs a valid mhi
device file node to write and test reads the same device node to get the
data back.
So test can not be run without having a MHI device connected over a
transport (in my testing MHI device is connected over PCIe). Could you
please suggest an option to use this test application as a standalone
test instead of being part of kselftest?
>
> thanks,
> -- Shuah
Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [Patch stable] netfilter: clear skb->next in NF_HOOK_LIST()
From: Cong Wang @ 2020-11-21 3:43 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, liuzx, Florian Westphal, Edward Cree, stable,
Greg Kroah-Hartman
From: Cong Wang <cong.wang@bytedance.com>
NF_HOOK_LIST() uses list_del() to remove skb from the linked list,
however, it is not sufficient as skb->next still points to other
skb. We should just call skb_list_del_init() to clear skb->next,
like the rest places which using skb list.
This has been fixed in upstream by commit ca58fbe06c54
("netfilter: add and use nf_hook_slow_list()").
Fixes: 9f17dbf04ddf ("netfilter: fix use-after-free in NF_HOOK_LIST")
Reported-by: liuzx@knownsec.com
Tested-by: liuzx@knownsec.com
Cc: Florian Westphal <fw@strlen.de>
Cc: Edward Cree <ecree@solarflare.com>
Cc: stable@vger.kernel.org # between 4.19 and 5.4
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
---
include/linux/netfilter.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 77ebb61faf48..4c0e6539effd 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -316,7 +316,7 @@ NF_HOOK_LIST(uint8_t pf, unsigned int hook, struct net *net, struct sock *sk,
INIT_LIST_HEAD(&sublist);
list_for_each_entry_safe(skb, next, head, list) {
- list_del(&skb->list);
+ skb_list_del_init(skb);
if (nf_hook(pf, hook, net, sk, skb, in, out, okfn) == 1)
list_add_tail(&skb->list, &sublist);
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH net-next 4/6] net: ipa: support retries on generic GSI commands
From: Alex Elder @ 2020-11-21 3:31 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, evgreen, subashab, cpratapa, bjorn.andersson, netdev,
linux-kernel
In-Reply-To: <20201120184923.404c30bb@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On 11/20/20 8:49 PM, Jakub Kicinski wrote:
> On Thu, 19 Nov 2020 16:49:27 -0600 Alex Elder wrote:
>> + do
>> + ret = gsi_generic_command(gsi, channel_id,
>> + GSI_GENERIC_HALT_CHANNEL);
>> + while (ret == -EAGAIN && retries--);
>
> This may well be the first time I've seen someone write a do while loop
> without the curly brackets!
I had them at one time, then saw I could get away
without them. I don't have a preference but I see
you accepted it as-is.
I really appreciate your timely responses.
-Alex
^ permalink raw reply
* Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel
From: Hemant Kumar @ 2020-11-21 3:18 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: gregkh, linux-arm-msm, linux-kernel, jhugo, bbhatt, loic.poulain,
netdev, skhan
In-Reply-To: <20201120061003.GA3909@work>
Hi Mani,
On 11/19/20 10:10 PM, Manivannan Sadhasivam wrote:
> On Mon, Nov 16, 2020 at 02:46:22PM -0800, Hemant Kumar wrote:
>> Loopback test opens the MHI device file node and writes
>> a data buffer to it. MHI UCI kernel space driver copies
>> the data and sends it to MHI uplink (Tx) LOOPBACK channel.
>> MHI device loops back the same data to MHI downlink (Rx)
>> LOOPBACK channel. This data is read by test application
>> and compared against the data sent. Test passes if data
>> buffer matches between Tx and Rx. Test application performs
>> open(), poll(), write(), read() and close() file operations.
>>
>> Signed-off-by: Hemant Kumar <hemantk@codeaurora.org>
>
> One nitpick below, with that addressed:
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
[..]
>
> Effectively this functions does parse and run, so this should be called
> as, "loopback_test_parse_run" or pthread creation should be moved here.
Done.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH v2] mdio_bus: suppress err message for reset gpio EPROBE_DEFER
From: Jakub Kicinski @ 2020-11-21 3:12 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Andrew Lunn, Heiner Kallweit,
linux-kernel
In-Reply-To: <20201119203446.20857-1-grygorii.strashko@ti.com>
On Thu, 19 Nov 2020 22:34:46 +0200 Grygorii Strashko wrote:
> The mdio_bus may have dependencies from GPIO controller and so got
> deferred. Now it will print error message every time -EPROBE_DEFER is
> returned which from:
> __mdiobus_register()
> |-devm_gpiod_get_optional()
> without actually identifying error code.
>
> "mdio_bus 4a101000.mdio: mii_bus 4a101000.mdio couldn't get reset GPIO"
>
> Hence, suppress error message for devm_gpiod_get_optional() returning
> -EPROBE_DEFER case by using dev_err_probe().
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Applied (with the line wrapped), thanks!
^ permalink raw reply
* [PATCH net-next v2 0/2] Add an assert in napi_consume_skb()
From: Yunsheng Lin @ 2020-11-21 3:06 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
This patch introduces a lockdep_assert_in_softirq() interface and
uses it to assert the case when napi_consume_skb() is not called in
the softirq context.
Changelog:
V2: Use lockdep instead of one-off Kconfig knob
Yunsheng Lin (2):
lockdep: Introduce in_softirq lockdep assert
net: Use lockdep_assert_in_softirq() in napi_consume_skb()
include/linux/lockdep.h | 7 +++++++
net/core/skbuff.c | 2 ++
2 files changed, 9 insertions(+)
--
2.8.1
^ permalink raw reply
* [PATCH net-next v2 1/2] lockdep: Introduce in_softirq lockdep assert
From: Yunsheng Lin @ 2020-11-21 3:06 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
In-Reply-To: <1605927976-232804-1-git-send-email-linyunsheng@huawei.com>
The current semantic for napi_consume_skb() is that caller need
to provide non-zero budget when calling from NAPI context, and
breaking this semantic will cause hard to debug problem, because
_kfree_skb_defer() need to run in atomic context in order to push
the skb to the particular cpu' napi_alloc_cache atomically.
So add the lockdep_assert_in_softirq() to assert when the running
context is not in_softirq, in_softirq means softirq is serving or
BH is disabled. Because the softirq context can be interrupted by
hard IRQ or NMI context, so lockdep_assert_in_softirq() need to
assert about hard IRQ or NMI context too.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
include/linux/lockdep.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index f559487..f5e3d81 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -594,6 +594,12 @@ do { \
this_cpu_read(hardirqs_enabled))); \
} while (0)
+#define lockdep_assert_in_softirq() \
+do { \
+ WARN_ON_ONCE(__lockdep_enabled && \
+ (!in_softirq() || in_irq() || in_nmi())); \
+} while (0)
+
#else
# define might_lock(lock) do { } while (0)
# define might_lock_read(lock) do { } while (0)
@@ -605,6 +611,7 @@ do { \
# define lockdep_assert_preemption_enabled() do { } while (0)
# define lockdep_assert_preemption_disabled() do { } while (0)
+# define lockdep_assert_in_softirq() do { } while (0)
#endif
#ifdef CONFIG_PROVE_RAW_LOCK_NESTING
--
2.8.1
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: Use lockdep_assert_in_softirq() in napi_consume_skb()
From: Yunsheng Lin @ 2020-11-21 3:06 UTC (permalink / raw)
To: peterz, mingo, will, viro, kyk.segfault, davem, kuba, linmiaohe,
martin.varghese, pabeni, pshelar, fw, gnault, steffen.klassert,
vladimir.oltean, edumazet, saeed
Cc: netdev, linux-kernel, linuxarm
In-Reply-To: <1605927976-232804-1-git-send-email-linyunsheng@huawei.com>
Use napi_consume_skb() to assert the case when it is not called
in a atomic softirq context.
Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
---
net/core/skbuff.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ffe3dcc..effa19d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -902,6 +902,8 @@ void napi_consume_skb(struct sk_buff *skb, int budget)
return;
}
+ lockdep_assert_in_softirq();
+
if (!skb_unref(skb))
return;
--
2.8.1
^ permalink raw reply related
* Re: [PATCH net 0/4] s390/qeth: fixes 2020-11-20
From: Jakub Kicinski @ 2020-11-21 3:03 UTC (permalink / raw)
To: Julian Wiedmann
Cc: David Miller, linux-netdev, linux-s390, Heiko Carstens,
Karsten Graul
In-Reply-To: <20201120090939.101406-1-jwi@linux.ibm.com>
On Fri, 20 Nov 2020 10:09:35 +0100 Julian Wiedmann wrote:
> please apply the following patch series to netdev's net tree.
>
> This brings several fixes for qeth's af_iucv-specific code paths.
>
> Also one fix by Alexandra for the recently added BR_LEARNING_SYNC
> support. We want to trust the feature indication bit, so that HW can
> mask it out if there's any issues on their end.
Applied, thank you!
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: ipa: add a driver shutdown callback
From: patchwork-bot+netdevbpf @ 2020-11-21 3:00 UTC (permalink / raw)
To: Alex Elder
Cc: davem, kuba, evgreen, subashab, cpratapa, bjorn.andersson, netdev,
linux-kernel
In-Reply-To: <20201119224929.23819-1-elder@linaro.org>
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Thu, 19 Nov 2020 16:49:23 -0600 you wrote:
> The final patch in this series adds a driver shutdown callback for
> the IPA driver. The patches leading up to that address some issues
> encountered while ensuring that callback worked as expected:
> - The first just reports a little more information when channels
> or event rings are in unexpected states
> - The second patch recognizes a condition where an as-yet-unused
> channel does not require a reset during teardown
> - The third patch explicitly ignores a certain error condition,
> because it can't be avoided, and is harmless if it occurs
> - The fourth properly handles requests to retry a channel HALT
> request
> - The fifth makes a second attempt to stop modem activity during
> shutdown if it's busy
>
> [...]
Here is the summary with links:
- [net-next,1/6] net: ipa: print channel/event ring number on error
https://git.kernel.org/netdev/net-next/c/f8d3bdd561a7
- [net-next,2/6] net: ipa: don't reset an ALLOCATED channel
https://git.kernel.org/netdev/net-next/c/5d28913d4ee6
- [net-next,3/6] net: ipa: ignore CHANNEL_NOT_RUNNING errors
https://git.kernel.org/netdev/net-next/c/f849afcc8c3b
- [net-next,4/6] net: ipa: support retries on generic GSI commands
https://git.kernel.org/netdev/net-next/c/1136145660f3
- [net-next,5/6] net: ipa: retry modem stop if busy
https://git.kernel.org/netdev/net-next/c/7c80e83829db
- [net-next,6/6] net: ipa: add driver shutdown callback
https://git.kernel.org/netdev/net-next/c/ae1d72f9779f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: ipa: platform-specific clock and interconnect rates
From: patchwork-bot+netdevbpf @ 2020-11-21 3:00 UTC (permalink / raw)
To: Alex Elder
Cc: davem, kuba, evgreen, subashab, cpratapa, bjorn.andersson, netdev,
linux-kernel
In-Reply-To: <20201119224041.16066-1-elder@linaro.org>
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Thu, 19 Nov 2020 16:40:38 -0600 you wrote:
> This series changes the way the IPA core clock rate and the
> bandwidth parameters for interconnects are specified. Previously
> these were specified with hard-wired constants, with the same values
> used for the SDM845 and SC7180 platforms. Now these parameters are
> recorded in platform-specific configuration data.
>
> For the SC7180 this means we use an all-new core clock rate and
> interconnect parameters.
>
> [...]
Here is the summary with links:
- [net-next,1/3] net: ipa: define clock and interconnect data
https://git.kernel.org/netdev/net-next/c/dfccb8b13c0c
- [net-next,2/3] net: ipa: populate clock and interconnect data
https://git.kernel.org/netdev/net-next/c/f08c99226458
- [net-next,3/3] net: ipa: use config data for clocking
https://git.kernel.org/netdev/net-next/c/91d02f955150
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH bpf-next v7 32/34] bpf: eliminate rlimit-based memory accounting infra for bpf maps
From: Roman Gushchin @ 2020-11-21 2:59 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, ast, daniel, netdev, andrii, akpm, linux-mm, linux-kernel,
kernel-team
In-Reply-To: <20201121025227.dypweojhaz7elwb3@ast-mbp>
On Fri, Nov 20, 2020 at 06:52:27PM -0800, Alexei Starovoitov wrote:
> On Thu, Nov 19, 2020 at 09:37:52AM -0800, Roman Gushchin wrote:
> > static void bpf_map_put_uref(struct bpf_map *map)
> > @@ -619,7 +562,7 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)
> > "value_size:\t%u\n"
> > "max_entries:\t%u\n"
> > "map_flags:\t%#x\n"
> > - "memlock:\t%llu\n"
> > + "memlock:\t%llu\n" /* deprecated */
> > "map_id:\t%u\n"
> > "frozen:\t%u\n",
> > map->map_type,
> > @@ -627,7 +570,7 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)
> > map->value_size,
> > map->max_entries,
> > map->map_flags,
> > - map->memory.pages * 1ULL << PAGE_SHIFT,
> > + 0LLU,
>
> The set looks great to me overall, but above change is problematic.
> There are tools out there that read this value.
> Returning zero might cause oncall alarms to trigger.
> I think we can be more accurate here.
> Instead of zero the kernel can return
> round_up(max_entries * round_up(key_size + value_size, 8), PAGE_SIZE)
> It's not the same as before, but at least the numbers won't suddenly
> go to zero and comparison between maps is still relevant.
> Of course we can introduce a page size calculating callback per map type,
> but imo that would be overkill. These monitoring tools don't care about
> precise number, but rather about relative value and growth from one
> version of the application to another.
>
> If Daniel doesn't find other issues this can be fixed in the follow up.
Makes total sense. I'll prepare a follow-up patch.
Thanks!
^ 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