* Re: Help with the BPF verifier
From: Arnaldo Carvalho de Melo @ 2018-11-03 11:32 UTC (permalink / raw)
To: Edward Cree
Cc: Yonghong Song, Daniel Borkmann, Jiri Olsa, Martin Lau,
Alexei Starovoitov, Linux Networking Development Mailing List
In-Reply-To: <20181103112934.GH20495@kernel.org>
Em Sat, Nov 03, 2018 at 08:29:34AM -0300, Arnaldo Carvalho de Melo escreveu:
> PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
> Preprocessed source(s) and associated run script(s) are located at:
> clang-7: note: diagnostic msg: /tmp/augmented_raw_syscalls-7444d9.c
> clang-7: note: diagnostic msg: /tmp/augmented_raw_syscalls-7444d9.sh
> clang-7: note: diagnostic msg:
>
> ********************
> ERROR: unable to compile tools/perf/examples/bpf/augmented_raw_syscalls.c
> Hint: Check error message shown above.
> Hint: You can also pre-compile it into .o using:
> clang -target bpf -O2 -c tools/perf/examples/bpf/augmented_raw_syscalls.c
> with proper -I and -D options.
> event syntax error: 'tools/perf/examples/bpf/augmented_raw_syscalls.c'
> \___ Failed to load tools/perf/examples/bpf/augmented_raw_syscalls.c from source: Error when compiling BPF scriptlet
>
> (add -v to see detail)
> Run 'perf list' for a list of valid events
>
> Usage: perf trace [<options>] [<command>]
> or: perf trace [<options>] -- <command> [<options>]
> or: perf trace record [<options>] [<command>]
> or: perf trace record [<options>] -- <command> [<options>]
>
> -e, --event <event> event/syscall selector. use 'perf list' to list available events
> [root@seventh perf]#
>
> Trying with -O1...
-O1 doesn't get clang confused, its just the verifier that doesn't like
the result, i.e. we're back to that optimization, that isn't disabled
with -O1
llvm compiling command : /usr/local/bin/clang -D__KERNEL__ -D__NR_CPUS__=4 -DLINUX_VERSION_CODE=0x41300 -I/home/acme/lib/perf/include/bpf -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/7/include -I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated -I/home/acme/git/linux/include -I./include -I/home/acme/git/linux/arch/x86/include/uapi -I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi -I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h -Wno-unused-value -Wno-pointer-sign -working-directory /lib/modules/4.19.0-rc8-00014-gc0cff31be705/build -c /home/acme/git/perf/tools/perf/examples/bpf/augmented_raw_syscalls.c -target bpf -O1 -o -
libbpf: loading object 'tools/perf/examples/bpf/augmented_raw_syscalls.c' from buffer
libbpf: section(1) .strtab, size 168, link 0, flags 0, type=3
libbpf: skip section(1) .strtab
libbpf: section(2) .text, size 0, link 0, flags 6, type=1
libbpf: skip section(2) .text
libbpf: section(3) raw_syscalls:sys_enter, size 344, link 0, flags 6, type=1
libbpf: found program raw_syscalls:sys_enter
libbpf: section(4) .relraw_syscalls:sys_enter, size 16, link 10, flags 0, type=9
libbpf: section(5) raw_syscalls:sys_exit, size 16, link 0, flags 6, type=1
libbpf: found program raw_syscalls:sys_exit
libbpf: section(6) maps, size 56, link 0, flags 3, type=1
libbpf: section(7) license, size 4, link 0, flags 3, type=1
libbpf: license of tools/perf/examples/bpf/augmented_raw_syscalls.c is GPL
libbpf: section(8) version, size 4, link 0, flags 3, type=1
libbpf: kernel version of tools/perf/examples/bpf/augmented_raw_syscalls.c is 41300
libbpf: section(9) .llvm_addrsig, size 6, link 10, flags 80000000, type=1879002115
libbpf: skip section(9) .llvm_addrsig
libbpf: section(10) .symtab, size 240, link 1, flags 0, type=2
libbpf: maps in tools/perf/examples/bpf/augmented_raw_syscalls.c: 2 maps in 56 bytes
libbpf: map 0 is "__augmented_syscalls__"
libbpf: map 1 is "__bpf_stdout__"
libbpf: collecting relocating info for: 'raw_syscalls:sys_enter'
libbpf: relo for 4 value 28 name 124
libbpf: relocation: insn_idx=35
libbpf: relocation: find map 1 (__augmented_syscalls__) for insn 35
Added extra kernel map __entry_SYSCALL_64_trampoline fffffe0000006000-fffffe0000007000
Added extra kernel map __entry_SYSCALL_64_trampoline fffffe0000032000-fffffe0000033000
Added extra kernel map __entry_SYSCALL_64_trampoline fffffe000005e000-fffffe000005f000
Added extra kernel map __entry_SYSCALL_64_trampoline fffffe000008a000-fffffe000008b000
bpf: config program 'raw_syscalls:sys_enter'
bpf: config program 'raw_syscalls:sys_exit'
libbpf: create map __bpf_stdout__: fd=3
libbpf: create map __augmented_syscalls__: fd=4
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
0: (bf) r6 = r1
1: (bf) r1 = r10
2: (07) r1 += -328
3: (b7) r7 = 64
4: (b7) r2 = 64
5: (bf) r3 = r6
6: (85) call bpf_probe_read#4
7: (79) r1 = *(u64 *)(r10 -320)
8: (15) if r1 == 0x101 goto pc+4
R0=inv(id=0) R1=inv(id=0) R6=ctx(id=0,off=0,imm=0) R7=inv64 R10=fp0,call_-1
9: (55) if r1 != 0x2 goto pc+22
R0=inv(id=0) R1=inv2 R6=ctx(id=0,off=0,imm=0) R7=inv64 R10=fp0,call_-1
10: (bf) r1 = r6
11: (07) r1 += 16
12: (05) goto pc+2
15: (79) r3 = *(u64 *)(r1 +0)
dereference of modified ctx ptr R1 off=16 disallowed
libbpf: -- END LOG --
libbpf: failed to load program 'raw_syscalls:sys_enter'
libbpf: failed to load object 'tools/perf/examples/bpf/augmented_raw_syscalls.c'
bpf: load objects failed: err=-4007: (Kernel verifier blocks program loading)
event syntax error: 'tools/perf/examples/bpf/augmented_raw_syscalls.c'
\___ Kernel verifier blocks program loading
(add -v to see detail)
Run 'perf list' for a list of valid events
Usage: perf trace [<options>] [<command>]
or: perf trace [<options>] -- <command> [<options>]
or: perf trace record [<options>] [<command>]
or: perf trace record [<options>] -- <command> [<options>]
-e, --event <event> event/syscall selector. use 'perf list' to list available events
[root@seventh perf]#
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-03 12:01 UTC (permalink / raw)
To: Aaron Lu, Jesper Dangaard Brouer
Cc: Saeed Mahameed, eric.dumazet@gmail.com, netdev@vger.kernel.org,
Tariq Toukan, ilias.apalodimas@linaro.org, yoel@kviknet.dk,
mgorman@techsingularity.net
In-Reply-To: <c5135749-4bf8-47b4-dec4-326324ab6e1d@itcare.pl>
W dniu 03.11.2018 o 01:16, Paweł Staszewski pisze:
>
>
> W dniu 02.11.2018 o 20:02, Paweł Staszewski pisze:
>>
>>
>> W dniu 02.11.2018 o 15:20, Aaron Lu pisze:
>>> On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote:
>>>> On Fri, 2 Nov 2018 13:23:56 +0800
>>>> Aaron Lu <aaron.lu@intel.com> wrote:
>>>>
>>>>> On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
>>>>>> On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
>>>>>>> On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
>>>>>>> wrote:
>>>>>>> ... ...
>>>>>>>> Section copied out:
>>>>>>>>
>>>>>>>> mlx5e_poll_tx_cq
>>>>>>>> |
>>>>>>>> --16.34%--napi_consume_skb
>>>>>>>> |
>>>>>>>> |--12.65%--__free_pages_ok
>>>>>>>> | |
>>>>>>>> | --11.86%--free_one_page
>>>>>>>> | |
>>>>>>>> | |--10.10%
>>>>>>>> --queued_spin_lock_slowpath
>>>>>>>> | |
>>>>>>>> | --0.65%--_raw_spin_lock
>>>>>>> This callchain looks like it is freeing higher order pages than
>>>>>>> order
>>>>>>> 0:
>>>>>>> __free_pages_ok is only called for pages whose order are bigger
>>>>>>> than
>>>>>>> 0.
>>>>>> mlx5 rx uses only order 0 pages, so i don't know where these high
>>>>>> order
>>>>>> tx SKBs are coming from..
>>>>> Perhaps here:
>>>>> __netdev_alloc_skb(), __napi_alloc_skb(), __netdev_alloc_frag() and
>>>>> __napi_alloc_frag() will all call page_frag_alloc(), which will use
>>>>> __page_frag_cache_refill() to get an order 3 page if possible, or
>>>>> fall
>>>>> back to an order 0 page if order 3 page is not available.
>>>>>
>>>>> I'm not sure if your workload will use the above code path though.
>>>> TL;DR: this is order-0 pages (code-walk trough proof below)
>>>>
>>>> To Aaron, the network stack *can* call __free_pages_ok() with order-0
>>>> pages, via:
>>>>
>>>> static void skb_free_head(struct sk_buff *skb)
>>>> {
>>>> unsigned char *head = skb->head;
>>>>
>>>> if (skb->head_frag)
>>>> skb_free_frag(head);
>>>> else
>>>> kfree(head);
>>>> }
>>>>
>>>> static inline void skb_free_frag(void *addr)
>>>> {
>>>> page_frag_free(addr);
>>>> }
>>>>
>>>> /*
>>>> * Frees a page fragment allocated out of either a compound or
>>>> order 0 page.
>>>> */
>>>> void page_frag_free(void *addr)
>>>> {
>>>> struct page *page = virt_to_head_page(addr);
>>>>
>>>> if (unlikely(put_page_testzero(page)))
>>>> __free_pages_ok(page, compound_order(page));
>>>> }
>>>> EXPORT_SYMBOL(page_frag_free);
>>> I think here is a problem - order 0 pages are freed directly to buddy,
>>> bypassing per-cpu-pages. This might be the reason lock contention
>>> appeared on free path. Can someone apply below diff and see if lock
>>> contention is gone?
>> Will test it tonight
>>
> Patch applied
> perf report:
> https://ufile.io/sytfh
>
>
>
> But i need to wait also with more traffic currently cpu's are sleeping
before patch:
| | | |
|--13.55%--mlx5e_poll_tx_cq
| | |
| | |
| | |
| | --10.32%--napi_consume_skb
| | |
| | |
| | |
| | |--8.52%--__free_pages_ok
| | |
| | | |
| | |
| | | --7.67%--free_one_page
| | |
| | | |
| | |
| | | |--6.05%--queued_spin_lock_slowpath
| | |
| | | |
| | |
| | | --0.64%--_raw_spin_lock
| | |
| | |
| | |
| | |--0.77%--skb_release_data
| | |
| | |
| | |
| | --0.72%--page_frag_free
after patch:
| | | | |
|--3.75%--mlx5e_poll_tx_cq
| | | |
| | |
| | | |
| | --1.53%--napi_consume_skb
| | | |
| | |
| | | |
| | --0.54%--skb_release_data
| | | |
| |
| | | | |
--3.09%--mlx5e_post_rx_wqes
| | | |
| |
| | | | |
--1.21%--__page_pool_alloc_pages_slow
| | | |
| |
| | | | |
--1.16%--__alloc_pages_nodemask
| | | |
| |
| | | | |
--1.05%--get_page_from_freelist
currently waiting for more traffic also
>
>
>
>
>>
>>
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index e2ef1c17942f..65c0ae13215a 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
>>> {
>>> struct page *page = virt_to_head_page(addr);
>>> - if (unlikely(put_page_testzero(page)))
>>> - __free_pages_ok(page, compound_order(page));
>>> + if (unlikely(put_page_testzero(page))) {
>>> + unsigned int order = compound_order(page);
>>> +
>>> + if (order == 0)
>>> + free_unref_page(page);
>>> + else
>>> + __free_pages_ok(page, order);
>>> + }
>>> }
>>> EXPORT_SYMBOL(page_frag_free);
>>>> Notice for the mlx5 driver it support several RX-memory models, so it
>>>> can be hard to follow, but from the perf report output we can see that
>>>> is uses mlx5e_skb_from_cqe_linear, which use build_skb.
>>>>
>>>> --13.63%--mlx5e_skb_from_cqe_linear
>>>> |
>>>> --5.02%--build_skb
>>>> |
>>>> --1.85%--__build_skb
>>>> |
>>>> --1.00%--kmem_cache_alloc
>>>>
>>>> /* build_skb() is wrapper over __build_skb(), that specifically
>>>> * takes care of skb->head and skb->pfmemalloc
>>>> * This means that if @frag_size is not zero, then @data must be
>>>> backed
>>>> * by a page fragment, not kmalloc() or vmalloc()
>>>> */
>>>> struct sk_buff *build_skb(void *data, unsigned int frag_size)
>>>> {
>>>> struct sk_buff *skb = __build_skb(data, frag_size);
>>>>
>>>> if (skb && frag_size) {
>>>> skb->head_frag = 1;
>>>> if (page_is_pfmemalloc(virt_to_head_page(data)))
>>>> skb->pfmemalloc = 1;
>>>> }
>>>> return skb;
>>>> }
>>>> EXPORT_SYMBOL(build_skb);
>>>>
>>>> It still doesn't prove, that the @data is backed by by a order-0 page.
>>>> For the mlx5 driver is uses mlx5e_page_alloc_mapped ->
>>>> page_pool_dev_alloc_pages(), and I can see perf report using
>>>> __page_pool_alloc_pages_slow().
>>>>
>>>> The setup for page_pool in mlx5 uses order=0.
>>>>
>>>> /* Create a page_pool and register it with rxq */
>>>> pp_params.order = 0;
>>>> pp_params.flags = 0; /* No-internal DMA mapping in
>>>> page_pool */
>>>> pp_params.pool_size = pool_size;
>>>> pp_params.nid = cpu_to_node(c->cpu);
>>>> pp_params.dev = c->pdev;
>>>> pp_params.dma_dir = rq->buff.map_dir;
>>>>
>>>> /* page_pool can be used even when there is no rq->xdp_prog,
>>>> * given page_pool does not handle DMA mapping there is no
>>>> * required state to clear. And page_pool gracefully handle
>>>> * elevated refcnt.
>>>> */
>>>> rq->page_pool = page_pool_create(&pp_params);
>>>> if (IS_ERR(rq->page_pool)) {
>>>> err = PTR_ERR(rq->page_pool);
>>>> rq->page_pool = NULL;
>>>> goto err_free;
>>>> }
>>>> err = xdp_rxq_info_reg_mem_model(&rq->xdp_rxq,
>>>> MEM_TYPE_PAGE_POOL, rq->page_pool);
>>> Thanks for the detailed analysis, I'll need more time to understand the
>>> whole picture :-)
>>>
>>
>>
>
>
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Jesper Dangaard Brouer @ 2018-11-03 12:53 UTC (permalink / raw)
To: Aaron Lu
Cc: Saeed Mahameed, pstaszewski@itcare.pl, eric.dumazet@gmail.com,
netdev@vger.kernel.org, Tariq Toukan, ilias.apalodimas@linaro.org,
yoel@kviknet.dk, mgorman@techsingularity.net, brouer
In-Reply-To: <20181102142024.GA18343@intel.com>
On Fri, 2 Nov 2018 22:20:24 +0800 Aaron Lu <aaron.lu@intel.com> wrote:
> On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote:
> > On Fri, 2 Nov 2018 13:23:56 +0800
> > Aaron Lu <aaron.lu@intel.com> wrote:
> >
> > > On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
> > > > On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
> > > > > On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
> > > > > wrote:
> > > > > ... ...
> > > > > > Section copied out:
> > > > > >
> > > > > > mlx5e_poll_tx_cq
> > > > > > |
> > > > > > --16.34%--napi_consume_skb
> > > > > > |
> > > > > > |--12.65%--__free_pages_ok
> > > > > > | |
> > > > > > | --11.86%--free_one_page
> > > > > > | |
> > > > > > | |--10.10%
> > > > > > --queued_spin_lock_slowpath
> > > > > > | |
> > > > > > | --0.65%--_raw_spin_lock
> > > > >
> > > > > This callchain looks like it is freeing higher order pages than order
> > > > > 0:
> > > > > __free_pages_ok is only called for pages whose order are bigger than
> > > > > 0.
> > > >
> > > > mlx5 rx uses only order 0 pages, so i don't know where these high order
> > > > tx SKBs are coming from..
> > >
> > > Perhaps here:
> > > __netdev_alloc_skb(), __napi_alloc_skb(), __netdev_alloc_frag() and
> > > __napi_alloc_frag() will all call page_frag_alloc(), which will use
> > > __page_frag_cache_refill() to get an order 3 page if possible, or fall
> > > back to an order 0 page if order 3 page is not available.
> > >
> > > I'm not sure if your workload will use the above code path though.
> >
> > TL;DR: this is order-0 pages (code-walk trough proof below)
> >
> > To Aaron, the network stack *can* call __free_pages_ok() with order-0
> > pages, via:
> >
> > static void skb_free_head(struct sk_buff *skb)
> > {
> > unsigned char *head = skb->head;
> >
> > if (skb->head_frag)
> > skb_free_frag(head);
> > else
> > kfree(head);
> > }
> >
> > static inline void skb_free_frag(void *addr)
> > {
> > page_frag_free(addr);
> > }
> >
> > /*
> > * Frees a page fragment allocated out of either a compound or order 0 page.
> > */
> > void page_frag_free(void *addr)
> > {
> > struct page *page = virt_to_head_page(addr);
> >
> > if (unlikely(put_page_testzero(page)))
> > __free_pages_ok(page, compound_order(page));
> > }
> > EXPORT_SYMBOL(page_frag_free);
>
> I think here is a problem - order 0 pages are freed directly to buddy,
> bypassing per-cpu-pages. This might be the reason lock contention
> appeared on free path.
OMG - you just found a significant issue with the network stacks
interaction with the page allocator! This explains why I could not get
the PCP (Per-Cpu-Pages) system to have good performance, in my
performance networking benchmarks. As we are basically only using the
alloc side of PCP, and not the free side.
We have spend years adding different driver level recycle tricks to
avoid this code path getting activated, exactly because it is rather
slow and problematic that we hit this zone->lock.
> Can someone apply below diff and see if lock contention is gone?
I have also applied and tested this patch, and yes the lock contention
is gone. As mentioned is it rather difficult to hit this code path, as
the driver page recycle mechanism tries to hide/avoid it, but mlx5 +
page_pool + CPU-map recycling have a known weakness that bypass the
driver page recycle scheme (that I've not fixed yet). I observed a 7%
speedup for this micro benchmark.
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index e2ef1c17942f..65c0ae13215a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
> {
> struct page *page = virt_to_head_page(addr);
>
> - if (unlikely(put_page_testzero(page)))
> - __free_pages_ok(page, compound_order(page));
> + if (unlikely(put_page_testzero(page))) {
> + unsigned int order = compound_order(page);
> +
> + if (order == 0)
> + free_unref_page(page);
> + else
> + __free_pages_ok(page, order);
> + }
> }
> EXPORT_SYMBOL(page_frag_free);
Thank you Aaron for spotting this!!!
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Jesper Dangaard Brouer @ 2018-11-03 12:58 UTC (permalink / raw)
To: Paweł Staszewski
Cc: Aaron Lu, Saeed Mahameed, eric.dumazet@gmail.com,
netdev@vger.kernel.org, Tariq Toukan, ilias.apalodimas@linaro.org,
yoel@kviknet.dk, mgorman@techsingularity.net, brouer
In-Reply-To: <c5135749-4bf8-47b4-dec4-326324ab6e1d@itcare.pl>
On Sat, 3 Nov 2018 01:16:08 +0100
Paweł Staszewski <pstaszewski@itcare.pl> wrote:
> W dniu 02.11.2018 o 20:02, Paweł Staszewski pisze:
> >
> >
> > W dniu 02.11.2018 o 15:20, Aaron Lu pisze:
> >> On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote:
> >>> On Fri, 2 Nov 2018 13:23:56 +0800
> >>> Aaron Lu <aaron.lu@intel.com> wrote:
> >>>
> >>>> On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
> >>>>> On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
> >>>>>> On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
> >>>>>> wrote:
> >>>>>> ... ...
[...]
> >>> TL;DR: this is order-0 pages (code-walk trough proof below)
> >>>
> >>> To Aaron, the network stack *can* call __free_pages_ok() with order-0
> >>> pages, via:
[...]
> >>
> >> I think here is a problem - order 0 pages are freed directly to buddy,
> >> bypassing per-cpu-pages. This might be the reason lock contention
> >> appeared on free path. Can someone apply below diff and see if lock
> >> contention is gone?
> >>
> > Will test it tonight
> >
> Patch applied
> perf report:
> https://ufile.io/sytfh
>
>
> But i need to wait also with more traffic currently cpu's are sleeping
>
Well, that would be the expected result, that the CPUs get more time to
sleep, if the lock contention is gone...
What is the measured bandwidth now?
Notice, you might still be limited by the PCIe bandwidth, but then your
CPUs might actually decide to sleep, as they are getting data fast
enough.
[...]
> >>
> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >> index e2ef1c17942f..65c0ae13215a 100644
> >> --- a/mm/page_alloc.c
> >> +++ b/mm/page_alloc.c
> >> @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
> >> {
> >> struct page *page = virt_to_head_page(addr);
> >> - if (unlikely(put_page_testzero(page)))
> >> - __free_pages_ok(page, compound_order(page));
> >> + if (unlikely(put_page_testzero(page))) {
> >> + unsigned int order = compound_order(page);
> >> +
> >> + if (order == 0)
> >> + free_unref_page(page);
> >> + else
> >> + __free_pages_ok(page, order);
> >> + }
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Masami Hiramatsu @ 2018-11-03 13:00 UTC (permalink / raw)
To: Steven Rostedt
Cc: Josh Poimboeuf, Aleksa Sarai, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Masami Hiramatsu, Jonathan Corbet,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Alexander Shishkin, Jiri Olsa, Namhyung Kim, Shuah Khan,
Alexei Starovoitov, Daniel Borkmann, Brendan Gregg,
Christian Brauner, Aleksa Sarai, netd
In-Reply-To: <20181102121307.32e99414@gandalf.local.home>
On Fri, 2 Nov 2018 12:13:07 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 2 Nov 2018 10:43:26 -0500
> Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> > > I'll hopefully have a prototype ready by plumbers.
> >
> > Why do we need multiple users? It would be a lot simpler if we could
> > just enforce a single user per fgraphed/kretprobed function (and return
> > -EBUSY if it's already being traced/probed).
>
> Because that means if function graph tracer is active, then you can't
> do a kretprobe, and vice versa. I'd really like to have it working for
> multiple users, then we could trace different graph functions and store
> them in different buffers. It would also allow for perf to use function
> graph tracer too.
Steve, how woul you allow multiple users on it? Something like this?
ret_trampoline_multiple(){
list_for_each(handler, &shadow_entry[i].handlers, list)
handler(shadow_entry[i]);
restore_retval_and_jump_to(shadow_entry[i].orig);
}
> > > And this too will require each architecture to probably change. As a
> > > side project to this, I'm going to try to consolidate the function
> > > graph code among all the architectures as well. Not an easy task.
> >
> > Do you mean implementing HAVE_FUNCTION_GRAPH_RET_ADDR_PTR for all the
> > arches? If so, I think have an old crusty patch which attempted to
> > that. I could try to dig it up if you're interested.
> >
>
> I'd like to have that, but it still requires some work. But I'd just
> the truly architecture dependent code be in the architecture (basically
> the asm code), and have the ability to move most of the duplicate code
> out of the archs.
I will also do that for kretprobe handlers.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-03 15:23 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Aaron Lu, Saeed Mahameed, eric.dumazet@gmail.com,
netdev@vger.kernel.org, Tariq Toukan, ilias.apalodimas@linaro.org,
yoel@kviknet.dk, mgorman@techsingularity.net
In-Reply-To: <20181103135831.180628ab@redhat.com>
W dniu 03.11.2018 o 13:58, Jesper Dangaard Brouer pisze:
> On Sat, 3 Nov 2018 01:16:08 +0100
> Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>
>> W dniu 02.11.2018 o 20:02, Paweł Staszewski pisze:
>>>
>>> W dniu 02.11.2018 o 15:20, Aaron Lu pisze:
>>>> On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer wrote:
>>>>> On Fri, 2 Nov 2018 13:23:56 +0800
>>>>> Aaron Lu <aaron.lu@intel.com> wrote:
>>>>>
>>>>>> On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
>>>>>>> On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
>>>>>>>> On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
>>>>>>>> wrote:
>>>>>>>> ... ...
> [...]
>>>>> TL;DR: this is order-0 pages (code-walk trough proof below)
>>>>>
>>>>> To Aaron, the network stack *can* call __free_pages_ok() with order-0
>>>>> pages, via:
> [...]
>>>>
>>>> I think here is a problem - order 0 pages are freed directly to buddy,
>>>> bypassing per-cpu-pages. This might be the reason lock contention
>>>> appeared on free path. Can someone apply below diff and see if lock
>>>> contention is gone?
>>>>
>>> Will test it tonight
>>>
>> Patch applied
>> perf report:
>> https://ufile.io/sytfh
>>
>>
>> But i need to wait also with more traffic currently cpu's are sleeping
>>
> Well, that would be the expected result, that the CPUs get more time to
> sleep, if the lock contention is gone...
>
> What is the measured bandwidth now?
30 RX /30 TX Gbit/s
>
> Notice, you might still be limited by the PCIe bandwidth, but then your
> CPUs might actually decide to sleep, as they are getting data fast
> enough.
Yes - i will replace network controller to two separate nic's in two
separate x16 pcie
But after monday.
But i dont think i hit pcie limit there - it looks like pcie x16 gen3
have 16GB/s RX and 16GB/s TX so bidirectional
>
> [...]
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index e2ef1c17942f..65c0ae13215a 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
>>>> {
>>>> struct page *page = virt_to_head_page(addr);
>>>> - if (unlikely(put_page_testzero(page)))
>>>> - __free_pages_ok(page, compound_order(page));
>>>> + if (unlikely(put_page_testzero(page))) {
>>>> + unsigned int order = compound_order(page);
>>>> +
>>>> + if (order == 0)
>>>> + free_unref_page(page);
>>>> + else
>>>> + __free_pages_ok(page, order);
>>>> + }
>
>
^ permalink raw reply
* Re: [PATCH] tcp: do not update snd_una if it is same with ack_seq
From: Eric Dumazet @ 2018-11-04 0:40 UTC (permalink / raw)
To: Yafang Shao, davem, edumazet; +Cc: netdev, linux-kernel
In-Reply-To: <1541264071-9905-1-git-send-email-laoar.shao@gmail.com>
On 11/03/2018 09:54 AM, Yafang Shao wrote:
> In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una,
> and under this condition we don't need to update the snd_una.
>
> Furthermore, tcp_ack_update_window() is only called in slow path,
> so introducing this check won't affect the fast path processing.
>
> By the way, '&' is a little faster than '-', so I replaced after() with
> "flag & FLAG_SND_UNA_ADVANCED".
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> net/ipv4/tcp_input.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 2868ef2..db5a6b7 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3376,7 +3376,8 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32
> }
> }
>
> - tcp_snd_una_update(tp, ack);
> + if (after(ack, tp->snd_una))
> + tcp_snd_una_update(tp, ack);
>
Adding this after() here is confusing, how ack could be before snd_una ?
That would be a serious bug.
I do not see why another conditional has any gain here.
You are trying to avoid very cheap operations :
u32 delta = ack - tp->snd_una;
tp->bytes_acked += delta;
tp->snd_una = ack;
Maybe the real reason for this patch is not explained in the changelog ?
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-03 15:43 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Aaron Lu, Saeed Mahameed, eric.dumazet@gmail.com,
netdev@vger.kernel.org, Tariq Toukan, ilias.apalodimas@linaro.org,
yoel@kviknet.dk, mgorman@techsingularity.net
In-Reply-To: <9f35c3f5-e865-54db-73bb-960ded60c1cc@itcare.pl>
W dniu 03.11.2018 o 16:23, Paweł Staszewski pisze:
>
>
> W dniu 03.11.2018 o 13:58, Jesper Dangaard Brouer pisze:
>> On Sat, 3 Nov 2018 01:16:08 +0100
>> Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>>
>>> W dniu 02.11.2018 o 20:02, Paweł Staszewski pisze:
>>>>
>>>> W dniu 02.11.2018 o 15:20, Aaron Lu pisze:
>>>>> On Fri, Nov 02, 2018 at 12:40:37PM +0100, Jesper Dangaard Brouer
>>>>> wrote:
>>>>>> On Fri, 2 Nov 2018 13:23:56 +0800
>>>>>> Aaron Lu <aaron.lu@intel.com> wrote:
>>>>>>> On Thu, Nov 01, 2018 at 08:23:19PM +0000, Saeed Mahameed wrote:
>>>>>>>> On Thu, 2018-11-01 at 23:27 +0800, Aaron Lu wrote:
>>>>>>>>> On Thu, Nov 01, 2018 at 10:22:13AM +0100, Jesper Dangaard Brouer
>>>>>>>>> wrote:
>>>>>>>>> ... ...
>> [...]
>>>>>> TL;DR: this is order-0 pages (code-walk trough proof below)
>>>>>>
>>>>>> To Aaron, the network stack *can* call __free_pages_ok() with
>>>>>> order-0
>>>>>> pages, via:
>> [...]
>>>>> I think here is a problem - order 0 pages are freed directly to
>>>>> buddy,
>>>>> bypassing per-cpu-pages. This might be the reason lock contention
>>>>> appeared on free path. Can someone apply below diff and see if lock
>>>>> contention is gone?
>>>>>
>>>> Will test it tonight
>>> Patch applied
>>> perf report:
>>> https://ufile.io/sytfh
>>>
>>>
>>> But i need to wait also with more traffic currently cpu's are sleeping
>>>
>> Well, that would be the expected result, that the CPUs get more time to
>> sleep, if the lock contention is gone...
>>
>> What is the measured bandwidth now?
> 30 RX /30 TX Gbit/s
>
>>
>> Notice, you might still be limited by the PCIe bandwidth, but then your
>> CPUs might actually decide to sleep, as they are getting data fast
>> enough.
> Yes - i will replace network controller to two separate nic's in two
> separate x16 pcie
> But after monday.
>
> But i dont think i hit pcie limit there - it looks like pcie x16 gen3
> have 16GB/s RX and 16GB/s TX so bidirectional
>
Was thinking that maybee memory limit - but also there is 4 channel DDR4
2666MHz - so total bandwidth for memory is bigger (48GB/s) than needed
for 100Gbit ethernet
>
>>
>> [...]
>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>>> index e2ef1c17942f..65c0ae13215a 100644
>>>>> --- a/mm/page_alloc.c
>>>>> +++ b/mm/page_alloc.c
>>>>> @@ -4554,8 +4554,14 @@ void page_frag_free(void *addr)
>>>>> {
>>>>> struct page *page = virt_to_head_page(addr);
>>>>> - if (unlikely(put_page_testzero(page)))
>>>>> - __free_pages_ok(page, compound_order(page));
>>>>> + if (unlikely(put_page_testzero(page))) {
>>>>> + unsigned int order = compound_order(page);
>>>>> +
>>>>> + if (order == 0)
>>>>> + free_unref_page(page);
>>>>> + else
>>>>> + __free_pages_ok(page, order);
>>>>> + }
>>
>>
>
>
^ permalink raw reply
* SACK compression patch causing performance drop
From: Jean-Louis Dupond @ 2018-11-03 15:59 UTC (permalink / raw)
To: netdev, edumazet
Hi All,
On recent kernels we noticed a way lower throughput to our SAN system
than before.
While on pre 4.18 kernels we had 400-700MB/sec read speed, on 4.18+ we
only had 70-120MB/sec.
The SAN is connected via iSCSI over a 10G network (ixgbe/X520 NICS if it
matters).
After some debugging, I tried to bisect between 4.17 and 4.18 to see
what commit caused the slowdown.
It showed that the addition of the SACK compression
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5d9f4262b7ea41ca9981cc790e37cca6e37c789e)
was the cause.
And indeed, if I set net.ipv4.tcp_comp_sack_nr to 0 on 4.19 for example,
the throughput is (almost) back to normal again.
So it seems like this change causes quite some performance issues.
Any ideas?
Thanks
Jean-Louis
^ permalink raw reply
* Re: [PATCH] tcp: do not update snd_una if it is same with ack_seq
From: Yafang Shao @ 2018-11-04 1:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Eric Dumazet, netdev, LKML
In-Reply-To: <5f585304-b6f5-5f49-adcf-eaed471c0d76@gmail.com>
On Sun, Nov 4, 2018 at 8:40 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 11/03/2018 09:54 AM, Yafang Shao wrote:
> > In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una,
> > and under this condition we don't need to update the snd_una.
> >
> > Furthermore, tcp_ack_update_window() is only called in slow path,
> > so introducing this check won't affect the fast path processing.
> >
> > By the way, '&' is a little faster than '-', so I replaced after() with
> > "flag & FLAG_SND_UNA_ADVANCED".
> >
> > Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> > ---
> > net/ipv4/tcp_input.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 2868ef2..db5a6b7 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -3376,7 +3376,8 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32
> > }
> > }
> >
> > - tcp_snd_una_update(tp, ack);
> > + if (after(ack, tp->snd_una))
> > + tcp_snd_una_update(tp, ack);
> >
>
> Adding this after() here is confusing, how ack could be before snd_una ?
> That would be a serious bug.
>
ack can't be before snd_una, but it can be equal with snd_una.
Seems bellow change would be more specific,
if (ack != tp->snd_una)
tcp_snd_una_update(tp, ack);
> I do not see why another conditional has any gain here.
>
> You are trying to avoid very cheap operations :
>
> u32 delta = ack - tp->snd_una;
>
> tp->bytes_acked += delta;
> tp->snd_una = ack;
>
> Maybe the real reason for this patch is not explained in the changelog ?
No additional reason. I just want to avoid these uneccessary operations.
Because I find that this uncessary operations always happen,
especially when head prediction fails and then the packet can't go to
fast path processing.
Thanks
Yafang
^ permalink raw reply
* Re: [PATCH] tcp: do not update snd_una if it is same with ack_seq
From: Yafang Shao @ 2018-11-04 1:27 UTC (permalink / raw)
To: joe; +Cc: David Miller, Eric Dumazet, netdev, LKML
In-Reply-To: <945a90d62aa45c4b53349ba0a104574759d40efe.camel@perches.com>
On Sun, Nov 4, 2018 at 1:04 AM Joe Perches <joe@perches.com> wrote:
>
> On Sun, 2018-11-04 at 00:54 +0800, Yafang Shao wrote:
> > In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una,
> > and under this condition we don't need to update the snd_una.
> >
> > Furthermore, tcp_ack_update_window() is only called in slow path,
> > so introducing this check won't affect the fast path processing.
> []
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> []
> > @@ -3610,7 +3611,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> > if (flag & FLAG_UPDATE_TS_RECENT)
> > tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
> >
> > - if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
> > + if (!(flag & FLAG_SLOWPATH) && flag & FLAG_SND_UNA_ADVANCED) {
>
> stylistic nit:
>
> While the precedence is correct in any case,
> perhaps adding parentheses around
> flag & FLAG_SND_UNA_ADVANCED
> would make it more obvious.
>
Sure. will change it.
Thanks
Yafang
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Masami Hiramatsu @ 2018-11-03 16:34 UTC (permalink / raw)
To: Steven Rostedt
Cc: Josh Poimboeuf, Aleksa Sarai, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jonathan Corbet, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
Brendan Gregg, Christian Brauner, Aleksa Sarai, netdev, linux-doc
In-Reply-To: <20181103091341.3d32683e@vmware.local.home>
On Sat, 3 Nov 2018 09:13:41 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 3 Nov 2018 22:00:12 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > On Fri, 2 Nov 2018 12:13:07 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> >
>
> > > Because that means if function graph tracer is active, then you can't
> > > do a kretprobe, and vice versa. I'd really like to have it working for
> > > multiple users, then we could trace different graph functions and store
> > > them in different buffers. It would also allow for perf to use function
> > > graph tracer too.
> >
> > Steve, how woul you allow multiple users on it? Something like this?
> >
> > ret_trampoline_multiple(){
> > list_for_each(handler, &shadow_entry[i].handlers, list)
> > handler(shadow_entry[i]);
> > restore_retval_and_jump_to(shadow_entry[i].orig);
> > }
> >
>
> Something like that. But since it's not a single mapping between shadow
> entries and handlers, that is we have multiple tasks with multiple
> shadow entries and multiple handlers, we can't use a link list (two
> different parents per handler).
Yes, I understand it.
> I was thinking of a bitmask that represents the handlers, and use that
> to map which handler gets called for which shadow entry for a
> particular task.
Hmm, I doubt that is too complicated and not scalable. I rather like to see
the open shadow entry...
entry: [[original_retaddr][function][modified_retaddr]]
So if there are many users on same function, the entries will be like this
[[original_return_address][function][trampoline_A]]
[[trampline_A][function][trampoline_B]]
[[trampline_B][function][trampoline_C]]
And on the top of the stack, there is trampline_C instead of original_return_address.
In this case, return to trampoline_C(), it jumps back to trampline_B() and then
it jumps back to trampline_A(). And eventually it jumps back to
original_return_address.
This way, we don't need allocate another bitmap/pages for the shadow stack.
We only need a shadow stack for each task.
Also, unwinder can easily find the trampline_C from the shadow stack and restores
original_return_address. (of course trampline_A,B,C must be registered so that
search function can skip it.)
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH] tcp: do not update snd_una if it is same with ack_seq
From: Yafang Shao @ 2018-11-03 16:54 UTC (permalink / raw)
To: davem, edumazet; +Cc: netdev, linux-kernel, Yafang Shao
In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una,
and under this condition we don't need to update the snd_una.
Furthermore, tcp_ack_update_window() is only called in slow path,
so introducing this check won't affect the fast path processing.
By the way, '&' is a little faster than '-', so I replaced after() with
"flag & FLAG_SND_UNA_ADVANCED".
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
net/ipv4/tcp_input.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2868ef2..db5a6b7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3376,7 +3376,8 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32
}
}
- tcp_snd_una_update(tp, ack);
+ if (after(ack, tp->snd_una))
+ tcp_snd_una_update(tp, ack);
return flag;
}
@@ -3610,7 +3611,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (flag & FLAG_UPDATE_TS_RECENT)
tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
- if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
+ if (!(flag & FLAG_SLOWPATH) && flag & FLAG_SND_UNA_ADVANCED) {
/* Window is constant, pure forward advance.
* No more checks are required.
* Note, we use the fact that SND.UNA>=SND.WL2.
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] tcp: do not update snd_una if it is same with ack_seq
From: Joe Perches @ 2018-11-03 17:04 UTC (permalink / raw)
To: Yafang Shao, davem, edumazet; +Cc: netdev, linux-kernel
In-Reply-To: <1541264071-9905-1-git-send-email-laoar.shao@gmail.com>
On Sun, 2018-11-04 at 00:54 +0800, Yafang Shao wrote:
> In the slow path, TCP_SKB_SB(skb)->ack_seq may be same with tp->snd_una,
> and under this condition we don't need to update the snd_una.
>
> Furthermore, tcp_ack_update_window() is only called in slow path,
> so introducing this check won't affect the fast path processing.
[]
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
[]
> @@ -3610,7 +3611,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
> if (flag & FLAG_UPDATE_TS_RECENT)
> tcp_replace_ts_recent(tp, TCP_SKB_CB(skb)->seq);
>
> - if (!(flag & FLAG_SLOWPATH) && after(ack, prior_snd_una)) {
> + if (!(flag & FLAG_SLOWPATH) && flag & FLAG_SND_UNA_ADVANCED) {
stylistic nit:
While the precedence is correct in any case,
perhaps adding parentheses around
flag & FLAG_SND_UNA_ADVANCED
would make it more obvious.
^ permalink raw reply
* Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
From: Masami Hiramatsu @ 2018-11-04 2:25 UTC (permalink / raw)
To: Steven Rostedt
Cc: Josh Poimboeuf, Aleksa Sarai, Naveen N. Rao, Anil S Keshavamurthy,
David S. Miller, Jonathan Corbet, Peter Zijlstra, Ingo Molnar,
Arnaldo Carvalho de Melo, Alexander Shishkin, Jiri Olsa,
Namhyung Kim, Shuah Khan, Alexei Starovoitov, Daniel Borkmann,
Brendan Gregg, Christian Brauner, Aleksa Sarai, netdev, linux-doc
In-Reply-To: <20181103133021.6676708c@vmware.local.home>
On Sat, 3 Nov 2018 13:30:21 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 4 Nov 2018 01:34:30 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > > I was thinking of a bitmask that represents the handlers, and use that
> > > to map which handler gets called for which shadow entry for a
> > > particular task.
> >
> > Hmm, I doubt that is too complicated and not scalable. I rather like to see
> > the open shadow entry...
>
> It can scale and not too complex (I already played a little with it).
> But that said, I'm not committed to it, and using the shadow stack is
> also an interesting idea.
>
> >
> > entry: [[original_retaddr][function][modified_retaddr]]
> >
> > So if there are many users on same function, the entries will be like this
> >
> > [[original_return_address][function][trampoline_A]]
> > [[trampline_A][function][trampoline_B]]
> > [[trampline_B][function][trampoline_C]]
> >
> > And on the top of the stack, there is trampline_C instead of original_return_address.
> > In this case, return to trampoline_C(), it jumps back to trampline_B() and then
> > it jumps back to trampline_A(). And eventually it jumps back to
> > original_return_address.
>
> Where are trampolines A, B, and C made? Do we also need to dynamically
> create them? If I register multiple function tracing ones, each one
> will need its own trampoline?
>
No, I think tramplines are very limited. currently we will only have ftrace
and kretprobe trampolines.
> > This way, we don't need allocate another bitmap/pages for the shadow stack.
> > We only need a shadow stack for each task.
> > Also, unwinder can easily find the trampline_C from the shadow stack and restores
> > original_return_address. (of course trampline_A,B,C must be registered so that
> > search function can skip it.)
>
> What I was thinking was to store a count and the functions to be called:
>
>
> [original_return_address]
> [function_A]
> [function_B]
> [function_C]
> [ 3 ]
>
> Then the trampoline that processes the return codes for ftrace (and
> kretprobes and everyone else) can simply do:
>
> count = pop_shadow_stack();
> for (i = 0; i < count; i++) {
> func = pop_shadow_stack();
> func(...);
> }
> return_address = pop_shadow_stack();
Ah, that's a good idea. I think we also have to store the called function
entry address with the number header, but basically I agree with you.
If we have a space to store a data with the function address, that is also
good to kretprobe. systemtap heavily uses "entry data" for saving some data
at function entry for exit handler.
> That way we only need to register a function to the return handler and
> it will be called, without worrying about making trampolines. There
> will just be a single trampoline that handles all the work.
OK, and could you make it independent from func graph tracer, so that
CONFIG_KPROBES=y but CONFIG_FUNCTION_GRAPH_TRACER=n kernel can support
kretprobes too.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH] openvswitch: fix linking without CONFIG_NF_CONNTRACK_LABELS
From: David Miller @ 2018-11-04 2:29 UTC (permalink / raw)
To: arnd-r2nGTMty4D4
Cc: dev-yBygre7rU0TnMu66kgdUjQ,
eswierk-FilZDy9cOaHkQYj/0HfcvtBPR1lH4CV8,
netdev-u79uwXL29TY76Z2rM5mHXA, fw-HFFVJYpyMKqzQB+pC5nmwQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Julia.Lawall-L2FTfq7BK8M,
gfree.wind-GveOkT2v/s5BDgjK7y7TUQ, thierry-EDrxZYgYwg/bB1ukUYwELw,
fbl-H+wXaHxf7aLQT0dZR+AlfA, pablo-Cap9r6Oaw4JrovVCs/uTlw
In-Reply-To: <20181102153705.1664786-1-arnd-r2nGTMty4D4@public.gmane.org>
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Date: Fri, 2 Nov 2018 16:36:55 +0100
> When CONFIG_CC_OPTIMIZE_FOR_DEBUGGING is enabled, the compiler
> fails to optimize out a dead code path, which leads to a link failure:
>
> net/openvswitch/conntrack.o: In function `ovs_ct_set_labels':
> conntrack.c:(.text+0x2e60): undefined reference to `nf_connlabels_replace'
>
> In this configuration, we can take a shortcut, and completely
> remove the contrack label code. This may also help the regular
> optimization.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Applied, thanks Arnd.
^ permalink raw reply
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: David Ahern @ 2018-11-03 17:32 UTC (permalink / raw)
To: Paweł Staszewski, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <a63df254-3791-cd5c-ddfc-6c9b8d38c559@itcare.pl>
On 11/1/18 11:30 AM, Paweł Staszewski wrote:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/bpf/xdp_fwd_kern.c
>>>>
>>>>
>>> I can try some tests on same hw but testlab configuration - will give it
>>> a try :)
>>>
>> That version does not work with VLANs. I have patches for it but it
>> needs a bit more work before sending out. Perhaps I can get back to it
>> next week.
>>
> Will be nice - next week i will be able to replace network controller
> and install separate two 100Gbit nics into two pciex x16 slots - so can
> test without hitting pcie bandwidth limits.
>
>
Does your setup have any other device types besides physical ports with
VLANs (e.g., any macvlans or bonds)?
^ permalink raw reply
* Re: [PATCH net 0/6] s390/qeth: fixes 2018-11-02
From: David Miller @ 2018-11-03 17:44 UTC (permalink / raw)
To: jwi; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20181102180413.53020-1-jwi@linux.ibm.com>
From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Fri, 2 Nov 2018 19:04:07 +0100
> please apply one round of qeth fixes for -net.
>
> Patch 1 is rather large and removes a use-after-free hazard from many of our
> debug trace entries.
> Patch 2 is yet another fix-up for the L3 subdriver's new IP address management
> code.
> Patch 3 and 4 resolve some fallout from the recent changes wrt how/when qeth
> allocates its net_device.
> Patch 5 makes sure we don't set reserved bits when building HW commands from
> user-provided data.
> And finally, patch 6 allows ethtool to play nice with new HW.
Series applied.
^ permalink raw reply
* Re: [PATCH v2] libertas: return errno from lbs_add_card()
From: Pavel Machek @ 2018-11-03 19:43 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Kalle Valo, David S. Miller, libertas-dev, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20181007003327.2806010-1-lkundrak@v3.sk>
[-- Attachment #1: Type: text/plain, Size: 431 bytes --]
On Sun 2018-10-07 02:33:27, Lubomir Rintel wrote:
> This makes the error handling somewhat cleaner -- lbs_add_card() does no
> logner throw away the errno and lets its callers propagate it.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Acked-by: Pavel Machek <pavel@ucw.cz>
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH v2 net 0/3] net: bql: better deal with GSO
From: David Miller @ 2018-11-03 22:40 UTC (permalink / raw)
To: edumazet; +Cc: netdev, tariqt, willemb, eric.dumazet
In-Reply-To: <20181031153914.132127-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 31 Oct 2018 08:39:11 -0700
> While BQL bulk dequeue works well for TSO packets, it is
> not very efficient as soon as GSO is involved.
>
> On a GSO only workload (UDP or TCP), this patch series
> can save about 8 % of cpu cycles on a 40Gbit mlx4 NIC,
> by keeping optimal batching, and avoiding expensive
> doorbells, qdisc requeues and reschedules.
>
> This patch series :
>
> - Add __netdev_tx_sent_queue() so that drivers
> can implement efficient BQL and xmit_more support.
>
> - Implement a work around in dev_hard_start_xmit()
> for drivers not using __netdev_tx_sent_queue()
>
> - changes mlx4 to use __netdev_tx_sent_queue()
>
> v2: Tariq and Willem feedback addressed.
> added __netdev_tx_sent_queue() (Willem suggestion)
Series applied, but I wonder how many other commonly used drivers we
should update the same way mlx4 is here?
^ permalink raw reply
* Re: [PATCH net] net: hns3: Fix for out-of-bounds access when setting pfc back pressure
From: David Miller @ 2018-11-03 22:42 UTC (permalink / raw)
To: linyunsheng
Cc: huangdaode, xuwei5, liguozhu, Yisen.Zhuang, john.garry, linuxarm,
salil.mehta, lipeng321, netdev, linux-kernel
In-Reply-To: <1541074363-98630-1-git-send-email-linyunsheng@huawei.com>
From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Thu, 1 Nov 2018 20:12:43 +0800
> The vport should be initialized to hdev->vport for each bp group,
> otherwise it will cause out-of-bounds access and bp setting not
> correct problem.
...
> Fixes: 67bf2541f4b9 ("net: hns3: Fixes the back pressure setting when sriov is enabled")
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 net 0/3] net: bql: better deal with GSO
From: Eric Dumazet @ 2018-11-03 23:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Tariq Toukan, Willem de Bruijn, Eric Dumazet
In-Reply-To: <20181103.154028.1864906755289829872.davem@davemloft.net>
On Sat, Nov 3, 2018 at 3:40 PM David Miller <davem@davemloft.net> wrote:
> Series applied, but I wonder how many other commonly used drivers we
> should update the same way mlx4 is here?
I can provide patches but can not test them, so probably we should get
Tested-by tags before applying them.
Thanks.
^ permalink raw reply
* [PATCH] e1000e: Change watchdog task to be delayed work
From: Robert Eshleman @ 2018-11-04 0:17 UTC (permalink / raw)
Cc: Robert Eshleman, Jeff Kirsher, David S. Miller, intel-wired-lan,
netdev, linux-kernel
This completes a pending TODO to use queue_delayed_work() instead of
schedule_work().
Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 16a73bd..a387b21 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -39,6 +39,8 @@ static int debug = -1;
module_param(debug, int, 0);
MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
+struct workqueue_struct *e1000e_workqueue;
+
static const struct e1000_info *e1000_info_tbl[] = {
[board_82571] = &e1000_82571_info,
[board_82572] = &e1000_82572_info,
@@ -5137,9 +5139,9 @@ static void e1000_watchdog(struct timer_list *t)
struct e1000_adapter *adapter = from_timer(adapter, t, watchdog_timer);
/* Do the rest outside of interrupt context */
- schedule_work(&adapter->watchdog_task);
+ struct delayed_work *dwork = to_delayed_work(&adapter->watchdog_task);
- /* TODO: make this use queue_delayed_work() */
+ queue_delayed_work(e1000e_workqueue, dwork, 1);
}
static void e1000_watchdog_task(struct work_struct *work)
@@ -7572,6 +7574,13 @@ static int __init e1000_init_module(void)
e1000e_driver_version);
pr_info("Copyright(c) 1999 - 2015 Intel Corporation.\n");
+ e1000e_workqueue = alloc_workqueue("%s", WQ_MEM_RECLAIM, 0,
+ e1000e_driver_name);
+ if (!e1000e_workqueue) {
+ pr_err("%s: Failed to create workqueue\n", e1000e_driver_name);
+ return -ENOMEM;
+ }
+
return pci_register_driver(&e1000_driver);
}
module_init(e1000_init_module);
@@ -7585,6 +7594,7 @@ module_init(e1000_init_module);
static void __exit e1000_exit_module(void)
{
pci_unregister_driver(&e1000_driver);
+ destroy_workqueue(e1000e_workqueue);
}
module_exit(e1000_exit_module);
--
2.7.4
^ permalink raw reply related
* Re: Kernel 4.19 network performance - forwarding/routing normal users traffic
From: Paweł Staszewski @ 2018-11-04 0:24 UTC (permalink / raw)
To: David Ahern, Jesper Dangaard Brouer; +Cc: netdev, Yoel Caspersen
In-Reply-To: <f6188534-38d7-fc3e-189d-443a76bcb47d@gmail.com>
n
W dniu 03.11.2018 o 18:32, David Ahern pisze:
> On 11/1/18 11:30 AM, Paweł Staszewski wrote:
>>>>>
>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/samples/bpf/xdp_fwd_kern.c
>>>>>
>>>>>
>>>> I can try some tests on same hw but testlab configuration - will give it
>>>> a try :)
>>>>
>>> That version does not work with VLANs. I have patches for it but it
>>> needs a bit more work before sending out. Perhaps I can get back to it
>>> next week.
>>>
>> Will be nice - next week i will be able to replace network controller
>> and install separate two 100Gbit nics into two pciex x16 slots - so can
>> test without hitting pcie bandwidth limits.
>>
>>
> Does your setup have any other device types besides physical ports with
> VLANs (e.g., any macvlans or bonds)?
>
>
no.
just
phy(mlnx)->vlans only config
And today again after allpy patch for page allocator - reached again
64/64 Gbit/s
with only 50-60% cpu load
today no slowpath hit for netwoking :)
But again dropped pckt at 64GbitRX and 64TX ....
And as it should not be pcie express limit -i think something more is
going on there - and hard to catch - cause perf top doestn chenged
besides there is no queued slowpath hit now
I ordered now also intel cards to compare - but 3 weeks eta
Faster - cause 3 days - i will have mellanox connectx 5 - so can
separate traffic to two different x16 pcie busses
^ permalink raw reply
* Re: [PATCH v2 net 0/3] net: bql: better deal with GSO
From: David Miller @ 2018-11-04 0:26 UTC (permalink / raw)
To: edumazet; +Cc: netdev, tariqt, willemb, eric.dumazet
In-Reply-To: <CANn89iLJcc4o7ftTL3Sb0GEd3tYvexKn-tHyDWzqEo2Pk1ECRA@mail.gmail.com>
From: Eric Dumazet <edumazet@google.com>
Date: Sat, 3 Nov 2018 16:00:39 -0700
> On Sat, Nov 3, 2018 at 3:40 PM David Miller <davem@davemloft.net> wrote:
>
>> Series applied, but I wonder how many other commonly used drivers we
>> should update the same way mlx4 is here?
>
> I can provide patches but can not test them, so probably we should get
> Tested-by tags before applying them.
Ok, agreed.
^ 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