* Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
From: Jakub Kicinski @ 2018-04-25 21:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, lkml, netdev, Quentin Monnet
In-Reply-To: <20180425174108.6586-4-jolsa@kernel.org>
On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> printf("tag ");
> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
3 nit picks:
Other "fields" are separated by two spaces between each other:
4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
^^ ^^ X
loaded_at Apr 25/11:20 uid 0
^^
xlated 16B not jited memlock 4096B
^^ ^^
Could you also update the example outputs in the man page:
tools/bpf/bpftool/Documentation/bpftool-prog.rst
Sorry about the bike shedding but I would also vote for:
"[not] GPL compatible"
rather than
"license GPL [NON] compatible"
for brevity..
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Randy Dunlap @ 2018-04-25 21:11 UTC (permalink / raw)
To: Mikulas Patocka
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
Michal Hocko, linux-mm, edumazet, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251656300.9428@file01.intranet.prod.int.rdu2.redhat.com>
On 04/25/2018 01:57 PM, Mikulas Patocka wrote:
>
>
> On Wed, 25 Apr 2018, Randy Dunlap wrote:
>
>> On 04/25/2018 01:02 PM, Mikulas Patocka wrote:
>>>
>>>
>>> From: Mikulas Patocka <mpatocka@redhat.com>
>>> Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
>>>
>>> This patch introduces a fault-injection option "kvmalloc_fallback". This
>>> option makes kvmalloc randomly fall back to vmalloc.
>>>
>>> Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
>>
>> Unfortunately,
>
> OK - here I fixed the typos:
>
>
> From: Mikulas Patocka <mpatocka@redhat.com>
> Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
>
> This patch introduces a fault-injection option "kvmalloc_fallback". This
> option makes kvmalloc randomly fall back to vmalloc.
>
> Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> code. This options helps to test for these bugs.
>
> The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> It can be enabled in distribution debug kernels, so that kvmalloc abuse
> can be tested by the users. The default can be overridden with
> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> Documentation/fault-injection/fault-injection.txt | 7 +++++
> include/linux/fault-inject.h | 9 +++---
> kernel/futex.c | 2 -
> lib/Kconfig.debug | 15 +++++++++++
> mm/failslab.c | 2 -
> mm/page_alloc.c | 2 -
> mm/util.c | 30 ++++++++++++++++++++++
> 7 files changed, 60 insertions(+), 7 deletions(-)
Acked-by: Randy Dunlap <rdunlap@infradead.org> # Documentation and Kconfig only
thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
From: Daniel Borkmann @ 2018-04-25 21:14 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Olsa
Cc: Alexei Starovoitov, lkml, netdev, Quentin Monnet
In-Reply-To: <20180425140346.3e0f3ba7@cakuba.netronome.com>
On 04/25/2018 11:03 PM, Jakub Kicinski wrote:
> On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
>> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
>> printf("tag ");
>> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
>> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
>> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
>
> 3 nit picks:
>
> Other "fields" are separated by two spaces between each other:
>
> 4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
> ^^ ^^ X
> loaded_at Apr 25/11:20 uid 0
> ^^
> xlated 16B not jited memlock 4096B
> ^^ ^^
>
> Could you also update the example outputs in the man page:
>
> tools/bpf/bpftool/Documentation/bpftool-prog.rst
>
> Sorry about the bike shedding but I would also vote for:
>
> "[not] GPL compatible"
>
> rather than
>
> "license GPL [NON] compatible"
>
> for brevity..
While we're at it, can we also squeeze this whole thing a bit? Feels like
huge string wasted for very little information compared to the rest of the
dump. Just append the string "gpl" at the end of the line if info->gpl_compatible
is set, otherwise just add nothing. This also allows to naturally grep
for it e.g. `bpftool p | grep gpl` if you need a quick summary.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: David Rientjes @ 2018-04-25 21:18 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Randy Dunlap, Michal Hocko, Matthew Wilcox, David Miller,
Andrew Morton, linux-mm, eric.dumazet, edumazet, netdev,
linux-kernel, mst, jasowang, virtualization, dm-devel,
Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251656300.9428@file01.intranet.prod.int.rdu2.redhat.com>
On Wed, 25 Apr 2018, Mikulas Patocka wrote:
> From: Mikulas Patocka <mpatocka@redhat.com>
> Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
>
> This patch introduces a fault-injection option "kvmalloc_fallback". This
> option makes kvmalloc randomly fall back to vmalloc.
>
> Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> code. This options helps to test for these bugs.
>
> The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> It can be enabled in distribution debug kernels, so that kvmalloc abuse
> can be tested by the users. The default can be overridden with
> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
>
Do we really need the new config option? This could just be manually
tunable via fault injection IIUC.
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> Documentation/fault-injection/fault-injection.txt | 7 +++++
> include/linux/fault-inject.h | 9 +++---
> kernel/futex.c | 2 -
> lib/Kconfig.debug | 15 +++++++++++
> mm/failslab.c | 2 -
> mm/page_alloc.c | 2 -
> mm/util.c | 30 ++++++++++++++++++++++
> 7 files changed, 60 insertions(+), 7 deletions(-)
>
> Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
> +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
> @@ -15,6 +15,12 @@ o fail_page_alloc
>
> injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
>
> +o kvmalloc_fallback
> +
> + makes the function kvmalloc randomly fall back to vmalloc. This could be used
> + to detects bugs such as using DMA-API on the result of kvmalloc or freeing
> + the result of kvmalloc with free.
> +
> o fail_futex
>
> injects futex deadlock and uaddr fault errors.
> @@ -167,6 +173,7 @@ use the boot option:
>
> failslab=
> fail_page_alloc=
> + kvmalloc_fallback=
> fail_make_request=
> fail_futex=
> mmc_core.fail_request=<interval>,<probability>,<space>,<times>
> Index: linux-2.6/include/linux/fault-inject.h
> ===================================================================
> --- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
> @@ -31,17 +31,18 @@ struct fault_attr {
> struct dentry *dname;
> };
>
> -#define FAULT_ATTR_INITIALIZER { \
> +#define FAULT_ATTR_INITIALIZER(p) { \
> + .probability = (p), \
> .interval = 1, \
> - .times = ATOMIC_INIT(1), \
> + .times = ATOMIC_INIT((p) ? -1 : 1), \
> + .verbose = (p) ? 0 : 2, \
> .require_end = ULONG_MAX, \
> .stacktrace_depth = 32, \
> .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
> - .verbose = 2, \
> .dname = NULL, \
> }
>
> -#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
> +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
> int setup_fault_attr(struct fault_attr *attr, char *str);
> bool should_fail(struct fault_attr *attr, ssize_t size);
>
> Index: linux-2.6/lib/Kconfig.debug
> ===================================================================
> --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
> +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
> @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
> help
> Provide fault-injection capability for alloc_pages().
>
> +config FAIL_KVMALLOC_FALLBACK_PROBABILITY
> + int "Default kvmalloc fallback probability"
> + depends on FAULT_INJECTION
> + range 0 100
> + default "0"
> + help
> + This option will make kvmalloc randomly fall back to vmalloc.
> + Normally, kvmalloc falls back to vmalloc only rarely, if memory
> + is fragmented.
> +
> + This option helps to detect hard-to-reproduce driver bugs, for
> + example using DMA API on the result of kvmalloc.
> +
> + The default may be overridden with the kvmalloc_fallback parameter.
> +
> config FAIL_MAKE_REQUEST
> bool "Fault-injection capability for disk IO"
> depends on FAULT_INJECTION && BLOCK
> Index: linux-2.6/mm/util.c
> ===================================================================
> --- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
> +++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
> @@ -14,6 +14,7 @@
> #include <linux/hugetlb.h>
> #include <linux/vmalloc.h>
> #include <linux/userfaultfd_k.h>
> +#include <linux/fault-inject.h>
>
> #include <asm/sections.h>
> #include <linux/uaccess.h>
> @@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
> }
> EXPORT_SYMBOL(vm_mmap);
>
> +#ifdef CONFIG_FAULT_INJECTION
> +
> +static struct fault_attr kvmalloc_fallback =
> + FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
> +
> +static int __init setup_kvmalloc_fallback(char *str)
> +{
> + return setup_fault_attr(&kvmalloc_fallback, str);
> +}
> +
> +__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
> +
> +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> +static int __init kvmalloc_fallback_debugfs_init(void)
> +{
> + fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
> + return 0;
> +}
> +late_initcall(kvmalloc_fallback_debugfs_init);
> +#endif
> +
> +#endif
> +
> /**
> * kvmalloc_node - attempt to allocate physically contiguous memory, but upon
> * failure, fall back to non-contiguous (vmalloc) allocation.
> @@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
> */
> WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
>
> +#ifdef CONFIG_FAULT_INJECTION
> + if (should_fail(&kvmalloc_fallback, size))
> + goto do_vmalloc;
> +#endif
> +
> /*
> * We want to attempt a large physically contiguous block first because
> * it is less likely to fragment multiple larger blocks and therefore
> @@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
> if (ret || size <= PAGE_SIZE)
> return ret;
>
> +do_vmalloc: __maybe_unused
> return __vmalloc_node_flags_caller(size, node, flags,
> __builtin_return_address(0));
> }
> Index: linux-2.6/kernel/futex.c
> ===================================================================
> --- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
> +++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
> @@ -288,7 +288,7 @@ static struct {
>
> bool ignore_private;
> } fail_futex = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_private = false,
> };
>
> Index: linux-2.6/mm/failslab.c
> ===================================================================
> --- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
> @@ -9,7 +9,7 @@ static struct {
> bool ignore_gfp_reclaim;
> bool cache_filter;
> } failslab = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_gfp_reclaim = true,
> .cache_filter = false,
> };
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
> +++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
> @@ -3055,7 +3055,7 @@ static struct {
> bool ignore_gfp_reclaim;
> u32 min_order;
> } fail_page_alloc = {
> - .attr = FAULT_ATTR_INITIALIZER,
> + .attr = FAULT_ATTR_INITIALIZER(0),
> .ignore_gfp_reclaim = true,
> .ignore_gfp_highmem = true,
> .min_order = 1,
>
>
^ permalink raw reply
* Re: [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 21:22 UTC (permalink / raw)
To: David Rientjes
Cc: Randy Dunlap, Michal Hocko, Matthew Wilcox, David Miller,
Andrew Morton, linux-mm, eric.dumazet, edumazet, netdev,
linux-kernel, mst, jasowang, virtualization, dm-devel,
Vlastimil Babka
In-Reply-To: <alpine.DEB.2.21.1804251417470.166306@chino.kir.corp.google.com>
On Wed, 25 Apr 2018, David Rientjes wrote:
> On Wed, 25 Apr 2018, Mikulas Patocka wrote:
>
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
> >
> > This patch introduces a fault-injection option "kvmalloc_fallback". This
> > option makes kvmalloc randomly fall back to vmalloc.
> >
> > Unfortunately, some kernel code has bugs - it uses kvmalloc and then
> > uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> > found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> > code. This options helps to test for these bugs.
> >
> > The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> > It can be enabled in distribution debug kernels, so that kvmalloc abuse
> > can be tested by the users. The default can be overridden with
> > "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
> >
>
> Do we really need the new config option? This could just be manually
> tunable via fault injection IIUC.
We do, because we want to enable it in RHEL and Fedora debugging kernels,
so that it will be tested by the users.
The users won't use some extra magic kernel options or debugfs files.
Mikulas
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> >
> > ---
> > Documentation/fault-injection/fault-injection.txt | 7 +++++
> > include/linux/fault-inject.h | 9 +++---
> > kernel/futex.c | 2 -
> > lib/Kconfig.debug | 15 +++++++++++
> > mm/failslab.c | 2 -
> > mm/page_alloc.c | 2 -
> > mm/util.c | 30 ++++++++++++++++++++++
> > 7 files changed, 60 insertions(+), 7 deletions(-)
> >
> > Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
> > +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
> > @@ -15,6 +15,12 @@ o fail_page_alloc
> >
> > injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
> >
> > +o kvmalloc_fallback
> > +
> > + makes the function kvmalloc randomly fall back to vmalloc. This could be used
> > + to detects bugs such as using DMA-API on the result of kvmalloc or freeing
> > + the result of kvmalloc with free.
> > +
> > o fail_futex
> >
> > injects futex deadlock and uaddr fault errors.
> > @@ -167,6 +173,7 @@ use the boot option:
> >
> > failslab=
> > fail_page_alloc=
> > + kvmalloc_fallback=
> > fail_make_request=
> > fail_futex=
> > mmc_core.fail_request=<interval>,<probability>,<space>,<times>
> > Index: linux-2.6/include/linux/fault-inject.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
> > @@ -31,17 +31,18 @@ struct fault_attr {
> > struct dentry *dname;
> > };
> >
> > -#define FAULT_ATTR_INITIALIZER { \
> > +#define FAULT_ATTR_INITIALIZER(p) { \
> > + .probability = (p), \
> > .interval = 1, \
> > - .times = ATOMIC_INIT(1), \
> > + .times = ATOMIC_INIT((p) ? -1 : 1), \
> > + .verbose = (p) ? 0 : 2, \
> > .require_end = ULONG_MAX, \
> > .stacktrace_depth = 32, \
> > .ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
> > - .verbose = 2, \
> > .dname = NULL, \
> > }
> >
> > -#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
> > +#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
> > int setup_fault_attr(struct fault_attr *attr, char *str);
> > bool should_fail(struct fault_attr *attr, ssize_t size);
> >
> > Index: linux-2.6/lib/Kconfig.debug
> > ===================================================================
> > --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
> > +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
> > @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
> > help
> > Provide fault-injection capability for alloc_pages().
> >
> > +config FAIL_KVMALLOC_FALLBACK_PROBABILITY
> > + int "Default kvmalloc fallback probability"
> > + depends on FAULT_INJECTION
> > + range 0 100
> > + default "0"
> > + help
> > + This option will make kvmalloc randomly fall back to vmalloc.
> > + Normally, kvmalloc falls back to vmalloc only rarely, if memory
> > + is fragmented.
> > +
> > + This option helps to detect hard-to-reproduce driver bugs, for
> > + example using DMA API on the result of kvmalloc.
> > +
> > + The default may be overridden with the kvmalloc_fallback parameter.
> > +
> > config FAIL_MAKE_REQUEST
> > bool "Fault-injection capability for disk IO"
> > depends on FAULT_INJECTION && BLOCK
> > Index: linux-2.6/mm/util.c
> > ===================================================================
> > --- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
> > +++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
> > @@ -14,6 +14,7 @@
> > #include <linux/hugetlb.h>
> > #include <linux/vmalloc.h>
> > #include <linux/userfaultfd_k.h>
> > +#include <linux/fault-inject.h>
> >
> > #include <asm/sections.h>
> > #include <linux/uaccess.h>
> > @@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
> > }
> > EXPORT_SYMBOL(vm_mmap);
> >
> > +#ifdef CONFIG_FAULT_INJECTION
> > +
> > +static struct fault_attr kvmalloc_fallback =
> > + FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
> > +
> > +static int __init setup_kvmalloc_fallback(char *str)
> > +{
> > + return setup_fault_attr(&kvmalloc_fallback, str);
> > +}
> > +
> > +__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
> > +
> > +#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
> > +static int __init kvmalloc_fallback_debugfs_init(void)
> > +{
> > + fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
> > + return 0;
> > +}
> > +late_initcall(kvmalloc_fallback_debugfs_init);
> > +#endif
> > +
> > +#endif
> > +
> > /**
> > * kvmalloc_node - attempt to allocate physically contiguous memory, but upon
> > * failure, fall back to non-contiguous (vmalloc) allocation.
> > @@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
> > */
> > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
> >
> > +#ifdef CONFIG_FAULT_INJECTION
> > + if (should_fail(&kvmalloc_fallback, size))
> > + goto do_vmalloc;
> > +#endif
> > +
> > /*
> > * We want to attempt a large physically contiguous block first because
> > * it is less likely to fragment multiple larger blocks and therefore
> > @@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
> > if (ret || size <= PAGE_SIZE)
> > return ret;
> >
> > +do_vmalloc: __maybe_unused
> > return __vmalloc_node_flags_caller(size, node, flags,
> > __builtin_return_address(0));
> > }
> > Index: linux-2.6/kernel/futex.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
> > +++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
> > @@ -288,7 +288,7 @@ static struct {
> >
> > bool ignore_private;
> > } fail_futex = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_private = false,
> > };
> >
> > Index: linux-2.6/mm/failslab.c
> > ===================================================================
> > --- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
> > @@ -9,7 +9,7 @@ static struct {
> > bool ignore_gfp_reclaim;
> > bool cache_filter;
> > } failslab = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_gfp_reclaim = true,
> > .cache_filter = false,
> > };
> > Index: linux-2.6/mm/page_alloc.c
> > ===================================================================
> > --- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
> > +++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
> > @@ -3055,7 +3055,7 @@ static struct {
> > bool ignore_gfp_reclaim;
> > u32 min_order;
> > } fail_page_alloc = {
> > - .attr = FAULT_ATTR_INITIALIZER,
> > + .attr = FAULT_ATTR_INITIALIZER(0),
> > .ignore_gfp_reclaim = true,
> > .ignore_gfp_highmem = true,
> > .min_order = 1,
> >
> >
>
^ permalink raw reply
* [bpf PATCH v2] bpf: fix for lex/yacc build error with gcc-5
From: John Fastabend @ 2018-04-25 21:22 UTC (permalink / raw)
To: ast, daniel, jbenc; +Cc: netdev
Fix build error found with Ubuntu shipped gcc-5
~/git/bpf/tools/bpf$ make all
Auto-detecting system features:
... libbfd: [ OFF ]
... disassembler-four-args: [ OFF ]
CC bpf_jit_disasm.o
LINK bpf_jit_disasm
CC bpf_dbg.o
/home/john/git/bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
/home/john/git/bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
} else if (matches(subcmd, "pcap") == 0) {
^
LINK bpf_dbg
CC bpf_asm.o
make: *** No rule to make target `bpf_exp.yacc.o', needed by `bpf_asm'. Stop.
Fixes: 5a8997f20715 ("tools: bpf: respect output directory during build")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/bpf/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 1ea5459..53b60ad 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -76,6 +76,8 @@ $(OUTPUT)bpf_asm: $(OUTPUT)bpf_asm.o $(OUTPUT)bpf_exp.yacc.o $(OUTPUT)bpf_exp.le
$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^
$(OUTPUT)bpf_exp.lex.c: $(OUTPUT)bpf_exp.yacc.c
+$(OUTPUT)bpf_exp.yacc.o: $(OUTPUT)bpf_exp.yacc.c
+$(OUTPUT)bpf_exp.lex.o: $(OUTPUT)bpf_exp.lex.c
clean: bpftool_clean
$(call QUIET_CLEAN, bpf-progs)
^ permalink raw reply related
* Re: simplify procfs code for seq_file instances
From: Alexey Dobriyan @ 2018-04-25 21:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-rtc, Alessandro Zummo, Alexandre Belloni, devel, linux-scsi,
Corey Minyard, linux-ide, Greg Kroah-Hartman, jfs-discussion,
linux-kernel, linux-acpi, netdev, netfilter-devel, Alexander Viro,
Jiri Slaby, Andrew Morton, linux-ext4, linux-afs,
megaraidlinux.pdl, drbd-dev
In-Reply-To: <20180424160652.GA28483@lst.de>
On Tue, Apr 24, 2018 at 06:06:53PM +0200, Christoph Hellwig wrote:
> On Tue, Apr 24, 2018 at 08:19:16AM -0700, Andrew Morton wrote:
> > > > I want to ask if it is time to start using poorman function overloading
> > > > with _b_c_e(). There are millions of allocation functions for example,
> > > > all slightly difference, and people will add more. Seeing /proc interfaces
> > > > doubled like this is painful.
> > >
> > > Function overloading is totally unacceptable.
> > >
> > > And I very much disagree with a tradeoff that keeps 5000 lines of
> > > code vs a few new helpers.
> >
> > OK, the curiosity and suspense are killing me. What the heck is
> > "function overloading with _b_c_e()"?
>
> The way I understood Alexey was to use have a proc_create macro
> that can take different ops types. Although the short cut for
> __builtin_types_compatible_p would be _b_t_c or similar, so maybe
> I misunderstood him.
That's correct.
I also think that several dozens kmalloc signatures are a problem.
And there will be more with pmalloc* stuff and more 2D/3D array
checked allocations and who knows what.
And I want to add typed kmalloc!
^ permalink raw reply
* [PATCH v2] selftests: net: add in_netns.sh TEST_GEN_PROGS_EXTENDED
From: Anders Roxell @ 2018-04-25 21:32 UTC (permalink / raw)
To: davem, shuah; +Cc: netdev, linux-kselftest, linux-kernel, Anders Roxell
In-Reply-To: <20180425.132513.1786379382285267245.davem@davemloft.net>
Script in_netns.sh is a utility function and not its own test so it
shouldn't be part of the TEST_PROGS. The in_netns.sh get used by
run_afpackettests.
To install in_netns.sh without being added to the main run_kselftest.sh
script use the TEST_GEN_PROGS_EXTENDED variable.
Fixes: 5ff9c1a3dd92 ("selftests: net: add in_netns.sh to TEST_PROGS")
Signed-off-by: Anders Roxell <anders.roxell@linaro.org>
---
respin against the 'net' tree.
tools/testing/selftests/net/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 8f1e13d2e547..daf5effec3f0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,7 +5,8 @@ CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
CFLAGS += -I../../../../usr/include/
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
-TEST_PROGS += fib_tests.sh fib-onlink-tests.sh in_netns.sh pmtu.sh
+TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh
+TEST_GEN_PROGS_EXTENDED := in_netns.sh
TEST_GEN_FILES = socket
TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: Siwei Liu @ 2018-04-25 21:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Alexander Duyck, virtio-dev, Jiri Pirko, Jakub Kicinski,
Sridhar Samudrala, virtualization, Netdev, David Miller
In-Reply-To: <20180423230037-mutt-send-email-mst@kernel.org>
On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >>
>> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> > > > >
>> >> > > > >I will NAK patches to change to common code for netvsc especially the
>> >> > > > >three device model. MS worked hard with distro vendors to support transparent
>> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> > > > >
>> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> > > >
>> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
>> >> > >
>> >> > > The network device model is a userspace API, and DPDK is a userspace application.
>> >> >
>> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> >> > AFAIK it's normally banging device registers directly.
>> >> >
>> >> > > You can't go breaking userspace even if you don't like the application.
>> >> >
>> >> > Could you please explain how is the proposed patchset breaking
>> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
>> >> > API at all.
>> >> >
>> >>
>> >> The DPDK has a device driver vdev_netvsc which scans the Linux network devices
>> >> to look for Linux netvsc device and the paired VF device and setup the
>> >> DPDK environment. This setup creates a DPDK failsafe (bondingish) instance
>> >> and sets up TAP support over the Linux netvsc device as well as the Mellanox
>> >> VF device.
>> >>
>> >> So it depends on existing 2 device model. You can't go to a 3 device model
>> >> or start hiding devices from userspace.
>> >
>> > Okay so how does the existing patch break that? IIUC does not go to
>> > a 3 device model since netvsc calls failover_register directly.
>> >
>> >> Also, I am working on associating netvsc and VF device based on serial number
>> >> rather than MAC address. The serial number is how Windows works now, and it makes
>> >> sense for Linux and Windows to use the same mechanism if possible.
>> >
>> > Maybe we should support same for virtio ...
>> > Which serial do you mean? From vpd?
>> >
>> > I guess you will want to keep supporting MAC for old hypervisors?
>> >
>> > It all seems like a reasonable thing to support in the generic core.
>>
>> That's the reason why I chose explicit identifier rather than rely on
>> MAC address to bind/pair a device. MAC address can change. Even if it
>> can't, malicious guest user can fake MAC address to skip binding.
>>
>> -Siwei
>
> Address should be sampled at device creation to prevent this
> kind of hack. Not that it buys the malicious user much:
> if you can poke at MAC addresses you probably already can
> break networking.
I don't understand why poking at MAC address may potentially break
networking. Unlike VF, passthrough PCI endpoint device has its freedom
to change the MAC address. Even on a VF setup it's not neccessarily
always safe to assume the VF's MAC address cannot or shouldn't be
changed. That depends on the specific need whether the host admin
wants to restrict guest from changing the MAC address, although in
most cases it's true.
I understand we can use the perm_addr to distinguish. But as said,
this will pose limitation of flexible configuration where one can
assign VFs with identical MAC address at all while each VF belongs to
different PF and/or different subnet for e.g. load balancing. And
furthermore, the QEMU device model never uses MAC address to be
interpreted as an identifier, which requires to be unique per VM
instance. Why we're introducing this inconsistency?
-Siwei
>
>
>
>
>>
>> >
>> > --
>> > MST
^ permalink raw reply
* Re: [PATCH][net-next] net: ip tos cgroup
From: kbuild test robot @ 2018-04-25 21:39 UTC (permalink / raw)
To: Li RongQing; +Cc: kbuild-all, netdev
In-Reply-To: <1523936161-11676-1-git-send-email-lirongqing@baidu.com>
[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]
Hi Li,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net/master]
[also build test ERROR on v4.17-rc2 next-20180424]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Li-RongQing/net-ip-tos-cgroup/20180417-204807
config: arm-dove_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
In file included from net/ipv4/af_inet.c:123:0:
>> include/net/tos_cgroup.h:10:29: error: field 'css' has incomplete type
struct cgroup_subsys_state css;
^~~
vim +/css +10 include/net/tos_cgroup.h
8
9 struct tos_cgroup_state {
> 10 struct cgroup_subsys_state css;
11 u32 tos;
12 };
13
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17596 bytes --]
^ permalink raw reply
* Re: [bpf PATCH v2] bpf: fix for lex/yacc build error with gcc-5
From: Daniel Borkmann @ 2018-04-25 21:42 UTC (permalink / raw)
To: John Fastabend, ast, jbenc; +Cc: netdev
In-Reply-To: <20180425212245.25999.21181.stgit@john-Precision-Tower-5810>
On 04/25/2018 11:22 PM, John Fastabend wrote:
> Fix build error found with Ubuntu shipped gcc-5
>
> ~/git/bpf/tools/bpf$ make all
>
> Auto-detecting system features:
> ... libbfd: [ OFF ]
> ... disassembler-four-args: [ OFF ]
>
> CC bpf_jit_disasm.o
> LINK bpf_jit_disasm
> CC bpf_dbg.o
> /home/john/git/bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
> /home/john/git/bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> } else if (matches(subcmd, "pcap") == 0) {
> ^
> LINK bpf_dbg
> CC bpf_asm.o
> make: *** No rule to make target `bpf_exp.yacc.o', needed by `bpf_asm'. Stop.
>
> Fixes: 5a8997f20715 ("tools: bpf: respect output directory during build")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Applied to bpf, thanks John! Will see to get that annoying bpf_dbg warn out
of the way if no one beats me to it. ;)
^ permalink raw reply
* [PATCH v2 net-next 0/2] tcp: mmap: rework zerocopy receive
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet
syzbot reported a lockdep issue caused by tcp mmap() support.
I implemented Andy Lutomirski nice suggestions to resolve the
issue and increase scalability as well.
First patch is adding a new setsockopt() operation and changes mmap()
behavior.
Second patch changes tcp_mmap reference program.
v2:
Added a missing page align of zc->length in tcp_zerocopy_receive()
Properly clear zc->recv_skip_hint in case user request was completed.
Eric Dumazet (2):
tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
selftests: net: tcp_mmap must use TCP_ZEROCOPY_RECEIVE
include/uapi/linux/tcp.h | 8 ++
net/ipv4/tcp.c | 189 +++++++++++++------------
tools/testing/selftests/net/tcp_mmap.c | 63 +++++----
3 files changed, 142 insertions(+), 118 deletions(-)
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply
* [PATCH v2 net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet, Soheil Hassas Yeganeh
In-Reply-To: <20180425214307.159264-1-edumazet@google.com>
When adding tcp mmap() implementation, I forgot that socket lock
had to be taken before current->mm->mmap_sem. syzbot eventually caught
the bug.
Since we can not lock the socket in tcp mmap() handler we have to
split the operation in two phases.
1) mmap() on a tcp socket simply reserves VMA space, and nothing else.
This operation does not involve any TCP locking.
2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements
the transfert of pages from skbs to one VMA.
This operation only uses down_read(¤t->mm->mmap_sem) after
holding TCP lock, thus solving the lockdep issue.
This new implementation was suggested by Andy Lutomirski with great details.
Benefits are :
- Better scalability, in case multiple threads reuse VMAS
(without mmap()/munmap() calls) since mmap_sem wont be write locked.
- Better error recovery.
The previous mmap() model had to provide the expected size of the
mapping. If for some reason one part could not be mapped (partial MSS),
the whole operation had to be aborted.
With the tcp_zerocopy_receive struct, kernel can report how
many bytes were successfuly mapped, and how many bytes should
be read to skip the problematic sequence.
- No more memory allocation to hold an array of page pointers.
16 MB mappings needed 32 KB for this array, potentially using vmalloc() :/
- skbs are freed while mmap_sem has been released
Following patch makes the change in tcp_mmap tool to demonstrate
one possible use of mmap() and setsockopt(... TCP_ZEROCOPY_RECEIVE ...)
Note that memcg might require additional changes.
Fixes: 93ab6cc69162 ("tcp: implement mmap() for zero copy receive")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Cc: linux-mm@kvack.org
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
include/uapi/linux/tcp.h | 8 ++
net/ipv4/tcp.c | 189 ++++++++++++++++++++-------------------
2 files changed, 106 insertions(+), 91 deletions(-)
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 379b08700a542d49bbce9b4b49b17879d00b69bb..e9e8373b34b9ddc735329341b91f455bf5c0b17c 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -122,6 +122,7 @@ enum {
#define TCP_MD5SIG_EXT 32 /* TCP MD5 Signature with extensions */
#define TCP_FASTOPEN_KEY 33 /* Set the key for Fast Open (cookie) */
#define TCP_FASTOPEN_NO_COOKIE 34 /* Enable TFO without a TFO cookie */
+#define TCP_ZEROCOPY_RECEIVE 35
struct tcp_repair_opt {
__u32 opt_code;
@@ -276,4 +277,11 @@ struct tcp_diag_md5sig {
__u8 tcpm_key[TCP_MD5SIG_MAXKEYLEN];
};
+/* setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) */
+
+struct tcp_zerocopy_receive {
+ __u64 address; /* in: address of mapping */
+ __u32 length; /* in/out: number of bytes to map/mapped */
+ __u32 recv_skip_hint; /* out: amount of bytes to skip */
+};
#endif /* _UAPI_LINUX_TCP_H */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dfd090ea54ad47112fc23c61180b5bf8edd2c736..9b9cbb837ff8d6cdd85515429f699e113c55b37b 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1726,118 +1726,111 @@ int tcp_set_rcvlowat(struct sock *sk, int val)
}
EXPORT_SYMBOL(tcp_set_rcvlowat);
-/* When user wants to mmap X pages, we first need to perform the mapping
- * before freeing any skbs in receive queue, otherwise user would be unable
- * to fallback to standard recvmsg(). This happens if some data in the
- * requested block is not exactly fitting in a page.
- *
- * We only support order-0 pages for the moment.
- * mmap() on TCP is very strict, there is no point
- * trying to accommodate with pathological layouts.
- */
+static const struct vm_operations_struct tcp_vm_ops = {
+};
+
int tcp_mmap(struct file *file, struct socket *sock,
struct vm_area_struct *vma)
{
- unsigned long size = vma->vm_end - vma->vm_start;
- unsigned int nr_pages = size >> PAGE_SHIFT;
- struct page **pages_array = NULL;
- u32 seq, len, offset, nr = 0;
- struct sock *sk = sock->sk;
- const skb_frag_t *frags;
+ if (vma->vm_flags & (VM_WRITE | VM_EXEC))
+ return -EPERM;
+ vma->vm_flags &= ~(VM_MAYWRITE | VM_MAYEXEC);
+
+ /* Instruct vm_insert_page() to not down_read(mmap_sem) */
+ vma->vm_flags |= VM_MIXEDMAP;
+
+ vma->vm_ops = &tcp_vm_ops;
+ return 0;
+}
+EXPORT_SYMBOL(tcp_mmap);
+
+static int tcp_zerocopy_receive(struct sock *sk,
+ struct tcp_zerocopy_receive *zc)
+{
+ unsigned long address = (unsigned long)zc->address;
+ const skb_frag_t *frags = NULL;
+ u32 length = 0, seq, offset;
+ struct vm_area_struct *vma;
+ struct sk_buff *skb = NULL;
struct tcp_sock *tp;
- struct sk_buff *skb;
int ret;
- if (vma->vm_pgoff || !nr_pages)
+ if (address & (PAGE_SIZE - 1) || address != zc->address)
return -EINVAL;
- if (vma->vm_flags & VM_WRITE)
- return -EPERM;
- /* TODO: Maybe the following is not needed if pages are COW */
- vma->vm_flags &= ~VM_MAYWRITE;
-
- lock_sock(sk);
-
- ret = -ENOTCONN;
if (sk->sk_state == TCP_LISTEN)
- goto out;
+ return -ENOTCONN;
sock_rps_record_flow(sk);
- if (tcp_inq(sk) < size) {
- ret = sock_flag(sk, SOCK_DONE) ? -EIO : -EAGAIN;
+ down_read(¤t->mm->mmap_sem);
+
+ ret = -EINVAL;
+ vma = find_vma(current->mm, address);
+ if (!vma || vma->vm_start > address || vma->vm_ops != &tcp_vm_ops)
goto out;
- }
+ zc->length = min_t(unsigned long, zc->length, vma->vm_end - address);
+
tp = tcp_sk(sk);
seq = tp->copied_seq;
- /* Abort if urgent data is in the area */
- if (unlikely(tp->urg_data)) {
- u32 urg_offset = tp->urg_seq - seq;
+ zc->length = min_t(u32, zc->length, tcp_inq(sk));
+ zc->length &= ~(PAGE_SIZE - 1);
- ret = -EINVAL;
- if (urg_offset < size)
- goto out;
- }
- ret = -ENOMEM;
- pages_array = kvmalloc_array(nr_pages, sizeof(struct page *),
- GFP_KERNEL);
- if (!pages_array)
- goto out;
- skb = tcp_recv_skb(sk, seq, &offset);
- ret = -EINVAL;
-skb_start:
- /* We do not support anything not in page frags */
- offset -= skb_headlen(skb);
- if ((int)offset < 0)
- goto out;
- if (skb_has_frag_list(skb))
- goto out;
- len = skb->data_len - offset;
- frags = skb_shinfo(skb)->frags;
- while (offset) {
- if (frags->size > offset)
- goto out;
- offset -= frags->size;
- frags++;
- }
- while (nr < nr_pages) {
- if (len) {
- if (len < PAGE_SIZE)
- goto out;
- if (frags->size != PAGE_SIZE || frags->page_offset)
- goto out;
- pages_array[nr++] = skb_frag_page(frags);
- frags++;
- len -= PAGE_SIZE;
- seq += PAGE_SIZE;
- continue;
- }
- skb = skb->next;
- offset = seq - TCP_SKB_CB(skb)->seq;
- goto skb_start;
- }
- /* OK, we have a full set of pages ready to be inserted into vma */
- for (nr = 0; nr < nr_pages; nr++) {
- ret = vm_insert_page(vma, vma->vm_start + (nr << PAGE_SHIFT),
- pages_array[nr]);
- if (ret)
- goto out;
- }
- /* operation is complete, we can 'consume' all skbs */
- tp->copied_seq = seq;
- tcp_rcv_space_adjust(sk);
-
- /* Clean up data we have read: This will do ACK frames. */
- tcp_recv_skb(sk, seq, &offset);
- tcp_cleanup_rbuf(sk, size);
+ zap_page_range(vma, address, zc->length);
+ zc->recv_skip_hint = 0;
ret = 0;
+ while (length + PAGE_SIZE <= zc->length) {
+ if (zc->recv_skip_hint < PAGE_SIZE) {
+ if (skb) {
+ skb = skb->next;
+ offset = seq - TCP_SKB_CB(skb)->seq;
+ } else {
+ skb = tcp_recv_skb(sk, seq, &offset);
+ }
+
+ zc->recv_skip_hint = skb->len - offset;
+ offset -= skb_headlen(skb);
+ if ((int)offset < 0 || skb_has_frag_list(skb))
+ break;
+ frags = skb_shinfo(skb)->frags;
+ while (offset) {
+ if (frags->size > offset)
+ goto out;
+ offset -= frags->size;
+ frags++;
+ }
+ }
+ if (frags->size != PAGE_SIZE || frags->page_offset)
+ break;
+ ret = vm_insert_page(vma, address + length,
+ skb_frag_page(frags));
+ if (ret)
+ break;
+ length += PAGE_SIZE;
+ seq += PAGE_SIZE;
+ zc->recv_skip_hint -= PAGE_SIZE;
+ frags++;
+ }
out:
- release_sock(sk);
- kvfree(pages_array);
+ up_read(¤t->mm->mmap_sem);
+ if (length) {
+ tp->copied_seq = seq;
+ tcp_rcv_space_adjust(sk);
+
+ /* Clean up data we have read: This will do ACK frames. */
+ tcp_recv_skb(sk, seq, &offset);
+ tcp_cleanup_rbuf(sk, length);
+ ret = 0;
+ if (length == zc->length)
+ zc->recv_skip_hint = 0;
+ } else {
+ if (!zc->recv_skip_hint && sock_flag(sk, SOCK_DONE))
+ ret = -EIO;
+ }
+ zc->length = length;
return ret;
}
-EXPORT_SYMBOL(tcp_mmap);
static void tcp_update_recv_tstamps(struct sk_buff *skb,
struct scm_timestamping *tss)
@@ -2738,6 +2731,20 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
return tcp_fastopen_reset_cipher(net, sk, key, sizeof(key));
}
+ case TCP_ZEROCOPY_RECEIVE: {
+ struct tcp_zerocopy_receive zc;
+
+ if (optlen != sizeof(zc))
+ return -EINVAL;
+ if (copy_from_user(&zc, optval, optlen))
+ return -EFAULT;
+ lock_sock(sk);
+ err = tcp_zerocopy_receive(sk, &zc);
+ release_sock(sk);
+ if (!err && copy_to_user(optval, &zc, optlen))
+ err = -EFAULT;
+ return err;
+ }
default:
/* fallthru */
break;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v2 net-next 2/2] selftests: net: tcp_mmap must use TCP_ZEROCOPY_RECEIVE
From: Eric Dumazet @ 2018-04-25 21:43 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Andy Lutomirski, linux-kernel, linux-mm, Eric Dumazet,
Eric Dumazet, Soheil Hassas Yeganeh
In-Reply-To: <20180425214307.159264-1-edumazet@google.com>
After prior kernel change, mmap() on TCP socket only reserves VMA.
We have to use setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...)
to perform the transfert of pages from skbs in TCP receive queue into such VMA.
struct tcp_zerocopy_receive {
__u64 address; /* in: address of mapping */
__u32 length; /* in/out: number of bytes to map/mapped */
__u32 recv_skip_hint; /* out: amount of bytes to skip */
};
After a successful setsockopt(...TCP_ZEROCOPY_RECEIVE...), @length contains
number of bytes that were mapped, and @recv_skip_hint contains number of bytes
that should be read using conventional read()/recv()/recvmsg() system calls,
to skip a sequence of bytes that can not be mapped, because not properly page
aligned.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
---
tools/testing/selftests/net/tcp_mmap.c | 63 +++++++++++++++-----------
1 file changed, 36 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/net/tcp_mmap.c b/tools/testing/selftests/net/tcp_mmap.c
index dea342fe6f4e88b5709d2ac37b2fc9a2a320bf44..5b381cdbdd6319556ba4e3dad530fae8f13f5a9b 100644
--- a/tools/testing/selftests/net/tcp_mmap.c
+++ b/tools/testing/selftests/net/tcp_mmap.c
@@ -76,9 +76,10 @@
#include <time.h>
#include <sys/time.h>
#include <netinet/in.h>
-#include <netinet/tcp.h>
#include <arpa/inet.h>
#include <poll.h>
+#include <linux/tcp.h>
+#include <assert.h>
#ifndef MSG_ZEROCOPY
#define MSG_ZEROCOPY 0x4000000
@@ -134,11 +135,12 @@ void hash_zone(void *zone, unsigned int length)
void *child_thread(void *arg)
{
unsigned long total_mmap = 0, total = 0;
+ struct tcp_zerocopy_receive zc;
unsigned long delta_usec;
int flags = MAP_SHARED;
struct timeval t0, t1;
char *buffer = NULL;
- void *oaddr = NULL;
+ void *addr = NULL;
double throughput;
struct rusage ru;
int lu, fd;
@@ -153,41 +155,45 @@ void *child_thread(void *arg)
perror("malloc");
goto error;
}
+ if (zflg) {
+ addr = mmap(NULL, chunk_size, PROT_READ, flags, fd, 0);
+ if (addr == (void *)-1)
+ zflg = 0;
+ }
while (1) {
struct pollfd pfd = { .fd = fd, .events = POLLIN, };
int sub;
poll(&pfd, 1, 10000);
if (zflg) {
- void *naddr;
+ int res;
- naddr = mmap(oaddr, chunk_size, PROT_READ, flags, fd, 0);
- if (naddr == (void *)-1) {
- if (errno == EAGAIN) {
- /* That is if SO_RCVLOWAT is buggy */
- usleep(1000);
- continue;
- }
- if (errno == EINVAL) {
- flags = MAP_SHARED;
- oaddr = NULL;
- goto fallback;
- }
- if (errno != EIO)
- perror("mmap()");
+ zc.address = (__u64)addr;
+ zc.length = chunk_size;
+ zc.recv_skip_hint = 0;
+ res = setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE,
+ &zc, sizeof(zc));
+ if (res == -1)
break;
+
+ if (zc.length) {
+ assert(zc.length <= chunk_size);
+ total_mmap += zc.length;
+ if (xflg)
+ hash_zone(addr, zc.length);
+ total += zc.length;
}
- total_mmap += chunk_size;
- if (xflg)
- hash_zone(naddr, chunk_size);
- total += chunk_size;
- if (!keepflag) {
- flags |= MAP_FIXED;
- oaddr = naddr;
+ if (zc.recv_skip_hint) {
+ assert(zc.recv_skip_hint <= chunk_size);
+ lu = read(fd, buffer, zc.recv_skip_hint);
+ if (lu > 0) {
+ if (xflg)
+ hash_zone(buffer, lu);
+ total += lu;
+ }
}
continue;
}
-fallback:
sub = 0;
while (sub < chunk_size) {
lu = read(fd, buffer + sub, chunk_size - sub);
@@ -228,6 +234,8 @@ void *child_thread(void *arg)
error:
free(buffer);
close(fd);
+ if (zflg)
+ munmap(addr, chunk_size);
pthread_exit(0);
}
@@ -371,7 +379,8 @@ int main(int argc, char *argv[])
setup_sockaddr(cfg_family, host, &listenaddr);
if (mss &&
- setsockopt(fdlisten, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
+ setsockopt(fdlisten, IPPROTO_TCP, TCP_MAXSEG,
+ &mss, sizeof(mss)) == -1) {
perror("setsockopt TCP_MAXSEG");
exit(1);
}
@@ -402,7 +411,7 @@ int main(int argc, char *argv[])
setup_sockaddr(cfg_family, host, &addr);
if (mss &&
- setsockopt(fd, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
+ setsockopt(fd, IPPROTO_TCP, TCP_MAXSEG, &mss, sizeof(mss)) == -1) {
perror("setsockopt TCP_MAXSEG");
exit(1);
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* Re: [PATCH net-next v2 2/2] openvswitch: Support conntrack zone limit
From: Yi-Hung Wei @ 2018-04-25 21:51 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAOrHB_DHAjUp8Mf1_d-uKiiJPqUmwOvQDCTveFuvEhj9oJ-DhQ@mail.gmail.com>
>> +#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
>> +#define OVS_CT_LIMIT_UNLIMITED 0
>> +#define OVS_CT_LIMIT_DEFAULT OVS_CT_LIMIT_UNLIMITED
>> +#define CT_LIMIT_HASH_BUCKETS 512
>> +
> Can you use static key when the limit is not set.
> This would avoid overhead in datapath when these limits are not used.
>
Thanks Parvin for the review. I'm not sure about the 'static key'
part, are you suggesting that say if we can have a flag to check if no
one has ever set the ct_limit? If the ct_limit feature is not used
so far, then instead of look up the hash table for the zone limit, we
just return the default unlimited value. So that we can avoid the
overhead of checking ct_limit.
>> +struct ovs_ct_limit {
>> + /* Elements in ovs_ct_limit_info->limits hash table */
>> + struct hlist_node hlist_node;
>> + struct rcu_head rcu;
>> + u16 zone;
>> + u32 limit;
>> +};
>> +
> ...
>> /* Lookup connection and confirm if unconfirmed. */
>> static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>> const struct ovs_conntrack_info *info,
>> @@ -1054,6 +1176,13 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key,
>> if (!ct)
>> return 0;
>>
>> +#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
>> + err = ovs_ct_check_limit(net, info,
>> + &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple);
>> + if (err)
>> + return err;
>> +#endif
>> +
>
> This could be checked during flow install time, so that only permitted
> flows would have 'ct commit' action, we can avoid per packet cost
> checking the limit.
It seems to me that it would be hard to check the # of connections of
a flow in the flow installation stage. For example, if we have a
datapath flow that performs “ct commit” action on all UDP traffic from
in_port 1, then it could be various combinations of 5-tuple that form
various # of connections. Therefore, it would be hard to do the
admission control there.
> returning error code form ovs_ct_commit() is lost in datapath and it
> would be hard to debug packet lost in case of the limit is reached. So
> another advantage of checking the limit in flow install be better
> traceability. datapath would return error to usespace and it can log
> the error code.
Thanks for pointing out the error code from ovs_ct_commit() will be
lost in the datapath. In this case, shall we consider to report the
packet drop by some rate_limit logging such as net_warn_ratelimited()
or net_info_ratelimited()?
Thanks,
-Yi-Hung
^ permalink raw reply
* Loan Offer
From: Coleman Krosberg @ 2018-04-25 21:53 UTC (permalink / raw)
Thank you for your time,
We are looking for clients in your area of residents with good business or project that requires financing
to execute, or who wants a loan to do business, we do give loans with low interest.
Please get back to me if you are interested in this or you know anybody who has good business ideas but
lack the necessary capital to fund his projects or business so we can establish working relationship.
Yours Sincerely,
Coleman Krosberg MBA, CFA
Ak Bank Financial Rep
Investment analyst.
^ permalink raw reply
* pull-request: bpf 2018-04-25
From: Daniel Borkmann @ 2018-04-25 22:04 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net* tree.
The main changes are:
1) Fix to clear the percpu metadata_dst that could otherwise carry
stale ip_tunnel_info, from William.
2) Fix that reduces the number of passes in x64 JIT with regards to
dead code sanitation to avoid risk of prog rejection, from Gianluca.
3) Several fixes of sockmap programs, besides others, fixing a double
page_put() in error path, missing refcount hold for pinned sockmap,
adding required -target bpf for clang in sample Makefile, from John.
4) Fix to disable preemption in __BPF_PROG_RUN_ARRAY() paths, from Roman.
5) Fix tools/bpf/ Makefile with regards to a lex/yacc build error
seen on older gcc-5, from John.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Would be great if you have a chance to merge net into net-next after
that since sockmap fixes are needed in bpf-next later on to avoid
ugly merge conflicts.
Thanks a lot!
----------------------------------------------------------------
The following changes since commit 77621f024d6be732e43366a42203486b6ec89acd:
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf (2018-04-23 16:22:24 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
for you to fetch changes up to 9c299a32ede98dc9faafb267034ed830a15304db:
bpf: fix for lex/yacc build error with gcc-5 (2018-04-25 23:27:46 +0200)
----------------------------------------------------------------
Daniel Borkmann (1):
Merge branch 'bpf-sockmap-fixes'
Gianluca Borello (1):
bpf, x64: fix JIT emission for dead code
John Fastabend (6):
bpf: Document sockmap '-target bpf' requirement for PROG_TYPE_SK_MSG
bpf: sockmap sample use clang flag, -target bpf
bpf: sockmap, map_release does not hold refcnt for pinned maps
bpf: sockmap, sk_wait_event needed to handle blocking cases
bpf: sockmap, fix double page_put on ENOMEM error in redirect path
bpf: fix for lex/yacc build error with gcc-5
Roman Gushchin (1):
bpf: disable and restore preemption in __BPF_PROG_RUN_ARRAY
William Tu (1):
bpf: clear the ip_tunnel_info.
Documentation/bpf/bpf_devel_QA.txt | 10 +++++++-
arch/x86/net/bpf_jit_comp.c | 12 ++++++++-
include/linux/bpf.h | 4 ++-
kernel/bpf/arraymap.c | 3 ++-
kernel/bpf/sockmap.c | 51 +++++++++++++++++++++++++++++++++++---
kernel/bpf/syscall.c | 4 +--
net/core/filter.c | 1 +
samples/sockmap/Makefile | 7 ++++--
tools/bpf/Makefile | 2 ++
9 files changed, 82 insertions(+), 12 deletions(-)
^ permalink raw reply
* [bpf PATCH] bpf: fix uninitialized variable in bpf tools
From: John Fastabend @ 2018-04-25 22:08 UTC (permalink / raw)
To: ast, daniel, jbenc; +Cc: netdev
Here the variable cont is used as the saved_pointer for a call to
strtok_r(). It is safe to use the value uninitialized in this
context however and the later reference is only ever used if
the strtok_r is successful. But, 'gcc-5' at least doesn't have all
this knowledge so initialize cont to NULL. Additionally, do the
natural NULL check before accessing just for completness.
The warning is the following:
./bpf/tools/bpf/bpf_dbg.c: In function ‘cmd_load’:
./bpf/tools/bpf/bpf_dbg.c:1077:13: warning: ‘cont’ may be used uninitialized in this function [-Wmaybe-uninitialized]
} else if (matches(subcmd, "pcap") == 0) {
Fixes: fd981e3c321a "filter: bpf_dbg: add minimal bpf debugger"
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
tools/bpf/bpf_dbg.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/bpf/bpf_dbg.c b/tools/bpf/bpf_dbg.c
index 4f254bc..61b9aa5 100644
--- a/tools/bpf/bpf_dbg.c
+++ b/tools/bpf/bpf_dbg.c
@@ -1063,7 +1063,7 @@ static int cmd_load_pcap(char *file)
static int cmd_load(char *arg)
{
- char *subcmd, *cont, *tmp = strdup(arg);
+ char *subcmd, *cont = NULL, *tmp = strdup(arg);
int ret = CMD_OK;
subcmd = strtok_r(tmp, " ", &cont);
@@ -1073,7 +1073,10 @@ static int cmd_load(char *arg)
bpf_reset();
bpf_reset_breakpoints();
- ret = cmd_load_bpf(cont);
+ if (!cont)
+ ret = CMD_ERR;
+ else
+ ret = cmd_load_bpf(cont);
} else if (matches(subcmd, "pcap") == 0) {
ret = cmd_load_pcap(cont);
} else {
^ permalink raw reply related
* Re: [dm-devel] [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: James Bottomley @ 2018-04-25 22:17 UTC (permalink / raw)
To: Mikulas Patocka, David Rientjes
Cc: dm-devel, eric.dumazet, mst, netdev, jasowang, Randy Dunlap,
linux-kernel, Matthew Wilcox, Michal Hocko, linux-mm, edumazet,
Andrew Morton, virtualization, David Miller, Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251720090.9428@file01.intranet.prod.int.rdu2.redhat.com>
On Wed, 2018-04-25 at 17:22 -0400, Mikulas Patocka wrote:
>
> On Wed, 25 Apr 2018, David Rientjes wrote:
>
> > On Wed, 25 Apr 2018, Mikulas Patocka wrote:
> >
> > > From: Mikulas Patocka <mpatocka@redhat.com>
> > > Subject: [PATCH] fault-injection: introduce kvmalloc fallback
> > > options
> > >
> > > This patch introduces a fault-injection option
> > > "kvmalloc_fallback". This option makes kvmalloc randomly fall
> > > back to vmalloc.
> > >
> > > Unfortunately, some kernel code has bugs - it uses kvmalloc and
> > > then uses DMA-API on the returned memory or frees it with kfree.
> > > Such bugs were found in the virtio-net driver, dm-integrity or
> > > RHEL7 powerpc-specific code. This options helps to test for these
> > > bugs.
> > >
> > > The patch introduces a config option
> > > FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> > > It can be enabled in distribution debug kernels, so that kvmalloc
> > > abuse can be tested by the users. The default can be overridden
> > > with "kvmalloc_fallback" parameter or in
> > > /sys/kernel/debug/kvmalloc_fallback/.
> > >
> >
> > Do we really need the new config option? This could just be
> > manually tunable via fault injection IIUC.
>
> We do, because we want to enable it in RHEL and Fedora debugging
> kernels, so that it will be tested by the users.
>
> The users won't use some extra magic kernel options or debugfs files.
If it can be enabled via a tunable, then the distro can turn it on
without the user having to do anything. If you want to present the
user with a different boot option, you can (just have the tunable set
on the command line), but being tunable driven means that you don't
have to choose that option, you could automatically enable it under a
range of circumstances. I think most sane distributions would want
that flexibility.
Kconfig proliferation, conversely, is a bit of a nightmare from both
the user and the tester's point of view, so we're trying to avoid it
unless absolutely necessary.
James
^ permalink raw reply
* Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: Michael S. Tsirkin @ 2018-04-25 22:22 UTC (permalink / raw)
To: Siwei Liu
Cc: Stephen Hemminger, Jiri Pirko, Sridhar Samudrala, David Miller,
Netdev, virtualization, virtio-dev, Brandeburg, Jesse,
Alexander Duyck, Jakub Kicinski, Jason Wang
In-Reply-To: <CADGSJ22WMxXEtR9QpRa9_ZqgmTbgWHZz2j4hsUnRJrM9zTsW4w@mail.gmail.com>
On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
> >> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >> >>
> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
> >> >> > > > >
> >> >> > > > >I will NAK patches to change to common code for netvsc especially the
> >> >> > > > >three device model. MS worked hard with distro vendors to support transparent
> >> >> > > > >mode, ans we really can't have a new model; or do backport.
> >> >> > > > >
> >> >> > > > >Plus, DPDK is now dependent on existing model.
> >> >> > > >
> >> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
> >> >> > >
> >> >> > > The network device model is a userspace API, and DPDK is a userspace application.
> >> >> >
> >> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
> >> >> > AFAIK it's normally banging device registers directly.
> >> >> >
> >> >> > > You can't go breaking userspace even if you don't like the application.
> >> >> >
> >> >> > Could you please explain how is the proposed patchset breaking
> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
> >> >> > API at all.
> >> >> >
> >> >>
> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux network devices
> >> >> to look for Linux netvsc device and the paired VF device and setup the
> >> >> DPDK environment. This setup creates a DPDK failsafe (bondingish) instance
> >> >> and sets up TAP support over the Linux netvsc device as well as the Mellanox
> >> >> VF device.
> >> >>
> >> >> So it depends on existing 2 device model. You can't go to a 3 device model
> >> >> or start hiding devices from userspace.
> >> >
> >> > Okay so how does the existing patch break that? IIUC does not go to
> >> > a 3 device model since netvsc calls failover_register directly.
> >> >
> >> >> Also, I am working on associating netvsc and VF device based on serial number
> >> >> rather than MAC address. The serial number is how Windows works now, and it makes
> >> >> sense for Linux and Windows to use the same mechanism if possible.
> >> >
> >> > Maybe we should support same for virtio ...
> >> > Which serial do you mean? From vpd?
> >> >
> >> > I guess you will want to keep supporting MAC for old hypervisors?
> >> >
> >> > It all seems like a reasonable thing to support in the generic core.
> >>
> >> That's the reason why I chose explicit identifier rather than rely on
> >> MAC address to bind/pair a device. MAC address can change. Even if it
> >> can't, malicious guest user can fake MAC address to skip binding.
> >>
> >> -Siwei
> >
> > Address should be sampled at device creation to prevent this
> > kind of hack. Not that it buys the malicious user much:
> > if you can poke at MAC addresses you probably already can
> > break networking.
>
> I don't understand why poking at MAC address may potentially break
> networking.
Set a MAC address to match another device on the same LAN,
packets will stop reaching that MAC.
> Unlike VF, passthrough PCI endpoint device has its freedom
> to change the MAC address. Even on a VF setup it's not neccessarily
> always safe to assume the VF's MAC address cannot or shouldn't be
> changed. That depends on the specific need whether the host admin
> wants to restrict guest from changing the MAC address, although in
> most cases it's true.
>
> I understand we can use the perm_addr to distinguish. But as said,
> this will pose limitation of flexible configuration where one can
> assign VFs with identical MAC address at all while each VF belongs to
> different PF and/or different subnet for e.g. load balancing.
> And
> furthermore, the QEMU device model never uses MAC address to be
> interpreted as an identifier, which requires to be unique per VM
> instance. Why we're introducing this inconsistency?
>
> -Siwei
Because it addresses most of the issues and is simple. That's already
much better than what we have now which is nothing unless guest
configures things manually.
I think ideally the infrastructure should suppport flexible matching of
NICs - netvsc is already reported to be moving to some kind of serial
address.
> >
> >
> >
> >
> >>
> >> >
> >> > --
> >> > MST
^ permalink raw reply
* Re: [dm-devel] [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 22:42 UTC (permalink / raw)
To: James Bottomley
Cc: David Rientjes, dm-devel, eric.dumazet, mst, netdev, jasowang,
Randy Dunlap, linux-kernel, Matthew Wilcox, Michal Hocko,
linux-mm, edumazet, Andrew Morton, virtualization, David Miller,
Vlastimil Babka
In-Reply-To: <1524694663.4100.21.camel@HansenPartnership.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2372 bytes --]
On Wed, 25 Apr 2018, James Bottomley wrote:
> On Wed, 2018-04-25 at 17:22 -0400, Mikulas Patocka wrote:
> >
> > On Wed, 25 Apr 2018, David Rientjes wrote:
> > >
> > > Do we really need the new config option? This could just be
> > > manually tunable via fault injection IIUC.
> >
> > We do, because we want to enable it in RHEL and Fedora debugging
> > kernels, so that it will be tested by the users.
> >
> > The users won't use some extra magic kernel options or debugfs files.
>
> If it can be enabled via a tunable, then the distro can turn it on
> without the user having to do anything.
You need to enable it on boot. Enabling it when the kernel starts to
execute userspace code is already too late (because you would miss
kvmalloc calls in the kernel boot path).
These are files in the kernel-debug rpm package. Where would you put the
extra kernel parameter to enable this feature? None of these files contain
kernel parameters.
kernel-debug /boot/.vmlinuz-3.10.0-693.21.1.el7.x86_64.debug.hmac
kernel-debug /boot/System.map-3.10.0-693.21.1.el7.x86_64.debug
kernel-debug /boot/config-3.10.0-693.21.1.el7.x86_64.debug
kernel-debug /boot/initramfs-3.10.0-693.21.1.el7.x86_64.debug.img
kernel-debug /boot/symvers-3.10.0-693.21.1.el7.x86_64.debug.gz
kernel-debug /boot/vmlinuz-3.10.0-693.21.1.el7.x86_64.debug
kernel-debug /etc/ld.so.conf.d/kernel-3.10.0-693.21.1.el7.x86_64.debug.conf
kernel-debug /lib/modules/3.10.0-693.21.1.el7.x86_64.debug
> If you want to present the user with a different boot option, you can
> (just have the tunable set on the command line), but being tunable
> driven means that you don't have to choose that option, you could
> automatically enable it under a range of circumstances. I think most
> sane distributions would want that flexibility.
>
> Kconfig proliferation, conversely, is a bit of a nightmare from both
> the user and the tester's point of view, so we're trying to avoid it
> unless absolutely necessary.
>
> James
I already offered that we don't need to introduce a new kernel option and
we can bind this feature to any other kernel option, that is enabled in
the debug kernel, for example CONFIG_DEBUG_SG. Michal said no and he said
that he wants a new kernel option instead.
Mikulas
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: phy: Enable C45 PHYs with vendor specific address space
From: kbuild test robot @ 2018-04-25 22:46 UTC (permalink / raw)
To: Vicentiu Galanopulo
Cc: kbuild-all, andrew, robh, netdev, linux-kernel, mark.rutland,
davem, marcel, devicetree, alexandru.marginean, madalin.bucur,
vicentiu.galanopulo
In-Reply-To: <20180417090233.21548-4-vicentiu.galanopulo@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 726 bytes --]
Hi Vicentiu,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Vicentiu-Galanopulo/net-phy-Enable-C45-vendor-specific-MDIO-register-addr-space/20180418-014927
config: x86_64-randconfig-b0-04260315 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "get_vend_spec_addr_phy_device" [drivers/of/of_mdio.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25865 bytes --]
^ permalink raw reply
* Re: [dm-devel] [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: David Rientjes @ 2018-04-25 22:49 UTC (permalink / raw)
To: Mikulas Patocka
Cc: James Bottomley, dm-devel, eric.dumazet, mst, netdev, jasowang,
Randy Dunlap, linux-kernel, Matthew Wilcox, Michal Hocko,
linux-mm, edumazet, Andrew Morton, virtualization, David Miller,
Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251830540.25124@file01.intranet.prod.int.rdu2.redhat.com>
On Wed, 25 Apr 2018, Mikulas Patocka wrote:
> You need to enable it on boot. Enabling it when the kernel starts to
> execute userspace code is already too late (because you would miss
> kvmalloc calls in the kernel boot path).
>
Is your motivation that since kvmalloc() never falls back to vmalloc() on
boot because fragmentation is not be an issue at boot that we should catch
bugs where it would matter if it had fallen back? If we are worrying
about falling back to vmalloc before even initscripts have run I think we
have bigger problems.
^ permalink raw reply
* Re: [dm-devel] [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 22:56 UTC (permalink / raw)
To: David Rientjes
Cc: James Bottomley, dm-devel, eric.dumazet, mst, netdev, jasowang,
Randy Dunlap, linux-kernel, Matthew Wilcox, Michal Hocko,
linux-mm, edumazet, Andrew Morton, virtualization, David Miller,
Vlastimil Babka
In-Reply-To: <alpine.DEB.2.21.1804251546240.58229@chino.kir.corp.google.com>
On Wed, 25 Apr 2018, David Rientjes wrote:
> On Wed, 25 Apr 2018, Mikulas Patocka wrote:
>
> > You need to enable it on boot. Enabling it when the kernel starts to
> > execute userspace code is already too late (because you would miss
> > kvmalloc calls in the kernel boot path).
>
> Is your motivation that since kvmalloc() never falls back to vmalloc() on
> boot because fragmentation is not be an issue at boot that we should catch
> bugs where it would matter if it had fallen back? If we are worrying
> about falling back to vmalloc before even initscripts have run I think we
> have bigger problems.
The same driver can be compiled directly into the kernel or be loaded as a
module. If the user (or the person preparing distro kernel) compiles the
driver directly into the kernel, kvmalloc should be tested on that driver,
because a different user or distribution can compile that driver as a
module.
Mikulas
^ permalink raw reply
* Re: [PATCH v7 net-next 4/4] netvsc: refactor notifier/event handling code to use the failover framework
From: Siwei Liu @ 2018-04-25 22:57 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stephen Hemminger, Jiri Pirko, Sridhar Samudrala, David Miller,
Netdev, virtualization, virtio-dev, Brandeburg, Jesse,
Alexander Duyck, Jakub Kicinski, Jason Wang
In-Reply-To: <20180426011221-mutt-send-email-mst@kernel.org>
On Wed, Apr 25, 2018 at 3:22 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Apr 25, 2018 at 02:38:57PM -0700, Siwei Liu wrote:
>> On Mon, Apr 23, 2018 at 1:06 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Mon, Apr 23, 2018 at 12:44:39PM -0700, Siwei Liu wrote:
>> >> On Mon, Apr 23, 2018 at 10:56 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> >> > On Mon, Apr 23, 2018 at 10:44:40AM -0700, Stephen Hemminger wrote:
>> >> >> On Mon, 23 Apr 2018 20:24:56 +0300
>> >> >> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>> >> >>
>> >> >> > On Mon, Apr 23, 2018 at 10:04:06AM -0700, Stephen Hemminger wrote:
>> >> >> > > > >
>> >> >> > > > >I will NAK patches to change to common code for netvsc especially the
>> >> >> > > > >three device model. MS worked hard with distro vendors to support transparent
>> >> >> > > > >mode, ans we really can't have a new model; or do backport.
>> >> >> > > > >
>> >> >> > > > >Plus, DPDK is now dependent on existing model.
>> >> >> > > >
>> >> >> > > > Sorry, but nobody here cares about dpdk or other similar oddities.
>> >> >> > >
>> >> >> > > The network device model is a userspace API, and DPDK is a userspace application.
>> >> >> >
>> >> >> > It is userspace but are you sure dpdk is actually poking at netdevs?
>> >> >> > AFAIK it's normally banging device registers directly.
>> >> >> >
>> >> >> > > You can't go breaking userspace even if you don't like the application.
>> >> >> >
>> >> >> > Could you please explain how is the proposed patchset breaking
>> >> >> > userspace? Ignoring DPDK for now, I don't think it changes the userspace
>> >> >> > API at all.
>> >> >> >
>> >> >>
>> >> >> The DPDK has a device driver vdev_netvsc which scans the Linux network devices
>> >> >> to look for Linux netvsc device and the paired VF device and setup the
>> >> >> DPDK environment. This setup creates a DPDK failsafe (bondingish) instance
>> >> >> and sets up TAP support over the Linux netvsc device as well as the Mellanox
>> >> >> VF device.
>> >> >>
>> >> >> So it depends on existing 2 device model. You can't go to a 3 device model
>> >> >> or start hiding devices from userspace.
>> >> >
>> >> > Okay so how does the existing patch break that? IIUC does not go to
>> >> > a 3 device model since netvsc calls failover_register directly.
>> >> >
>> >> >> Also, I am working on associating netvsc and VF device based on serial number
>> >> >> rather than MAC address. The serial number is how Windows works now, and it makes
>> >> >> sense for Linux and Windows to use the same mechanism if possible.
>> >> >
>> >> > Maybe we should support same for virtio ...
>> >> > Which serial do you mean? From vpd?
>> >> >
>> >> > I guess you will want to keep supporting MAC for old hypervisors?
>> >> >
>> >> > It all seems like a reasonable thing to support in the generic core.
>> >>
>> >> That's the reason why I chose explicit identifier rather than rely on
>> >> MAC address to bind/pair a device. MAC address can change. Even if it
>> >> can't, malicious guest user can fake MAC address to skip binding.
>> >>
>> >> -Siwei
>> >
>> > Address should be sampled at device creation to prevent this
>> > kind of hack. Not that it buys the malicious user much:
>> > if you can poke at MAC addresses you probably already can
>> > break networking.
>>
>> I don't understand why poking at MAC address may potentially break
>> networking.
>
> Set a MAC address to match another device on the same LAN,
> packets will stop reaching that MAC.
What I meant was guest users may create a virtual link, say veth that
has exactly the same MAC address as that for the VF, which can easily
get around of the binding procedure. There's no explicit flag to
identify a VF or pass-through device AFAIK. And sometimes this happens
maybe due to user misconfiguring the link. This process should be
hardened to avoid from any potential configuration errors.
>
>> Unlike VF, passthrough PCI endpoint device has its freedom
>> to change the MAC address. Even on a VF setup it's not neccessarily
>> always safe to assume the VF's MAC address cannot or shouldn't be
>> changed. That depends on the specific need whether the host admin
>> wants to restrict guest from changing the MAC address, although in
>> most cases it's true.
>>
>> I understand we can use the perm_addr to distinguish. But as said,
>> this will pose limitation of flexible configuration where one can
>> assign VFs with identical MAC address at all while each VF belongs to
>> different PF and/or different subnet for e.g. load balancing.
>> And
>> furthermore, the QEMU device model never uses MAC address to be
>> interpreted as an identifier, which requires to be unique per VM
>> instance. Why we're introducing this inconsistency?
>>
>> -Siwei
>
> Because it addresses most of the issues and is simple. That's already
> much better than what we have now which is nothing unless guest
> configures things manually.
Did you see my QEMU patch for using BDF as the grouping identifier?
And there can be others like what you suggested, but the point is that
it's requried to support explicit grouping mechanism from day one,
before the backup property cast into stones. This is orthogonal to
device model being proposed, be it 1-netdev or not. Delaying it would
just mean support and compatibility burden, appearing more like a
design flaw rather than a feature to add later on.
>
> I think ideally the infrastructure should suppport flexible matching of
> NICs - netvsc is already reported to be moving to some kind of serial
> address.
>
As Stephen said, Hyper-V supports the serial UUID thing from day-one.
It's just the Linux netvsc guest driver itself does not leverage that
ID from the very beginging.
Regards,
-Siwei
>
>> >
>> >
>> >
>> >
>> >>
>> >> >
>> >> > --
>> >> > MST
^ 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