* Re: [ovs-dev] Pravin Shelar
From: Joe Perches @ 2017-12-27 18:33 UTC (permalink / raw)
To: Ben Pfaff, Julia Lawall, Pravin Shelar; +Cc: netdev, dev
In-Reply-To: <20171227182540.GT13883@ovn.org>
On Wed, 2017-12-27 at 10:25 -0800, Ben Pfaff wrote:
> On Wed, Dec 27, 2017 at 04:22:55PM +0100, Julia Lawall wrote:
> > The email address pshelar@nicira.com listed for Pravin Shelar in
> > MAINTAINERS (OPENVSWITCH section) seems to bounce.
>
> Pravin has used a newer address recently, so I sent out a suggested
> update (for OVS):
> https://patchwork.ozlabs.org/patch/853232/
As Pravin is still active with acks but not any authored patches in
the
last year, this should still be updated in the linux-kernel's
MAINTAINERS
file too.
---
diff --git a/MAINTAINERS b/MAINTAINERS
index
a6e86e20761e..5869e5f0b930 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@
-10137,7 +10137,7 @@ F: drivers/irqchip/irq-ompic.c
F: dri
vers/irqchip/irq-or1k-*
OPENVSWITCH
-M: Pravin Shelar <pshelar@ni
cira.com>
+M: Pravin Shelar <pshelar@ovn.org>
L: netdev@vge
r.kernel.org
L: dev@openvswitch.org
W: http://openvswitch.org
^ permalink raw reply
* Re: [PATCH V2 net-next 0/3] rds bug fixes
From: David Miller @ 2017-12-27 18:38 UTC (permalink / raw)
To: sowmini.varadhan; +Cc: netdev, rds-devel, santosh.shilimkar
In-Reply-To: <cover.1513962765.git.sowmini.varadhan@oracle.com>
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Fri, 22 Dec 2017 09:38:58 -0800
> Ran into pre-existing bugs when working on the fix for
> https://www.spinics.net/lists/netdev/msg472849.html
>
> The bugs fixed in this patchset are unrelated to the syzbot
> failure (which I'm still testing and trying to reproduce) but
> meanwhile, let's get these fixes out of the way.
>
> V2: target net-next (rds:tcp patches have a dependancy on
> changes that are in net-next, but not yet in net)
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v2] sctp: Replace use of sockets_allocated with specified macro.
From: David Miller @ 2017-12-27 18:48 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: netdev, eric.dumazet
In-Reply-To: <1513966520-5429-1-git-send-email-xiangxia.m.yue@gmail.com>
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Date: Fri, 22 Dec 2017 10:15:20 -0800
> The patch(180d8cd942ce) replaces all uses of struct sock fields'
> memory_pressure, memory_allocated, sockets_allocated, and sysctl_mem
> to accessor macros. But the sockets_allocated field of sctp sock is
> not replaced at all. Then replace it now for unifying the code.
>
> Fixes: 180d8cd942ce ("foundations of per-cgroup memory pressure controlling.")
> Cc: Glauber Costa <glommer@parallels.com>
> Signed-off-by: Tonghao Zhang <zhangtonghao@didichuxing.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH 2/4] libbpf: add error reporting in XDP
From: Alexei Starovoitov @ 2017-12-27 18:57 UTC (permalink / raw)
To: Eric Leblond; +Cc: netdev, daniel, linux-kernel
In-Reply-To: <20171227180229.1926-3-eric@regit.org>
On Wed, Dec 27, 2017 at 07:02:27PM +0100, Eric Leblond wrote:
> Parse netlink ext attribute to get the error message returned by
> the card. Code is partially take from libnl.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/4] libbpf: add function to setup XDP
From: Alexei Starovoitov @ 2017-12-27 18:57 UTC (permalink / raw)
To: Eric Leblond; +Cc: netdev, daniel, linux-kernel
In-Reply-To: <20171227180229.1926-2-eric@regit.org>
On Wed, Dec 27, 2017 at 07:02:26PM +0100, Eric Leblond wrote:
> Most of the code is taken from set_link_xdp_fd() in bpf_load.c and
> slightly modified to be library compliant.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/4] libbpf: break loop earlier
From: Alexei Starovoitov @ 2017-12-27 19:00 UTC (permalink / raw)
To: Eric Leblond; +Cc: netdev, daniel, linux-kernel
In-Reply-To: <20171227180229.1926-4-eric@regit.org>
On Wed, Dec 27, 2017 at 07:02:28PM +0100, Eric Leblond wrote:
> Get out of the loop when we have a match.
>
> Signed-off-by: Eric Leblond <eric@regit.org>
> ---
> tools/lib/bpf/libbpf.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 5fe8aaa2123e..d263748aa341 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -412,6 +412,7 @@ bpf_object__init_prog_names(struct bpf_object *obj)
> prog->section_name);
> return -LIBBPF_ERRNO__LIBELF;
> }
> + break;
why this is needed?
The top of the loop is:
for (si = 0; si < symbols->d_size / sizeof(GElf_Sym) && !name;
so as soon as name is found the loop will exit.
I agree that the loop structure is non-standard is confusing,
but adding break here will make it even more so.
If 'break' is added then '&& !name' should be removed.
^ permalink raw reply
* Re: [PATCH 4/4] libbpf: add missing SPDX-License-Identifier
From: Alexei Starovoitov @ 2017-12-27 19:01 UTC (permalink / raw)
To: Eric Leblond; +Cc: netdev, daniel, linux-kernel
In-Reply-To: <20171227180229.1926-5-eric@regit.org>
On Wed, Dec 27, 2017 at 07:02:29PM +0100, Eric Leblond wrote:
> Signed-off-by: Eric Leblond <eric@regit.org>
thank you.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 bpf-next 2/2] tools/bpftool: fix bpftool build with bintutils >= 2.8
From: Roman Gushchin @ 2017-12-27 19:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Quentin Monnet, netdev, linux-kernel, kernel-team, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20171227023204.eulgkbg7epj7nl76@ast-mbp>
On Tue, Dec 26, 2017 at 06:32:05PM -0800, Alexei Starovoitov wrote:
> On Fri, Dec 22, 2017 at 06:50:01PM +0000, Quentin Monnet wrote:
> > Hi Roman,
> >
> > 2017-12-22 16:11 UTC+0000 ~ Roman Gushchin <guro@fb.com>
> > > Bpftool build is broken with binutils version 2.28 and later.
> >
> > Could you check the binutils version? I believe it changed in 2.29
> > instead of 2.28. Could you update your commit log and subject
> > accordingly, please?
Yes, you're right. Thanks!
> >
> > > The cause is commit 003ca0fd2286 ("Refactor disassembler selection")
> > > in the binutils repo, which changed the disassembler() function
> > > signature.
> > >
> > > Fix this by adding a new "feature" to the tools/build/features
> > > infrastructure and make it responsible for decision which
> > > disassembler() function signature to use.
> > >
> > > Signed-off-by: Roman Gushchin <guro@fb.com>
> > > Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > Cc: Alexei Starovoitov <ast@kernel.org>
> > > Cc: Daniel Borkmann <daniel@iogearbox.net>
> > > ---
> > > tools/bpf/Makefile | 29 +++++++++++++++++++++++
> > > tools/bpf/bpf_jit_disasm.c | 7 ++++++
> > > tools/bpf/bpftool/Makefile | 24 +++++++++++++++++++
> > > tools/bpf/bpftool/jit_disasm.c | 7 ++++++
> > > tools/build/feature/Makefile | 4 ++++
> > > tools/build/feature/test-disassembler-four-args.c | 15 ++++++++++++
> > > 6 files changed, 86 insertions(+)
> > > create mode 100644 tools/build/feature/test-disassembler-four-args.c
> > >
> > > diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> > > index 07a6697466ef..c8ec0ae16bf0 100644
> > > --- a/tools/bpf/Makefile
> > > +++ b/tools/bpf/Makefile
> > > @@ -9,6 +9,35 @@ MAKE = make
> > > CFLAGS += -Wall -O2
> > > CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
> > >
> > > +ifeq ($(srctree),)
> > > +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> > > +srctree := $(patsubst %/,%,$(dir $(srctree)))
> > > +endif
> > > +
> > > +FEATURE_USER = .bpf
> > > +FEATURE_TESTS = libbfd disassembler-four-args
> > > +FEATURE_DISPLAY = libbfd disassembler-four-args
> >
> > Thanks for adding libbfd as I requested. However, you do not use it in
> > the Makefile to prevent compilation if the feature is not detected (see
> > "bpfdep" or "elfdep" in tools/lib/bpf/Makefile. Sorry, I should have
> > pointed it in my previous review.
> >
> > But actually, I have another issue related to the libbfd feature: since
> > commit 280e7c48c3b8 ("perf tools: fix BFD detection on opensuse") it
> > requires libiberty so that libbfd is correctly detected, but libiberty
> > is not needed on all distros (at least Ubuntu can have libbfd without
> > libiberty). Typically, detection fails on my setup, although I do have
> > libbfd installed. So forcing libbfd feature here may eventually force
> > users to install libraries they do not need to compile bpftool, which is
> > not what we want.
> >
> > I do not have a clean work around to suggest. Maybe have one
> > "libbfd-something" feature that tries to compile without libiberty, then
> > another one that tries with it, and compile the tools if at least one of
> > them succeeds. But it's probably for another patch series. In the
> > meantime, would you please simply remove libbfd detection here and
> > accept my apologies for suggesting to add it in the previous review?
>
> I think since libbfd is already used by bpftool it's a good thing
> to add feature detection. Even if it's not perfect on some setups.
Agree, we can enhance it later.
>
> Roman,
> I think you still need to do one more respin to address commit log nit?
>
Sure, will send soon-ish.
Thanks!
^ permalink raw reply
* Re: WARNING in strp_data_ready
From: Tom Herbert @ 2017-12-27 19:09 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: John Fastabend, syzbot, David S. Miller, Eric Biggers, LKML,
Linux Kernel Network Developers, syzkaller-bugs, Tom Herbert,
Cong Wang
In-Reply-To: <CACT4Y+bFsT4ZVvZSZvQdcKXb8n-79gy2tLB02jBm8wwp32YoAA@mail.gmail.com>
Did you try the patch I posted?
On Wed, Dec 27, 2017 at 10:25 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Wed, Dec 6, 2017 at 4:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>> <john.fastabend@gmail.com> wrote:
>>>> On 10/24/2017 08:20 AM, syzbot wrote:
>>>>> Hello,
>>>>>
>>>>> syzkaller hit the following crash on 73d3393ada4f70fa3df5639c8d438f2f034c0ecb
>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>>>>> compiler: gcc (GCC) 7.1.1 20170620
>>>>> .config is attached
>>>>> Raw console output is attached.
>>>>> C reproducer is attached
>>>>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>>>>> for information about syzkaller reproducers
>>>>>
>>>>>
>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>> Kernel panic - not syncing: panic_on_warn set ...
>>>>>
>>>>> CPU: 0 PID: 2996 Comm: syzkaller142210 Not tainted 4.14.0-rc5+ #138
>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>>>> Call Trace:
>>>>> <IRQ>
>>>>> __dump_stack lib/dump_stack.c:16 [inline]
>>>>> dump_stack+0x194/0x257 lib/dump_stack.c:52
>>>>> panic+0x1e4/0x417 kernel/panic.c:181
>>>>> __warn+0x1c4/0x1d9 kernel/panic.c:542
>>>>> report_bug+0x211/0x2d0 lib/bug.c:183
>>>>> fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
>>>>> do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
>>>>> do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
>>>>> do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
>>>>> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
>>>>> invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
>>>>> RIP: 0010:sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>> RIP: 0010:sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>> RIP: 0010:strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>> RSP: 0018:ffff8801db206b18 EFLAGS: 00010206
>>>>> RAX: ffff8801d1e02080 RBX: ffff8801dad74c48 RCX: 0000000000000000
>>>>> RDX: 0000000000000100 RSI: ffff8801d29fa0a0 RDI: ffffffff85cbede0
>>>>> RBP: ffff8801db206b38 R08: 0000000000000005 R09: 1ffffffff0ce0bcd
>>>>> R10: ffff8801db206a00 R11: dffffc0000000000 R12: ffff8801d29fa000
>>>>> R13: ffff8801dad74c50 R14: ffff8801d4350a92 R15: 0000000000000001
>>>>> psock_data_ready+0x56/0x70 net/kcm/kcmsock.c:353
>>>>
>>>> Looks like KCM is calling sk_data_ready() without first taking the
>>>> sock lock.
>>>>
>>>> /* Called with lower sock held */
>>>> static void kcm_rcv_strparser(struct strparser *strp, struct sk_buff *skb)
>>>> {
>>>> [...]
>>>> if (kcm_queue_rcv_skb(&kcm->sk, skb)) {
>>>>
>>>> In this case kcm->sk is not the same lock the comment is referring to.
>>>> And kcm_queue_rcv_skb() will eventually call sk_data_ready().
>>>>
>>>> @Tom, how about wrapping the sk_data_ready call in {lock|release}_sock?
>>>> I don't have anything better in mind immediately.
>>>>
>>> The sock locks are taken in reverse order in the send path so so
>>> grabbing kcm sock lock with lower lock held to call sk_data_ready may
>>> lead to deadlock like I think.
>>>
>>> It might be possible to change the order in the send path to do this.
>>> Something like:
>>>
>>> trylock on lower socket lock
>>> -if trylock fails
>>> - release kcm sock lock
>>> - lock lower sock
>>> - lock kcm sock
>>> - call sendpage locked function
>>>
>>> I admit that dealing with two levels of socket locks in the data path
>>> is quite a pain :-)
>>
>> up
>>
>> still happening and we've lost 50K+ test VMs on this
>
> up
>
> Still happens and number of crashes crossed 60K, can we do something
> with this please?
^ permalink raw reply
* [PATCH v3 bpf-next 1/2] tools/bpftool: use version from the kernel source tree
From: Roman Gushchin @ 2017-12-27 19:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, Roman Gushchin, Alexei Starovoitov,
Daniel Borkmann
Bpftool determines it's own version based on the kernel
version, which is picked from the linux/version.h header.
It's strange to use the version of the installed kernel
headers, and makes much more sense to use the version
of the actual source tree, where bpftool sources are.
Fix this by building kernelversion target and use
the resulting string as bpftool version.
Example:
before:
$ bpftool version
bpftool v4.14.6
after:
$ bpftool version
bpftool v4.15.0-rc3
$bpftool version --json
{"version":"4.15.0-rc3"}
Signed-off-by: Roman Gushchin <guro@fb.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
tools/bpf/bpftool/Makefile | 3 +++
tools/bpf/bpftool/main.c | 13 ++-----------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 3f17ad317512..f8f31a8d9269 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -23,6 +23,8 @@ endif
LIBBPF = $(BPF_PATH)libbpf.a
+BPFTOOL_VERSION=$(shell make --no-print-directory -sC ../../.. kernelversion)
+
$(LIBBPF): FORCE
$(Q)$(MAKE) -C $(BPF_DIR) OUTPUT=$(OUTPUT) $(OUTPUT)libbpf.a FEATURES_DUMP=$(FEATURE_DUMP_EXPORT)
@@ -38,6 +40,7 @@ CC = gcc
CFLAGS += -O2
CFLAGS += -W -Wall -Wextra -Wno-unused-parameter -Wshadow
CFLAGS += -D__EXPORTED_HEADERS__ -I$(srctree)/tools/include/uapi -I$(srctree)/tools/include -I$(srctree)/tools/lib/bpf -I$(srctree)/kernel/bpf/
+CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
INSTALL ?= install
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index ecd53ccf1239..3a0396d87c42 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -38,7 +38,6 @@
#include <errno.h>
#include <getopt.h>
#include <linux/bpf.h>
-#include <linux/version.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -95,21 +94,13 @@ static int do_help(int argc, char **argv)
static int do_version(int argc, char **argv)
{
- unsigned int version[3];
-
- version[0] = LINUX_VERSION_CODE >> 16;
- version[1] = LINUX_VERSION_CODE >> 8 & 0xf;
- version[2] = LINUX_VERSION_CODE & 0xf;
-
if (json_output) {
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "version");
- jsonw_printf(json_wtr, "\"%u.%u.%u\"",
- version[0], version[1], version[2]);
+ jsonw_printf(json_wtr, "\"%s\"", BPFTOOL_VERSION);
jsonw_end_object(json_wtr);
} else {
- printf("%s v%u.%u.%u\n", bin_name,
- version[0], version[1], version[2]);
+ printf("%s v%s\n", bin_name, BPFTOOL_VERSION);
}
return 0;
}
--
2.14.3
^ permalink raw reply related
* [PATCH v3 bpf-next 2/2] tools/bpftool: fix bpftool build with bintutils >= 2.9
From: Roman Gushchin @ 2017-12-27 19:16 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel-team, Roman Gushchin, Jakub Kicinski,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20171227191629.4920-1-guro@fb.com>
Bpftool build is broken with binutils version 2.29 and later.
The cause is commit 003ca0fd2286 ("Refactor disassembler selection")
in the binutils repo, which changed the disassembler() function
signature.
Fix this by adding a new "feature" to the tools/build/features
infrastructure and make it responsible for decision which
disassembler() function signature to use.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
---
tools/bpf/Makefile | 29 +++++++++++++++++++++++
tools/bpf/bpf_jit_disasm.c | 7 ++++++
tools/bpf/bpftool/Makefile | 24 +++++++++++++++++++
tools/bpf/bpftool/jit_disasm.c | 7 ++++++
tools/build/feature/Makefile | 4 ++++
tools/build/feature/test-disassembler-four-args.c | 15 ++++++++++++
6 files changed, 86 insertions(+)
create mode 100644 tools/build/feature/test-disassembler-four-args.c
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 07a6697466ef..c8ec0ae16bf0 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -9,6 +9,35 @@ MAKE = make
CFLAGS += -Wall -O2
CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
+FEATURE_USER = .bpf
+FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_DISPLAY = libbfd disassembler-four-args
+
+check_feat := 1
+NON_CHECK_FEAT_TARGETS := clean bpftool_clean
+ifdef MAKECMDGOALS
+ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
+ check_feat := 0
+endif
+endif
+
+ifeq ($(check_feat),1)
+ifeq ($(FEATURES_DUMP),)
+include $(srctree)/tools/build/Makefile.feature
+else
+include $(FEATURES_DUMP)
+endif
+endif
+
+ifeq ($(feature-disassembler-four-args), 1)
+CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
+endif
+
%.yacc.c: %.y
$(YACC) -o $@ -d $<
diff --git a/tools/bpf/bpf_jit_disasm.c b/tools/bpf/bpf_jit_disasm.c
index 75bf526a0168..30044bc4f389 100644
--- a/tools/bpf/bpf_jit_disasm.c
+++ b/tools/bpf/bpf_jit_disasm.c
@@ -72,7 +72,14 @@ static void get_asm_insns(uint8_t *image, size_t len, int opcodes)
disassemble_init_for_target(&info);
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+ disassemble = disassembler(info.arch,
+ bfd_big_endian(bfdf),
+ info.mach,
+ bfdf);
+#else
disassemble = disassembler(bfdf);
+#endif
assert(disassemble);
do {
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index f8f31a8d9269..2237bc43f71c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -46,6 +46,30 @@ LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
INSTALL ?= install
RM ?= rm -f
+FEATURE_USER = .bpftool
+FEATURE_TESTS = libbfd disassembler-four-args
+FEATURE_DISPLAY = libbfd disassembler-four-args
+
+check_feat := 1
+NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
+ifdef MAKECMDGOALS
+ifeq ($(filter-out $(NON_CHECK_FEAT_TARGETS),$(MAKECMDGOALS)),)
+ check_feat := 0
+endif
+endif
+
+ifeq ($(check_feat),1)
+ifeq ($(FEATURES_DUMP),)
+include $(srctree)/tools/build/Makefile.feature
+else
+include $(FEATURES_DUMP)
+endif
+endif
+
+ifeq ($(feature-disassembler-four-args), 1)
+CFLAGS += -DDISASM_FOUR_ARGS_SIGNATURE
+endif
+
include $(wildcard *.d)
all: $(OUTPUT)bpftool
diff --git a/tools/bpf/bpftool/jit_disasm.c b/tools/bpf/bpftool/jit_disasm.c
index 1551d3918d4c..57d32e8a1391 100644
--- a/tools/bpf/bpftool/jit_disasm.c
+++ b/tools/bpf/bpftool/jit_disasm.c
@@ -107,7 +107,14 @@ void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes)
disassemble_init_for_target(&info);
+#ifdef DISASM_FOUR_ARGS_SIGNATURE
+ disassemble = disassembler(info.arch,
+ bfd_big_endian(bfdf),
+ info.mach,
+ bfdf);
+#else
disassemble = disassembler(bfdf);
+#endif
assert(disassemble);
if (json_output)
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index 96982640fbf8..17f2c73fff8b 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -13,6 +13,7 @@ FILES= \
test-hello.bin \
test-libaudit.bin \
test-libbfd.bin \
+ test-disassembler-four-args.bin \
test-liberty.bin \
test-liberty-z.bin \
test-cplus-demangle.bin \
@@ -188,6 +189,9 @@ $(OUTPUT)test-libpython-version.bin:
$(OUTPUT)test-libbfd.bin:
$(BUILD) -DPACKAGE='"perf"' -lbfd -lz -liberty -ldl
+$(OUTPUT)test-disassembler-four-args.bin:
+ $(BUILD) -lbfd -lopcodes
+
$(OUTPUT)test-liberty.bin:
$(CC) $(CFLAGS) -Wall -Werror -o $@ test-libbfd.c -DPACKAGE='"perf"' $(LDFLAGS) -lbfd -ldl -liberty
diff --git a/tools/build/feature/test-disassembler-four-args.c b/tools/build/feature/test-disassembler-four-args.c
new file mode 100644
index 000000000000..45ce65cfddf0
--- /dev/null
+++ b/tools/build/feature/test-disassembler-four-args.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <bfd.h>
+#include <dis-asm.h>
+
+int main(void)
+{
+ bfd *abfd = bfd_openr(NULL, NULL);
+
+ disassembler(bfd_get_arch(abfd),
+ bfd_big_endian(abfd),
+ bfd_get_mach(abfd),
+ abfd);
+
+ return 0;
+}
--
2.14.3
^ permalink raw reply related
* Re: WARNING in strp_data_ready
From: Dmitry Vyukov @ 2017-12-27 19:20 UTC (permalink / raw)
To: Tom Herbert
Cc: John Fastabend, syzbot, David S. Miller, Eric Biggers, LKML,
Linux Kernel Network Developers, syzkaller-bugs, Tom Herbert,
Cong Wang
In-Reply-To: <CALx6S37RTEzd5pABpULPfrsoe-Huj0GZtQpOUfG=tT9dn5wL_A@mail.gmail.com>
On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <tom@herbertland.com> wrote:
> Did you try the patch I posted?
Hi Tom,
No. And I didn't know I need to. Why?
If you think the patch needs additional testing, you can ask syzbot to
test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
Otherwise proceed with committing it. Or what are we waiting for?
Thanks
> On Wed, Dec 27, 2017 at 10:25 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Wed, Dec 6, 2017 at 4:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> <john.fastabend@gmail.com> wrote:
>>>>> On 10/24/2017 08:20 AM, syzbot wrote:
>>>>>> Hello,
>>>>>>
>>>>>> syzkaller hit the following crash on 73d3393ada4f70fa3df5639c8d438f2f034c0ecb
>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>>>>>> compiler: gcc (GCC) 7.1.1 20170620
>>>>>> .config is attached
>>>>>> Raw console output is attached.
>>>>>> C reproducer is attached
>>>>>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>>>>>> for information about syzkaller reproducers
>>>>>>
>>>>>>
>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>>> Kernel panic - not syncing: panic_on_warn set ...
>>>>>>
>>>>>> CPU: 0 PID: 2996 Comm: syzkaller142210 Not tainted 4.14.0-rc5+ #138
>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>>>>> Call Trace:
>>>>>> <IRQ>
>>>>>> __dump_stack lib/dump_stack.c:16 [inline]
>>>>>> dump_stack+0x194/0x257 lib/dump_stack.c:52
>>>>>> panic+0x1e4/0x417 kernel/panic.c:181
>>>>>> __warn+0x1c4/0x1d9 kernel/panic.c:542
>>>>>> report_bug+0x211/0x2d0 lib/bug.c:183
>>>>>> fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
>>>>>> do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
>>>>>> do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
>>>>>> do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
>>>>>> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
>>>>>> invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
>>>>>> RIP: 0010:sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>>> RIP: 0010:sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>>> RIP: 0010:strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>>> RSP: 0018:ffff8801db206b18 EFLAGS: 00010206
>>>>>> RAX: ffff8801d1e02080 RBX: ffff8801dad74c48 RCX: 0000000000000000
>>>>>> RDX: 0000000000000100 RSI: ffff8801d29fa0a0 RDI: ffffffff85cbede0
>>>>>> RBP: ffff8801db206b38 R08: 0000000000000005 R09: 1ffffffff0ce0bcd
>>>>>> R10: ffff8801db206a00 R11: dffffc0000000000 R12: ffff8801d29fa000
>>>>>> R13: ffff8801dad74c50 R14: ffff8801d4350a92 R15: 0000000000000001
>>>>>> psock_data_ready+0x56/0x70 net/kcm/kcmsock.c:353
>>>>>
>>>>> Looks like KCM is calling sk_data_ready() without first taking the
>>>>> sock lock.
>>>>>
>>>>> /* Called with lower sock held */
>>>>> static void kcm_rcv_strparser(struct strparser *strp, struct sk_buff *skb)
>>>>> {
>>>>> [...]
>>>>> if (kcm_queue_rcv_skb(&kcm->sk, skb)) {
>>>>>
>>>>> In this case kcm->sk is not the same lock the comment is referring to.
>>>>> And kcm_queue_rcv_skb() will eventually call sk_data_ready().
>>>>>
>>>>> @Tom, how about wrapping the sk_data_ready call in {lock|release}_sock?
>>>>> I don't have anything better in mind immediately.
>>>>>
>>>> The sock locks are taken in reverse order in the send path so so
>>>> grabbing kcm sock lock with lower lock held to call sk_data_ready may
>>>> lead to deadlock like I think.
>>>>
>>>> It might be possible to change the order in the send path to do this.
>>>> Something like:
>>>>
>>>> trylock on lower socket lock
>>>> -if trylock fails
>>>> - release kcm sock lock
>>>> - lock lower sock
>>>> - lock kcm sock
>>>> - call sendpage locked function
>>>>
>>>> I admit that dealing with two levels of socket locks in the data path
>>>> is quite a pain :-)
>>>
>>> up
>>>
>>> still happening and we've lost 50K+ test VMs on this
>>
>> up
>>
>> Still happens and number of crashes crossed 60K, can we do something
>> with this please?
^ permalink raw reply
* Re: [PATCH net] phylink: ensure we report link down when LOS asserted
From: Florian Fainelli @ 2017-12-27 19:27 UTC (permalink / raw)
To: Russell King, Andrew Lunn; +Cc: netdev
In-Reply-To: <E1eTyRE-0007ZC-Ap@rmk-PC.armlinux.org.uk>
On 12/26/2017 03:15 PM, Russell King wrote:
> Although we disable the netdev carrier, we fail to report in the kernel
> log that the link went down. Fix this.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
--
Florian
^ permalink raw reply
* Re: [PATCH net] sfp: fix sfp-bus oops when removing socket/upstream
From: Florian Fainelli @ 2017-12-27 19:29 UTC (permalink / raw)
To: Russell King, Andrew Lunn; +Cc: netdev
In-Reply-To: <E1eTyRJ-0007ZJ-IS@rmk-PC.armlinux.org.uk>
On 12/26/2017 03:15 PM, Russell King wrote:
> When we remove a socket or upstream, and the other side isn't
> registered, we dereference a NULL pointer, causing a kernel oops.
> Fix this.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Fixes: ce0aa27ff3f6 ("sfp: add sfp-bus to bridge between network devices
and sfp cages")
--
Florian
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Andrew Lunn @ 2017-12-27 19:31 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, netdev, davem, arkadis, mlxsw, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck, linville,
gospo, steven.lin1, yuvalm, ogerlitz, roopa
In-Reply-To: <20171227131531.GE1997@nanopsycho>
> Hmm. That documents mainly sysfs. No mention of Netlink at all. But
> maybe I missed it. Also, that defines the interface as is. However we
> are talking about the data exchanged over the interface, not the
> interface itself. I don't see how ASIC/HW specific thing, like for
> example KVD in our case could be part of kernel ABI.
You need to be very careful here. As soon as somebody starts using it,
it might become an ABI. Or you need to clearly document it is not ABI,
there is no guarantee it will not disappear or change its meaning in
the next kernel, and it should be used with extreme caution.
Personally, if DSA drivers were to expose such settings, i would
consider them ABI, which people can rely on to remain stable.
Andrew
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Roopa Prabhu @ 2017-12-27 19:43 UTC (permalink / raw)
To: David Ahern
Cc: Jiri Pirko, netdev, David Miller, Arkadi Sharshevsky, mlxsw,
Andrew Lunn, Vivien Didelot, Florian Fainelli, Michael Chan,
ganeshgr, Saeed Mahameed, matanb, leonro, Ido Schimmel,
jakub.kicinski, ast, Daniel Borkmann, Simon Horman,
pieter.jansenvanvuuren, john.hurley, Alexander Duyck,
John W. Linville, Andy Gospodarek <gospo@
In-Reply-To: <ae70d810-8277-899b-b2a9-6b2dbdd5eb21@cumulusnetworks.com>
On Wed, Dec 27, 2017 at 8:34 AM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 12/27/17 2:09 AM, Jiri Pirko wrote:
>> Wed, Dec 27, 2017 at 05:05:09AM CET, dsa@cumulusnetworks.com wrote:
>>> On 12/26/17 5:23 AM, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> Many of the ASIC's internal resources are limited and are shared between
>>>> several hardware procedures. For example, unified hash-based memory can
>>>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>>>> can provide a partitioning scheme for such a resource in order to perform
>>>> fine tuning for his application. In such cases performing driver reload is
>>>> needed for the changes to take place, thus this patchset also adds support
>>>> for hot reload.
>>>>
>>>> Such an abstraction can be coupled with devlink's dpipe interface, which
>>>> models the ASIC's pipeline as a graph of match/action tables. By modeling
>>>> the hardware resource object, and by coupling it to several dpipe tables,
>>>> further visibility can be achieved in order to debug ASIC-wide issues.
>>>>
>>>> The proposed interface will provide the user the ability to understand the
>>>> limitations of the hardware, and receive notification regarding its occupancy.
>>>> Furthermore, monitoring the resource occupancy can be done in real-time and
>>>> can be useful in many cases.
>>>
>>> In the last RFC (not v1, but RFC) I asked for some kind of description
>>> for each resource, and you and Arkadi have pushed back. Let's walk
>>> through an example to see what I mean:
>>>
>>> $ devlink resource show pci/0000:03:00.0
>>> pci/0000:03:00.0:
>>> name kvd size 245760 size_valid true
>>> resources:
>>> name linear size 98304 occ 0
>>> name hash_double size 60416
>>> name hash_single size 87040
>>>
>>> So this 2700 has 3 resources that can be managed -- some table or
>>> resource or something named 'kvd' with linear, hash_double and
>>> hash_single sub-resources. What are these names referring too? The above
>>> output gives no description, and 'kvd' is not an industry term. Further,
>>
>> This are internal resources specific to the ASIC. Would you like some
>> description to each or something like that?
>
> devlink has some nice self-documenting capabilities. What's missing here
> is a description of what the resource is used for in standard terms --
> ipv4 host routes, fdb, nexthops, rifs, etc. Even if the description is a
> short list versus an exhaustive list of everything it is used for. e.g.,
> Why would a user decrease linear and increase hash_single or vice versa?
Arkadi, on what david says above, can the resource names and ids not
be driver specific, but moved up to the switchdev layer and just map
to fdb, host routes, nexthops table sizes etc ?.
Can these generic networking resources then in-turn be mapped to kvd
sizes by the driver ?
^ permalink raw reply
* Re: [patch iproute2 v3 3/4] tc: Add -bs option to batch mode
From: Marcelo Ricardo Leitner @ 2017-12-27 19:56 UTC (permalink / raw)
To: Chris Mi; +Cc: netdev, gerlitz.or, stephen, dsahern
In-Reply-To: <20171225084658.24076-4-chrism@mellanox.com>
On Mon, Dec 25, 2017 at 05:46:57PM +0900, Chris Mi wrote:
> @@ -267,6 +287,7 @@ int main(int argc, char **argv)
> {
> int ret;
> char *batch_file = NULL;
> + int batch_size = 1;
>
> while (argc > 1) {
> if (argv[1][0] != '-')
> @@ -297,6 +318,14 @@ int main(int argc, char **argv)
> if (argc <= 1)
> usage();
> batch_file = argv[1];
> + } else if (matches(argv[1], "-batchsize") == 0 ||
> + matches(argv[1], "-bs") == 0) {
> + argc--; argv++;
> + if (argc <= 1)
> + usage();
> + batch_size = atoi(argv[1]);
> + if (batch_size > MSG_IOV_MAX)
> + batch_size = MSG_IOV_MAX;
what about
if (batch_size < 1)
batch_size = 1;
> } else if (matches(argv[1], "-netns") == 0) {
> NEXT_ARG();
> if (netns_switch(argv[1]))
^ permalink raw reply
* Re: WARNING in strp_data_ready
From: Dmitry Vyukov @ 2017-12-27 20:14 UTC (permalink / raw)
To: Ozgur
Cc: Tom Herbert, John Fastabend, syzbot, David S. Miller,
Eric Biggers, LKML, Linux Kernel Network Developers,
syzkaller-bugs@googlegroups.com, Tom Herbert, Cong Wang
In-Reply-To: <1164631514405294@web52j.yandex.ru>
On Wed, Dec 27, 2017 at 9:08 PM, Ozgur <ozgur@goosey.org> wrote:
>
>
> 27.12.2017, 22:21, "Dmitry Vyukov" <dvyukov@google.com>:
>> On Wed, Dec 27, 2017 at 8:09 PM, Tom Herbert <tom@herbertland.com> wrote:
>>> Did you try the patch I posted?
>>
>> Hi Tom,
>
> Hello Dmitry,
>
>> No. And I didn't know I need to. Why?
>> If you think the patch needs additional testing, you can ask syzbot to
>> test it. See https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
>> Otherwise proceed with committing it. Or what are we waiting for?
>>
>> Thanks
>
> I think we need to fixed patch for crash, in fact check to patch code and test solve the bug.
> How do test it because there is no patch in the following bug?
Hi Ozgur,
I am not sure I completely understand what you mean. But the
reproducer for this bug (which one can use for testing) is here:
https://groups.google.com/forum/#!topic/syzkaller-bugs/Kxs05ziCpgY
Tom also mentions there is some patch for this, but I don't know where
it is, it doesn't seem to be referenced from this thread.
> The fix patch should be for this net/kcm/kcmsock.c file and lock functions must be added calling sk_data_ready ().
> Regards
>
> Ozgur
>
>>> On Wed, Dec 27, 2017 at 10:25 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>> On Wed, Dec 6, 2017 at 4:44 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>>>>>> <john.fastabend@gmail.com> wrote:
>>>>>>> On 10/24/2017 08:20 AM, syzbot wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> syzkaller hit the following crash on 73d3393ada4f70fa3df5639c8d438f2f034c0ecb
>>>>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>>>>>>>> compiler: gcc (GCC) 7.1.1 20170620
>>>>>>>> .config is attached
>>>>>>>> Raw console output is attached.
>>>>>>>> C reproducer is attached
>>>>>>>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>>>>>>>> for information about syzkaller reproducers
>>>>>>>>
>>>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>>>>> WARNING: CPU: 0 PID: 2996 at ./include/net/sock.h:1505 strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>>>>> Kernel panic - not syncing: panic_on_warn set ...
>>>>>>>>
>>>>>>>> CPU: 0 PID: 2996 Comm: syzkaller142210 Not tainted 4.14.0-rc5+ #138
>>>>>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>>>>>>>> Call Trace:
>>>>>>>> <IRQ>
>>>>>>>> __dump_stack lib/dump_stack.c:16 [inline]
>>>>>>>> dump_stack+0x194/0x257 lib/dump_stack.c:52
>>>>>>>> panic+0x1e4/0x417 kernel/panic.c:181
>>>>>>>> __warn+0x1c4/0x1d9 kernel/panic.c:542
>>>>>>>> report_bug+0x211/0x2d0 lib/bug.c:183
>>>>>>>> fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:178
>>>>>>>> do_trap_no_signal arch/x86/kernel/traps.c:212 [inline]
>>>>>>>> do_trap+0x260/0x390 arch/x86/kernel/traps.c:261
>>>>>>>> do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:298
>>>>>>>> do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:311
>>>>>>>> invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:905
>>>>>>>> RIP: 0010:sock_owned_by_me include/net/sock.h:1505 [inline]
>>>>>>>> RIP: 0010:sock_owned_by_user include/net/sock.h:1511 [inline]
>>>>>>>> RIP: 0010:strp_data_ready+0x2b7/0x390 net/strparser/strparser.c:404
>>>>>>>> RSP: 0018:ffff8801db206b18 EFLAGS: 00010206
>>>>>>>> RAX: ffff8801d1e02080 RBX: ffff8801dad74c48 RCX: 0000000000000000
>>>>>>>> RDX: 0000000000000100 RSI: ffff8801d29fa0a0 RDI: ffffffff85cbede0
>>>>>>>> RBP: ffff8801db206b38 R08: 0000000000000005 R09: 1ffffffff0ce0bcd
>>>>>>>> R10: ffff8801db206a00 R11: dffffc0000000000 R12: ffff8801d29fa000
>>>>>>>> R13: ffff8801dad74c50 R14: ffff8801d4350a92 R15: 0000000000000001
>>>>>>>> psock_data_ready+0x56/0x70 net/kcm/kcmsock.c:353
>>>>>>>
>>>>>>> Looks like KCM is calling sk_data_ready() without first taking the
>>>>>>> sock lock.
>>>>>>>
>>>>>>> /* Called with lower sock held */
>>>>>>> static void kcm_rcv_strparser(struct strparser *strp, struct sk_buff *skb)
>>>>>>> {
>>>>>>> [...]
>>>>>>> if (kcm_queue_rcv_skb(&kcm->sk, skb)) {
>>>>>>>
>>>>>>> In this case kcm->sk is not the same lock the comment is referring to.
>>>>>>> And kcm_queue_rcv_skb() will eventually call sk_data_ready().
>>>>>>>
>>>>>>> @Tom, how about wrapping the sk_data_ready call in {lock|release}_sock?
>>>>>>> I don't have anything better in mind immediately.
>>>>>> The sock locks are taken in reverse order in the send path so so
>>>>>> grabbing kcm sock lock with lower lock held to call sk_data_ready may
>>>>>> lead to deadlock like I think.
>>>>>>
>>>>>> It might be possible to change the order in the send path to do this.
>>>>>> Something like:
>>>>>>
>>>>>> trylock on lower socket lock
>>>>>> -if trylock fails
>>>>>> - release kcm sock lock
>>>>>> - lock lower sock
>>>>>> - lock kcm sock
>>>>>> - call sendpage locked function
>>>>>>
>>>>>> I admit that dealing with two levels of socket locks in the data path
>>>>>> is quite a pain :-)
>>>>>
>>>>> up
>>>>>
>>>>> still happening and we've lost 50K+ test VMs on this
>>>>
>>>> up
>>>>
>>>> Still happens and number of crashes crossed 60K, can we do something
>>>> with this please?
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Arkadi Sharshevsky @ 2017-12-27 20:15 UTC (permalink / raw)
To: David Ahern, Jiri Pirko
Cc: netdev, davem, mlxsw, andrew, vivien.didelot, f.fainelli,
michael.chan, ganeshgr, saeedm, matanb, leonro, idosch,
jakub.kicinski, ast, daniel, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, linville, gospo, steven.lin1,
yuvalm, ogerlitz, roopa
In-Reply-To: <ae70d810-8277-899b-b2a9-6b2dbdd5eb21@cumulusnetworks.com>
On 12/27/2017 06:34 PM, David Ahern wrote:
> On 12/27/17 2:09 AM, Jiri Pirko wrote:
>> Wed, Dec 27, 2017 at 05:05:09AM CET, dsa@cumulusnetworks.com wrote:
>>> On 12/26/17 5:23 AM, Jiri Pirko wrote:
>>>> From: Jiri Pirko <jiri@mellanox.com>
>>>>
>>>> Many of the ASIC's internal resources are limited and are shared between
>>>> several hardware procedures. For example, unified hash-based memory can
>>>> be used for many lookup purposes, like FDB and LPM. In many cases the user
>>>> can provide a partitioning scheme for such a resource in order to perform
>>>> fine tuning for his application. In such cases performing driver reload is
>>>> needed for the changes to take place, thus this patchset also adds support
>>>> for hot reload.
>>>>
>>>> Such an abstraction can be coupled with devlink's dpipe interface, which
>>>> models the ASIC's pipeline as a graph of match/action tables. By modeling
>>>> the hardware resource object, and by coupling it to several dpipe tables,
>>>> further visibility can be achieved in order to debug ASIC-wide issues.
>>>>
>>>> The proposed interface will provide the user the ability to understand the
>>>> limitations of the hardware, and receive notification regarding its occupancy.
>>>> Furthermore, monitoring the resource occupancy can be done in real-time and
>>>> can be useful in many cases.
>>>
>>> In the last RFC (not v1, but RFC) I asked for some kind of description
>>> for each resource, and you and Arkadi have pushed back. Let's walk
>>> through an example to see what I mean:
>>>
>>> $ devlink resource show pci/0000:03:00.0
>>> pci/0000:03:00.0:
>>> name kvd size 245760 size_valid true
>>> resources:
>>> name linear size 98304 occ 0
>>> name hash_double size 60416
>>> name hash_single size 87040
>>>
>>> So this 2700 has 3 resources that can be managed -- some table or
>>> resource or something named 'kvd' with linear, hash_double and
>>> hash_single sub-resources. What are these names referring too? The above
>>> output gives no description, and 'kvd' is not an industry term. Further,
>>
>> This are internal resources specific to the ASIC. Would you like some
>> description to each or something like that?
>
> devlink has some nice self-documenting capabilities. What's missing here
> is a description of what the resource is used for in standard terms --
> ipv4 host routes, fdb, nexthops, rifs, etc. Even if the description is a
> short list versus an exhaustive list of everything it is used for. e.g.,
> Why would a user decrease linear and increase hash_single or vice versa?
>
>>
>>
>>> what are these sizes that a user can control? The output contains no
>>> units, no description, nothing. In short, the above output provides
>>> random numbers associated with random names.
>>
>> Units are now exposed from kernel, just this version of iproute2 patch
>> does not display it.
>
> please provide an iproute2 patch that does so the full context if this
> patch set can be reviewed from a user perspective.
>
>>
>>
>>>
>>> I can see dpipe tables exported by this device:
>>>
>>> $ devlink dpipe header show pci/0000:03:00.0
>>>
>>> pci/0000:03:00.0:
>>> name mlxsw_meta
>>> field:
>>> name erif_port bitwidth 32 mapping_type ifindex
>>> name l3_forward bitwidth 1
>>> name l3_drop bitwidth 1
>>> name adj_index bitwidth 32
>>> name adj_size bitwidth 32
>>> name adj_hash_index bitwidth 32
>>>
>>> name ipv6
>>> field:
>>> name destination ip bitwidth 128
>>>
>>> name ipv4
>>> field:
>>> name destination ip bitwidth 32
>>>
>>> name ethernet
>>> field:
>>> name destination mac bitwidth 48
>>>
>>> but none mention 'kvd' or 'linear' or 'hash" and none of the other
>>> various devlink options:
>>>
>>> $ devlink
>>> Usage: devlink [ OPTIONS ] OBJECT { COMMAND | help }
>>> where OBJECT := { dev | port | sb | monitor | dpipe }
>>>
>>> seem to related to resources.
>>>
>>> So how does a user know what they are controlling by this 'resource'
>>> option? Is the user expected to have a PRM or user guide on hand for the
>>> specific device model that is being configured?
>>
>> The relation of specific dpipe table to specific resource is exposed by
>> the kernel as well. Probably the iproute2 patch just does not display
>> it.
>
> please provide an iproute2 patch that does so the full context if this
> patch set can be reviewed from a user perspective.
>
As Yuval stated you are using the wrong command here.
You are printing the headers not the tables. On each dpipe
table you can see the resource it is using (the resource
path aka host table uses /kvd/hash_single for example).
This is already working. Just try it.
>>
>>
>>>
>>> Again, I have no objections to kvd, linear, hash, etc terms as they do
>>> relate to Mellanox products. But kvd/linear, for example, does correlate
>>> to industry standard concepts in some way. My request is that the
>>> resource listing guide the user in some way, stating what these
>>> resources mean.
>>
>> So the showed relation to dpipe table would be enougn or you would still
>> like to see some description? I don't like the description concept here
>> as the relations to dpipe table should tell user exactly what he needs
>> to know.
>
> I believe it is useful to have a 1-line, short description that gives
> the user some memory jogger as to what the resource is used for. It does
> not have to be an exhaustive list, but the user should not have to do
> mental jumping jacks running a bunch of commands to understand the
> resources for vendor specific asics.
>
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: David Ahern @ 2017-12-27 20:16 UTC (permalink / raw)
To: Andrew Lunn, Jiri Pirko
Cc: netdev, davem, arkadis, mlxsw, vivien.didelot, f.fainelli,
michael.chan, ganeshgr, saeedm, matanb, leonro, idosch,
jakub.kicinski, ast, daniel, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, linville, gospo, steven.lin1,
yuvalm, ogerlitz, roopa
In-Reply-To: <20171227193110.GA5494@lunn.ch>
On 12/27/17 1:31 PM, Andrew Lunn wrote:
>> Hmm. That documents mainly sysfs. No mention of Netlink at all. But
>> maybe I missed it. Also, that defines the interface as is. However we
>> are talking about the data exchanged over the interface, not the
>> interface itself. I don't see how ASIC/HW specific thing, like for
>> example KVD in our case could be part of kernel ABI.
>
> You need to be very careful here. As soon as somebody starts using it,
> it might become an ABI. Or you need to clearly document it is not ABI,
> there is no guarantee it will not disappear or change its meaning in
> the next kernel, and it should be used with extreme caution.
>
+1
Once the names go in, people can write scripts that invoke devlink at
boot to partition resources. With the proposed patch set, the name
(e.g., kvd/linear) becomes part of the ABI.
^ permalink raw reply
* Re: [PATCHv3 0/2] capability controlled user-namespaces
From: Michael Kerrisk (man-pages) @ 2017-12-27 20:23 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: James Morris, LKML, Netdev, Kernel-hardening, Linux API,
Kees Cook, Serge Hallyn, Eric W . Biederman, Eric Dumazet,
David Miller, Mahesh Bandewar
In-Reply-To: <CAF2d9jit74_VCdD-pEFy3bJo2W1-0cDo0BOJC5beJiy8yFPCWg@mail.gmail.com>
Hello Mahesh,
On 27 December 2017 at 18:09, Mahesh Bandewar (महेश बंडेवार)
<maheshb@google.com> wrote:
> Hello James,
>
> Seems like I missed your name to be added into the review of this
> patch series. Would you be willing be pull this into the security
> tree? Serge Hallyn has already ACKed it.
We seem to have no formal documentation/specification of this feature.
I think that should be written up before this patch goes into
mainline...
Cheers,
Michael
>
> On Tue, Dec 5, 2017 at 2:30 PM, Mahesh Bandewar <mahesh@bandewar.net> wrote:
>> From: Mahesh Bandewar <maheshb@google.com>
>>
>> TL;DR version
>> -------------
>> Creating a sandbox environment with namespaces is challenging
>> considering what these sandboxed processes can engage into. e.g.
>> CVE-2017-6074, CVE-2017-7184, CVE-2017-7308 etc. just to name few.
>> Current form of user-namespaces, however, if changed a bit can allow
>> us to create a sandbox environment without locking down user-
>> namespaces.
>>
>> Detailed version
>> ----------------
>>
>> Problem
>> -------
>> User-namespaces in the current form have increased the attack surface as
>> any process can acquire capabilities which are not available to them (by
>> default) by performing combination of clone()/unshare()/setns() syscalls.
>>
>> #define _GNU_SOURCE
>> #include <stdio.h>
>> #include <sched.h>
>> #include <netinet/in.h>
>>
>> int main(int ac, char **av)
>> {
>> int sock = -1;
>>
>> printf("Attempting to open RAW socket before unshare()...\n");
>> sock = socket(AF_INET6, SOCK_RAW, IPPROTO_RAW);
>> if (sock < 0) {
>> perror("socket() SOCK_RAW failed: ");
>> } else {
>> printf("Successfully opened RAW-Sock before unshare().\n");
>> close(sock);
>> sock = -1;
>> }
>>
>> if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0) {
>> perror("unshare() failed: ");
>> return 1;
>> }
>>
>> printf("Attempting to open RAW socket after unshare()...\n");
>> sock = socket(AF_INET6, SOCK_RAW, IPPROTO_RAW);
>> if (sock < 0) {
>> perror("socket() SOCK_RAW failed: ");
>> } else {
>> printf("Successfully opened RAW-Sock after unshare().\n");
>> close(sock);
>> sock = -1;
>> }
>>
>> return 0;
>> }
>>
>> The above example shows how easy it is to acquire NET_RAW capabilities
>> and once acquired, these processes could take benefit of above mentioned
>> or similar issues discovered/undiscovered with malicious intent. Note
>> that this is just an example and the problem/solution is not limited
>> to NET_RAW capability *only*.
>>
>> The easiest fix one can apply here is to lock-down user-namespaces which
>> many of the distros do (i.e. don't allow users to create user namespaces),
>> but unfortunately that prevents everyone from using them.
>>
>> Approach
>> --------
>> Introduce a notion of 'controlled' user-namespaces. Every process on
>> the host is allowed to create user-namespaces (governed by the limit
>> imposed by per-ns sysctl) however, mark user-namespaces created by
>> sandboxed processes as 'controlled'. Use this 'mark' at the time of
>> capability check in conjunction with a global capability whitelist.
>> If the capability is not whitelisted, processes that belong to
>> controlled user-namespaces will not be allowed.
>>
>> Once a user-ns is marked as 'controlled'; all its child user-
>> namespaces are marked as 'controlled' too.
>>
>> A global whitelist is list of capabilities governed by the
>> sysctl which is available to (privileged) user in init-ns to modify
>> while it's applicable to all controlled user-namespaces on the host.
>>
>> Marking user-namespaces controlled without modifying the whitelist is
>> equivalent of the current behavior. The default value of whitelist includes
>> all capabilities so that the compatibility is maintained. However it gives
>> admins fine-grained ability to control various capabilities system wide
>> without locking down user-namespaces.
>>
>> Please see individual patches in this series.
>>
>> Mahesh Bandewar (2):
>> capability: introduce sysctl for controlled user-ns capability whitelist
>> userns: control capabilities of some user namespaces
>>
>> Documentation/sysctl/kernel.txt | 21 +++++++++++++++++
>> include/linux/capability.h | 7 ++++++
>> include/linux/user_namespace.h | 25 ++++++++++++++++++++
>> kernel/capability.c | 52 +++++++++++++++++++++++++++++++++++++++++
>> kernel/sysctl.c | 5 ++++
>> kernel/user_namespace.c | 4 ++++
>> security/commoncap.c | 8 +++++++
>> 7 files changed, 122 insertions(+)
>>
>> --
>> 2.15.0.531.g2ccb3012c9-goog
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH 3/4] libbpf: break loop earlier
From: Eric Leblond @ 2017-12-27 20:30 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: netdev, daniel, linux-kernel
In-Reply-To: <20171227190042.hwxk6ccazdtnob77@ast-mbp>
Hello,
On Wed, 2017-12-27 at 11:00 -0800, Alexei Starovoitov wrote:
> On Wed, Dec 27, 2017 at 07:02:28PM +0100, Eric Leblond wrote:
> > Get out of the loop when we have a match.
> >
> > Signed-off-by: Eric Leblond <eric@regit.org>
> > ---
> > tools/lib/bpf/libbpf.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 5fe8aaa2123e..d263748aa341 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -412,6 +412,7 @@ bpf_object__init_prog_names(struct bpf_object
> > *obj)
> > prog->section_name);
> > return -LIBBPF_ERRNO__LIBELF;
> > }
> > + break;
>
> why this is needed?
It was just cosmetic, no related bug.
> The top of the loop is:
> for (si = 0; si < symbols->d_size / sizeof(GElf_Sym) && !name;
>
> so as soon as name is found the loop will exit.
OK, I've missed that. Please disregard this patch.
BR,
--
Eric Leblond <eric@regit.org>
^ permalink raw reply
* Re: [PATCH v3 1/4] security: Add support for SCTP security hooks
From: Paul Moore @ 2017-12-27 20:35 UTC (permalink / raw)
To: Richard Haines
Cc: Marcelo Ricardo Leitner, Casey Schaufler, selinux, netdev,
linux-sctp, linux-security-module, Vlad Yasevich, nhorman,
Stephen Smalley, Eric Paris
In-Reply-To: <1514391741.2780.4.camel@btinternet.com>
On Wed, Dec 27, 2017 at 11:22 AM, Richard Haines
<richard_c_haines@btinternet.com> wrote:
> On Fri, 2017-12-22 at 15:45 -0200, Marcelo Ricardo Leitner wrote:
>> On Fri, Dec 22, 2017 at 09:20:45AM -0800, Casey Schaufler wrote:
>> > On 12/22/2017 5:05 AM, Marcelo Ricardo Leitner wrote:
>> > > From: Richard Haines <richard_c_haines@btinternet.com>
>> > >
>> > > The SCTP security hooks are explained in:
>> > > Documentation/security/LSM-sctp.rst
>>
>> Thanks Casey for your comments. However, I'm not that acquainted with
>> these area of codes and I cannot work on them. I'll just wait for
>> Richard then.
>
> I'm back online and will post a V4 set of patches within a week. These
> will address Paul's comments as per [1] and Casey's regarding the
> documentation.
> Sorry for the delay
No worries, thanks.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [patch iproute2 v3 3/4] tc: Add -bs option to batch mode
From: Marcelo Ricardo Leitner @ 2017-12-27 20:39 UTC (permalink / raw)
To: David Ahern; +Cc: Chris Mi, netdev, gerlitz.or, stephen
In-Reply-To: <28563dd5-01be-9198-2911-658bbd0ba3d7@gmail.com>
On Wed, Dec 27, 2017 at 09:39:15AM -0600, David Ahern wrote:
> On 12/25/17 2:46 AM, Chris Mi wrote:
> > Signed-off-by: Chris Mi <chrism@mellanox.com>
> > ---
> > tc/m_action.c | 91 +++++++++++++++++++++++++++++++++----------
> > tc/tc.c | 47 ++++++++++++++++++----
> > tc/tc_common.h | 8 +++-
> > tc/tc_filter.c | 121 +++++++++++++++++++++++++++++++++++++++++----------------
> > 4 files changed, 204 insertions(+), 63 deletions(-)
> >
> > diff --git a/tc/m_action.c b/tc/m_action.c
> > index fc422364..c4c3b862 100644
> > --- a/tc/m_action.c
> > +++ b/tc/m_action.c
> > @@ -23,6 +23,7 @@
> > #include <arpa/inet.h>
> > #include <string.h>
> > #include <dlfcn.h>
> > +#include <errno.h>
> >
> > #include "utils.h"
> > #include "tc_common.h"
> > @@ -546,40 +547,88 @@ bad_val:
> > return ret;
> > }
> >
> > +typedef struct {
> > + struct nlmsghdr n;
> > + struct tcamsg t;
> > + char buf[MAX_MSG];
> > +} tc_action_req;
> > +
> > +static tc_action_req *action_reqs;
> > +static struct iovec msg_iov[MSG_IOV_MAX];
> > +
> > +void free_action_reqs(void)
> > +{
> > + free(action_reqs);
> > +}
> > +
> > +static tc_action_req *get_action_req(int batch_size, int index)
> > +{
> > + tc_action_req *req;
> > +
> > + if (action_reqs == NULL) {
> > + action_reqs = malloc(batch_size * sizeof (tc_action_req));
> > + if (action_reqs == NULL)
> > + return NULL;
> > + }
> > + req = &action_reqs[index];
> > + memset(req, 0, sizeof (*req));
> > +
> > + return req;
> > +}
> > +
> > static int tc_action_modify(int cmd, unsigned int flags,
> > - int *argc_p, char ***argv_p)
> > + int *argc_p, char ***argv_p,
> > + int batch_size, int index, bool send)
> > {
> > int argc = *argc_p;
> > char **argv = *argv_p;
> > int ret = 0;
> > - struct {
> > - struct nlmsghdr n;
> > - struct tcamsg t;
> > - char buf[MAX_MSG];
> > - } req = {
> > - .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
> > - .n.nlmsg_flags = NLM_F_REQUEST | flags,
> > - .n.nlmsg_type = cmd,
> > - .t.tca_family = AF_UNSPEC,
> > + tc_action_req *req;
> > + struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> > + struct iovec *iov = &msg_iov[index];
> > +
> > + req = get_action_req(batch_size, index);
> > + if (req == NULL) {
> > + fprintf(stderr, "get_action_req error: not enough buffer\n");
> > + return -ENOMEM;
> > + }
> > +
> > + req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
> > + req->n.nlmsg_flags = NLM_F_REQUEST | flags;
> > + req->n.nlmsg_type = cmd;
> > + req->t.tca_family = AF_UNSPEC;
> > + struct rtattr *tail = NLMSG_TAIL(&req->n);
> > +
> > + struct msghdr msg = {
> > + .msg_name = &nladdr,
> > + .msg_namelen = sizeof(nladdr),
> > + .msg_iov = msg_iov,
> > + .msg_iovlen = index + 1,
> > };
> > - struct rtattr *tail = NLMSG_TAIL(&req.n);
> >
> > argc -= 1;
> > argv += 1;
> > - if (parse_action(&argc, &argv, TCA_ACT_TAB, &req.n)) {
> > + if (parse_action(&argc, &argv, TCA_ACT_TAB, &req->n)) {
> > fprintf(stderr, "Illegal \"action\"\n");
> > return -1;
> > }
> > - tail->rta_len = (void *) NLMSG_TAIL(&req.n) - (void *) tail;
> > + tail->rta_len = (void *) NLMSG_TAIL(&req->n) - (void *) tail;
> > +
> > + *argc_p = argc;
> > + *argv_p = argv;
> > +
> > + iov->iov_base = &req->n;
> > + iov->iov_len = req->n.nlmsg_len;
> > +
> > + if (!send)
> > + return 0;
> >
> > - if (rtnl_talk(&rth, &req.n, NULL) < 0) {
> > + ret = rtnl_talk_msg(&rth, &msg, NULL);
> > + if (ret < 0) {
> > fprintf(stderr, "We have an error talking to the kernel\n");
> > ret = -1;
> > }
> >
> > - *argc_p = argc;
> > - *argv_p = argv;
> > -
> > return ret;
> > }
> >
> > @@ -679,7 +728,7 @@ bad_val:
> > return ret;
> > }
> >
> > -int do_action(int argc, char **argv)
> > +int do_action(int argc, char **argv, int batch_size, int index, bool send)
> > {
> >
> > int ret = 0;
> > @@ -689,12 +738,14 @@ int do_action(int argc, char **argv)
> > if (matches(*argv, "add") == 0) {
> > ret = tc_action_modify(RTM_NEWACTION,
> > NLM_F_EXCL | NLM_F_CREATE,
> > - &argc, &argv);
> > + &argc, &argv, batch_size,
> > + index, send);
> > } else if (matches(*argv, "change") == 0 ||
> > matches(*argv, "replace") == 0) {
> > ret = tc_action_modify(RTM_NEWACTION,
> > NLM_F_CREATE | NLM_F_REPLACE,
> > - &argc, &argv);
> > + &argc, &argv, batch_size,
> > + index, send);
> > } else if (matches(*argv, "delete") == 0) {
> > argc -= 1;
> > argv += 1;
> > diff --git a/tc/tc.c b/tc/tc.c
> > index ad9f07e9..7ea2fc89 100644
> > --- a/tc/tc.c
> > +++ b/tc/tc.c
> > @@ -189,20 +189,20 @@ static void usage(void)
> > fprintf(stderr, "Usage: tc [ OPTIONS ] OBJECT { COMMAND | help }\n"
> > " tc [-force] -batch filename\n"
> > "where OBJECT := { qdisc | class | filter | action | monitor | exec }\n"
> > - " OPTIONS := { -s[tatistics] | -d[etails] | -r[aw] | -p[retty] | -b[atch] [filename] | -n[etns] name |\n"
> > + " OPTIONS := { -s[tatistics] | -d[etails] | -r[aw] | -p[retty] | -b[atch] [filename] | -bs | -batchsize [size] | -n[etns] name |\n"
> > " -nm | -nam[es] | { -cf | -conf } path } | -j[son]\n");
> > }
> >
> > -static int do_cmd(int argc, char **argv)
> > +static int do_cmd(int argc, char **argv, int batch_size, int index, bool send)
> > {
> > if (matches(*argv, "qdisc") == 0)
> > return do_qdisc(argc-1, argv+1);
> > if (matches(*argv, "class") == 0)
> > return do_class(argc-1, argv+1);
> > if (matches(*argv, "filter") == 0)
> > - return do_filter(argc-1, argv+1);
> > + return do_filter(argc-1, argv+1, batch_size, index, send);
> > if (matches(*argv, "actions") == 0)
> > - return do_action(argc-1, argv+1);
> > + return do_action(argc-1, argv+1, batch_size, index, send);
> > if (matches(*argv, "monitor") == 0)
> > return do_tcmonitor(argc-1, argv+1);
> > if (matches(*argv, "exec") == 0)
> > @@ -217,11 +217,16 @@ static int do_cmd(int argc, char **argv)
> > return -1;
> > }
> >
> > -static int batch(const char *name)
> > +static int batch(const char *name, int batch_size)
> > {
> > + int msg_iov_index = 0;
> > char *line = NULL;
> > size_t len = 0;
> > int ret = 0;
> > + bool send;
> > +
> > + if (batch_size > 1)
> > + setcmdlinetotal(name);
> >
> > batch_mode = 1;
> > if (name && strcmp(name, "-") != 0) {
> > @@ -248,15 +253,30 @@ static int batch(const char *name)
> > if (largc == 0)
> > continue; /* blank line */
> >
> > - if (do_cmd(largc, largv)) {
> > + /*
> > + * In batch mode, if we haven't accumulated enough commands
> > + * and this is not the last command, don't send the message
> > + * immediately.
> > + */
> > + if (batch_size > 1 && msg_iov_index + 1 != batch_size
> > + && cmdlineno != cmdlinetotal)
I think you can replace the cmdlineno check with a simple !feof(stdin)
> > + send = false;
> > + else
> > + send = true;
> > +
> > + ret = do_cmd(largc, largv, batch_size, msg_iov_index++, send);
>
> What happens if tc commands are interlaced in the file -- qdisc add,
> class add, filter add, then a delete, show, exec, etc.? Right now each
> command is handled one at a time so an add followed by a delete will
> work. Your proposed batching loop won't work for this case as some
> commands are executed when that line is reached and others are batched
> for later send. Not all of the tc commands need to be batched in a
> single message so perhaps those commands cause the queue to be flushed
> (ie, message sent), then that command is executed and you start the
> batching over.
>
> Further, I really think the batching can be done without the global
> variables and without the command handlers knowing it is batching or
> part of an iov. e.g., in the case of batching try having the commands
> malloc the request buffer and return the pointer back to this loop in
> which case this loop calls rtnl_talk_msg and frees the buffers.
Sounds like the batching is being done at the wrong level. If it was
done by rtnl_talk(), it should be easier.
We can keep rtnl_talk() for previous users and make rtnl_talk_msg() do
the batching, mostly independent of which kind of msg it it.
As you need to inform it that it was the last entry, that may be
detected with feof(stdin). Just add a 'bool flush' parameter to it.
rtnl_talk_msg(...., flush=feof(stdin));
Next step then would be to add a memory manager layer to it, so
libnetlink wouldn't need to copy the messages but recycle pointers:
rtnl_get_msgbuf(): returns a buffer that one can use to fill in the
msg and use with rtnl_talk_msg()
and the free is done by libnetlink itself when the message is
finally sent, so no need to keep track of what one needs to free or
can reuse.
>
> > + if (ret < 0) {
> > fprintf(stderr, "Command failed %s:%d\n", name, cmdlineno);
> > ret = 1;
> > if (!force)
> > break;
> > }
> > + msg_iov_index %= batch_size;
> > }
> > if (line)
> > free(line);
> > + free_filter_reqs();
> > + free_action_reqs();
> >
> > rtnl_close(&rth);
> > return ret;
>
^ permalink raw reply
* [PATCH] Staging: ipx: fixed several no space before tabs coding style issues
From: Jianshen Liu @ 2017-12-27 21:25 UTC (permalink / raw)
To: gregkh; +Cc: devel, netdev, linux-kernel, Jianshen Liu
Fixed several coding style warnings of "please, no space before tabs".
Signed-off-by: Jianshen Liu <ljishen@gmail.com>
---
drivers/staging/ipx/af_ipx.c | 56 ++++++++++++++++++++---------------------
drivers/staging/ipx/ipx_proc.c | 2 +-
drivers/staging/ipx/ipx_route.c | 6 ++---
3 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/staging/ipx/af_ipx.c b/drivers/staging/ipx/af_ipx.c
index d21a9d1..d8be06c 100644
--- a/drivers/staging/ipx/af_ipx.c
+++ b/drivers/staging/ipx/af_ipx.c
@@ -2,7 +2,7 @@
* Implements an IPX socket layer.
*
* This code is derived from work by
- * Ross Biro : Writing the original IP stack
+ * Ross Biro : Writing the original IP stack
* Fred Van Kempen : Tidying up the TCP/IP
*
* Many thanks go to Keith Baker, Institute For Industrial Information
@@ -20,7 +20,7 @@
* provide warranty for any of this software. This material is provided
* "AS-IS" and at no charge.
*
- * Portions Copyright (c) 1995 Caldera, Inc. <greg@caldera.com>
+ * Portions Copyright (c) 1995 Caldera, Inc. <greg@caldera.com>
* Neither Greg Page nor Caldera, Inc. admit liability nor provide
* warranty for any of this software. This material is provided
* "AS-IS" and at no charge.
@@ -758,7 +758,7 @@ static void ipxitf_discover_netnum(struct ipx_interface *intrfc,
/**
* ipxitf_pprop - Process packet propagation IPX packet type 0x14, used for
- * NetBIOS broadcasts
+ * NetBIOS broadcasts
* @intrfc: IPX interface receiving this packet
* @skb: Received packet
*
@@ -870,11 +870,11 @@ static struct ipx_interface *ipxitf_alloc(struct net_device *dev, __be32 netnum,
if (intrfc) {
intrfc->if_dev = dev;
intrfc->if_netnum = netnum;
- intrfc->if_dlink_type = dlink_type;
- intrfc->if_dlink = dlink;
- intrfc->if_internal = internal;
- intrfc->if_ipx_offset = ipx_offset;
- intrfc->if_sknum = IPX_MIN_EPHEMERAL_SOCKET;
+ intrfc->if_dlink_type = dlink_type;
+ intrfc->if_dlink = dlink;
+ intrfc->if_internal = internal;
+ intrfc->if_ipx_offset = ipx_offset;
+ intrfc->if_sknum = IPX_MIN_EPHEMERAL_SOCKET;
INIT_HLIST_HEAD(&intrfc->if_sklist);
refcount_set(&intrfc->refcnt, 1);
spin_lock_init(&intrfc->if_sklist_lock);
@@ -965,23 +965,23 @@ static int ipxitf_create(struct ipx_interface_definition *idef)
switch (idef->ipx_dlink_type) {
case IPX_FRAME_8022:
- dlink_type = htons(ETH_P_802_2);
- datalink = p8022_datalink;
+ dlink_type = htons(ETH_P_802_2);
+ datalink = p8022_datalink;
break;
case IPX_FRAME_ETHERII:
if (dev->type != ARPHRD_IEEE802) {
- dlink_type = htons(ETH_P_IPX);
- datalink = pEII_datalink;
+ dlink_type = htons(ETH_P_IPX);
+ datalink = pEII_datalink;
break;
}
/* fall through */
case IPX_FRAME_SNAP:
- dlink_type = htons(ETH_P_SNAP);
- datalink = pSNAP_datalink;
+ dlink_type = htons(ETH_P_SNAP);
+ datalink = pSNAP_datalink;
break;
case IPX_FRAME_8023:
- dlink_type = htons(ETH_P_802_3);
- datalink = p8023_datalink;
+ dlink_type = htons(ETH_P_802_3);
+ datalink = p8023_datalink;
break;
case IPX_FRAME_NONE:
default:
@@ -1522,7 +1522,7 @@ static int ipx_connect(struct socket *sock, struct sockaddr *uaddr,
struct ipx_route *rt;
sk->sk_state = TCP_CLOSE;
- sock->state = SS_UNCONNECTED;
+ sock->state = SS_UNCONNECTED;
lock_sock(sk);
if (addr_len != sizeof(*addr))
@@ -1534,7 +1534,7 @@ static int ipx_connect(struct socket *sock, struct sockaddr *uaddr,
struct sockaddr_ipx uaddr;
uaddr.sipx_port = 0;
- uaddr.sipx_network = 0;
+ uaddr.sipx_network = 0;
#ifdef CONFIG_IPX_INTERN
rc = -ENETDOWN;
@@ -1563,8 +1563,8 @@ static int ipx_connect(struct socket *sock, struct sockaddr *uaddr,
ipxs->type = addr->sipx_type;
if (sock->type == SOCK_DGRAM) {
- sock->state = SS_CONNECTED;
- sk->sk_state = TCP_ESTABLISHED;
+ sock->state = SS_CONNECTED;
+ sk->sk_state = TCP_ESTABLISHED;
}
if (rt)
@@ -1736,10 +1736,10 @@ static int ipx_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
goto out;
usipx = &local_sipx;
- usipx->sipx_family = AF_IPX;
- usipx->sipx_type = ipxs->type;
- usipx->sipx_port = ipxs->dest_addr.sock;
- usipx->sipx_network = ipxs->dest_addr.net;
+ usipx->sipx_family = AF_IPX;
+ usipx->sipx_type = ipxs->type;
+ usipx->sipx_port = ipxs->dest_addr.sock;
+ usipx->sipx_network = ipxs->dest_addr.net;
memcpy(usipx->sipx_node, ipxs->dest_addr.node, IPX_NODE_LEN);
}
@@ -1769,7 +1769,7 @@ static int ipx_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
struct sockaddr_ipx uaddr;
uaddr.sipx_port = 0;
- uaddr.sipx_network = 0;
+ uaddr.sipx_network = 0;
#ifdef CONFIG_IPX_INTERN
rc = -ENETDOWN;
@@ -1798,8 +1798,8 @@ static int ipx_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
goto out;
}
- ipx = ipx_hdr(skb);
- copied = ntohs(ipx->ipx_pktsize) - sizeof(struct ipxhdr);
+ ipx = ipx_hdr(skb);
+ copied = ntohs(ipx->ipx_pktsize) - sizeof(struct ipxhdr);
if (copied > size) {
copied = size;
msg->msg_flags |= MSG_TRUNC;
@@ -1816,7 +1816,7 @@ static int ipx_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
sipx->sipx_port = ipx->ipx_source.sock;
memcpy(sipx->sipx_node, ipx->ipx_source.node, IPX_NODE_LEN);
sipx->sipx_network = IPX_SKB_CB(skb)->ipx_source_net;
- sipx->sipx_type = ipx->ipx_type;
+ sipx->sipx_type = ipx->ipx_type;
sipx->sipx_zero = 0;
msg->msg_namelen = sizeof(*sipx);
}
diff --git a/drivers/staging/ipx/ipx_proc.c b/drivers/staging/ipx/ipx_proc.c
index 38a3d51..893b2ab 100644
--- a/drivers/staging/ipx/ipx_proc.c
+++ b/drivers/staging/ipx/ipx_proc.c
@@ -2,7 +2,7 @@
/*
* IPX proc routines
*
- * Copyright(C) Arnaldo Carvalho de Melo <acme@conectiva.com.br>, 2002
+ * Copyright(C) Arnaldo Carvalho de Melo <acme@conectiva.com.br>, 2002
*/
#include <linux/init.h>
diff --git a/drivers/staging/ipx/ipx_route.c b/drivers/staging/ipx/ipx_route.c
index 3cf93aa9..833206c 100644
--- a/drivers/staging/ipx/ipx_route.c
+++ b/drivers/staging/ipx/ipx_route.c
@@ -71,8 +71,8 @@ int ipxrtr_add_route(__be32 network, struct ipx_interface *intrfc,
goto out_put;
}
- rt->ir_net = network;
- rt->ir_intrfc = intrfc;
+ rt->ir_net = network;
+ rt->ir_intrfc = intrfc;
if (!node) {
memset(rt->ir_router_node, '\0', IPX_NODE_LEN);
rt->ir_routed = 0;
@@ -207,7 +207,7 @@ int ipxrtr_route_packet(struct sock *sk, struct sockaddr_ipx *usipx,
ipx = ipx_hdr(skb);
ipx->ipx_pktsize = htons(len + sizeof(struct ipxhdr));
IPX_SKB_CB(skb)->ipx_tctrl = 0;
- ipx->ipx_type = usipx->sipx_type;
+ ipx->ipx_type = usipx->sipx_type;
IPX_SKB_CB(skb)->last_hop.index = -1;
#ifdef CONFIG_IPX_INTERN
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox