* Re: [PATCH v2] net: Add trace events for all receive exit points
From: Mathieu Desnoyers @ 2018-11-12 20:56 UTC (permalink / raw)
To: rostedt; +Cc: Geneviève Bastien, David S. Miller, netdev, Ingo Molnar
In-Reply-To: <1354786248.4048.1542055613213.JavaMail.zimbra@efficios.com>
----- On Nov 12, 2018, at 3:46 PM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:
> ----- On Nov 12, 2018, at 3:40 PM, rostedt rostedt@goodmis.org wrote:
>
>> On Mon, 12 Nov 2018 15:20:55 -0500 (EST)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>
>>>
>>> Hrm, looking at this again, I notice that there is a single DEFINE_EVENT
>>> using net_dev_template_simple.
>>>
>>> We could simply turn netif_receive_skb_list_exit into a TRACE_EVENT(),
>>> remove the net_dev_template_simple, and rename the net_dev_template_return
>>> to net_dev_template ?
>>
>> This too is only cosmetic and doesn't affect the code at all, because a
>> TRACE_EVENT() is really just:
>>
>> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
>> DECLARE_EVENT_CLASS(name, \
>> PARAMS(proto), \
>> PARAMS(args), \
>> PARAMS(tstruct), \
>> PARAMS(assign), \
>> PARAMS(print)); \
>> DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));
>>
>> -- Steve
>>
>
> Of course.
>
> I also notice that in two cases, a "gro_result_t" is implicitly cast
> to "int". I usually frown upon this kind of stuff, because it's asking
> for trouble if gro_result_t typedef to something else than "int" in the
> future.
>
> I would recommend going for two templates, one which takes a "int"
> ret parameter, and the other a "gro_result_t" ret parameter.
>
> Or am I being too cautious ?
Digging further, gro_result_t maps to:
enum gro_result {
GRO_MERGED,
GRO_MERGED_FREE,
GRO_HELD,
GRO_NORMAL,
GRO_DROP,
GRO_CONSUMED,
};
typedef enum gro_result gro_result_t;
So we should add a TRACE_DEFINE_ENUM() for those.
Thanks,
Mathieu
>
> Thanks,
>
> Mathieu
>
>
>>>
>>> It's pretty clear from the prototype that it expects a "ret" argument,
>>> so I don't see the need to also state it in the template name.
>>>
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH bpf-next] bpftool: make libbfd optional
From: Stanislav Fomichev @ 2018-11-12 21:30 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Stanislav Fomichev, netdev, ast, daniel, quentin.monnet
In-Reply-To: <20181112123533.19da3b36@cakuba.netronome.com>
On 11/12, Jakub Kicinski wrote:
> On Mon, 12 Nov 2018 11:58:27 -0800, Stanislav Fomichev wrote:
> > On 11/12, Jakub Kicinski wrote:
> > > On Mon, 12 Nov 2018 10:53:28 -0800, Stanislav Fomichev wrote:
> > > > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> > > > index 61d82020af58..ec1bc2ae3c71 100644
> > > > --- a/tools/bpf/bpftool/main.h
> > > > +++ b/tools/bpf/bpftool/main.h
> > > > @@ -147,8 +147,19 @@ int prog_parse_fd(int *argc, char ***argv);
> > > > int map_parse_fd(int *argc, char ***argv);
> > > > int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
> > > >
> > > > +#ifdef HAVE_LIBBFD_SUPPORT
> > > > void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > > const char *arch, const char *disassembler_options);
> > > > +void disasm_init(void);
> > > > +#else
> > > > +static inline
> > > > +void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> > > > + const char *arch, const char *disassembler_options)
> > > > +{
> > > > + p_err("No libbfd support");
> > > > +}
> > >
> > > I think an error per instruction is a bit much, could we make sure we
> > > error out earlier?
> >
> > I can return int from disasm_print_insn as an indication to
> > stop/continue. Let me know if you have better ideas, will post v2 later
> > today.
>
> Why not simply:
Makes sense. I think I can actually repurpose disasm_init for that (the
function I added to wrap bfd_init, I think I can move the callsite here
without any problems).
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 5302ee282409..44edf6705ad2 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -466,6 +466,10 @@ static int do_dump(int argc, char **argv)
> int fd;
>
> if (is_prefix(*argv, "jited")) {
> +#ifndef HAVE_LIBBFD_SUPPORT
> + p_err("No libbfd support");
> + return -1;
> +#endif
> member_len = &info.jited_prog_len;
> member_ptr = &info.jited_prog_insns;
> } else if (is_prefix(*argv, "xlated")) {
>
> Perhaps wrapped into some helper in the header to avoid the yuckiness.
^ permalink raw reply
* [PATCH bpf-next v2] bpftool: make libbfd optional
From: Stanislav Fomichev @ 2018-11-12 21:44 UTC (permalink / raw)
To: netdev, ast, daniel, jakub.kicinski, quentin.monnet; +Cc: Stanislav Fomichev
Make it possible to build bpftool without libbfd. libbfd and libopcodes are
typically provided in dev/dbg packages (binutils-dev in debian) which we
usually don't have installed on the fleet machines and we'd like a way to have
bpftool version that works without installing any additional packages.
This excludes support for disassembling jit-ted code and prints an error if
the user tries to use these features.
Tested by:
cat > FEATURES_DUMP.bpftool <<EOF
feature-libbfd=0
feature-disassembler-four-args=1
feature-reallocarray=0
feature-libelf=1
feature-libelf-mmap=1
feature-bpf=1
EOF
FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
ldd bpftool | grep libbfd
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/bpf/bpftool/Makefile | 13 +++++++++++--
tools/bpf/bpftool/jit_disasm.c | 8 +++++++-
tools/bpf/bpftool/main.c | 3 ---
tools/bpf/bpftool/main.h | 14 ++++++++++++++
tools/bpf/bpftool/prog.c | 3 +++
5 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index dac7eff4c7e5..1bea6b979082 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -53,7 +53,7 @@ ifneq ($(EXTRA_LDFLAGS),)
LDFLAGS += $(EXTRA_LDFLAGS)
endif
-LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
+LIBS = -lelf $(LIBBPF)
INSTALL ?= install
RM ?= rm -f
@@ -90,7 +90,16 @@ include $(wildcard $(OUTPUT)*.d)
all: $(OUTPUT)bpftool
-SRCS = $(wildcard *.c)
+BFD_SRCS = jit_disasm.c
+
+SRCS = $(filter-out $(BFD_SRCS),$(wildcard *.c))
+
+ifeq ($(feature-libbfd),1)
+CFLAGS += -DHAVE_LIBBFD_SUPPORT
+SRCS += $(BFD_SRCS)
+LIBS += -lbfd -lopcodes
+endif
+
OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
$(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index c75ffd9ce2bb..b2ed5ee1af5f 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -109,7 +109,7 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
if (inf) {
bfdf->arch_info = inf;
} else {
- p_err("No libfd support for %s", arch);
+ p_err("No libbfd support for %s", arch);
return;
}
}
@@ -183,3 +183,9 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
bfd_close(bfdf);
}
+
+int disasm_init(void)
+{
+ bfd_init();
+ return 0;
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 75a3296dc0bc..5c4c1cd5a7ba 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -31,7 +31,6 @@
* SOFTWARE.
*/
-#include <bfd.h>
#include <ctype.h>
#include <errno.h>
#include <getopt.h>
@@ -399,8 +398,6 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
- bfd_init();
-
ret = cmd_select(cmds, argc, argv, do_help);
if (json_output)
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 61d82020af58..10c6c16fae29 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -147,8 +147,22 @@ int prog_parse_fd(int *argc, char ***argv);
int map_parse_fd(int *argc, char ***argv);
int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 *info_len);
+#ifdef HAVE_LIBBFD_SUPPORT
void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
const char *arch, const char *disassembler_options);
+int disasm_init(void);
+#else
+static inline
+void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
+ const char *arch, const char *disassembler_options)
+{
+}
+static inline int disasm_init(void)
+{
+ p_err("No libbfd support");
+ return -1;
+}
+#endif
void print_data_json(uint8_t *data, size_t len);
void print_hex_data_json(uint8_t *data, size_t len);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 5ff5544596e7..c176e1aa66fe 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -467,6 +467,9 @@ static int do_dump(int argc, char **argv)
int fd;
if (is_prefix(*argv, "jited")) {
+ if (disasm_init())
+ return -1;
+
member_len = &info.jited_prog_len;
member_ptr = &info.jited_prog_insns;
} else if (is_prefix(*argv, "xlated")) {
--
2.19.1.930.g4563a0d9d0-goog
^ permalink raw reply related
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Martin Lau @ 2018-11-12 21:53 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev@vger.kernel.org, linux-kselftest@vger.kernel.org,
ast@kernel.org, daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, sandipan@linux.vnet.ibm.com
In-Reply-To: <20181109162146.78019-3-sdf@google.com>
On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
[ ... ]
> @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> }
>
> static struct bpf_program *
> -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> {
> - size_t idx;
> + ssize_t idx;
>
> if (!obj->programs)
> return NULL;
> - /* First handler */
> - if (prev == NULL)
> - return &obj->programs[0];
>
> - if (prev->obj != obj) {
> + if (p->obj != obj) {
> pr_warning("error: program handler doesn't match object\n");
> return NULL;
> }
>
> - idx = (prev - obj->programs) + 1;
> - if (idx >= obj->nr_programs)
> + idx = (p - obj->programs) + i;
> + if (idx >= obj->nr_programs || idx < 0)
> return NULL;
> return &obj->programs[idx];
> }
> @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> {
> struct bpf_program *prog = prev;
>
> + if (prev == NULL)
> + return obj->programs;
> +
This patch breaks the behavior introduced in
commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
"Make bpf_program__next() skip over '.text' section if object file
has pseudo calls. The '.text' section is hardly a program in that
case, it's more of a storage for code of functions other than main."
For example, the userspace could have been doing:
prog = bpf_program__next(NULL, obj);
bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
bpf_object__load(obj);
For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
the prog returned by bpf_program__next() could be in ".text" instead of
the main bpf program. The next bpf_program__set_type() has
no effect to the main program. The following bpf_object__load()
will catch user in surprise with the main bpf prog in
the wrong BPF_PROG_TYPE.
> do {
> - prog = __bpf_program__next(prog, obj);
> + prog = __bpf_program__iter(prog, obj, 1);
> + } while (prog && bpf_program__is_function_storage(prog, obj));
> +
> + return prog;
> +}
> +
> +struct bpf_program *
> +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> +{
> + struct bpf_program *prog = next;
> +
> + if (next == NULL) {
> + if (!obj->nr_programs)
> + return NULL;
> + return obj->programs + obj->nr_programs - 1;
> + }
> +
> + do {
> + prog = __bpf_program__iter(prog, obj, -1);
> } while (prog && bpf_program__is_function_storage(prog, obj));
>
> return prog;
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpftool: make libbfd optional
From: Jakub Kicinski @ 2018-11-12 22:02 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: netdev, ast, daniel, quentin.monnet, Jiong Wang
In-Reply-To: <20181112214410.14270-1-sdf@google.com>
On Mon, 12 Nov 2018 13:44:10 -0800, Stanislav Fomichev wrote:
> Make it possible to build bpftool without libbfd. libbfd and libopcodes are
> typically provided in dev/dbg packages (binutils-dev in debian) which we
> usually don't have installed on the fleet machines and we'd like a way to have
> bpftool version that works without installing any additional packages.
> This excludes support for disassembling jit-ted code and prints an error if
> the user tries to use these features.
>
> Tested by:
> cat > FEATURES_DUMP.bpftool <<EOF
> feature-libbfd=0
> feature-disassembler-four-args=1
> feature-reallocarray=0
> feature-libelf=1
> feature-libelf-mmap=1
> feature-bpf=1
> EOF
> FEATURES_DUMP=$PWD/FEATURES_DUMP.bpftool make
> ldd bpftool | grep libbfd
>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Seems reasonable, thanks!
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH v5 bpf-next 2/7] libbpf: cleanup after partial failure in bpf_object__pin
From: Stanislav Fomichev @ 2018-11-12 22:10 UTC (permalink / raw)
To: Martin Lau
Cc: Stanislav Fomichev, netdev@vger.kernel.org,
linux-kselftest@vger.kernel.org, ast@kernel.org,
daniel@iogearbox.net, shuah@kernel.org,
jakub.kicinski@netronome.com, quentin.monnet@netronome.com,
Roman Gushchin, jiong.wang@netronome.com,
bhole_prashant_q7@lab.ntt.co.jp, john.fastabend@gmail.com,
jbenc@redhat.com, treeze.taeung@gmail.com, Yonghong Song,
Okash Khawaja, "sandipan@linux
In-Reply-To: <20181112215337.5ztvb5v7cf4kzr2h@kafai-mbp.dhcp.thefacebook.com>
On 11/12, Martin Lau wrote:
> On Fri, Nov 09, 2018 at 08:21:41AM -0800, Stanislav Fomichev wrote:
> [ ... ]
> > @@ -1918,23 +2160,20 @@ void *bpf_object__priv(struct bpf_object *obj)
> > }
> >
> > static struct bpf_program *
> > -__bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > +__bpf_program__iter(struct bpf_program *p, struct bpf_object *obj, int i)
> > {
> > - size_t idx;
> > + ssize_t idx;
> >
> > if (!obj->programs)
> > return NULL;
> > - /* First handler */
> > - if (prev == NULL)
> > - return &obj->programs[0];
> >
> > - if (prev->obj != obj) {
> > + if (p->obj != obj) {
> > pr_warning("error: program handler doesn't match object\n");
> > return NULL;
> > }
> >
> > - idx = (prev - obj->programs) + 1;
> > - if (idx >= obj->nr_programs)
> > + idx = (p - obj->programs) + i;
> > + if (idx >= obj->nr_programs || idx < 0)
> > return NULL;
> > return &obj->programs[idx];
> > }
> > @@ -1944,8 +2183,29 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
> > {
> > struct bpf_program *prog = prev;
> >
> > + if (prev == NULL)
> > + return obj->programs;
> > +
> This patch breaks the behavior introduced in
> commit eac7d84519a3 ("tools: libbpf: don't return '.text' as a program for multi-function programs"):
> "Make bpf_program__next() skip over '.text' section if object file
> has pseudo calls. The '.text' section is hardly a program in that
> case, it's more of a storage for code of functions other than main."
>
> For example, the userspace could have been doing:
> prog = bpf_program__next(NULL, obj);
> bpf_program__set_type(prog, BPF_PROG_TYPE_TRACEPOINT);
> bpf_object__load(obj);
>
> For the bpf_prog.o that has pseudo calls, after this patch in bpf-next,
> the prog returned by bpf_program__next() could be in ".text" instead of
> the main bpf program. The next bpf_program__set_type() has
> no effect to the main program. The following bpf_object__load()
> will catch user in surprise with the main bpf prog in
> the wrong BPF_PROG_TYPE.
Will something like the following fix your concern? (plus, assuming the
same for prev):
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -2216,8 +2216,11 @@ bpf_program__next(struct bpf_program *prev, struct bpf_object *obj)
{
struct bpf_program *prog = prev;
- if (prev == NULL)
- return obj->programs;
+ if (prev == NULL) {
+ prog = obj->programs;
+ if (!prog || !bpf_program__is_function_storage(prog, obj))
+ return prog;
+ }
do {
prog = __bpf_program__iter(prog, obj, 1);
Any suggestions for a better way to do it?
> > do {
> > - prog = __bpf_program__next(prog, obj);
> > + prog = __bpf_program__iter(prog, obj, 1);
> > + } while (prog && bpf_program__is_function_storage(prog, obj));
> > +
> > + return prog;
> > +}
> > +
> > +struct bpf_program *
> > +bpf_program__prev(struct bpf_program *next, struct bpf_object *obj)
> > +{
> > + struct bpf_program *prog = next;
> > +
> > + if (next == NULL) {
> > + if (!obj->nr_programs)
> > + return NULL;
> > + return obj->programs + obj->nr_programs - 1;
> > + }
> > +
> > + do {
> > + prog = __bpf_program__iter(prog, obj, -1);
> > } while (prog && bpf_program__is_function_storage(prog, obj));
> >
> > return prog;
^ permalink raw reply
* Re: [PATCH] bpf: Remove unused variable in nsim_bpf
From: Jakub Kicinski @ 2018-11-12 22:14 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet, netdev,
linux-kernel
In-Reply-To: <20181112221042.11160-1-natechancellor@gmail.com>
On Mon, 12 Nov 2018 15:10:42 -0700, Nathan Chancellor wrote:
> Clang warns:
>
> drivers/net/netdevsim/bpf.c:557:30: error: unused variable 'state'
> [-Werror,-Wunused-variable]
> struct nsim_bpf_bound_prog *state;
> ^
> 1 error generated.
>
> The declaration should have been removed in commit b07ade27e933 ("bpf:
> pass translate() as a callback and remove its ndo_bpf subcommand").
>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* [iproute PATCH] man: ip-route.8: Document nexthop limit
From: Phil Sutter @ 2018-11-12 22:21 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Add a note to 'nexthop' description stating the maximum number of
nexthops per command and pointing at 'append' command as a workaround.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
man/man8/ip-route.8.in | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index a33ce1f0f4006..383178c11331e 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -589,6 +589,13 @@ argument lists:
route reflecting its relative bandwidth or quality.
.in -8
+The internal buffer used in iproute2 limits the maximum number of nexthops to
+be specified in one go. If only a gateway address is given, the current buffer
+size allows for 144 IPv6 nexthops and 253 IPv4 ones. If more are required, they
+may be added to the existing route using
+.B "ip route append"
+command.
+
.TP
.BI scope " SCOPE_VAL"
the scope of the destinations covered by the route prefix.
--
2.19.0
^ permalink raw reply related
* Re: [Patch net-next] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-12 22:41 UTC (permalink / raw)
To: David Miller; +Cc: Linux Kernel Network Developers
In-Reply-To: <20181109.201630.1672707103795948980.davem@davemloft.net>
On Fri, Nov 9, 2018 at 8:16 PM David Miller <davem@davemloft.net> wrote:
>
> Didn't you move this function into net/core/skbuff.c? :-)
At the time when I created this patch, that patch didn't hit net-next yet. :)
>
> Please respin.
Sure. I will send v2, as I need to cleanup the mixed commas and
spaces too.
Thanks.
^ permalink raw reply
* [Patch net-next v2] net: dump more useful information in netdev_rx_csum_fault()
From: Cong Wang @ 2018-11-12 22:47 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang
Currently netdev_rx_csum_fault() only shows a device name,
we need more information about the skb for debugging csum
failures.
Sample output:
ens3: hw csum failure
dev features: 0x0000000000014b89
skb len=84 data_len=0 pkt_type=0 gso_size=0 gso_type=0 nr_frags=0 ip_summed=0 csum=0 csum_complete_sw=0 csum_valid=0 csum_level=0
Note, I use pr_err() just to be consistent with the existing one.
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/linux/netdevice.h | 5 +++--
net/core/datagram.c | 2 +-
net/core/dev.c | 11 +++++++++--
net/core/skbuff.c | 4 ++--
net/sunrpc/socklib.c | 2 +-
5 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 487fa5e0e165..5a97ffbff932 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4342,9 +4342,10 @@ static inline bool can_checksum_protocol(netdev_features_t features,
}
#ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev);
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb);
#else
-static inline void netdev_rx_csum_fault(struct net_device *dev)
+static inline void netdev_rx_csum_fault(struct net_device *dev,
+ struct sk_buff *skb)
{
}
#endif
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 07983b90d2bd..4bf62b1afa3b 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -767,7 +767,7 @@ int skb_copy_and_csum_datagram_msg(struct sk_buff *skb,
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(NULL);
+ netdev_rx_csum_fault(NULL, skb);
}
return 0;
fault:
diff --git a/net/core/dev.c b/net/core/dev.c
index bf7e0a471186..5927f6a7c301 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3091,10 +3091,17 @@ EXPORT_SYMBOL(__skb_gso_segment);
/* Take action when hardware reception checksum errors are detected. */
#ifdef CONFIG_BUG
-void netdev_rx_csum_fault(struct net_device *dev)
+void netdev_rx_csum_fault(struct net_device *dev, struct sk_buff *skb)
{
if (net_ratelimit()) {
pr_err("%s: hw csum failure\n", dev ? dev->name : "<unknown>");
+ if (dev)
+ pr_err("dev features: %pNF\n", &dev->features);
+ pr_err("skb len=%u data_len=%u pkt_type=%u gso_size=%u gso_type=%u nr_frags=%u ip_summed=%u csum=%x csum_complete_sw=%d csum_valid=%d csum_level=%u\n",
+ skb->len, skb->data_len, skb->pkt_type,
+ skb_shinfo(skb)->gso_size, skb_shinfo(skb)->gso_type,
+ skb_shinfo(skb)->nr_frags, skb->ip_summed, skb->csum,
+ skb->csum_complete_sw, skb->csum_valid, skb->csum_level);
dump_stack();
}
}
@@ -5781,7 +5788,7 @@ __sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
NAPI_GRO_CB(skb)->csum = wsum;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 396fcb3baad0..fcb1155a00ec 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2653,7 +2653,7 @@ __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
if (!skb_shared(skb))
skb->csum_valid = !sum;
@@ -2673,7 +2673,7 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb)
if (likely(!sum)) {
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
}
if (!skb_shared(skb)) {
diff --git a/net/sunrpc/socklib.c b/net/sunrpc/socklib.c
index 9062967575c4..7e55cfc69697 100644
--- a/net/sunrpc/socklib.c
+++ b/net/sunrpc/socklib.c
@@ -175,7 +175,7 @@ int csum_partial_copy_to_xdr(struct xdr_buf *xdr, struct sk_buff *skb)
return -1;
if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
!skb->csum_complete_sw)
- netdev_rx_csum_fault(skb->dev);
+ netdev_rx_csum_fault(skb->dev, skb);
return 0;
no_checksum:
if (xdr_partial_copy_from_skb(xdr, 0, &desc, xdr_skb_read_bits) < 0)
--
2.19.1
^ permalink raw reply related
* [PATCH net-next 00/13] nfp: abm: track all Qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
Hi!
Our Qdisc offload so far has been very simplistic. We held
and array of marking thresholds and statistics sized to the
number of PF queues. This was sufficient since the only
configuration we supported was single layer of RED Qdiscs
(on top of MQ or not, but MQ isn't really about queuing).
As we move to add more Qdiscs it's time to actually try to
track the full Qdisc hierarchy. This allows us to make sure
our offloaded configuration reflects the SW path better.
We add graft notifications to MQ and RED (PRIO already sends
them) to allow drivers offloading those to learn how Qdiscs
are linked. MQ graft gives us the obvious advantage of being
able to track when Qdiscs are shared or moved. It seems
unlikely HW would offload RED's child Qdiscs but since the
behaviour would change based on linked child we should
stop offloading REDs with modified child. RED will also
handle the child differently during reconfig when limit
parameter is set - so we have to inform the drivers about
the limit, and have them reset the child state when
appropriate.
The NFP driver will now allocate a structure to track each
Qdisc and link it to its children. We will also maintain
a shadow copy of threshold settings - to save device writes
and make it easier to apply defaults when config is
re-evaluated.
Jakub Kicinski (13):
nfp: abm: rename qdiscs -> red_qdiscs
nfp: abm: keep track of all RED thresholds
nfp: abm: track all offload-enabled qdiscs
net: sched: provide notification for graft on root
nfp: abm: remember which Qdisc is root
nfp: abm: allocate Qdisc child table
net: sched: red: offload a graft notification
net: sched: mq: offload a graft notification
nfp: abm: build full Qdisc hierarchy based on graft notifications
net: sched: red: notify drivers about RED's limit parameter
nfp: abm: reset RED's child based on limit
nfp: abm: save RED's parameters
nfp: abm: restructure Qdisc handling
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 105 +--
drivers/net/ethernet/netronome/nfp/abm/main.c | 42 +-
drivers/net/ethernet/netronome/nfp/abm/main.h | 108 ++-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 712 +++++++++++++-----
include/linux/netdevice.h | 1 +
include/net/pkt_cls.h | 24 +-
net/sched/sch_api.c | 17 +
net/sched/sch_mq.c | 9 +
net/sched/sch_red.c | 18 +
9 files changed, 740 insertions(+), 296 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next 01/13] nfp: abm: rename qdiscs -> red_qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Rename qdiscs member to red_qdiscs. One of following patches will
use the name qdiscs for tracking all qdisc types.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 11 +++---
drivers/net/ethernet/netronome/nfp/abm/main.h | 4 +-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 38 ++++++++++---------
3 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 3d15de0ae271..5e749602989e 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -309,9 +309,10 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
alink->id = id;
alink->parent = TC_H_ROOT;
alink->total_queues = alink->vnic->max_rx_rings;
- alink->qdiscs = kvcalloc(alink->total_queues, sizeof(*alink->qdiscs),
- GFP_KERNEL);
- if (!alink->qdiscs) {
+ alink->red_qdiscs = kvcalloc(alink->total_queues,
+ sizeof(*alink->red_qdiscs),
+ GFP_KERNEL);
+ if (!alink->red_qdiscs) {
err = -ENOMEM;
goto err_free_alink;
}
@@ -331,7 +332,7 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
return 0;
err_free_qdiscs:
- kvfree(alink->qdiscs);
+ kvfree(alink->red_qdiscs);
err_free_alink:
kfree(alink);
return err;
@@ -342,7 +343,7 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
struct nfp_abm_link *alink = nn->app_priv;
nfp_abm_kill_reprs(alink->abm, alink);
- kvfree(alink->qdiscs);
+ kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 3774c063e419..a09090004f82 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -82,7 +82,7 @@ struct nfp_red_qdisc {
* @total_queues: number of PF queues
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
- * @qdiscs: array of qdiscs
+ * @red_qdiscs: array of qdiscs
*/
struct nfp_abm_link {
struct nfp_abm *abm;
@@ -92,7 +92,7 @@ struct nfp_abm_link {
unsigned int total_queues;
u32 parent;
unsigned int num_qdiscs;
- struct nfp_red_qdisc *qdiscs;
+ struct nfp_red_qdisc *red_qdiscs;
};
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index bb05f9ee0401..abda392880e0 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -18,7 +18,8 @@ __nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
int ret;
ret = nfp_abm_ctrl_set_all_q_lvls(alink, init_val);
- memset(alink->qdiscs, 0, sizeof(*alink->qdiscs) * alink->num_qdiscs);
+ memset(alink->red_qdiscs, 0,
+ sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
@@ -46,7 +47,8 @@ nfp_abm_red_find(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
else
return -EOPNOTSUPP;
- if (i >= alink->num_qdiscs || opt->handle != alink->qdiscs[i].handle)
+ if (i >= alink->num_qdiscs ||
+ opt->handle != alink->red_qdiscs[i].handle)
return -EOPNOTSUPP;
return i;
@@ -59,7 +61,7 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
unsigned int i;
for (i = 0; i < alink->num_qdiscs; i++)
- if (handle == alink->qdiscs[i].handle)
+ if (handle == alink->red_qdiscs[i].handle)
break;
if (i == alink->num_qdiscs)
return;
@@ -68,7 +70,7 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
} else {
nfp_abm_ctrl_set_q_lvl(alink, i, NFP_ABM_LVL_INFINITY);
- memset(&alink->qdiscs[i], 0, sizeof(*alink->qdiscs));
+ memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
}
}
@@ -139,37 +141,39 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
return -EINVAL;
}
/* Set the handle to try full clean up, in case IO failed */
- alink->qdiscs[i].handle = opt->handle;
+ alink->red_qdiscs[i].handle = opt->handle;
if (err)
goto err_destroy;
if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink, &alink->qdiscs[i].stats);
+ err = nfp_abm_ctrl_read_stats(alink,
+ &alink->red_qdiscs[i].stats);
else
err = nfp_abm_ctrl_read_q_stats(alink, i,
- &alink->qdiscs[i].stats);
+ &alink->red_qdiscs[i].stats);
if (err)
goto err_destroy;
if (opt->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_xstats(alink,
- &alink->qdiscs[i].xstats);
+ &alink->red_qdiscs[i].xstats);
else
err = nfp_abm_ctrl_read_q_xstats(alink, i,
- &alink->qdiscs[i].xstats);
+ &alink->red_qdiscs[i].xstats);
if (err)
goto err_destroy;
- alink->qdiscs[i].stats.backlog_pkts = 0;
- alink->qdiscs[i].stats.backlog_bytes = 0;
+ alink->red_qdiscs[i].stats.backlog_pkts = 0;
+ alink->red_qdiscs[i].stats.backlog_bytes = 0;
return 0;
err_destroy:
/* If the qdisc keeps on living, but we can't offload undo changes */
if (existing) {
- opt->set.qstats->qlen -= alink->qdiscs[i].stats.backlog_pkts;
+ opt->set.qstats->qlen -=
+ alink->red_qdiscs[i].stats.backlog_pkts;
opt->set.qstats->backlog -=
- alink->qdiscs[i].stats.backlog_bytes;
+ alink->red_qdiscs[i].stats.backlog_bytes;
}
nfp_abm_red_destroy(netdev, alink, opt->handle);
@@ -198,7 +202,7 @@ nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
i = nfp_abm_red_find(alink, opt);
if (i < 0)
return i;
- prev_stats = &alink->qdiscs[i].stats;
+ prev_stats = &alink->red_qdiscs[i].stats;
if (alink->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_stats(alink, &stats);
@@ -224,7 +228,7 @@ nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
i = nfp_abm_red_find(alink, opt);
if (i < 0)
return i;
- prev_xstats = &alink->qdiscs[i].xstats;
+ prev_xstats = &alink->red_qdiscs[i].xstats;
if (alink->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_xstats(alink, &xstats);
@@ -267,14 +271,14 @@ nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
int err;
for (i = 0; i < alink->num_qdiscs; i++) {
- if (alink->qdiscs[i].handle == TC_H_UNSPEC)
+ if (alink->red_qdiscs[i].handle == TC_H_UNSPEC)
continue;
err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
if (err)
return err;
- nfp_abm_update_stats(&stats, &alink->qdiscs[i].stats,
+ nfp_abm_update_stats(&stats, &alink->red_qdiscs[i].stats,
&opt->stats);
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 02/13] nfp: abm: keep track of all RED thresholds
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Instead of writing the threshold out when Qdisc is configured
and not remembering it move to a scheme where we remember all
thresholds. When configuration changes parse the offloaded
Qdiscs and set thresholds appropriately.
This will help future extensions.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 32 +++----
drivers/net/ethernet/netronome/nfp/abm/main.c | 25 +++++-
drivers/net/ethernet/netronome/nfp/abm/main.h | 16 +++-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 88 ++++++++++++++-----
4 files changed, 120 insertions(+), 41 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 3c661f422688..564427e8a6e8 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -71,35 +71,37 @@ nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
return 0;
}
-int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i, u32 val)
+int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val)
{
- struct nfp_cpp *cpp = alink->abm->app->cpp;
+ struct nfp_cpp *cpp = abm->app->cpp;
u64 sym_offset;
int err;
- sym_offset = (alink->queue_base + i) * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
- err = __nfp_rtsym_writel(cpp, alink->abm->q_lvls, 4, 0,
- sym_offset, val);
+ __clear_bit(id, abm->threshold_undef);
+ if (abm->thresholds[id] == val)
+ return 0;
+
+ sym_offset = id * NFP_QLVL_STRIDE + NFP_QLVL_THRS;
+ err = __nfp_rtsym_writel(cpp, abm->q_lvls, 4, 0, sym_offset, val);
if (err) {
- nfp_err(cpp, "RED offload setting level failed on vNIC %d queue %d\n",
- alink->id, i);
+ nfp_err(cpp,
+ "RED offload setting level failed on subqueue %d\n",
+ id);
return err;
}
+ abm->thresholds[id] = val;
return 0;
}
-int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val)
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
+ u32 val)
{
- int i, err;
+ unsigned int threshold;
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- err = nfp_abm_ctrl_set_q_lvl(alink, i, val);
- if (err)
- return err;
- }
+ threshold = alink->queue_base + queue;
- return 0;
+ return __nfp_abm_ctrl_set_q_lvl(alink->abm, threshold, val);
}
u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 5e749602989e..c6ae0ac9a154 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -2,6 +2,7 @@
/* Copyright (C) 2018 Netronome Systems, Inc. */
#include <linux/bitfield.h>
+#include <linux/bitmap.h>
#include <linux/etherdevice.h>
#include <linux/lockdep.h>
#include <linux/netdevice.h>
@@ -399,6 +400,7 @@ static int nfp_abm_init(struct nfp_app *app)
struct nfp_pf *pf = app->pf;
struct nfp_reprs *reprs;
struct nfp_abm *abm;
+ unsigned int i;
int err;
if (!pf->eth_tbl) {
@@ -425,15 +427,28 @@ static int nfp_abm_init(struct nfp_app *app)
if (err)
goto err_free_abm;
+ err = -ENOMEM;
+ abm->num_thresholds = NFP_NET_MAX_RX_RINGS;
+ abm->threshold_undef = bitmap_zalloc(abm->num_thresholds, GFP_KERNEL);
+ if (!abm->threshold_undef)
+ goto err_free_abm;
+
+ abm->thresholds = kvcalloc(abm->num_thresholds,
+ sizeof(*abm->thresholds), GFP_KERNEL);
+ if (!abm->thresholds)
+ goto err_free_thresh_umap;
+ for (i = 0; i < NFP_NET_MAX_RX_RINGS; i++)
+ __nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+
/* We start in legacy mode, make sure advanced queuing is disabled */
err = nfp_abm_ctrl_qm_disable(abm);
if (err)
- goto err_free_abm;
+ goto err_free_thresh;
err = -ENOMEM;
reprs = nfp_reprs_alloc(pf->max_data_vnics);
if (!reprs)
- goto err_free_abm;
+ goto err_free_thresh;
RCU_INIT_POINTER(app->reprs[NFP_REPR_TYPE_PHYS_PORT], reprs);
reprs = nfp_reprs_alloc(pf->max_data_vnics);
@@ -445,6 +460,10 @@ static int nfp_abm_init(struct nfp_app *app)
err_free_phys:
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PHYS_PORT);
+err_free_thresh:
+ kvfree(abm->thresholds);
+err_free_thresh_umap:
+ bitmap_free(abm->threshold_undef);
err_free_abm:
kfree(abm);
app->priv = NULL;
@@ -458,6 +477,8 @@ static void nfp_abm_clean(struct nfp_app *app)
nfp_abm_eswitch_clean_up(abm);
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PF);
nfp_reprs_clean_and_free_by_type(app, NFP_REPR_TYPE_PHYS_PORT);
+ bitmap_free(abm->threshold_undef);
+ kvfree(abm->thresholds);
kfree(abm);
app->priv = NULL;
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index a09090004f82..15732ad7c202 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -20,6 +20,11 @@ struct nfp_net;
* struct nfp_abm - ABM NIC app structure
* @app: back pointer to nfp_app
* @pf_id: ID of our PF link
+ *
+ * @thresholds: current threshold configuration
+ * @threshold_undef: bitmap of thresholds which have not been set
+ * @num_thresholds: number of @thresholds and bits in @threshold_undef
+ *
* @eswitch_mode: devlink eswitch mode, advanced functions only visible
* in switchdev mode
* @q_lvls: queue level control area
@@ -28,6 +33,11 @@ struct nfp_net;
struct nfp_abm {
struct nfp_app *app;
unsigned int pf_id;
+
+ u32 *thresholds;
+ unsigned long *threshold_undef;
+ size_t num_thresholds;
+
enum devlink_eswitch_mode eswitch_mode;
const struct nfp_rtsym *q_lvls;
const struct nfp_rtsym *qm_stats;
@@ -64,11 +74,13 @@ struct nfp_alink_xstats {
/**
* struct nfp_red_qdisc - representation of single RED Qdisc
* @handle: handle of currently offloaded RED Qdisc
+ * @threshold: marking threshold of this Qdisc
* @stats: statistics from last refresh
* @xstats: base of extended statistics
*/
struct nfp_red_qdisc {
u32 handle;
+ u32 threshold;
struct nfp_alink_stats stats;
struct nfp_alink_xstats xstats;
};
@@ -102,8 +114,8 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
void nfp_abm_ctrl_read_params(struct nfp_abm_link *alink);
int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
-int nfp_abm_ctrl_set_all_q_lvls(struct nfp_abm_link *alink, u32 val);
-int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int i,
+int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val);
+int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
u32 val);
int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
struct nfp_alink_stats *stats);
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index abda392880e0..abb0a24c7fac 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -7,25 +7,73 @@
#include "../nfpcore/nfp_cpp.h"
#include "../nfp_app.h"
+#include "../nfp_main.h"
+#include "../nfp_net.h"
#include "../nfp_port.h"
#include "main.h"
-static int
+static void
+nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
+ struct nfp_red_qdisc *qdisc, unsigned int queue)
+{
+ if (!qdisc->handle)
+ return;
+
+ nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->threshold);
+}
+
+static void nfp_abm_offload_compile_one(struct nfp_abm_link *alink)
+{
+ unsigned int i;
+ bool is_mq;
+
+ is_mq = alink->num_qdiscs > 1;
+
+ for (i = 0; i < alink->total_queues; i++) {
+ struct nfp_red_qdisc *next;
+
+ if (is_mq && !alink->red_qdiscs[i].handle)
+ continue;
+
+ next = is_mq ? &alink->red_qdiscs[i] : &alink->red_qdiscs[0];
+ nfp_abm_offload_compile_red(alink, next, i);
+ }
+}
+
+static void nfp_abm_offload_update(struct nfp_abm *abm)
+{
+ struct nfp_abm_link *alink = NULL;
+ struct nfp_pf *pf = abm->app->pf;
+ struct nfp_net *nn;
+ size_t i;
+
+ /* Mark all thresholds as unconfigured */
+ __bitmap_set(abm->threshold_undef, 0, abm->num_thresholds);
+
+ /* Configure all offloads */
+ list_for_each_entry(nn, &pf->vnics, vnic_list) {
+ alink = nn->app_priv;
+ nfp_abm_offload_compile_one(alink);
+ }
+
+ /* Reset the unconfigured thresholds */
+ for (i = 0; i < abm->num_thresholds; i++)
+ if (test_bit(i, abm->threshold_undef))
+ __nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+}
+
+static void
__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
u32 handle, unsigned int qs, u32 init_val)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
- int ret;
- ret = nfp_abm_ctrl_set_all_q_lvls(alink, init_val);
memset(alink->red_qdiscs, 0,
sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
port->tc_offload_cnt = qs;
-
- return ret;
}
static void
@@ -66,12 +114,12 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
if (i == alink->num_qdiscs)
return;
- if (alink->parent == TC_H_ROOT) {
+ if (alink->parent == TC_H_ROOT)
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- } else {
- nfp_abm_ctrl_set_q_lvl(alink, i, NFP_ABM_LVL_INFINITY);
+ else
memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
- }
+
+ nfp_abm_offload_update(alink->abm);
}
static bool
@@ -121,29 +169,19 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
}
if (existing) {
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_set_all_q_lvls(alink, opt->set.min);
- else
- err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
- if (err)
- goto err_destroy;
+ nfp_abm_offload_update(alink->abm);
return 0;
}
if (opt->parent == TC_H_ROOT) {
i = 0;
- err = __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1,
- opt->set.min);
+ __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1, opt->set.min);
} else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent)) {
i = TC_H_MIN(opt->parent) - 1;
- err = nfp_abm_ctrl_set_q_lvl(alink, i, opt->set.min);
} else {
return -EINVAL;
}
- /* Set the handle to try full clean up, in case IO failed */
alink->red_qdiscs[i].handle = opt->handle;
- if (err)
- goto err_destroy;
if (opt->parent == TC_H_ROOT)
err = nfp_abm_ctrl_read_stats(alink,
@@ -163,9 +201,12 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
if (err)
goto err_destroy;
+ alink->red_qdiscs[i].threshold = opt->set.min;
alink->red_qdiscs[i].stats.backlog_pkts = 0;
alink->red_qdiscs[i].stats.backlog_bytes = 0;
+ nfp_abm_offload_update(alink->abm);
+
return 0;
err_destroy:
/* If the qdisc keeps on living, but we can't offload undo changes */
@@ -292,10 +333,13 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
case TC_MQ_CREATE:
nfp_abm_reset_root(netdev, alink, opt->handle,
alink->total_queues);
+ nfp_abm_offload_update(alink->abm);
return 0;
case TC_MQ_DESTROY:
- if (opt->handle == alink->parent)
+ if (opt->handle == alink->parent) {
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
+ nfp_abm_offload_update(alink->abm);
+ }
return 0;
case TC_MQ_STATS:
return nfp_abm_mq_stats(alink, opt);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 03/13] nfp: abm: track all offload-enabled qdiscs
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Allocate an object corresponding to any offloaded qdisc we are
informed about by the kernel. Not only the qdiscs we have a
chance of offloading.
The count of created objects will be used to decide whether
the ethtool TC offload can be disabled, since otherwise we may
miss destroy commands.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 2 +
drivers/net/ethernet/netronome/nfp/abm/main.h | 23 ++++
.../net/ethernet/netronome/nfp/abm/qdisc.c | 111 +++++++++++++++++-
3 files changed, 132 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index c6ae0ac9a154..35f3a6054569 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -329,6 +329,7 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
nfp_abm_vnic_set_mac(app->pf, abm, nn, id);
nfp_abm_ctrl_read_params(alink);
+ INIT_RADIX_TREE(&alink->qdiscs, GFP_KERNEL);
return 0;
@@ -344,6 +345,7 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
struct nfp_abm_link *alink = nn->app_priv;
nfp_abm_kill_reprs(alink->abm, alink);
+ WARN(!radix_tree_empty(&alink->qdiscs), "left over qdiscs\n");
kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 15732ad7c202..64cb5ebcf80e 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -5,6 +5,7 @@
#define __NFP_ABM_H__ 1
#include <linux/bits.h>
+#include <linux/radix-tree.h>
#include <net/devlink.h>
#include <net/pkt_cls.h>
@@ -71,6 +72,26 @@ struct nfp_alink_xstats {
u64 pdrop;
};
+enum nfp_qdisc_type {
+ NFP_QDISC_NONE = 0,
+ NFP_QDISC_MQ,
+ NFP_QDISC_RED,
+};
+
+/**
+ * struct nfp_qdisc - tracked TC Qdisc
+ * @netdev: netdev on which Qdisc was created
+ * @type: Qdisc type
+ * @handle: handle of this Qdisc
+ * @parent_handle: handle of the parent (unreliable if Qdisc was grafted)
+ */
+struct nfp_qdisc {
+ struct net_device *netdev;
+ enum nfp_qdisc_type type;
+ u32 handle;
+ u32 parent_handle;
+};
+
/**
* struct nfp_red_qdisc - representation of single RED Qdisc
* @handle: handle of currently offloaded RED Qdisc
@@ -95,6 +116,7 @@ struct nfp_red_qdisc {
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
* @red_qdiscs: array of qdiscs
+ * @qdiscs: all qdiscs recorded by major part of the handle
*/
struct nfp_abm_link {
struct nfp_abm *abm;
@@ -105,6 +127,7 @@ struct nfp_abm_link {
u32 parent;
unsigned int num_qdiscs;
struct nfp_red_qdisc *red_qdiscs;
+ struct radix_tree_root qdiscs;
};
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index abb0a24c7fac..a6f95924656d 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -63,17 +63,97 @@ static void nfp_abm_offload_update(struct nfp_abm *abm)
}
static void
-__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs, u32 init_val)
+nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
+ if (!qdisc)
+ return;
+ WARN_ON(radix_tree_delete(&alink->qdiscs,
+ TC_H_MAJ(qdisc->handle)) != qdisc);
+ kfree(qdisc);
+
+ port->tc_offload_cnt--;
+}
+
+static struct nfp_qdisc *
+nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle)
+{
+ struct nfp_port *port = nfp_port_from_netdev(netdev);
+ struct nfp_qdisc *qdisc;
+ int err;
+
+ qdisc = kzalloc(sizeof(*qdisc), GFP_KERNEL);
+ if (!qdisc)
+ return NULL;
+
+ qdisc->netdev = netdev;
+ qdisc->type = type;
+ qdisc->parent_handle = parent_handle;
+ qdisc->handle = handle;
+
+ err = radix_tree_insert(&alink->qdiscs, TC_H_MAJ(qdisc->handle), qdisc);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "Qdisc insertion into radix tree failed: %d\n", err);
+ goto err_free_qdisc;
+ }
+
+ port->tc_offload_cnt++;
+ return qdisc;
+
+err_free_qdisc:
+ kfree(qdisc);
+ return NULL;
+}
+
+static struct nfp_qdisc *
+nfp_abm_qdisc_find(struct nfp_abm_link *alink, u32 handle)
+{
+ return radix_tree_lookup(&alink->qdiscs, TC_H_MAJ(handle));
+}
+
+static int
+nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
+ struct nfp_qdisc **qdisc)
+{
+ *qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (*qdisc) {
+ if (WARN_ON((*qdisc)->type != type))
+ return -EINVAL;
+ return 0;
+ }
+
+ *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle,
+ handle);
+ return *qdisc ? 0 : -ENOMEM;
+}
+
+static void
+nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
+ u32 handle)
+{
+ struct nfp_qdisc *qdisc;
+
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return;
+
+ nfp_abm_qdisc_free(netdev, alink, qdisc);
+}
+
+static void
+__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ u32 handle, unsigned int qs, u32 init_val)
+{
memset(alink->red_qdiscs, 0,
sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
alink->parent = handle;
alink->num_qdiscs = qs;
- port->tc_offload_cnt = qs;
}
static void
@@ -108,6 +188,8 @@ nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
{
unsigned int i;
+ nfp_abm_qdisc_destroy(netdev, alink, handle);
+
for (i = 0; i < alink->num_qdiscs; i++)
if (handle == alink->red_qdiscs[i].handle)
break;
@@ -157,12 +239,22 @@ static int
nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt)
{
+ struct nfp_qdisc *qdisc;
bool existing;
int i, err;
+ int ret;
+
+ ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
+ opt->handle, &qdisc);
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
+ if (ret) {
+ err = ret;
+ goto err_destroy;
+ }
+
if (!nfp_abm_red_check_params(alink, opt)) {
err = -EINVAL;
goto err_destroy;
@@ -326,6 +418,16 @@ nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
return 0;
}
+static int
+nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_mq_qopt_offload *opt)
+{
+ struct nfp_qdisc *qdisc;
+
+ return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
+ TC_H_ROOT, opt->handle, &qdisc);
+}
+
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
@@ -334,8 +436,9 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_reset_root(netdev, alink, opt->handle,
alink->total_queues);
nfp_abm_offload_update(alink->abm);
- return 0;
+ return nfp_abm_mq_create(netdev, alink, opt);
case TC_MQ_DESTROY:
+ nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
if (opt->handle == alink->parent) {
nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
nfp_abm_offload_update(alink->abm);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 04/13] net: sched: provide notification for graft on root
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers are currently not notified when a Qdisc is grafted as root.
This requires special casing Qdiscs added with parent = TC_H_ROOT in
the driver. Also there is no notification sent to the driver when
an existing Qdisc is grafted as root.
Add this very simple notifications, drivers should now be able to
track their Qdisc tree fully.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/linux/netdevice.h | 1 +
include/net/pkt_cls.h | 10 ++++++++++
net/sched/sch_api.c | 17 +++++++++++++++++
3 files changed, 28 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 487fa5e0e165..97b4233120e4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -845,6 +845,7 @@ enum tc_setup_type {
TC_SETUP_QDISC_PRIO,
TC_SETUP_QDISC_MQ,
TC_SETUP_QDISC_ETF,
+ TC_SETUP_ROOT_QDISC,
};
/* These structures hold the attributes of bpf state that are being passed
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index f6c0cd29dea4..fa31d034231d 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -889,4 +889,14 @@ struct tc_prio_qopt_offload {
};
};
+enum tc_root_command {
+ TC_ROOT_GRAFT,
+};
+
+struct tc_root_qopt_offload {
+ enum tc_root_command command;
+ u32 handle;
+ bool ingress;
+};
+
#endif
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index f55bc50cd0a9..9c88cec7e8a2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -860,6 +860,21 @@ void qdisc_offload_graft_helper(struct net_device *dev, struct Qdisc *sch,
}
EXPORT_SYMBOL(qdisc_offload_graft_helper);
+static void qdisc_offload_graft_root(struct net_device *dev,
+ struct Qdisc *new, struct Qdisc *old,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_root_qopt_offload graft_offload = {
+ .command = TC_ROOT_GRAFT,
+ .handle = new ? new->handle : 0,
+ .ingress = (new && new->flags & TCQ_F_INGRESS) ||
+ (old && old->flags & TCQ_F_INGRESS),
+ };
+
+ qdisc_offload_graft_helper(dev, NULL, new, old,
+ TC_SETUP_ROOT_QDISC, &graft_offload, extack);
+}
+
static int tc_fill_qdisc(struct sk_buff *skb, struct Qdisc *q, u32 clid,
u32 portid, u32 seq, u16 flags, int event)
{
@@ -1026,6 +1041,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
if (dev->flags & IFF_UP)
dev_deactivate(dev);
+ qdisc_offload_graft_root(dev, new, old, extack);
+
if (new && new->ops->attach)
goto skip;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 05/13] nfp: abm: remember which Qdisc is root
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Keep track of which Qdisc is currently root. We need to implement
TC_SETUP_ROOT_QDISC handling, and for completeness also clear the
root Qdisc pointer when it's freed. TC_SETUP_ROOT_QDISC isn't always
sent when device is dismantled.
Remembering the root Qdisc will allow us to build the entire hierarchy
in following patches.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.c | 2 ++
drivers/net/ethernet/netronome/nfp/abm/main.h | 4 ++++
drivers/net/ethernet/netronome/nfp/abm/qdisc.c | 13 +++++++++++++
3 files changed, 19 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index 35f3a6054569..da5394886a78 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -37,6 +37,8 @@ nfp_abm_setup_tc(struct nfp_app *app, struct net_device *netdev,
return -EOPNOTSUPP;
switch (type) {
+ case TC_SETUP_ROOT_QDISC:
+ return nfp_abm_setup_root(netdev, repr->app_priv, type_data);
case TC_SETUP_QDISC_MQ:
return nfp_abm_setup_tc_mq(netdev, repr->app_priv, type_data);
case TC_SETUP_QDISC_RED:
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 64cb5ebcf80e..48d519989886 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -116,6 +116,7 @@ struct nfp_red_qdisc {
* @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
* @num_qdiscs: number of currently used qdiscs
* @red_qdiscs: array of qdiscs
+ * @root_qdisc: pointer to the current root of the Qdisc hierarchy
* @qdiscs: all qdiscs recorded by major part of the handle
*/
struct nfp_abm_link {
@@ -127,9 +128,12 @@ struct nfp_abm_link {
u32 parent;
unsigned int num_qdiscs;
struct nfp_red_qdisc *red_qdiscs;
+ struct nfp_qdisc *root_qdisc;
struct radix_tree_root qdiscs;
};
+int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_root_qopt_offload *opt);
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt);
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index a6f95924656d..ba6ce2d1eda2 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -143,6 +143,9 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
return;
nfp_abm_qdisc_free(netdev, alink, qdisc);
+
+ if (alink->root_qdisc == qdisc)
+ alink->root_qdisc = NULL;
}
static void
@@ -450,3 +453,13 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
return -EOPNOTSUPP;
}
}
+
+int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct tc_root_qopt_offload *opt)
+{
+ if (opt->ingress)
+ return -EOPNOTSUPP;
+ alink->root_qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+
+ return 0;
+}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 06/13] nfp: abm: allocate Qdisc child table
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
To keep track of Qdisc hierarchy allocate a table for children
for each Qdisc. RED Qdisc can only have one child.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 6 +++++
.../net/ethernet/netronome/nfp/abm/qdisc.c | 25 +++++++++++++------
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index 48d519989886..adffa36981e0 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -84,12 +84,18 @@ enum nfp_qdisc_type {
* @type: Qdisc type
* @handle: handle of this Qdisc
* @parent_handle: handle of the parent (unreliable if Qdisc was grafted)
+ * @use_cnt: number of attachment points in the hierarchy
+ * @num_children: current size of the @children array
+ * @children: pointers to children
*/
struct nfp_qdisc {
struct net_device *netdev;
enum nfp_qdisc_type type;
u32 handle;
u32 parent_handle;
+ unsigned int use_cnt;
+ unsigned int num_children;
+ struct nfp_qdisc **children;
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index ba6ce2d1eda2..3ecb63060429 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -72,6 +72,8 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
return;
WARN_ON(radix_tree_delete(&alink->qdiscs,
TC_H_MAJ(qdisc->handle)) != qdisc);
+
+ kfree(qdisc->children);
kfree(qdisc);
port->tc_offload_cnt--;
@@ -79,7 +81,8 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
static struct nfp_qdisc *
nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
- enum nfp_qdisc_type type, u32 parent_handle, u32 handle)
+ enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
+ unsigned int children)
{
struct nfp_port *port = nfp_port_from_netdev(netdev);
struct nfp_qdisc *qdisc;
@@ -89,21 +92,28 @@ nfp_abm_qdisc_alloc(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return NULL;
+ qdisc->children = kcalloc(children, sizeof(void *), GFP_KERNEL);
+ if (!qdisc->children)
+ goto err_free_qdisc;
+
qdisc->netdev = netdev;
qdisc->type = type;
qdisc->parent_handle = parent_handle;
qdisc->handle = handle;
+ qdisc->num_children = children;
err = radix_tree_insert(&alink->qdiscs, TC_H_MAJ(qdisc->handle), qdisc);
if (err) {
nfp_err(alink->abm->app->cpp,
"Qdisc insertion into radix tree failed: %d\n", err);
- goto err_free_qdisc;
+ goto err_free_child_tbl;
}
port->tc_offload_cnt++;
return qdisc;
+err_free_child_tbl:
+ kfree(qdisc->children);
err_free_qdisc:
kfree(qdisc);
return NULL;
@@ -118,7 +128,7 @@ nfp_abm_qdisc_find(struct nfp_abm_link *alink, u32 handle)
static int
nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
enum nfp_qdisc_type type, u32 parent_handle, u32 handle,
- struct nfp_qdisc **qdisc)
+ unsigned int children, struct nfp_qdisc **qdisc)
{
*qdisc = nfp_abm_qdisc_find(alink, handle);
if (*qdisc) {
@@ -127,8 +137,8 @@ nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
return 0;
}
- *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle,
- handle);
+ *qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle, handle,
+ children);
return *qdisc ? 0 : -ENOMEM;
}
@@ -248,7 +258,7 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
int ret;
ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
- opt->handle, &qdisc);
+ opt->handle, 1, &qdisc);
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
@@ -428,7 +438,8 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct nfp_qdisc *qdisc;
return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
- TC_H_ROOT, opt->handle, &qdisc);
+ TC_H_ROOT, opt->handle,
+ alink->total_queues, &qdisc);
}
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 07/13] net: sched: red: offload a graft notification
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers offloading Qdiscs should have reasonable certainty
the offloaded behaviour matches the SW path. This is impossible
if the driver does not know about all Qdiscs or when Qdiscs move
and are reused. Send a graft notification from RED. The drivers
are expected to simply stop offloading the Qdisc, if a non-standard
child is ever grafted onto it.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 2 ++
net/sched/sch_red.c | 17 +++++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index fa31d034231d..01f2802b7aee 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -834,6 +834,7 @@ enum tc_red_command {
TC_RED_DESTROY,
TC_RED_STATS,
TC_RED_XSTATS,
+ TC_RED_GRAFT,
};
struct tc_red_qopt_offload_params {
@@ -853,6 +854,7 @@ struct tc_red_qopt_offload {
struct tc_red_qopt_offload_params set;
struct tc_qopt_offload_stats stats;
struct red_stats *xstats;
+ u32 child_handle;
};
};
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index a1d08bdd9357..4b5ca172ee2d 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -367,6 +367,21 @@ static int red_dump_class(struct Qdisc *sch, unsigned long cl,
return 0;
}
+static void red_graft_offload(struct Qdisc *sch,
+ struct Qdisc *new, struct Qdisc *old,
+ struct netlink_ext_ack *extack)
+{
+ struct tc_red_qopt_offload graft_offload = {
+ .handle = sch->handle,
+ .parent = sch->parent,
+ .child_handle = new->handle,
+ .command = TC_RED_GRAFT,
+ };
+
+ qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, old,
+ TC_SETUP_QDISC_RED, &graft_offload, extack);
+}
+
static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
struct Qdisc **old, struct netlink_ext_ack *extack)
{
@@ -376,6 +391,8 @@ static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
new = &noop_qdisc;
*old = qdisc_replace(sch, new, &q->qdisc);
+
+ red_graft_offload(sch, new, *old, extack);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 08/13] net: sched: mq: offload a graft notification
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Drivers offloading Qdiscs should have reasonable certainty
the offloaded behaviour matches the SW path. This is impossible
if the driver does not know about all Qdiscs or when Qdiscs move
and are reused. Send a graft notification from MQ.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 11 ++++++++++-
net/sched/sch_mq.c | 9 +++++++++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 01f2802b7aee..5d31820b7e80 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -821,12 +821,21 @@ enum tc_mq_command {
TC_MQ_CREATE,
TC_MQ_DESTROY,
TC_MQ_STATS,
+ TC_MQ_GRAFT,
+};
+
+struct tc_mq_opt_offload_graft_params {
+ unsigned long queue;
+ u32 child_handle;
};
struct tc_mq_qopt_offload {
enum tc_mq_command command;
u32 handle;
- struct tc_qopt_offload_stats stats;
+ union {
+ struct tc_qopt_offload_stats stats;
+ struct tc_mq_opt_offload_graft_params graft_params;
+ };
};
enum tc_red_command {
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index 1db5c1bf6ddd..203659bc3906 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -193,6 +193,7 @@ static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
struct Qdisc **old, struct netlink_ext_ack *extack)
{
struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
+ struct tc_mq_qopt_offload graft_offload;
struct net_device *dev = qdisc_dev(sch);
if (dev->flags & IFF_UP)
@@ -203,6 +204,14 @@ static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
new->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
if (dev->flags & IFF_UP)
dev_activate(dev);
+
+ graft_offload.handle = sch->handle;
+ graft_offload.graft_params.queue = cl - 1;
+ graft_offload.graft_params.child_handle = new ? new->handle : 0;
+ graft_offload.command = TC_MQ_GRAFT;
+
+ qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, *old,
+ TC_SETUP_QDISC_MQ, &graft_offload, extack);
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 09/13] nfp: abm: build full Qdisc hierarchy based on graft notifications
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Using graft notifications recreate in the driver the full Qdisc
hierarchy. Keep track of how many times each Qdisc is attached
to the hierarchy to make sure we don't offload Qdiscs which are
attached multiple times (device queues can't be shared). For
graft events of Qdiscs we don't know exist make the child as
invalid/untracked.
Note that MQ Qdisc doesn't send destruction events reliably when
device is dismantled, so we need to manually clean out the
children otherwise we'd think Qdiscs which are still in use
are getting freed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 2 +
.../net/ethernet/netronome/nfp/abm/qdisc.c | 105 ++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index adffa36981e0..daca93e90099 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -78,6 +78,8 @@ enum nfp_qdisc_type {
NFP_QDISC_RED,
};
+#define NFP_QDISC_UNTRACKED ((struct nfp_qdisc *)1UL)
+
/**
* struct nfp_qdisc - tracked TC Qdisc
* @netdev: netdev on which Qdisc was created
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 3ecb63060429..151d2dafbc76 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
/* Copyright (C) 2018 Netronome Systems, Inc. */
+#include <linux/rtnetlink.h>
#include <net/pkt_cls.h>
#include <net/pkt_sched.h>
#include <net/red.h>
@@ -12,6 +13,66 @@
#include "../nfp_port.h"
#include "main.h"
+static bool nfp_abm_qdisc_child_valid(struct nfp_qdisc *qdisc, unsigned int id)
+{
+ return qdisc->children[id] &&
+ qdisc->children[id] != NFP_QDISC_UNTRACKED;
+}
+
+static void *nfp_abm_qdisc_tree_deref_slot(void __rcu **slot)
+{
+ return rtnl_dereference(*slot);
+}
+
+static void
+nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
+ unsigned int start, unsigned int end)
+{
+ unsigned int i;
+
+ for (i = start; i < end; i++)
+ if (nfp_abm_qdisc_child_valid(qdisc, i)) {
+ qdisc->children[i]->use_cnt--;
+ qdisc->children[i] = NULL;
+ }
+}
+
+static void
+nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
+{
+ struct radix_tree_iter iter;
+ unsigned int mq_refs = 0;
+ void __rcu **slot;
+
+ if (!qdisc->use_cnt)
+ return;
+ /* MQ doesn't notify well on destruction, we need special handling of
+ * MQ's children.
+ */
+ if (qdisc->type == NFP_QDISC_MQ &&
+ qdisc == alink->root_qdisc &&
+ netdev->reg_state == NETREG_UNREGISTERING)
+ return;
+
+ /* Count refs held by MQ instances and clear pointers */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
+ unsigned int i;
+
+ if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
+ continue;
+ for (i = 0; i < mq->num_children; i++)
+ if (mq->children[i] == qdisc) {
+ mq->children[i] = NULL;
+ mq_refs++;
+ }
+ }
+
+ WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
+ qdisc->use_cnt, mq_refs);
+}
+
static void
nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
struct nfp_red_qdisc *qdisc, unsigned int queue)
@@ -70,6 +131,7 @@ nfp_abm_qdisc_free(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return;
+ nfp_abm_qdisc_clear_mq(netdev, alink, qdisc);
WARN_ON(radix_tree_delete(&alink->qdiscs,
TC_H_MAJ(qdisc->handle)) != qdisc);
@@ -152,12 +214,44 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
if (!qdisc)
return;
+ /* We don't get TC_SETUP_ROOT_QDISC w/ MQ when netdev is unregistered */
+ if (alink->root_qdisc == qdisc)
+ qdisc->use_cnt--;
+
+ nfp_abm_qdisc_unlink_children(qdisc, 0, qdisc->num_children);
nfp_abm_qdisc_free(netdev, alink, qdisc);
if (alink->root_qdisc == qdisc)
alink->root_qdisc = NULL;
}
+static int
+nfp_abm_qdisc_graft(struct nfp_abm_link *alink, u32 handle, u32 child_handle,
+ unsigned int id)
+{
+ struct nfp_qdisc *parent, *child;
+
+ parent = nfp_abm_qdisc_find(alink, handle);
+ if (!parent)
+ return 0;
+
+ if (WARN(id >= parent->num_children,
+ "graft child out of bound %d >= %d\n",
+ id, parent->num_children))
+ return -EINVAL;
+
+ nfp_abm_qdisc_unlink_children(parent, id, id + 1);
+
+ child = nfp_abm_qdisc_find(alink, child_handle);
+ if (child)
+ child->use_cnt++;
+ else
+ child = NFP_QDISC_UNTRACKED;
+ parent->children[id] = child;
+
+ return 0;
+}
+
static void
__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
u32 handle, unsigned int qs, u32 init_val)
@@ -404,6 +498,9 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
return nfp_abm_red_stats(alink, opt);
case TC_RED_XSTATS:
return nfp_abm_red_xstats(alink, opt);
+ case TC_RED_GRAFT:
+ return nfp_abm_qdisc_graft(alink, opt->handle,
+ opt->child_handle, 0);
default:
return -EOPNOTSUPP;
}
@@ -460,6 +557,10 @@ int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
return 0;
case TC_MQ_STATS:
return nfp_abm_mq_stats(alink, opt);
+ case TC_MQ_GRAFT:
+ return nfp_abm_qdisc_graft(alink, opt->handle,
+ opt->graft_params.child_handle,
+ opt->graft_params.queue);
default:
return -EOPNOTSUPP;
}
@@ -470,7 +571,11 @@ int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
{
if (opt->ingress)
return -EOPNOTSUPP;
+ if (alink->root_qdisc)
+ alink->root_qdisc->use_cnt--;
alink->root_qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+ if (alink->root_qdisc)
+ alink->root_qdisc->use_cnt++;
return 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 10/13] net: sched: red: notify drivers about RED's limit parameter
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
RED qdisc's limit parameter changes the behaviour of the qdisc,
for instance if it's set to 0 qdisc will drop all the packets.
When replace operation happens and parameter is set to non-0
a new fifo qdisc will be instantiated and replace the old child
qdisc which will be destroyed.
Drivers need to know the parameter, even if they don't impose
the actual limit to be able to reliably reconstruct the Qdisc
hierarchy.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
include/net/pkt_cls.h | 1 +
net/sched/sch_red.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 5d31820b7e80..c497ada7f591 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -850,6 +850,7 @@ struct tc_red_qopt_offload_params {
u32 min;
u32 max;
u32 probability;
+ u32 limit;
bool is_ecn;
bool is_harddrop;
struct gnet_stats_queue *qstats;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 4b5ca172ee2d..9df9942340ea 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -166,6 +166,7 @@ static int red_offload(struct Qdisc *sch, bool enable)
opt.set.min = q->parms.qth_min >> q->parms.Wlog;
opt.set.max = q->parms.qth_max >> q->parms.Wlog;
opt.set.probability = q->parms.max_P;
+ opt.set.limit = q->limit;
opt.set.is_ecn = red_use_ecn(q);
opt.set.is_harddrop = red_use_harddrop(q);
opt.set.qstats = &sch->qstats;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 11/13] nfp: abm: reset RED's child based on limit
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
RED qdisc will replace its child Qdisc with a new FIFO queue if
it is reconfigured and the limit parameter is not 0.
This means that when it's created with limit of 0 it will have no FIFO,
and all packets will be dropped. If it's changed and limit is specified
it will loose its existing child (implicit graft). Make sure we mark
RED Qdisc child as NFP_QDISC_UNTRACKED if its not the expected FIFO.
nfp_abm_qdisc_replace() will return 1 if Qdisc already existed.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
.../net/ethernet/netronome/nfp/abm/qdisc.c | 27 +++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 151d2dafbc76..1b3c0b5b52bf 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -196,7 +196,7 @@ nfp_abm_qdisc_replace(struct net_device *netdev, struct nfp_abm_link *alink,
if (*qdisc) {
if (WARN_ON((*qdisc)->type != type))
return -EINVAL;
- return 0;
+ return 1;
}
*qdisc = nfp_abm_qdisc_alloc(netdev, alink, type, parent_handle, handle,
@@ -357,11 +357,24 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
i = nfp_abm_red_find(alink, opt);
existing = i >= 0;
- if (ret) {
+ if (ret < 0) {
err = ret;
goto err_destroy;
}
+ /* If limit != 0 child gets reset */
+ if (opt->set.limit) {
+ if (nfp_abm_qdisc_child_valid(qdisc, 0))
+ qdisc->children[0]->use_cnt--;
+ qdisc->children[0] = NULL;
+ } else {
+ /* Qdisc was just allocated without a limit will use noop_qdisc,
+ * i.e. a block hole.
+ */
+ if (!ret)
+ qdisc->children[0] = NFP_QDISC_UNTRACKED;
+ }
+
if (!nfp_abm_red_check_params(alink, opt)) {
err = -EINVAL;
goto err_destroy;
@@ -533,10 +546,14 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
struct nfp_qdisc *qdisc;
+ int ret;
- return nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
- TC_H_ROOT, opt->handle,
- alink->total_queues, &qdisc);
+ ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_MQ,
+ TC_H_ROOT, opt->handle, alink->total_queues,
+ &qdisc);
+ if (ret < 0)
+ return ret;
+ return 0;
}
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 12/13] nfp: abm: save RED's parameters
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
Use the new driver Qdisc structure to keep track of parameters
of RED Qdiscs. This way as the Qdisc moves around in the hierarchy
we will be able to configure the HW appropriately.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/main.h | 14 ++++++++++++++
drivers/net/ethernet/netronome/nfp/abm/qdisc.c | 5 ++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index daca93e90099..d0d85f82bd1c 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -89,6 +89,11 @@ enum nfp_qdisc_type {
* @use_cnt: number of attachment points in the hierarchy
* @num_children: current size of the @children array
* @children: pointers to children
+ *
+ * @params_ok: parameters of this Qdisc are OK for offload
+ *
+ * @red: RED Qdisc specific parameters and state
+ * @red.threshold: ECN marking threshold
*/
struct nfp_qdisc {
struct net_device *netdev;
@@ -98,6 +103,15 @@ struct nfp_qdisc {
unsigned int use_cnt;
unsigned int num_children;
struct nfp_qdisc **children;
+
+ bool params_ok;
+
+ union {
+ /* TC_SETUP_QDISC_RED */
+ struct {
+ u32 threshold;
+ } red;
+ };
};
/**
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index 1b3c0b5b52bf..fb68038ec1da 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -375,7 +375,10 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
qdisc->children[0] = NFP_QDISC_UNTRACKED;
}
- if (!nfp_abm_red_check_params(alink, opt)) {
+ qdisc->params_ok = nfp_abm_red_check_params(alink, opt);
+ if (qdisc->params_ok) {
+ qdisc->red.threshold = opt->set.min;
+ } else {
err = -EINVAL;
goto err_destroy;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 13/13] nfp: abm: restructure Qdisc handling
From: Jakub Kicinski @ 2018-11-12 22:58 UTC (permalink / raw)
To: davem; +Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, Jakub Kicinski
In-Reply-To: <20181112225819.29823-1-jakub.kicinski@netronome.com>
In preparation of handling more Qdisc types switch to a different
offload strategy. We have now recreated the Qdisc hierarchy in
the driver. Every time the hierarchy changes parse it, and update
the configuration of the HW accordingly.
While at it drop the support of pretending that we can instantiate
a single queue on a multi-queue device in HW/FW. MQ is now required,
and each queue will have its own instance of RED.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: John Hurley <john.hurley@netronome.com>
---
drivers/net/ethernet/netronome/nfp/abm/ctrl.c | 73 ---
drivers/net/ethernet/netronome/nfp/abm/main.c | 14 +-
drivers/net/ethernet/netronome/nfp/abm/main.h | 57 +-
.../net/ethernet/netronome/nfp/abm/qdisc.c | 548 ++++++++++--------
4 files changed, 340 insertions(+), 352 deletions(-)
diff --git a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
index 564427e8a6e8..1629b07f727b 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/ctrl.c
@@ -50,27 +50,6 @@ nfp_abm_ctrl_stat(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
return 0;
}
-static int
-nfp_abm_ctrl_stat_all(struct nfp_abm_link *alink, const struct nfp_rtsym *sym,
- unsigned int stride, unsigned int offset, bool is_u64,
- u64 *res)
-{
- u64 val, sum = 0;
- unsigned int i;
- int err;
-
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- err = nfp_abm_ctrl_stat(alink, sym, stride, offset, i,
- is_u64, &val);
- if (err)
- return err;
- sum += val;
- }
-
- *res = sum;
- return 0;
-}
-
int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val)
{
struct nfp_cpp *cpp = abm->app->cpp;
@@ -155,42 +134,6 @@ int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
i, true, &stats->overlimits);
}
-int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
- struct nfp_alink_stats *stats)
-{
- u64 pkts = 0, bytes = 0;
- int i, err;
-
- for (i = 0; i < alink->vnic->max_rx_rings; i++) {
- pkts += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i));
- bytes += nn_readq(alink->vnic, NFP_NET_CFG_RXR_STATS(i) + 8);
- }
- stats->tx_pkts = pkts;
- stats->tx_bytes = bytes;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
- NFP_QLVL_STRIDE, NFP_QLVL_BLOG_BYTES,
- false, &stats->backlog_bytes);
- if (err)
- return err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->q_lvls,
- NFP_QLVL_STRIDE, NFP_QLVL_BLOG_PKTS,
- false, &stats->backlog_pkts);
- if (err)
- return err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
- true, &stats->drops);
- if (err)
- return err;
-
- return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
- true, &stats->overlimits);
-}
-
int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_xstats *xstats)
{
@@ -207,22 +150,6 @@ int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
i, true, &xstats->ecn_marked);
}
-int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
- struct nfp_alink_xstats *xstats)
-{
- int err;
-
- err = nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_DROP,
- true, &xstats->pdrop);
- if (err)
- return err;
-
- return nfp_abm_ctrl_stat_all(alink, alink->abm->qm_stats,
- NFP_QMSTAT_STRIDE, NFP_QMSTAT_ECN,
- true, &xstats->ecn_marked);
-}
-
int nfp_abm_ctrl_qm_enable(struct nfp_abm *abm)
{
return nfp_mbox_cmd(abm->app->pf, NFP_MBOX_PCIE_ABM_ENABLE,
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.c b/drivers/net/ethernet/netronome/nfp/abm/main.c
index da5394886a78..a5732d3bd1b7 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.c
@@ -7,6 +7,7 @@
#include <linux/lockdep.h>
#include <linux/netdevice.h>
#include <linux/rcupdate.h>
+#include <linux/rtnetlink.h>
#include <linux/slab.h>
#include "../nfpcore/nfp.h"
@@ -310,22 +311,14 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
alink->abm = abm;
alink->vnic = nn;
alink->id = id;
- alink->parent = TC_H_ROOT;
alink->total_queues = alink->vnic->max_rx_rings;
- alink->red_qdiscs = kvcalloc(alink->total_queues,
- sizeof(*alink->red_qdiscs),
- GFP_KERNEL);
- if (!alink->red_qdiscs) {
- err = -ENOMEM;
- goto err_free_alink;
- }
/* This is a multi-host app, make sure MAC/PHY is up, but don't
* make the MAC/PHY state follow the state of any of the ports.
*/
err = nfp_eth_set_configured(app->cpp, eth_port->index, true);
if (err < 0)
- goto err_free_qdiscs;
+ goto err_free_alink;
netif_keep_dst(nn->dp.netdev);
@@ -335,8 +328,6 @@ nfp_abm_vnic_alloc(struct nfp_app *app, struct nfp_net *nn, unsigned int id)
return 0;
-err_free_qdiscs:
- kvfree(alink->red_qdiscs);
err_free_alink:
kfree(alink);
return err;
@@ -348,7 +339,6 @@ static void nfp_abm_vnic_free(struct nfp_app *app, struct nfp_net *nn)
nfp_abm_kill_reprs(alink->abm, alink);
WARN(!radix_tree_empty(&alink->qdiscs), "left over qdiscs\n");
- kvfree(alink->red_qdiscs);
kfree(alink);
}
diff --git a/drivers/net/ethernet/netronome/nfp/abm/main.h b/drivers/net/ethernet/netronome/nfp/abm/main.h
index d0d85f82bd1c..240e2c8683fe 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/main.h
+++ b/drivers/net/ethernet/netronome/nfp/abm/main.h
@@ -9,6 +9,11 @@
#include <net/devlink.h>
#include <net/pkt_cls.h>
+/* Dump of 64 PRIOs and 256 REDs seems to take 850us on Xeon v4 @ 2.20GHz;
+ * 2.5ms / 400Hz seems more than sufficient for stats resolution.
+ */
+#define NFP_ABM_STATS_REFRESH_IVAL (2500 * 1000) /* ns */
+
#define NFP_ABM_LVL_INFINITY S32_MAX
struct nfp_app;
@@ -91,9 +96,19 @@ enum nfp_qdisc_type {
* @children: pointers to children
*
* @params_ok: parameters of this Qdisc are OK for offload
+ * @offload_mark: offload refresh state - selected for offload
+ * @offloaded: Qdisc is currently offloaded to the HW
+ *
+ * @mq: MQ Qdisc specific parameters and state
+ * @mq.stats: current stats of the MQ Qdisc
+ * @mq.prev_stats: previously reported @mq.stats
*
* @red: RED Qdisc specific parameters and state
* @red.threshold: ECN marking threshold
+ * @red.stats: current stats of the RED Qdisc
+ * @red.prev_stats: previously reported @red.stats
+ * @red.xstats: extended stats for RED - current
+ * @red.prev_xstats: extended stats for RED - previously reported
*/
struct nfp_qdisc {
struct net_device *netdev;
@@ -105,29 +120,26 @@ struct nfp_qdisc {
struct nfp_qdisc **children;
bool params_ok;
+ bool offload_mark;
+ bool offloaded;
union {
+ /* NFP_QDISC_MQ */
+ struct {
+ struct nfp_alink_stats stats;
+ struct nfp_alink_stats prev_stats;
+ } mq;
/* TC_SETUP_QDISC_RED */
struct {
u32 threshold;
+ struct nfp_alink_stats stats;
+ struct nfp_alink_stats prev_stats;
+ struct nfp_alink_xstats xstats;
+ struct nfp_alink_xstats prev_xstats;
} red;
};
};
-/**
- * struct nfp_red_qdisc - representation of single RED Qdisc
- * @handle: handle of currently offloaded RED Qdisc
- * @threshold: marking threshold of this Qdisc
- * @stats: statistics from last refresh
- * @xstats: base of extended statistics
- */
-struct nfp_red_qdisc {
- u32 handle;
- u32 threshold;
- struct nfp_alink_stats stats;
- struct nfp_alink_xstats xstats;
-};
-
/**
* struct nfp_abm_link - port tuple of a ABM NIC
* @abm: back pointer to nfp_abm
@@ -135,9 +147,9 @@ struct nfp_red_qdisc {
* @id: id of the data vNIC
* @queue_base: id of base to host queue within PCIe (not QC idx)
* @total_queues: number of PF queues
- * @parent: handle of expected parent, i.e. handle of MQ, or TC_H_ROOT
- * @num_qdiscs: number of currently used qdiscs
- * @red_qdiscs: array of qdiscs
+ *
+ * @last_stats_update: ktime of last stats update
+ *
* @root_qdisc: pointer to the current root of the Qdisc hierarchy
* @qdiscs: all qdiscs recorded by major part of the handle
*/
@@ -147,13 +159,14 @@ struct nfp_abm_link {
unsigned int id;
unsigned int queue_base;
unsigned int total_queues;
- u32 parent;
- unsigned int num_qdiscs;
- struct nfp_red_qdisc *red_qdiscs;
+
+ u64 last_stats_update;
+
struct nfp_qdisc *root_qdisc;
struct radix_tree_root qdiscs;
};
+void nfp_abm_qdisc_offload_update(struct nfp_abm_link *alink);
int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_root_qopt_offload *opt);
int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
@@ -166,12 +179,8 @@ int nfp_abm_ctrl_find_addrs(struct nfp_abm *abm);
int __nfp_abm_ctrl_set_q_lvl(struct nfp_abm *abm, unsigned int id, u32 val);
int nfp_abm_ctrl_set_q_lvl(struct nfp_abm_link *alink, unsigned int queue,
u32 val);
-int nfp_abm_ctrl_read_stats(struct nfp_abm_link *alink,
- struct nfp_alink_stats *stats);
int nfp_abm_ctrl_read_q_stats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_stats *stats);
-int nfp_abm_ctrl_read_xstats(struct nfp_abm_link *alink,
- struct nfp_alink_xstats *xstats);
int nfp_abm_ctrl_read_q_xstats(struct nfp_abm_link *alink, unsigned int i,
struct nfp_alink_xstats *xstats);
u64 nfp_abm_ctrl_stat_non_sto(struct nfp_abm_link *alink, unsigned int i);
diff --git a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
index fb68038ec1da..16c4afe3a37f 100644
--- a/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
+++ b/drivers/net/ethernet/netronome/nfp/abm/qdisc.c
@@ -13,6 +13,11 @@
#include "../nfp_port.h"
#include "main.h"
+static bool nfp_abm_qdisc_is_red(struct nfp_qdisc *qdisc)
+{
+ return qdisc->type == NFP_QDISC_RED;
+}
+
static bool nfp_abm_qdisc_child_valid(struct nfp_qdisc *qdisc, unsigned int id)
{
return qdisc->children[id] &&
@@ -24,6 +29,74 @@ static void *nfp_abm_qdisc_tree_deref_slot(void __rcu **slot)
return rtnl_dereference(*slot);
}
+static void
+nfp_abm_stats_propagate(struct nfp_alink_stats *parent,
+ struct nfp_alink_stats *child)
+{
+ parent->tx_pkts += child->tx_pkts;
+ parent->tx_bytes += child->tx_bytes;
+ parent->backlog_pkts += child->backlog_pkts;
+ parent->backlog_bytes += child->backlog_bytes;
+ parent->overlimits += child->overlimits;
+ parent->drops += child->drops;
+}
+
+static void
+nfp_abm_stats_update_red(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
+{
+ struct nfp_cpp *cpp = alink->abm->app->cpp;
+ int err;
+
+ if (!qdisc->offloaded)
+ return;
+
+ err = nfp_abm_ctrl_read_q_stats(alink, queue, &qdisc->red.stats);
+ if (err)
+ nfp_err(cpp, "RED stats (%d) read failed with error %d\n",
+ queue, err);
+
+ err = nfp_abm_ctrl_read_q_xstats(alink, queue, &qdisc->red.xstats);
+ if (err)
+ nfp_err(cpp, "RED xstats (%d) read failed with error %d\n",
+ queue, err);
+}
+
+static void
+nfp_abm_stats_update_mq(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
+{
+ unsigned int i;
+
+ if (qdisc->type != NFP_QDISC_MQ)
+ return;
+
+ for (i = 0; i < alink->total_queues; i++)
+ if (nfp_abm_qdisc_child_valid(qdisc, i))
+ nfp_abm_stats_update_red(alink, qdisc->children[i], i);
+}
+
+static void __nfp_abm_stats_update(struct nfp_abm_link *alink, u64 time_now)
+{
+ alink->last_stats_update = time_now;
+ if (alink->root_qdisc)
+ nfp_abm_stats_update_mq(alink, alink->root_qdisc);
+}
+
+static void nfp_abm_stats_update(struct nfp_abm_link *alink)
+{
+ u64 now;
+
+ /* Limit the frequency of updates - stats of non-leaf qdiscs are a sum
+ * of all their leafs, so we would read the same stat multiple times
+ * for every dump.
+ */
+ now = ktime_get();
+ if (now - alink->last_stats_update < NFP_ABM_STATS_REFRESH_IVAL)
+ return;
+
+ __nfp_abm_stats_update(alink, now);
+}
+
static void
nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
unsigned int start, unsigned int end)
@@ -38,89 +111,174 @@ nfp_abm_qdisc_unlink_children(struct nfp_qdisc *qdisc,
}
static void
-nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
- struct nfp_qdisc *qdisc)
+nfp_abm_qdisc_offload_stop(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
{
- struct radix_tree_iter iter;
- unsigned int mq_refs = 0;
- void __rcu **slot;
+ /* Don't complain when qdisc is getting unlinked */
+ if (qdisc->use_cnt)
+ nfp_warn(alink->abm->app->cpp, "Offload of '%08x' stopped\n",
+ qdisc->handle);
- if (!qdisc->use_cnt)
+ if (!nfp_abm_qdisc_is_red(qdisc))
return;
- /* MQ doesn't notify well on destruction, we need special handling of
- * MQ's children.
+
+ qdisc->red.stats.backlog_pkts = 0;
+ qdisc->red.stats.backlog_bytes = 0;
+}
+
+static int
+__nfp_abm_stats_init(struct nfp_abm_link *alink,
+ unsigned int queue, struct nfp_alink_stats *prev_stats,
+ struct nfp_alink_xstats *prev_xstats)
+{
+ u64 backlog_pkts, backlog_bytes;
+ int err;
+
+ /* Don't touch the backlog, backlog can only be reset after it has
+ * been reported back to the tc qdisc stats.
*/
- if (qdisc->type == NFP_QDISC_MQ &&
- qdisc == alink->root_qdisc &&
- netdev->reg_state == NETREG_UNREGISTERING)
- return;
+ backlog_pkts = prev_stats->backlog_pkts;
+ backlog_bytes = prev_stats->backlog_bytes;
- /* Count refs held by MQ instances and clear pointers */
- radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
- struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
- unsigned int i;
+ err = nfp_abm_ctrl_read_q_stats(alink, queue, prev_stats);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "RED stats init (%d) failed with error %d\n",
+ queue, err);
+ return err;
+ }
- if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
- continue;
- for (i = 0; i < mq->num_children; i++)
- if (mq->children[i] == qdisc) {
- mq->children[i] = NULL;
- mq_refs++;
- }
+ err = nfp_abm_ctrl_read_q_xstats(alink, queue, prev_xstats);
+ if (err) {
+ nfp_err(alink->abm->app->cpp,
+ "RED xstats init (%d) failed with error %d\n",
+ queue, err);
+ return err;
}
- WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
- qdisc->use_cnt, mq_refs);
+ prev_stats->backlog_pkts = backlog_pkts;
+ prev_stats->backlog_bytes = backlog_bytes;
+ return 0;
+}
+
+static int
+nfp_abm_stats_init(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
+{
+ return __nfp_abm_stats_init(alink, queue,
+ &qdisc->red.prev_stats,
+ &qdisc->red.prev_xstats);
}
static void
-nfp_abm_offload_compile_red(struct nfp_abm_link *alink,
- struct nfp_red_qdisc *qdisc, unsigned int queue)
+nfp_abm_offload_compile_red(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc,
+ unsigned int queue)
{
- if (!qdisc->handle)
+ qdisc->offload_mark = qdisc->type == NFP_QDISC_RED &&
+ qdisc->params_ok &&
+ qdisc->use_cnt == 1 &&
+ !qdisc->children[0];
+
+ /* If we are starting offload init prev_stats */
+ if (qdisc->offload_mark && !qdisc->offloaded)
+ if (nfp_abm_stats_init(alink, qdisc, queue))
+ qdisc->offload_mark = false;
+
+ if (!qdisc->offload_mark)
return;
- nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->threshold);
+ nfp_abm_ctrl_set_q_lvl(alink, queue, qdisc->red.threshold);
}
-static void nfp_abm_offload_compile_one(struct nfp_abm_link *alink)
+static void
+nfp_abm_offload_compile_mq(struct nfp_abm_link *alink, struct nfp_qdisc *qdisc)
{
unsigned int i;
- bool is_mq;
- is_mq = alink->num_qdiscs > 1;
+ qdisc->offload_mark = qdisc->type == NFP_QDISC_MQ;
+ if (!qdisc->offload_mark)
+ return;
for (i = 0; i < alink->total_queues; i++) {
- struct nfp_red_qdisc *next;
+ struct nfp_qdisc *child = qdisc->children[i];
- if (is_mq && !alink->red_qdiscs[i].handle)
+ if (!nfp_abm_qdisc_child_valid(qdisc, i))
continue;
- next = is_mq ? &alink->red_qdiscs[i] : &alink->red_qdiscs[0];
- nfp_abm_offload_compile_red(alink, next, i);
+ nfp_abm_offload_compile_red(alink, child, i);
}
}
-static void nfp_abm_offload_update(struct nfp_abm *abm)
+void nfp_abm_qdisc_offload_update(struct nfp_abm_link *alink)
{
- struct nfp_abm_link *alink = NULL;
- struct nfp_pf *pf = abm->app->pf;
- struct nfp_net *nn;
+ struct nfp_abm *abm = alink->abm;
+ struct radix_tree_iter iter;
+ struct nfp_qdisc *qdisc;
+ void __rcu **slot;
size_t i;
/* Mark all thresholds as unconfigured */
- __bitmap_set(abm->threshold_undef, 0, abm->num_thresholds);
+ __bitmap_set(abm->threshold_undef,
+ alink->queue_base, alink->total_queues);
- /* Configure all offloads */
- list_for_each_entry(nn, &pf->vnics, vnic_list) {
- alink = nn->app_priv;
- nfp_abm_offload_compile_one(alink);
+ /* Clear offload marks */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ qdisc = nfp_abm_qdisc_tree_deref_slot(slot);
+ qdisc->offload_mark = false;
+ }
+
+ if (alink->root_qdisc)
+ nfp_abm_offload_compile_mq(alink, alink->root_qdisc);
+
+ /* Refresh offload status */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ qdisc = nfp_abm_qdisc_tree_deref_slot(slot);
+ if (!qdisc->offload_mark && qdisc->offloaded)
+ nfp_abm_qdisc_offload_stop(alink, qdisc);
+ qdisc->offloaded = qdisc->offload_mark;
}
/* Reset the unconfigured thresholds */
for (i = 0; i < abm->num_thresholds; i++)
if (test_bit(i, abm->threshold_undef))
__nfp_abm_ctrl_set_q_lvl(abm, i, NFP_ABM_LVL_INFINITY);
+
+ __nfp_abm_stats_update(alink, ktime_get());
+}
+
+static void
+nfp_abm_qdisc_clear_mq(struct net_device *netdev, struct nfp_abm_link *alink,
+ struct nfp_qdisc *qdisc)
+{
+ struct radix_tree_iter iter;
+ unsigned int mq_refs = 0;
+ void __rcu **slot;
+
+ if (!qdisc->use_cnt)
+ return;
+ /* MQ doesn't notify well on destruction, we need special handling of
+ * MQ's children.
+ */
+ if (qdisc->type == NFP_QDISC_MQ &&
+ qdisc == alink->root_qdisc &&
+ netdev->reg_state == NETREG_UNREGISTERING)
+ return;
+
+ /* Count refs held by MQ instances and clear pointers */
+ radix_tree_for_each_slot(slot, &alink->qdiscs, &iter, 0) {
+ struct nfp_qdisc *mq = nfp_abm_qdisc_tree_deref_slot(slot);
+ unsigned int i;
+
+ if (mq->type != NFP_QDISC_MQ || mq->netdev != netdev)
+ continue;
+ for (i = 0; i < mq->num_children; i++)
+ if (mq->children[i] == qdisc) {
+ mq->children[i] = NULL;
+ mq_refs++;
+ }
+ }
+
+ WARN(qdisc->use_cnt != mq_refs, "non-zero qdisc use count: %d (- %d)\n",
+ qdisc->use_cnt, mq_refs);
}
static void
@@ -221,8 +379,13 @@ nfp_abm_qdisc_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
nfp_abm_qdisc_unlink_children(qdisc, 0, qdisc->num_children);
nfp_abm_qdisc_free(netdev, alink, qdisc);
- if (alink->root_qdisc == qdisc)
+ if (alink->root_qdisc == qdisc) {
alink->root_qdisc = NULL;
+ /* Only root change matters, other changes are acted upon on
+ * the graft notification.
+ */
+ nfp_abm_qdisc_offload_update(alink);
+ }
}
static int
@@ -249,66 +412,73 @@ nfp_abm_qdisc_graft(struct nfp_abm_link *alink, u32 handle, u32 child_handle,
child = NFP_QDISC_UNTRACKED;
parent->children[id] = child;
+ nfp_abm_qdisc_offload_update(alink);
+
return 0;
}
static void
-__nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs, u32 init_val)
+nfp_abm_stats_calculate(struct nfp_alink_stats *new,
+ struct nfp_alink_stats *old,
+ struct gnet_stats_basic_packed *bstats,
+ struct gnet_stats_queue *qstats)
{
- memset(alink->red_qdiscs, 0,
- sizeof(*alink->red_qdiscs) * alink->num_qdiscs);
-
- alink->parent = handle;
- alink->num_qdiscs = qs;
+ _bstats_update(bstats, new->tx_bytes - old->tx_bytes,
+ new->tx_pkts - old->tx_pkts);
+ qstats->qlen += new->backlog_pkts - old->backlog_pkts;
+ qstats->backlog += new->backlog_bytes - old->backlog_bytes;
+ qstats->overlimits += new->overlimits - old->overlimits;
+ qstats->drops += new->drops - old->drops;
}
static void
-nfp_abm_reset_root(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle, unsigned int qs)
+nfp_abm_stats_red_calculate(struct nfp_alink_xstats *new,
+ struct nfp_alink_xstats *old,
+ struct red_stats *stats)
{
- __nfp_abm_reset_root(netdev, alink, handle, qs, NFP_ABM_LVL_INFINITY);
+ stats->forced_mark += new->ecn_marked - old->ecn_marked;
+ stats->pdrop += new->pdrop - old->pdrop;
}
static int
-nfp_abm_red_find(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
+nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
{
- unsigned int i = TC_H_MIN(opt->parent) - 1;
+ struct nfp_qdisc *qdisc;
- if (opt->parent == TC_H_ROOT)
- i = 0;
- else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent))
- i = TC_H_MIN(opt->parent) - 1;
- else
- return -EOPNOTSUPP;
+ nfp_abm_stats_update(alink);
- if (i >= alink->num_qdiscs ||
- opt->handle != alink->red_qdiscs[i].handle)
+ qdisc = nfp_abm_qdisc_find(alink, opt->handle);
+ if (!qdisc || !qdisc->offloaded)
return -EOPNOTSUPP;
- return i;
+ nfp_abm_stats_red_calculate(&qdisc->red.xstats,
+ &qdisc->red.prev_xstats,
+ opt->xstats);
+ qdisc->red.prev_xstats = qdisc->red.xstats;
+ return 0;
}
-static void
-nfp_abm_red_destroy(struct net_device *netdev, struct nfp_abm_link *alink,
- u32 handle)
+static int
+nfp_abm_red_stats(struct nfp_abm_link *alink, u32 handle,
+ struct tc_qopt_offload_stats *stats)
{
- unsigned int i;
+ struct nfp_qdisc *qdisc;
- nfp_abm_qdisc_destroy(netdev, alink, handle);
+ nfp_abm_stats_update(alink);
- for (i = 0; i < alink->num_qdiscs; i++)
- if (handle == alink->red_qdiscs[i].handle)
- break;
- if (i == alink->num_qdiscs)
- return;
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return -EOPNOTSUPP;
+ /* If the qdisc offload has stopped we may need to adjust the backlog
+ * counters back so carry on even if qdisc is not currently offloaded.
+ */
- if (alink->parent == TC_H_ROOT)
- nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- else
- memset(&alink->red_qdiscs[i], 0, sizeof(*alink->red_qdiscs));
+ nfp_abm_stats_calculate(&qdisc->red.stats,
+ &qdisc->red.prev_stats,
+ stats->bstats, stats->qstats);
+ qdisc->red.prev_stats = qdisc->red.stats;
- nfp_abm_offload_update(alink->abm);
+ return qdisc->offloaded ? 0 : -EOPNOTSUPP;
}
static bool
@@ -347,20 +517,12 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_red_qopt_offload *opt)
{
struct nfp_qdisc *qdisc;
- bool existing;
- int i, err;
int ret;
ret = nfp_abm_qdisc_replace(netdev, alink, NFP_QDISC_RED, opt->parent,
opt->handle, 1, &qdisc);
-
- i = nfp_abm_red_find(alink, opt);
- existing = i >= 0;
-
- if (ret < 0) {
- err = ret;
- goto err_destroy;
- }
+ if (ret < 0)
+ return ret;
/* If limit != 0 child gets reset */
if (opt->set.limit) {
@@ -376,127 +538,11 @@ nfp_abm_red_replace(struct net_device *netdev, struct nfp_abm_link *alink,
}
qdisc->params_ok = nfp_abm_red_check_params(alink, opt);
- if (qdisc->params_ok) {
+ if (qdisc->params_ok)
qdisc->red.threshold = opt->set.min;
- } else {
- err = -EINVAL;
- goto err_destroy;
- }
-
- if (existing) {
- nfp_abm_offload_update(alink->abm);
- return 0;
- }
-
- if (opt->parent == TC_H_ROOT) {
- i = 0;
- __nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 1, opt->set.min);
- } else if (TC_H_MAJ(alink->parent) == TC_H_MAJ(opt->parent)) {
- i = TC_H_MIN(opt->parent) - 1;
- } else {
- return -EINVAL;
- }
- alink->red_qdiscs[i].handle = opt->handle;
-
- if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink,
- &alink->red_qdiscs[i].stats);
- else
- err = nfp_abm_ctrl_read_q_stats(alink, i,
- &alink->red_qdiscs[i].stats);
- if (err)
- goto err_destroy;
-
- if (opt->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_xstats(alink,
- &alink->red_qdiscs[i].xstats);
- else
- err = nfp_abm_ctrl_read_q_xstats(alink, i,
- &alink->red_qdiscs[i].xstats);
- if (err)
- goto err_destroy;
-
- alink->red_qdiscs[i].threshold = opt->set.min;
- alink->red_qdiscs[i].stats.backlog_pkts = 0;
- alink->red_qdiscs[i].stats.backlog_bytes = 0;
-
- nfp_abm_offload_update(alink->abm);
-
- return 0;
-err_destroy:
- /* If the qdisc keeps on living, but we can't offload undo changes */
- if (existing) {
- opt->set.qstats->qlen -=
- alink->red_qdiscs[i].stats.backlog_pkts;
- opt->set.qstats->backlog -=
- alink->red_qdiscs[i].stats.backlog_bytes;
- }
- nfp_abm_red_destroy(netdev, alink, opt->handle);
-
- return err;
-}
-
-static void
-nfp_abm_update_stats(struct nfp_alink_stats *new, struct nfp_alink_stats *old,
- struct tc_qopt_offload_stats *stats)
-{
- _bstats_update(stats->bstats, new->tx_bytes - old->tx_bytes,
- new->tx_pkts - old->tx_pkts);
- stats->qstats->qlen += new->backlog_pkts - old->backlog_pkts;
- stats->qstats->backlog += new->backlog_bytes - old->backlog_bytes;
- stats->qstats->overlimits += new->overlimits - old->overlimits;
- stats->qstats->drops += new->drops - old->drops;
-}
-
-static int
-nfp_abm_red_stats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
-{
- struct nfp_alink_stats *prev_stats;
- struct nfp_alink_stats stats;
- int i, err;
-
- i = nfp_abm_red_find(alink, opt);
- if (i < 0)
- return i;
- prev_stats = &alink->red_qdiscs[i].stats;
-
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_stats(alink, &stats);
- else
- err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
- if (err)
- return err;
-
- nfp_abm_update_stats(&stats, prev_stats, &opt->stats);
- *prev_stats = stats;
-
- return 0;
-}
-
-static int
-nfp_abm_red_xstats(struct nfp_abm_link *alink, struct tc_red_qopt_offload *opt)
-{
- struct nfp_alink_xstats *prev_xstats;
- struct nfp_alink_xstats xstats;
- int i, err;
-
- i = nfp_abm_red_find(alink, opt);
- if (i < 0)
- return i;
- prev_xstats = &alink->red_qdiscs[i].xstats;
-
- if (alink->parent == TC_H_ROOT)
- err = nfp_abm_ctrl_read_xstats(alink, &xstats);
- else
- err = nfp_abm_ctrl_read_q_xstats(alink, i, &xstats);
- if (err)
- return err;
-
- opt->xstats->forced_mark += xstats.ecn_marked - prev_xstats->ecn_marked;
- opt->xstats->pdrop += xstats.pdrop - prev_xstats->pdrop;
-
- *prev_xstats = xstats;
+ if (qdisc->use_cnt == 1)
+ nfp_abm_qdisc_offload_update(alink);
return 0;
}
@@ -508,10 +554,10 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
case TC_RED_REPLACE:
return nfp_abm_red_replace(netdev, alink, opt);
case TC_RED_DESTROY:
- nfp_abm_red_destroy(netdev, alink, opt->handle);
+ nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
return 0;
case TC_RED_STATS:
- return nfp_abm_red_stats(alink, opt);
+ return nfp_abm_red_stats(alink, opt->handle, &opt->stats);
case TC_RED_XSTATS:
return nfp_abm_red_xstats(alink, opt);
case TC_RED_GRAFT:
@@ -522,28 +568,6 @@ int nfp_abm_setup_tc_red(struct net_device *netdev, struct nfp_abm_link *alink,
}
}
-static int
-nfp_abm_mq_stats(struct nfp_abm_link *alink, struct tc_mq_qopt_offload *opt)
-{
- struct nfp_alink_stats stats;
- unsigned int i;
- int err;
-
- for (i = 0; i < alink->num_qdiscs; i++) {
- if (alink->red_qdiscs[i].handle == TC_H_UNSPEC)
- continue;
-
- err = nfp_abm_ctrl_read_q_stats(alink, i, &stats);
- if (err)
- return err;
-
- nfp_abm_update_stats(&stats, &alink->red_qdiscs[i].stats,
- &opt->stats);
- }
-
- return 0;
-}
-
static int
nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
@@ -556,27 +580,63 @@ nfp_abm_mq_create(struct net_device *netdev, struct nfp_abm_link *alink,
&qdisc);
if (ret < 0)
return ret;
+
+ qdisc->params_ok = true;
+ qdisc->offloaded = true;
+ nfp_abm_qdisc_offload_update(alink);
return 0;
}
+static int
+nfp_abm_mq_stats(struct nfp_abm_link *alink, u32 handle,
+ struct tc_qopt_offload_stats *stats)
+{
+ struct nfp_qdisc *qdisc, *red;
+ unsigned int i;
+
+ qdisc = nfp_abm_qdisc_find(alink, handle);
+ if (!qdisc)
+ return -EOPNOTSUPP;
+
+ nfp_abm_stats_update(alink);
+
+ /* MQ stats are summed over the children in the core, so we need
+ * to add up the unreported child values.
+ */
+ memset(&qdisc->mq.stats, 0, sizeof(qdisc->mq.stats));
+ memset(&qdisc->mq.prev_stats, 0, sizeof(qdisc->mq.prev_stats));
+
+ for (i = 0; i < qdisc->num_children; i++) {
+ if (!nfp_abm_qdisc_child_valid(qdisc, i))
+ continue;
+
+ if (!nfp_abm_qdisc_is_red(qdisc->children[i]))
+ continue;
+ red = qdisc->children[i];
+
+ nfp_abm_stats_propagate(&qdisc->mq.stats,
+ &red->red.stats);
+ nfp_abm_stats_propagate(&qdisc->mq.prev_stats,
+ &red->red.prev_stats);
+ }
+
+ nfp_abm_stats_calculate(&qdisc->mq.stats, &qdisc->mq.prev_stats,
+ stats->bstats, stats->qstats);
+
+ return qdisc->offloaded ? 0 : -EOPNOTSUPP;
+}
+
int nfp_abm_setup_tc_mq(struct net_device *netdev, struct nfp_abm_link *alink,
struct tc_mq_qopt_offload *opt)
{
switch (opt->command) {
case TC_MQ_CREATE:
- nfp_abm_reset_root(netdev, alink, opt->handle,
- alink->total_queues);
- nfp_abm_offload_update(alink->abm);
return nfp_abm_mq_create(netdev, alink, opt);
case TC_MQ_DESTROY:
nfp_abm_qdisc_destroy(netdev, alink, opt->handle);
- if (opt->handle == alink->parent) {
- nfp_abm_reset_root(netdev, alink, TC_H_ROOT, 0);
- nfp_abm_offload_update(alink->abm);
- }
return 0;
case TC_MQ_STATS:
- return nfp_abm_mq_stats(alink, opt);
+ return nfp_abm_mq_stats(alink, opt->handle, &opt->stats);
case TC_MQ_GRAFT:
return nfp_abm_qdisc_graft(alink, opt->handle,
opt->graft_params.child_handle,
@@ -597,5 +657,7 @@ int nfp_abm_setup_root(struct net_device *netdev, struct nfp_abm_link *alink,
if (alink->root_qdisc)
alink->root_qdisc->use_cnt++;
+ nfp_abm_qdisc_offload_update(alink);
+
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v2] geneve: Use empty braces for addr6 initializer
From: Stefano Brivio @ 2018-11-13 9:02 UTC (permalink / raw)
To: Nathan Chancellor
Cc: David S. Miller, Sabrina Dubroca, netdev, linux-kernel,
Joe Perches
In-Reply-To: <20181113061146.12182-1-natechancellor@gmail.com>
On Mon, 12 Nov 2018 23:11:47 -0700
Nathan Chancellor <natechancellor@gmail.com> wrote:
> Clang warns:
>
> drivers/net/geneve.c:428:29: error: suggest braces around initialization
> of subobject [-Werror,-Wmissing-braces]
> struct in6_addr addr6 = { 0 };
> ^
> {}
>
> Most initializations of structs in the kernel seem to use this format.
>
> Fixes: a07966447f39 ("geneve: ICMP error lookup handler")
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Also,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ 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