* Re: [PATCH v3 net-next 1/4] net: dsa: microchip: prepare PHY for proper advertisement
From: Andrew Lunn @ 2019-02-21 22:15 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-2-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 21, 2019 at 02:03:14PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Prepare PHY for proper advertisement as sometimes the PHY in the switch
> has its own problems even though it may share the PHY id from regular PHY
> but the fixes in the PHY driver do not apply.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v3 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Andrew Lunn @ 2019-02-21 22:19 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-3-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 21, 2019 at 02:03:15PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add background MIB counter reading support.
>
> Port MIB counters should only be read when there is link. Otherwise it is
> a waste of time as hardware never increases those counters. There are
> exceptions as some switches keep track of dropped counts no matter what.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v3 net-next 3/4] net: dsa: microchip: get port link status
From: Andrew Lunn @ 2019-02-21 22:20 UTC (permalink / raw)
To: Tristram.Ha
Cc: Sergio Paracuellos, Florian Fainelli, Pavel Machek,
UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-4-git-send-email-Tristram.Ha@microchip.com>
On Thu, Feb 21, 2019 at 02:03:16PM -0800, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Get port link status to know whether to read MIB counters when the link
> is going down. Add port_cleanup function to read MIB counters the last
> time as when the port is disabled the PHY is also powered down.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* [PATCH] x86, retpolines: raise limit for generating indirect calls from switch-case
From: Daniel Borkmann @ 2019-02-21 22:19 UTC (permalink / raw)
To: tglx
Cc: linux-kernel, netdev, Daniel Borkmann, David S . Miller,
Björn Töpel, Magnus Karlsson, Jesper Dangaard Brouer,
Alexei Starovoitov, Peter Zijlstra, David Woodhouse,
Andy Lutomirski, Borislav Petkov, Linus Torvalds
From networking side, there are numerous attempts to get rid of
indirect calls in fast-path wherever feasible in order to avoid
the cost of retpolines, for example, just to name a few:
* 283c16a2dfd3 ("indirect call wrappers: helpers to speed-up indirect calls of builtin")
* aaa5d90b395a ("net: use indirect call wrappers at GRO network layer")
* 028e0a476684 ("net: use indirect call wrappers at GRO transport layer")
* 356da6d0cde3 ("dma-mapping: bypass indirect calls for dma-direct")
* 09772d92cd5a ("bpf: avoid retpoline for lookup/update/delete calls on maps")
* 10870dd89e95 ("netfilter: nf_tables: add direct calls for all builtin expressions")
[...]
Recent work on XDP from Björn and Magnus additionally found that
manually transforming the XDP return code switch statement with
more than 5 cases into if-else combination would result in a
considerable speedup in XDP layer due to avoidance of indirect
calls in CONFIG_RETPOLINE enabled builds. On i40e driver with
XDP prog attached, a 20-26% speedup has been observed [0]. Aside
from XDP, there are many other places later in the networking
stack's critical path with similar switch-case processing. Rather
than fixing every XDP-enabled driver and locations in stack by
hand, it would be good to instead raise the limit where gcc would
emit expensive indirect calls from the switch under retpolines
and stick with the default as-is in case of !retpoline configured
kernels. This would also have the advantage that for archs where
this is not necessary, we let compiler select the underlying
target optimization for these constructs and avoid potential
slow-downs by if-else hand-rewrite.
In case of gcc, this setting is controlled by case-values-threshold
which has an architecture global default that selects 4 or 5 (latter
if target does not have a case insn that compares the bounds) where
some arch back ends like arm64 or s390 override it with their own
target hooks, for example, in gcc commit db7a90aa0de5 ("S/390:
Disable prediction of indirect branches") the threshold pretty
much disables jump tables by limit of 20 under retpoline builds.
Comparing gcc's and clang's default code generation on x86-64 under
O2 level with retpoline build results in the following outcome
for 5 switch cases:
* gcc with -mindirect-branch=thunk-inline -mindirect-branch-register:
# gdb -batch -ex 'disassemble dispatch' ./c-switch
Dump of assembler code for function dispatch:
0x0000000000400be0 <+0>: cmp $0x4,%edi
0x0000000000400be3 <+3>: ja 0x400c35 <dispatch+85>
0x0000000000400be5 <+5>: lea 0x915f8(%rip),%rdx # 0x4921e4
0x0000000000400bec <+12>: mov %edi,%edi
0x0000000000400bee <+14>: movslq (%rdx,%rdi,4),%rax
0x0000000000400bf2 <+18>: add %rdx,%rax
0x0000000000400bf5 <+21>: callq 0x400c01 <dispatch+33>
0x0000000000400bfa <+26>: pause
0x0000000000400bfc <+28>: lfence
0x0000000000400bff <+31>: jmp 0x400bfa <dispatch+26>
0x0000000000400c01 <+33>: mov %rax,(%rsp)
0x0000000000400c05 <+37>: retq
0x0000000000400c06 <+38>: nopw %cs:0x0(%rax,%rax,1)
0x0000000000400c10 <+48>: jmpq 0x400c90 <fn_3>
0x0000000000400c15 <+53>: nopl (%rax)
0x0000000000400c18 <+56>: jmpq 0x400c70 <fn_2>
0x0000000000400c1d <+61>: nopl (%rax)
0x0000000000400c20 <+64>: jmpq 0x400c50 <fn_1>
0x0000000000400c25 <+69>: nopl (%rax)
0x0000000000400c28 <+72>: jmpq 0x400c40 <fn_0>
0x0000000000400c2d <+77>: nopl (%rax)
0x0000000000400c30 <+80>: jmpq 0x400cb0 <fn_4>
0x0000000000400c35 <+85>: push %rax
0x0000000000400c36 <+86>: callq 0x40dd80 <abort>
End of assembler dump.
* clang with -mretpoline emitting search tree:
# gdb -batch -ex 'disassemble dispatch' ./c-switch
Dump of assembler code for function dispatch:
0x0000000000400b30 <+0>: cmp $0x1,%edi
0x0000000000400b33 <+3>: jle 0x400b44 <dispatch+20>
0x0000000000400b35 <+5>: cmp $0x2,%edi
0x0000000000400b38 <+8>: je 0x400b4d <dispatch+29>
0x0000000000400b3a <+10>: cmp $0x3,%edi
0x0000000000400b3d <+13>: jne 0x400b52 <dispatch+34>
0x0000000000400b3f <+15>: jmpq 0x400c50 <fn_3>
0x0000000000400b44 <+20>: test %edi,%edi
0x0000000000400b46 <+22>: jne 0x400b5c <dispatch+44>
0x0000000000400b48 <+24>: jmpq 0x400c20 <fn_0>
0x0000000000400b4d <+29>: jmpq 0x400c40 <fn_2>
0x0000000000400b52 <+34>: cmp $0x4,%edi
0x0000000000400b55 <+37>: jne 0x400b66 <dispatch+54>
0x0000000000400b57 <+39>: jmpq 0x400c60 <fn_4>
0x0000000000400b5c <+44>: cmp $0x1,%edi
0x0000000000400b5f <+47>: jne 0x400b66 <dispatch+54>
0x0000000000400b61 <+49>: jmpq 0x400c30 <fn_1>
0x0000000000400b66 <+54>: push %rax
0x0000000000400b67 <+55>: callq 0x40dd20 <abort>
End of assembler dump.
For sake of comparison, clang without -mretpoline:
# gdb -batch -ex 'disassemble dispatch' ./c-switch
Dump of assembler code for function dispatch:
0x0000000000400b30 <+0>: cmp $0x4,%edi
0x0000000000400b33 <+3>: ja 0x400b57 <dispatch+39>
0x0000000000400b35 <+5>: mov %edi,%eax
0x0000000000400b37 <+7>: jmpq *0x492148(,%rax,8)
0x0000000000400b3e <+14>: jmpq 0x400bf0 <fn_0>
0x0000000000400b43 <+19>: jmpq 0x400c30 <fn_4>
0x0000000000400b48 <+24>: jmpq 0x400c10 <fn_2>
0x0000000000400b4d <+29>: jmpq 0x400c20 <fn_3>
0x0000000000400b52 <+34>: jmpq 0x400c00 <fn_1>
0x0000000000400b57 <+39>: push %rax
0x0000000000400b58 <+40>: callq 0x40dcf0 <abort>
End of assembler dump.
Raising the cases to a high number (e.g. 100) will still result
in similar code generation pattern with clang and gcc as above,
in other words clang generally turns off jump table emission by
having an extra expansion pass under retpoline build to turn
indirectbr instructions from their IR into switch instructions
as a built-in -mno-jump-table lowering of a switch (in this
case, even if IR input already contained an indirect branch).
For gcc, adding --param=case-values-threshold=20 as in similar
fashion as s390 in order to raise the limit for x86 retpoline
enabled builds results in a small vmlinux size increase of only
0.13% (before=18,027,528 after=18,051,192). For clang this
option is ignored due to i) not being needed as mentioned and
ii) not having above cmdline parameter. Non-retpoline-enabled
builds with gcc continue to use the default case-values-threshold
setting, so nothing changes here.
[0] https://lore.kernel.org/netdev/20190129095754.9390-1-bjorn.topel@gmail.com/
and "The Path to DPDK Speeds for AF_XDP", LPC 2018, networking track:
- http://vger.kernel.org/lpc_net2018_talks/lpc18_pres_af_xdp_perf-v3.pdf
- http://vger.kernel.org/lpc_net2018_talks/lpc18_paper_af_xdp_perf-v2.pdf
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Björn Töpel <bjorn.topel@intel.com>
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
arch/x86/Makefile | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 9c5a67d1b9c1..f55420a67164 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -217,6 +217,11 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
# Avoid indirect branches in kernel to deal with Spectre
ifdef CONFIG_RETPOLINE
KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
+ # Additionally, avoid generating expensive indirect jumps which
+ # are subject to retpolines for small number of switch cases.
+ # clang turns off jump table generation by default when under
+ # retpoline builds, however, gcc does not for x86.
+ KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
endif
archscripts: scripts_basic
--
2.17.1
^ permalink raw reply related
* Re: [RESEND PATCH 3/7] mm/gup: Change GUP fast to use flags rather than a write 'bool'
From: Ira Weiny @ 2019-02-21 22:24 UTC (permalink / raw)
To: Souptick Joarder
Cc: John Hubbard, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
Paul Mackerras, David S. Miller, Martin Schwidefsky,
Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Ralf Baechle, Paul Burton,
James Hogan, linux-kernel, Linux-MM, linux-mips, linuxppc-dev,
linux-s390, linux-sh, sparclinux, kvm-ppc, kvm, linux-fpga,
dri-devel, linux-rdma, linux-media, linux-scsi,
open list:ANDROID DRIVERS, virtualization, netdev, linux-fbdev,
xen-devel, devel, ceph-devel, rds-devel
In-Reply-To: <CAFqt6zYvkhKw3GExRQB2e_c16KQqrKT=GoiqErA06txUQa2bMQ@mail.gmail.com>
On Thu, Feb 21, 2019 at 08:48:41AM +0530, Souptick Joarder wrote:
> Hi Ira,
>
> On Wed, Feb 20, 2019 at 11:01 AM <ira.weiny@intel.com> wrote:
> >
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > To facilitate additional options to get_user_pages_fast() change the
> > singular write parameter to be gup_flags.
> >
> > This patch does not change any functionality. New functionality will
> > follow in subsequent patches.
> >
> > Some of the get_user_pages_fast() call sites were unchanged because they
> > already passed FOLL_WRITE or 0 for the write parameter.
> >
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > ---
[snip]
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > index bd2dcfbf00cd..8fcb0a921e46 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > @@ -582,7 +582,7 @@ int kvmppc_book3s_hv_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
> > /* If writing != 0, then the HPTE must allow writing, if we get here */
> > write_ok = writing;
> > hva = gfn_to_hva_memslot(memslot, gfn);
> > - npages = get_user_pages_fast(hva, 1, writing, pages);
> > + npages = get_user_pages_fast(hva, 1, writing ? FOLL_WRITE : 0, pages);
>
> Just requesting for opinion,
> * writing ? FOLL_WRITE : 0 * is used in many places. How about placing it in a
> macro/ inline ?
I don't really think this would gain much. And I don't think it would be more
clear. In fact I can't even think of a macro name which would make sense. I'm
inclined to leave this as written.
Ira
>
> > if (npages < 1) {
> > /* Check if it's an I/O mapping */
> > down_read(¤t->mm->mmap_sem);
> > @@ -1175,7 +1175,7 @@ void *kvmppc_pin_guest_page(struct kvm *kvm, unsigned long gpa,
> > if (!memslot || (memslot->flags & KVM_MEMSLOT_INVALID))
> > goto err;
> > hva = gfn_to_hva_memslot(memslot, gfn);
> > - npages = get_user_pages_fast(hva, 1, 1, pages);
> > + npages = get_user_pages_fast(hva, 1, FOLL_WRITE, pages);
> > if (npages < 1)
> > goto err;
> > page = pages[0];
> > diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> > index 24296f4cadc6..e0af53fd78c5 100644
> > --- a/arch/powerpc/kvm/e500_mmu.c
> > +++ b/arch/powerpc/kvm/e500_mmu.c
> > @@ -783,7 +783,7 @@ int kvm_vcpu_ioctl_config_tlb(struct kvm_vcpu *vcpu,
> > if (!pages)
> > return -ENOMEM;
> >
> > - ret = get_user_pages_fast(cfg->array, num_pages, 1, pages);
> > + ret = get_user_pages_fast(cfg->array, num_pages, FOLL_WRITE, pages);
> > if (ret < 0)
> > goto free_pages;
> >
> > diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
> > index a712a650a8b6..acb0990c8364 100644
> > --- a/arch/powerpc/mm/mmu_context_iommu.c
> > +++ b/arch/powerpc/mm/mmu_context_iommu.c
> > @@ -190,7 +190,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
> > for (i = 0; i < entries; ++i) {
> > cur_ua = ua + (i << PAGE_SHIFT);
> > if (1 != get_user_pages_fast(cur_ua,
> > - 1/* pages */, 1/* iswrite */, &page)) {
> > + 1/* pages */, FOLL_WRITE, &page)) {
> > ret = -EFAULT;
> > for (j = 0; j < i; ++j)
> > put_page(pfn_to_page(mem->hpas[j] >>
> > @@ -209,7 +209,7 @@ static long mm_iommu_do_alloc(struct mm_struct *mm, unsigned long ua,
> > if (mm_iommu_move_page_from_cma(page))
> > goto populate;
> > if (1 != get_user_pages_fast(cur_ua,
> > - 1/* pages */, 1/* iswrite */,
> > + 1/* pages */, FOLL_WRITE,
> > &page)) {
> > ret = -EFAULT;
> > for (j = 0; j < i; ++j)
> > diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> > index fcb55b02990e..69d9366b966c 100644
> > --- a/arch/s390/kvm/interrupt.c
> > +++ b/arch/s390/kvm/interrupt.c
> > @@ -2278,7 +2278,7 @@ static int kvm_s390_adapter_map(struct kvm *kvm, unsigned int id, __u64 addr)
> > ret = -EFAULT;
> > goto out;
> > }
> > - ret = get_user_pages_fast(map->addr, 1, 1, &map->page);
> > + ret = get_user_pages_fast(map->addr, 1, FOLL_WRITE, &map->page);
> > if (ret < 0)
> > goto out;
> > BUG_ON(ret != 1);
> > diff --git a/arch/s390/mm/gup.c b/arch/s390/mm/gup.c
> > index 2809d11c7a28..0a6faf3d9960 100644
> > --- a/arch/s390/mm/gup.c
> > +++ b/arch/s390/mm/gup.c
> > @@ -265,7 +265,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * get_user_pages_fast() - pin user pages in memory
> > * @start: starting user address
> > * @nr_pages: number of pages from start to pin
> > - * @write: whether pages will be written to
> > + * @gup_flags: flags modifying pin behaviour
> > * @pages: array that receives pointers to the pages pinned.
> > * Should be at least nr_pages long.
> > *
> > @@ -277,22 +277,22 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * requested. If nr_pages is 0 or negative, returns 0. If no pages
> > * were pinned, returns -errno.
> > */
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages)
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > {
> > int nr, ret;
> >
> > might_sleep();
> > start &= PAGE_MASK;
> > - nr = __get_user_pages_fast(start, nr_pages, write, pages);
> > + nr = __get_user_pages_fast(start, nr_pages, gup_flags & FOLL_WRITE,
> > + pages);
> > if (nr == nr_pages)
> > return nr;
> >
> > /* Try to get the remaining pages with get_user_pages */
> > start += nr << PAGE_SHIFT;
> > pages += nr;
> > - ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
> > - write ? FOLL_WRITE : 0);
> > + ret = get_user_pages_unlocked(start, nr_pages - nr, pages, gup_flags);
> > /* Have to be a bit careful with return values */
> > if (nr > 0)
> > ret = (ret < 0) ? nr : ret + nr;
> > diff --git a/arch/sh/mm/gup.c b/arch/sh/mm/gup.c
> > index 3e27f6d1f1ec..277c882f7489 100644
> > --- a/arch/sh/mm/gup.c
> > +++ b/arch/sh/mm/gup.c
> > @@ -204,7 +204,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * get_user_pages_fast() - pin user pages in memory
> > * @start: starting user address
> > * @nr_pages: number of pages from start to pin
> > - * @write: whether pages will be written to
> > + * @gup_flags: flags modifying pin behaviour
> > * @pages: array that receives pointers to the pages pinned.
> > * Should be at least nr_pages long.
> > *
> > @@ -216,8 +216,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * requested. If nr_pages is 0 or negative, returns 0. If no pages
> > * were pinned, returns -errno.
> > */
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages)
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > {
> > struct mm_struct *mm = current->mm;
> > unsigned long addr, len, end;
> > @@ -241,7 +241,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > next = pgd_addr_end(addr, end);
> > if (pgd_none(pgd))
> > goto slow;
> > - if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
> > + if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
> > + pages, &nr))
> > goto slow;
> > } while (pgdp++, addr = next, addr != end);
> > local_irq_enable();
> > @@ -261,7 +262,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> >
> > ret = get_user_pages_unlocked(start,
> > (end - start) >> PAGE_SHIFT, pages,
> > - write ? FOLL_WRITE : 0);
> > + gup_flags);
> >
> > /* Have to be a bit careful with return values */
> > if (nr > 0) {
> > diff --git a/arch/sparc/mm/gup.c b/arch/sparc/mm/gup.c
> > index aee6dba83d0e..1e770a517d4a 100644
> > --- a/arch/sparc/mm/gup.c
> > +++ b/arch/sparc/mm/gup.c
> > @@ -245,8 +245,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > return nr;
> > }
> >
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages)
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > {
> > struct mm_struct *mm = current->mm;
> > unsigned long addr, len, end;
> > @@ -303,7 +303,8 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > next = pgd_addr_end(addr, end);
> > if (pgd_none(pgd))
> > goto slow;
> > - if (!gup_pud_range(pgd, addr, next, write, pages, &nr))
> > + if (!gup_pud_range(pgd, addr, next, gup_flags & FOLL_WRITE,
> > + pages, &nr))
> > goto slow;
> > } while (pgdp++, addr = next, addr != end);
> >
> > @@ -324,7 +325,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> >
> > ret = get_user_pages_unlocked(start,
> > (end - start) >> PAGE_SHIFT, pages,
> > - write ? FOLL_WRITE : 0);
> > + gup_flags);
> >
> > /* Have to be a bit careful with return values */
> > if (nr > 0) {
> > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> > index 6bdca39829bc..08715034e315 100644
> > --- a/arch/x86/kvm/paging_tmpl.h
> > +++ b/arch/x86/kvm/paging_tmpl.h
> > @@ -140,7 +140,7 @@ static int FNAME(cmpxchg_gpte)(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
> > pt_element_t *table;
> > struct page *page;
> >
> > - npages = get_user_pages_fast((unsigned long)ptep_user, 1, 1, &page);
> > + npages = get_user_pages_fast((unsigned long)ptep_user, 1, FOLL_WRITE, &page);
> > /* Check if the user is doing something meaningless. */
> > if (unlikely(npages != 1))
> > return -EFAULT;
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index f13a3a24d360..173596a020cb 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -1803,7 +1803,7 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
> > return NULL;
> >
> > /* Pin the user virtual address. */
> > - npinned = get_user_pages_fast(uaddr, npages, write ? FOLL_WRITE : 0, pages);
> > + npinned = get_user_pages_fast(uaddr, npages, FOLL_WRITE, pages);
> > if (npinned != npages) {
> > pr_err("SEV: Failure locking %lu pages.\n", npages);
> > goto err;
> > diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
> > index e18a786fc943..c438722bf4e1 100644
> > --- a/drivers/fpga/dfl-afu-dma-region.c
> > +++ b/drivers/fpga/dfl-afu-dma-region.c
> > @@ -102,7 +102,7 @@ static int afu_dma_pin_pages(struct dfl_feature_platform_data *pdata,
> > goto unlock_vm;
> > }
> >
> > - pinned = get_user_pages_fast(region->user_addr, npages, 1,
> > + pinned = get_user_pages_fast(region->user_addr, npages, FOLL_WRITE,
> > region->pages);
> > if (pinned < 0) {
> > ret = pinned;
> > diff --git a/drivers/gpu/drm/via/via_dmablit.c b/drivers/gpu/drm/via/via_dmablit.c
> > index 345bda4494e1..0c8b09602910 100644
> > --- a/drivers/gpu/drm/via/via_dmablit.c
> > +++ b/drivers/gpu/drm/via/via_dmablit.c
> > @@ -239,7 +239,8 @@ via_lock_all_dma_pages(drm_via_sg_info_t *vsg, drm_via_dmablit_t *xfer)
> > if (NULL == vsg->pages)
> > return -ENOMEM;
> > ret = get_user_pages_fast((unsigned long)xfer->mem_addr,
> > - vsg->num_pages, vsg->direction == DMA_FROM_DEVICE,
> > + vsg->num_pages,
> > + vsg->direction == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
> > vsg->pages);
> > if (ret != vsg->num_pages) {
> > if (ret < 0)
> > diff --git a/drivers/infiniband/hw/hfi1/user_pages.c b/drivers/infiniband/hw/hfi1/user_pages.c
> > index 24b592c6522e..78ccacaf97d0 100644
> > --- a/drivers/infiniband/hw/hfi1/user_pages.c
> > +++ b/drivers/infiniband/hw/hfi1/user_pages.c
> > @@ -105,7 +105,8 @@ int hfi1_acquire_user_pages(struct mm_struct *mm, unsigned long vaddr, size_t np
> > {
> > int ret;
> >
> > - ret = get_user_pages_fast(vaddr, npages, writable, pages);
> > + ret = get_user_pages_fast(vaddr, npages, writable ? FOLL_WRITE : 0,
> > + pages);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/drivers/misc/genwqe/card_utils.c b/drivers/misc/genwqe/card_utils.c
> > index 25265fd0fd6e..89cff9d1012b 100644
> > --- a/drivers/misc/genwqe/card_utils.c
> > +++ b/drivers/misc/genwqe/card_utils.c
> > @@ -603,7 +603,7 @@ int genwqe_user_vmap(struct genwqe_dev *cd, struct dma_mapping *m, void *uaddr,
> > /* pin user pages in memory */
> > rc = get_user_pages_fast(data & PAGE_MASK, /* page aligned addr */
> > m->nr_pages,
> > - m->write, /* readable/writable */
> > + m->write ? FOLL_WRITE : 0, /* readable/writable */
> > m->page_list); /* ptrs to pages */
> > if (rc < 0)
> > goto fail_get_user_pages;
> > diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
> > index 997f92543dd4..422d08da3244 100644
> > --- a/drivers/misc/vmw_vmci/vmci_host.c
> > +++ b/drivers/misc/vmw_vmci/vmci_host.c
> > @@ -242,7 +242,7 @@ static int vmci_host_setup_notify(struct vmci_ctx *context,
> > /*
> > * Lock physical page backing a given user VA.
> > */
> > - retval = get_user_pages_fast(uva, 1, 1, &context->notify_page);
> > + retval = get_user_pages_fast(uva, 1, FOLL_WRITE, &context->notify_page);
> > if (retval != 1) {
> > context->notify_page = NULL;
> > return VMCI_ERROR_GENERIC;
> > diff --git a/drivers/misc/vmw_vmci/vmci_queue_pair.c b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> > index 264f4ed8eef2..c5396ee32e51 100644
> > --- a/drivers/misc/vmw_vmci/vmci_queue_pair.c
> > +++ b/drivers/misc/vmw_vmci/vmci_queue_pair.c
> > @@ -666,7 +666,8 @@ static int qp_host_get_user_memory(u64 produce_uva,
> > int err = VMCI_SUCCESS;
> >
> > retval = get_user_pages_fast((uintptr_t) produce_uva,
> > - produce_q->kernel_if->num_pages, 1,
> > + produce_q->kernel_if->num_pages,
> > + FOLL_WRITE,
> > produce_q->kernel_if->u.h.header_page);
> > if (retval < (int)produce_q->kernel_if->num_pages) {
> > pr_debug("get_user_pages_fast(produce) failed (retval=%d)",
> > @@ -678,7 +679,8 @@ static int qp_host_get_user_memory(u64 produce_uva,
> > }
> >
> > retval = get_user_pages_fast((uintptr_t) consume_uva,
> > - consume_q->kernel_if->num_pages, 1,
> > + consume_q->kernel_if->num_pages,
> > + FOLL_WRITE,
> > consume_q->kernel_if->u.h.header_page);
> > if (retval < (int)consume_q->kernel_if->num_pages) {
> > pr_debug("get_user_pages_fast(consume) failed (retval=%d)",
> > diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> > index 321bc673c417..cef0133aa47a 100644
> > --- a/drivers/platform/goldfish/goldfish_pipe.c
> > +++ b/drivers/platform/goldfish/goldfish_pipe.c
> > @@ -274,7 +274,8 @@ static int pin_user_pages(unsigned long first_page,
> > *iter_last_page_size = last_page_size;
> > }
> >
> > - ret = get_user_pages_fast(first_page, requested_pages, !is_write,
> > + ret = get_user_pages_fast(first_page, requested_pages,
> > + !is_write ? FOLL_WRITE : 0,
> > pages);
> > if (ret <= 0)
> > return -EFAULT;
> > diff --git a/drivers/rapidio/devices/rio_mport_cdev.c b/drivers/rapidio/devices/rio_mport_cdev.c
> > index cbe467ff1aba..f681b3e9e970 100644
> > --- a/drivers/rapidio/devices/rio_mport_cdev.c
> > +++ b/drivers/rapidio/devices/rio_mport_cdev.c
> > @@ -868,7 +868,9 @@ rio_dma_transfer(struct file *filp, u32 transfer_mode,
> >
> > pinned = get_user_pages_fast(
> > (unsigned long)xfer->loc_addr & PAGE_MASK,
> > - nr_pages, dir == DMA_FROM_DEVICE, page_list);
> > + nr_pages,
> > + dir == DMA_FROM_DEVICE ? FOLL_WRITE : 0,
> > + page_list);
> >
> > if (pinned != nr_pages) {
> > if (pinned < 0) {
> > diff --git a/drivers/sbus/char/oradax.c b/drivers/sbus/char/oradax.c
> > index 6516bc3cb58b..790aa148670d 100644
> > --- a/drivers/sbus/char/oradax.c
> > +++ b/drivers/sbus/char/oradax.c
> > @@ -437,7 +437,7 @@ static int dax_lock_page(void *va, struct page **p)
> >
> > dax_dbg("uva %p", va);
> >
> > - ret = get_user_pages_fast((unsigned long)va, 1, 1, p);
> > + ret = get_user_pages_fast((unsigned long)va, 1, FOLL_WRITE, p);
> > if (ret == 1) {
> > dax_dbg("locked page %p, for VA %p", *p, va);
> > return 0;
> > diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
> > index 7ff22d3f03e3..871b25914c07 100644
> > --- a/drivers/scsi/st.c
> > +++ b/drivers/scsi/st.c
> > @@ -4918,7 +4918,8 @@ static int sgl_map_user_pages(struct st_buffer *STbp,
> >
> > /* Try to fault in all of the necessary pages */
> > /* rw==READ means read from drive, write into memory area */
> > - res = get_user_pages_fast(uaddr, nr_pages, rw == READ, pages);
> > + res = get_user_pages_fast(uaddr, nr_pages, rw == READ ? FOLL_WRITE : 0,
> > + pages);
> >
> > /* Errors and no page mapped should return here */
> > if (res < nr_pages)
> > diff --git a/drivers/staging/gasket/gasket_page_table.c b/drivers/staging/gasket/gasket_page_table.c
> > index 26755d9ca41d..f67fdf1d3817 100644
> > --- a/drivers/staging/gasket/gasket_page_table.c
> > +++ b/drivers/staging/gasket/gasket_page_table.c
> > @@ -486,8 +486,8 @@ static int gasket_perform_mapping(struct gasket_page_table *pg_tbl,
> > ptes[i].dma_addr = pg_tbl->coherent_pages[0].paddr +
> > off + i * PAGE_SIZE;
> > } else {
> > - ret = get_user_pages_fast(page_addr - offset, 1, 1,
> > - &page);
> > + ret = get_user_pages_fast(page_addr - offset, 1,
> > + FOLL_WRITE, &page);
> >
> > if (ret <= 0) {
> > dev_err(pg_tbl->device,
> > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> > index 0b9ab1d0dd45..49fd7312e2aa 100644
> > --- a/drivers/tee/tee_shm.c
> > +++ b/drivers/tee/tee_shm.c
> > @@ -273,7 +273,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
> > goto err;
> > }
> >
> > - rc = get_user_pages_fast(start, num_pages, 1, shm->pages);
> > + rc = get_user_pages_fast(start, num_pages, FOLL_WRITE, shm->pages);
> > if (rc > 0)
> > shm->num_pages = rc;
> > if (rc != num_pages) {
> > diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> > index c424913324e3..a4b10bb4086b 100644
> > --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> > +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> > @@ -532,7 +532,8 @@ static int tce_iommu_use_page(unsigned long tce, unsigned long *hpa)
> > enum dma_data_direction direction = iommu_tce_direction(tce);
> >
> > if (get_user_pages_fast(tce & PAGE_MASK, 1,
> > - direction != DMA_TO_DEVICE, &page) != 1)
> > + direction != DMA_TO_DEVICE ? FOLL_WRITE : 0,
> > + &page) != 1)
> > return -EFAULT;
> >
> > *hpa = __pa((unsigned long) page_address(page));
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 24a129fcdd61..72685b1659ff 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1700,7 +1700,7 @@ static int set_bit_to_user(int nr, void __user *addr)
> > int bit = nr + (log % PAGE_SIZE) * 8;
> > int r;
> >
> > - r = get_user_pages_fast(log, 1, 1, &page);
> > + r = get_user_pages_fast(log, 1, FOLL_WRITE, &page);
> > if (r < 0)
> > return r;
> > BUG_ON(r != 1);
> > diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
> > index 8a53d1de611d..41390c8e0f67 100644
> > --- a/drivers/video/fbdev/pvr2fb.c
> > +++ b/drivers/video/fbdev/pvr2fb.c
> > @@ -686,7 +686,7 @@ static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
> > if (!pages)
> > return -ENOMEM;
> >
> > - ret = get_user_pages_fast((unsigned long)buf, nr_pages, true, pages);
> > + ret = get_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);
> > if (ret < nr_pages) {
> > nr_pages = ret;
> > ret = -EINVAL;
> > diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
> > index 8ba726e600e9..6446bcab4185 100644
> > --- a/drivers/virt/fsl_hypervisor.c
> > +++ b/drivers/virt/fsl_hypervisor.c
> > @@ -244,7 +244,7 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
> >
> > /* Get the physical addresses of the source buffer */
> > num_pinned = get_user_pages_fast(param.local_vaddr - lb_offset,
> > - num_pages, param.source != -1, pages);
> > + num_pages, param.source != -1 ? FOLL_WRITE : 0, pages);
> >
> > if (num_pinned != num_pages) {
> > /* get_user_pages() failed */
> > diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> > index 5efc5eee9544..7b47f1e6aab4 100644
> > --- a/drivers/xen/gntdev.c
> > +++ b/drivers/xen/gntdev.c
> > @@ -852,7 +852,7 @@ static int gntdev_get_page(struct gntdev_copy_batch *batch, void __user *virt,
> > unsigned long xen_pfn;
> > int ret;
> >
> > - ret = get_user_pages_fast(addr, 1, writeable, &page);
> > + ret = get_user_pages_fast(addr, 1, writeable ? FOLL_WRITE : 0, &page);
> > if (ret < 0)
> > return ret;
> >
> > diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c
> > index 443bcd8c3c19..5a7c4fda682f 100644
> > --- a/fs/orangefs/orangefs-bufmap.c
> > +++ b/fs/orangefs/orangefs-bufmap.c
> > @@ -269,7 +269,7 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap,
> >
> > /* map the pages */
> > ret = get_user_pages_fast((unsigned long)user_desc->ptr,
> > - bufmap->page_count, 1, bufmap->page_array);
> > + bufmap->page_count, FOLL_WRITE, bufmap->page_array);
> >
> > if (ret < 0)
> > return ret;
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 05a105d9d4c3..8e1f3cd7482a 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -1537,8 +1537,8 @@ long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> > struct page **pages, unsigned int gup_flags);
> >
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages);
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages);
> >
> > /* Container for pinned pfns / pages */
> > struct frame_vector {
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index fdd312da0992..e10209946f8b 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -546,7 +546,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, enum futex_a
> > if (unlikely(should_fail_futex(fshared)))
> > return -EFAULT;
> >
> > - err = get_user_pages_fast(address, 1, 1, &page);
> > + err = get_user_pages_fast(address, 1, FOLL_WRITE, &page);
> > /*
> > * If write access is not required (eg. FUTEX_WAIT), try
> > * and get read-only access.
> > diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> > index be4bd627caf0..6dbae0692719 100644
> > --- a/lib/iov_iter.c
> > +++ b/lib/iov_iter.c
> > @@ -1280,7 +1280,9 @@ ssize_t iov_iter_get_pages(struct iov_iter *i,
> > len = maxpages * PAGE_SIZE;
> > addr &= ~(PAGE_SIZE - 1);
> > n = DIV_ROUND_UP(len, PAGE_SIZE);
> > - res = get_user_pages_fast(addr, n, iov_iter_rw(i) != WRITE, pages);
> > + res = get_user_pages_fast(addr, n,
> > + iov_iter_rw(i) != WRITE ? FOLL_WRITE : 0,
> > + pages);
> > if (unlikely(res < 0))
> > return res;
> > return (res == n ? len : res * PAGE_SIZE) - *start;
> > @@ -1361,7 +1363,8 @@ ssize_t iov_iter_get_pages_alloc(struct iov_iter *i,
> > p = get_pages_array(n);
> > if (!p)
> > return -ENOMEM;
> > - res = get_user_pages_fast(addr, n, iov_iter_rw(i) != WRITE, p);
> > + res = get_user_pages_fast(addr, n,
> > + iov_iter_rw(i) != WRITE ? FOLL_WRITE : 0, p);
> > if (unlikely(res < 0)) {
> > kvfree(p);
> > return res;
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 681388236106..6f32d36b3c5b 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -1863,7 +1863,7 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * get_user_pages_fast() - pin user pages in memory
> > * @start: starting user address
> > * @nr_pages: number of pages from start to pin
> > - * @write: whether pages will be written to
> > + * @gup_flags: flags modifying pin behaviour
> > * @pages: array that receives pointers to the pages pinned.
> > * Should be at least nr_pages long.
> > *
> > @@ -1875,8 +1875,8 @@ int __get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > * requested. If nr_pages is 0 or negative, returns 0. If no pages
> > * were pinned, returns -errno.
> > */
> > -int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > - struct page **pages)
> > +int get_user_pages_fast(unsigned long start, int nr_pages,
> > + unsigned int gup_flags, struct page **pages)
> > {
> > unsigned long addr, len, end;
> > int nr = 0, ret = 0;
> > @@ -1894,7 +1894,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> >
> > if (gup_fast_permitted(start, nr_pages)) {
> > local_irq_disable();
> > - gup_pgd_range(addr, end, write ? FOLL_WRITE : 0, pages, &nr);
> > + gup_pgd_range(addr, end, gup_flags, pages, &nr);
> > local_irq_enable();
> > ret = nr;
> > }
> > @@ -1905,7 +1905,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
> > pages += nr;
> >
> > ret = get_user_pages_unlocked(start, nr_pages - nr, pages,
> > - write ? FOLL_WRITE : 0);
> > + gup_flags);
> >
> > /* Have to be a bit careful with return values */
> > if (nr > 0) {
> > diff --git a/mm/util.c b/mm/util.c
> > index 1ea055138043..01ffe145c62b 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -306,7 +306,7 @@ EXPORT_SYMBOL_GPL(__get_user_pages_fast);
> > * get_user_pages_fast() - pin user pages in memory
> > * @start: starting user address
> > * @nr_pages: number of pages from start to pin
> > - * @write: whether pages will be written to
> > + * @gup_flags: flags modifying pin behaviour
> > * @pages: array that receives pointers to the pages pinned.
> > * Should be at least nr_pages long.
> > *
> > @@ -327,10 +327,10 @@ EXPORT_SYMBOL_GPL(__get_user_pages_fast);
> > * get_user_pages_fast simply falls back to get_user_pages.
> > */
> > int __weak get_user_pages_fast(unsigned long start,
> > - int nr_pages, int write, struct page **pages)
> > + int nr_pages, unsigned int gup_flags,
> > + struct page **pages)
> > {
> > - return get_user_pages_unlocked(start, nr_pages, pages,
> > - write ? FOLL_WRITE : 0);
> > + return get_user_pages_unlocked(start, nr_pages, pages, gup_flags);
> > }
> > EXPORT_SYMBOL_GPL(get_user_pages_fast);
> >
> > diff --git a/net/ceph/pagevec.c b/net/ceph/pagevec.c
> > index d3736f5bffec..74cafc0142ea 100644
> > --- a/net/ceph/pagevec.c
> > +++ b/net/ceph/pagevec.c
> > @@ -27,7 +27,7 @@ struct page **ceph_get_direct_page_vector(const void __user *data,
> > while (got < num_pages) {
> > rc = get_user_pages_fast(
> > (unsigned long)data + ((unsigned long)got * PAGE_SIZE),
> > - num_pages - got, write_page, pages + got);
> > + num_pages - got, write_page ? FOLL_WRITE : 0, pages + got);
> > if (rc < 0)
> > break;
> > BUG_ON(rc == 0);
> > diff --git a/net/rds/info.c b/net/rds/info.c
> > index e367a97a18c8..03f6fd56d237 100644
> > --- a/net/rds/info.c
> > +++ b/net/rds/info.c
> > @@ -193,7 +193,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval,
> > ret = -ENOMEM;
> > goto out;
> > }
> > - ret = get_user_pages_fast(start, nr_pages, 1, pages);
> > + ret = get_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
> > if (ret != nr_pages) {
> > if (ret > 0)
> > nr_pages = ret;
> > diff --git a/net/rds/rdma.c b/net/rds/rdma.c
> > index 182ab8430594..b340ed4fc43a 100644
> > --- a/net/rds/rdma.c
> > +++ b/net/rds/rdma.c
> > @@ -158,7 +158,8 @@ static int rds_pin_pages(unsigned long user_addr, unsigned int nr_pages,
> > {
> > int ret;
> >
> > - ret = get_user_pages_fast(user_addr, nr_pages, write, pages);
> > + ret = get_user_pages_fast(user_addr, nr_pages, write ? FOLL_WRITE : 0,
> > + pages);
> >
> > if (ret >= 0 && ret < nr_pages) {
> > while (ret--)
> > --
> > 2.20.1
> >
>
^ permalink raw reply
* Re: [PATCH bpf-next v1] bpf, lpm: fix lookup bug in map_delete_elem
From: Martin Lau @ 2019-02-21 22:23 UTC (permalink / raw)
To: Alban Crequy
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, alban@kinvolk.io, iago@kinvolk.io,
Craig Gallek
In-Reply-To: <20190221163926.15221-1-alban@kinvolk.io>
On Thu, Feb 21, 2019 at 05:39:26PM +0100, Alban Crequy wrote:
> From: Alban Crequy <alban@kinvolk.io>
>
> trie_delete_elem() was deleting an entry even though it was not matching
> if the prefixlen was correct. This patch adds a check on matchlen.
>
> Reproducer:
>
> $ sudo bpftool map create /sys/fs/bpf/mylpm type lpm_trie key 8 value 1 entries 128 name mylpm flags 1
> $ sudo bpftool map update pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 aa bb cc dd value hex 01
> $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
> key: 10 00 00 00 aa bb cc dd value: 01
> Found 1 element
> $ sudo bpftool map delete pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 ff ff ff ff
> $ echo $?
> 0
> $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
> Found 0 elements
The change makes sense to me. Can you add this reproducer to
tools/testing/selftests/bpf/test_lpm_map.c?
Bug fix should be for the "bpf" tree instead of "bpf-next"
Fixes tag is also required, like
Fixes: e454cf595853 ("bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE")
Cc: Craig Gallek <kraig@google.com>
>
> Signed-off-by: Alban Crequy <alban@kinvolk.io>
> ---
> kernel/bpf/lpm_trie.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
> index abf1002080df..93a5cbbde421 100644
> --- a/kernel/bpf/lpm_trie.c
> +++ b/kernel/bpf/lpm_trie.c
> @@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key)
> }
>
> if (!node || node->prefixlen != key->prefixlen ||
> + node->prefixlen != matchlen ||
> (node->flags & LPM_TREE_NODE_FLAG_IM)) {
> ret = -ENOENT;
> goto out;
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH] x86, retpolines: raise limit for generating indirect calls from switch-case
From: Linus Torvalds @ 2019-02-21 22:27 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Thomas Gleixner, Linux List Kernel Mailing, Netdev,
David S . Miller, Björn Töpel, Magnus Karlsson,
Jesper Dangaard Brouer, Alexei Starovoitov, Peter Zijlstra,
David Woodhouse, Andy Lutomirski, Borislav Petkov
In-Reply-To: <20190221221941.29358-1-daniel@iogearbox.net>
On Thu, Feb 21, 2019 at 2:20 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> In case of gcc, this setting is controlled by case-values-threshold
> which has an architecture global default that selects 4 or 5 (
Ack. For retpoline, that's much too low.
Patch looks sane, although it would be good to verify just which
versions of gcc this works for. All versions with retpoline?
Linus
^ permalink raw reply
* Re: [PATCH v3 net-next 1/4] net: dsa: microchip: prepare PHY for proper advertisement
From: Florian Fainelli @ 2019-02-21 22:33 UTC (permalink / raw)
To: Tristram.Ha, Sergio Paracuellos, Andrew Lunn, Pavel Machek
Cc: UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-2-git-send-email-Tristram.Ha@microchip.com>
On 2/21/19 2:03 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Prepare PHY for proper advertisement as sometimes the PHY in the switch
> has its own problems even though it may share the PHY id from regular PHY
> but the fixes in the PHY driver do not apply.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: mlx5 stable backport help
From: Saeed Mahameed @ 2019-02-21 22:35 UTC (permalink / raw)
To: gerlitz.or@gmail.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, Or Gerlitz
In-Reply-To: <CAJ3xEMibTfuFkdoV5wjq4s3aV4WCBmaKGQij-tH=5S4t7ZoSrQ@mail.gmail.com>
On Thu, 2019-02-21 at 12:21 +0200, Or Gerlitz wrote:
> On Wed, Feb 20, 2019 at 10:03 PM Saeed Mahameed <saeedm@mellanox.com>
> wrote:
> > On Tue, 2019-02-19 at 22:36 -0800, David Miller wrote:
> > > I need help backporting two changes for -stable. Namely:
> > Hi Dave Thanks for trying,
>
> here too
>
> > > 6707f74be8621ae067d2cf1c4485900e2742c20f ("net/mlx5e: Update hw
> > > flows
> > > when encap source mac changed")
> >
> > The fixes tag of the above commit is wrong and it should have been
> >
> > commit e32ee6c78efa6a32bff782bbe7a9970b018996ca
> > Author: Eli Britstein <elibr@mellanox.com>
> > Date: Mon Dec 3 17:09:54 2018 +0200
> >
> > net/mlx5e: Support tunnel encap over tagged Ethernet
> >
> > Before this patch we didn't have the support for "tunnel encap over
> > tagged Ethernet" which introduced the whole route_dev logic to
> > generate
> > the encap headers, so the patch should only exist in -rc, let's
> > just skip it.
>
> Hi Saeed,
>
> The issue exists prior to the commit you mentioned, we are doing this
> code since day one of
> tc tunnel offloads support which lacked the neigh update flow
>
> linux-stable.git]# git grep -pe "eth->h_source"
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c=static void
> gen_vxlan_header_ipv4(struct net_device *out_dev,
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:
> ether_addr_copy(eth->h_source, out_dev->dev_addr);
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c=static void
> gen_vxlan_header_ipv6(struct net_device *out_dev,
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:
> ether_addr_copy(eth->h_source, out_dev->dev_addr);
>
> later when 232c001398ae ("net/mlx5e: Add support to neighbour update
> flow") was done, we went short
> and did not address the case of source address change. Until the
> commit you mentioned, we didn't have the
> out_dev vs route_dev notion, but the problem exists and it's possible
> to backport the patch by
>
So we all agree that the offending patch is "net/mlx5e: Support tunnel
encap over tagged Ethernet" even if the issue existed before,
in order to fix the issue you will have to port not only this patch but
the whole series which claimed to fix the issue, so the fixes tag was
wrong.. this patch on its own is no good.
> [1] using e->out_dev instead of e->route_dev
> [2] use the hunks from mlx5e_tc_tun_create_header_ipv4/6() in
> mlx5e_create_encap_header_ipv4/6()
>
> Or.
^ permalink raw reply
* Re: [iproute PATCH] ip-address: Use correct max attribute value in print_vf_stats64()
From: Stephen Hemminger @ 2019-02-21 22:40 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
In-Reply-To: <20190221183751.10604-1-phil@nwl.cc>
On Thu, 21 Feb 2019 19:37:51 +0100
Phil Sutter <phil@nwl.cc> wrote:
> IFLA_VF_MAX is larger than the highest valid index in vf array.
>
> Fixes: a1b99717c7cd7 ("Add displaying VF traffic statistics")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
Good catch. It worked because VF_MAX is 12 and VF_STATS_MAX is 8.
Applied.
^ permalink raw reply
* Re: [PATCH] samples/bpf: Fix dummy program unloading for xdp_redirect samples
From: Martin Lau @ 2019-02-21 22:47 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: Toke Høiland-Jørgensen, netdev@vger.kernel.org
In-Reply-To: <20190221173054.00006df2@gmail.com>
On Thu, Feb 21, 2019 at 05:30:54PM +0100, Maciej Fijalkowski wrote:
> On Thu, 21 Feb 2019 17:05:39 +0100
> Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> > The xdp_redirect and xdp_redirect_map sample programs both load a dummy
> > program onto the egress interfaces. However, the unload code checks these
> > programs against the wrong fd number, and thus refuses to unload them. Fix
s/fd/id/
> > the comparison to avoid this.
The change LGTM.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [PATCH iproute2 v2] bridge: make mcast_flood description consistent
From: Stephen Hemminger @ 2019-02-21 22:48 UTC (permalink / raw)
To: Vivien Didelot; +Cc: netdev, Russell King
In-Reply-To: <20190220163357.25968-1-vivien.didelot@gmail.com>
On Wed, 20 Feb 2019 11:33:57 -0500
Vivien Didelot <vivien.didelot@gmail.com> wrote:
> This patch simply changes the description of the mcast_flood flag
> with "flood" instead of "be flooded with" to avoid confusion, and be
> consistent with the description of the flooding flag, which "Controls
> whether a given port will *flood* unicast traffic for which there is
> no FDB entry."
>
> At the same time, fix the documentation for the "flood" flag which
> is incorrectly described as "flooding on" or "flooding off".
>
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
Applied
^ permalink raw reply
* Re: [PATCH iproute2] ss: fix compilation under glibc < 2.18
From: Stephen Hemminger @ 2019-02-21 22:49 UTC (permalink / raw)
To: Thomas De Schampheleire; +Cc: netdev, Thomas De Schampheleire
In-Reply-To: <CAAXf6LUAS5c-21P55gSG9-1yXZ_a8nEYU8GgeF=xgZQtmyohpQ@mail.gmail.com>
On Thu, 21 Feb 2019 08:43:08 +0100
Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
> El jue., 21 feb. 2019 a las 2:53, Stephen Hemminger
> (<stephen@networkplumber.org>) escribió:
> >
> > On Wed, 20 Feb 2019 15:41:51 +0100
> > Thomas De Schampheleire <patrickdepinguin@gmail.com> wrote:
> >
> > > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > >
> > > Commit c759116a0b2b6da8df9687b0a40ac69050132c77 introduced support for
> > > AF_VSOCK. This define is only provided since glibc version 2.18, so
> > > compilation fails when using older toolchains.
> > >
> > > Provide the necessary definitions if needed.
> > >
> > > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Not sure why you would want new iproute2 with a 5 year old version of glibc?
> > Yes that means update your tool chain.
> >
>
> This problem is noticed for an embedded system. It has up-to-date
> applications and libraries (built via Buildroot) but the toolchain and
> kernel are supplied by the SoC vendor (in this case Marvell, formerly
> Cavium Networks). Unfortunately their toolchain is lagging behind and
> is still using glibc 2.16.
>
> I could handle this patch locally, but I think there may be other
> people in a similar situation which would benefit from an upstream
> fix.
OK applied, just don't want to keep lots of old cruft in the code
since it can lead to future suprise bugs.
^ permalink raw reply
* Re: [patch iproute2 v2] devlink: relax dpipe table show dependency on resources
From: Stephen Hemminger @ 2019-02-21 22:49 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, dsahern, mlxsw
In-Reply-To: <20190221105556.1315-1-jiri@resnulli.us>
On Thu, 21 Feb 2019 11:55:56 +0100
Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Dpipe table show command has a depencency on getting resources.
> If resource get command is not supported by the driver, dpipe table
> show fails. However, resource is only additional information
> in dpipe table show output. So relax the dependency and let
> the dpipe tables be shown even if resources get command fails.
>
> Fixes: ead180274caf ("devlink: Add support for resource/dpipe relation")
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied
^ permalink raw reply
* Re: [PATCH v3 net-next 3/4] net: dsa: microchip: get port link status
From: Florian Fainelli @ 2019-02-21 22:55 UTC (permalink / raw)
To: Tristram.Ha, Sergio Paracuellos, Andrew Lunn, Pavel Machek
Cc: UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-4-git-send-email-Tristram.Ha@microchip.com>
On 2/21/19 2:03 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Get port link status to know whether to read MIB counters when the link
> is going down. Add port_cleanup function to read MIB counters the last
> time as when the port is disabled the PHY is also powered down.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
> ---
Some comments here and there again, not against a merge, but would be
nice to address in the future.
[snip]
> +void ksz_port_cleanup(struct ksz_device *dev, int port)
> +{
> + /* Read all MIB counters when the link is going down. */
> + if (dev->live_ports & (1 << port)) {
> + struct ksz_port *p = &dev->ports[port];
> +
> + p->read = true;
> + schedule_work(&dev->mib_read);
> + }
> +
> + /* Common code for port cleanup. */
> + dev->on_ports &= ~(1 << port);
It seems to be that dev->on_ports is a shorthand for a port is enabled
whether its link is up or down, which is equivalent to using:
!dsa_port_is_unused(ds, port)) unless I am mistaken about the intention
of the code.
In general you seem to be very prone to adding a lot of these
bookkeeping variables which just creates more opportunities for bugs to
creep in.
> + dev->live_ports &= ~(1 << port);
> +}
> +EXPORT_SYMBOL_GPL(ksz_port_cleanup);
> +
> void ksz_update_port_member(struct ksz_device *dev, int port)
> {
> struct ksz_port *p;
> @@ -156,6 +172,26 @@ int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
> }
> EXPORT_SYMBOL_GPL(ksz_phy_write16);
>
> +void ksz_adjust_link(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct ksz_device *dev = ds->priv;
> + struct ksz_port *p = &dev->ports[port];
> +
> + if (!phydev->link) {
> + /* Read all MIB counters when the link is going down. */
> + if (dev->live_ports & (1 << port)) {
> + p->read = true;
> + schedule_work(&dev->mib_read);
> + }
> + dev->live_ports &= ~(1 << port);
> + } else {
> + /* Remember which port is connected and active. */
> + dev->live_ports |= (1 << port) & dev->on_ports;
> + }
How about something simpler than this:
if (phydev->link != p->old_link) {
p->read = !!phydev->link;
schedule_work(&dev->mib_read);
}
This is also super racy because you schedule the workqueue without any
locking against the state machine which runs with the PHY device's mutex
held, so there is the possibility of the following happening:
Thread 0 Thread 1
ksz_adjust_link()
phydev->link = 0
schedule_work()
ksz_adjust_link()
p->read = true
mib_read_workqueue()
and so we don't read counters anymore. It is probably fine since this is
clearly entirely opportunistic and in order to minimize register bandwidth.
--
Florian
^ permalink raw reply
* Re: [PATCH] x86, retpolines: raise limit for generating indirect calls from switch-case
From: Daniel Borkmann @ 2019-02-21 22:56 UTC (permalink / raw)
To: Linus Torvalds
Cc: Thomas Gleixner, Linux List Kernel Mailing, Netdev,
David S . Miller, Björn Töpel, Magnus Karlsson,
Jesper Dangaard Brouer, Alexei Starovoitov, Peter Zijlstra,
David Woodhouse, Andy Lutomirski, Borislav Petkov
In-Reply-To: <CAHk-=wgNfnC-oe7aJGgNWoTbssc0LPYK6FyR=6LO4_U_dsphbg@mail.gmail.com>
On 02/21/2019 11:27 PM, Linus Torvalds wrote:
> On Thu, Feb 21, 2019 at 2:20 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> In case of gcc, this setting is controlled by case-values-threshold
>> which has an architecture global default that selects 4 or 5 (
>
> Ack. For retpoline, that's much too low.
>
> Patch looks sane, although it would be good to verify just which
> versions of gcc this works for. All versions with retpoline?
The feature was first added in gcc 4.7 [0], under "General Optimizer Improvements":
Support for a new parameter --param case-values-threshold=n was added to allow
users to control the cutoff between doing switch statements as a series of if
statements and using a jump table.
From what I can tell, original author (H.J. Lu) provided backports up to gcc 4.8
and distros seem to have pulled it from his github branch [1] as upstream gcc does
not handle backports for stable versions that old.
Thanks,
Daniel
[0] https://www.gnu.org/software/gcc/gcc-4.7/changes.html
[1] https://bugs.launchpad.net/ubuntu/+source/gcc-4.8/+bug/1749261
https://github.com/hjl-tools/gcc/tree/hjl/indirect/gcc-4_8-branch/master
^ permalink raw reply
* Re: [PATCH v3 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Florian Fainelli @ 2019-02-21 23:01 UTC (permalink / raw)
To: Tristram.Ha, Sergio Paracuellos, Andrew Lunn, Pavel Machek
Cc: UNGLinuxDriver, netdev
In-Reply-To: <1550786597-591-3-git-send-email-Tristram.Ha@microchip.com>
On 2/21/19 2:03 PM, Tristram.Ha@microchip.com wrote:
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add background MIB counter reading support.
>
> Port MIB counters should only be read when there is link. Otherwise it is
> a waste of time as hardware never increases those counters. There are
> exceptions as some switches keep track of dropped counts no matter what.
Some minor comments below, none of them need to be addressed right now,
but it might be a good idea to do it at a later time.
[snip]
> +
> +static void mib_monitor(struct timer_list *t)
> +{
> + struct ksz_device *dev = from_timer(dev, t, mib_read_timer);
> + const struct dsa_port *dp;
> + struct net_device *netdev;
> + struct ksz_port_mib *mib;
> + struct ksz_port *p;
> + int i;
> +
> + mod_timer(&dev->mib_read_timer, jiffies + dev->mib_read_interval);
> +
> + /* Check which port needs to read MIB counters. */
> + for (i = 0; i < dev->mib_port_cnt; i++) {
> + p = &dev->ports[i];
> + if (!p->on)
> + continue;
Would not using dsa_port_is_unused() accomplish the same thing here?
Your setup function should not have enabled non-existent/connected to
the outside world ports anyway.
> + dp = dsa_to_port(dev->ds, i);
> + netdev = dp->slave;
> +
> + mib = &p->mib;
> + mutex_lock(&mib->cnt_mutex);
Have you built this with CONFIG_DEBUG_SLEEP_ATOMIC? timer executes in BH
context (atomic), you cannot hold a mutex here which would be allowed to
sleep.
> +
> + /* Read only dropped counters when link is not up. */
> + if (netdev && !netif_carrier_ok(netdev))> + mib->cnt_ptr = dev->reg_mib_cnt;
> + mutex_unlock(&mib->cnt_mutex);
> + p->read = true;
> + }
> + schedule_work(&dev->mib_read);
> +}
[snip]
> +
> int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg)
> {
> struct ksz_device *dev = ds->priv;
> @@ -72,6 +167,26 @@ int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
> }
> EXPORT_SYMBOL_GPL(ksz_sset_count);
>
> +void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
> +{
> + const struct dsa_port *dp = dsa_to_port(ds, port);
> + struct ksz_device *dev = ds->priv;
> + struct net_device *netdev;
> + struct ksz_port_mib *mib;
> +
> + mib = &dev->ports[port].mib;
> + mutex_lock(&mib->cnt_mutex);
> +
> + /* Only read dropped counters if no link. */
> + netdev = dp->slave;
> + if (netdev && !netif_carrier_ok(netdev))
> + mib->cnt_ptr = dev->reg_mib_cnt;
The netdev is guaranteed to exist, otherwise you would not be in this
function, and we are executing with RTNL held, so the device can't
vanish under your feet.
[snip]
>
> +/* Modify from readx_poll_timeout in iopoll.h. */
> +#define ksz_pread_poll_timeout(op, dev, p, addr, val, cond, sleep_us, \
> + timeout_us) \
> +({ \
> + ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
> + might_sleep_if(sleep_us); \
> + for (;;) { \
> + op(dev, p, addr, &(val)); \
I don't think you need to create your own custom macro, which you seem
to need such that you can pass dev and p as arguments, instead you can
create a specific helper function, pass it down to op() as the "addr"
argument, since that is a macro that does not type checking at all. So
something like this:
struct ksz_read_ctx {
struct ksz_device *dev;
struct ksz_port *p;
u16 reg;
};
static int ksz_pread32_poll(struct ksz_read_ctx *ctx)
{
return ksz_pread32(ctx->dev, ctx->p, ctx->reg);
}
static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
u64 *cnt)
{
struct ksz_port *p = &dev->ports[port];
struct ksz_read_ctx ctx = {
.dev = dev,
.p = &dev->ports[port],
.reg = REG_PORT_MIB_CTRL_STAT__4
};
u32 data;
int ret;
/* retain the flush/freeze bit */
data = p->freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
data |= MIB_COUNTER_READ;
data |= (addr << MIB_COUNTER_INDEX_S);
ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
ret = readx_poll_timeout(ksz_pread32_poll, &ctx, data, !(data &
MIB_COUNTER_READ), 10, 1000);
Completely not compile tested, but you get the idea.
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/2] xdp: Add devmap_idx map type for looking up devices by ifindex
From: Toke Høiland-Jørgensen @ 2019-02-21 23:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190221134923.53c40b11@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> A common pattern when using xdp_redirect_map() is to create a device map
>> where the lookup key is simply ifindex. Because device maps are arrays,
>> this leaves holes in the map, and the map has to be sized to fit the
>> largest ifindex, regardless of how many devices actually are actually
>> needed in the map.
>>
>> This patch adds a second type of device map where the key is interpreted as
>> an ifindex and looked up using a hashmap, instead of being used as an array
>> index. This leads to maps being densely packed, so they can be smaller.
>>
>> The default maps used by xdp_redirect() are changed to use the new map
>> type, which means that xdp_redirect() is no longer limited to ifindex < 64,
>> but instead to 64 total simultaneous interfaces per network namespace. This
>> also provides an easy way to compare the performance of devmap and
>> devmap_idx:
>>
>> xdp_redirect_map (devmap): 8394560 pkt/s
>> xdp_redirect (devmap_idx): 8179480 pkt/s
>>
>> Difference: 215080 pkt/s or 3.1 nanoseconds per packet.
>
> Could you share what the ifindex mix was here, to arrive at these
> numbers? How does it compare to using an array but not keying with
> ifindex?
Just the standard set on my test machine; ifindex 1 through 9, except 8
in this case. So certainly no more than 1 ifindex in each hash bucket
for those numbers.
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
>> +static int dev_map_idx_update_elem(struct bpf_map *map, void *key, void *value,
>> + u64 map_flags)
>> +{
>> + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
>> + struct bpf_dtab_netdev *dev, *old_dev;
>> + u32 idx = *(u32 *)key;
>> + u32 val = *(u32 *)value;
>> + u32 bit;
>> +
>> + if (unlikely(map_flags > BPF_EXIST))
>> + return -EINVAL;
>> + if (unlikely(map_flags == BPF_NOEXIST))
>> + return -EEXIST;
>> +
>> + old_dev = __dev_map_idx_lookup_elem(map, idx);
>> + if (!val) {
>> + if (!old_dev)
>> + return 0;
>
> IMHO this is a fairly strange mix of array and hashmap semantics. I
> think you should stick to hashmap behaviour AFA flags and
> update/delete goes.
Yeah, the double book-keeping is a bit strange, but it allows the actual
forwarding and flush code to be reused between both types of maps. I
think this is worth the slight semantic confusion :)
-Toke
^ permalink raw reply
* Re: [PATCH RFC 3/5] sched/cpufreq: Fix incorrect RCU API usage
From: Rafael J. Wysocki @ 2019-02-21 23:05 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ido Schimmel,
Ingo Molnar, moderated list:INTEL ETHERNET DRIVERS,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, Lai Jiangshan,
Martin KaFai Lau, Mathieu Desnoyers, netdev, Paul E. McKenney,
Peter Zijlstra, rcu, Song Liu, Steven Rostedt, xdp-newbies,
Yonghong Song
In-Reply-To: <20190221054942.132388-4-joel@joelfernandes.org>
On Thursday, February 21, 2019 6:49:40 AM CET Joel Fernandes (Google) wrote:
> Recently I added an RCU annotation check to rcu_assign_pointer(). All
> pointers assigned to RCU protected data are to be annotated with __rcu
> inorder to be able to use rcu_assign_pointer() similar to checks in
> other RCU APIs.
>
> This resulted in a sparse error: kernel//sched/cpufreq.c:41:9: sparse:
> error: incompatible types in comparison expression (different address
> spaces)
>
> Fix this by using the correct APIs for RCU accesses. This will
> potentially avoid any future bugs in the code. If it is felt that RCU
> protection is not needed here, then the rcu_assign_pointer call can be
> dropped and replaced with, say, WRITE_ONCE or smp_store_release. Or, may
> be we add a new API to do it. But calls rcu_assign_pointer seems an
> abuse of the RCU API unless RCU is being used.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> kernel/sched/cpufreq.c | 8 ++++++--
> kernel/sched/sched.h | 2 +-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/cpufreq.c b/kernel/sched/cpufreq.c
> index 22bd8980f32f..c9aeb3bf5dc2 100644
> --- a/kernel/sched/cpufreq.c
> +++ b/kernel/sched/cpufreq.c
> @@ -7,7 +7,7 @@
> */
> #include "sched.h"
>
> -DEFINE_PER_CPU(struct update_util_data *, cpufreq_update_util_data);
> +DEFINE_PER_CPU(struct update_util_data __rcu *, cpufreq_update_util_data);
>
> /**
> * cpufreq_add_update_util_hook - Populate the CPU's update_util_data pointer.
> @@ -34,8 +34,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
> if (WARN_ON(!data || !func))
> return;
>
> - if (WARN_ON(per_cpu(cpufreq_update_util_data, cpu)))
> + rcu_read_lock();
> + if (WARN_ON(rcu_dereference(per_cpu(cpufreq_update_util_data, cpu)))) {
> + rcu_read_unlock();
> return;
> + }
> + rcu_read_unlock();
As Steve said, this is not a read-side critical section, so the rcu_read_lock()
and rcu_read_unlock() don't help.
But rcu_access_pointer() should work here AFAICS.
Cheers,
Rafael
^ permalink raw reply
* Re: [PATCH v1 iproute2-next 2/4] Sync up rdma_netlink.h
From: Jason Gunthorpe @ 2019-02-21 23:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Steve Wise, dsahern, leon, netdev, linux-rdma
In-Reply-To: <20190221105631.7dfa5217@shemminger-XPS-13-9360>
On Thu, Feb 21, 2019 at 10:56:31AM -0800, Stephen Hemminger wrote:
> On Thu, 21 Feb 2019 08:19:07 -0800
> Steve Wise <swise@opengridcomputing.com> wrote:
>
> > Pull in the latest rdma_netlink.h to get the RDMA_NLDEV_CMD_NEWLINK /
> > RDMA_NLDEV_CMD_DELLINK API.
> >
> > Signed-off-by: Steve Wise <swise@opengridcomputing.com>
> > rdma/include/uapi/rdma/rdma_netlink.h | 74 +++++++++++++++++++++++++++--------
> > 1 file changed, 58 insertions(+), 16 deletions(-)
> >
> > diff --git a/rdma/include/uapi/rdma/rdma_netlink.h b/rdma/include/uapi/rdma/rdma_netlink.h
> > index 04c80cebef49..23a90ad52485 100644
> > +++ b/rdma/include/uapi/rdma/rdma_netlink.h
> > @@ -5,8 +5,7 @@
> > #include <linux/types.h>
> >
> > enum {
> > - RDMA_NL_RDMA_CM = 1,
> > - RDMA_NL_IWCM,
> > + RDMA_NL_IWCM = 2,
> > RDMA_NL_RSVD,
> > RDMA_NL_LS, /* RDMA Local Services */
> > RDMA_NL_NLDEV, /* RDMA device interface */
>
> You can't just drop elements from user ABI headers.
> That is a break of kernel ABI guarantee.
The ABI didn't change..
We don't promise unlimited source code compatibility in the uapi
headers.
If the kernel doesn't support something better to remove all traces of
it so userspace users are aware of the change when their compile
breaks.
Jason
^ permalink raw reply
* [PATCH 1/2] nl80211: Allow change CW to Ad-Hock network
From: Andrea Greco @ 2019-02-21 23:11 UTC (permalink / raw)
To: johannes.berg
Cc: Andrea Greco, Johannes Berg, David S. Miller, linux-wireless,
netdev, linux-kernel
From: Andrea Greco <a.greco@4sigma.it>
Add net-link support for change CW in Ad-Hock network
Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
net/wireless/nl80211.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d91a408db113..4fcc63fa4380 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2731,7 +2731,8 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
if (netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
- netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
+ netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO &&
+ netdev->ieee80211_ptr->iftype != NL80211_IFTYPE_ADHOC)
return -EINVAL;
if (!netif_running(netdev))
--
2.17.2
^ permalink raw reply related
* [PATCH 2/2] ath9K: debugfs: Fix SPUR-DOWN field
From: Andrea Greco @ 2019-02-21 23:12 UTC (permalink / raw)
To: johannes.berg
Cc: Andrea Greco, QCA ath9k Development, Kalle Valo, David S. Miller,
linux-wireless, netdev, linux-kernel
From: Andrea Greco <a.greco@4sigma.it>
SPUR DOWN field return spurup inside of spurdown
Signed-off-by: Andrea Greco <a.greco@4sigma.it>
---
drivers/net/wireless/ath/ath9k/debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 4399e9ad058f..d4a2cdf9212c 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -148,7 +148,7 @@ static ssize_t read_file_ani(struct file *file, char __user *user_buf,
{ "OFDM LEVEL", ah->ani.ofdmNoiseImmunityLevel },
{ "CCK LEVEL", ah->ani.cckNoiseImmunityLevel },
{ "SPUR UP", ah->stats.ast_ani_spurup },
- { "SPUR DOWN", ah->stats.ast_ani_spurup },
+ { "SPUR DOWN", ah->stats.ast_ani_spurdown },
{ "OFDM WS-DET ON", ah->stats.ast_ani_ofdmon },
{ "OFDM WS-DET OFF", ah->stats.ast_ani_ofdmoff },
{ "MRC-CCK ON", ah->stats.ast_ani_ccklow },
--
2.17.2
^ permalink raw reply related
* Re: [PATCH v1 iproute2-next 3/4] rdma: add 'link add/delete' commands
From: Jason Gunthorpe @ 2019-02-21 23:14 UTC (permalink / raw)
To: Steve Wise; +Cc: dsahern, leon, stephen, netdev, linux-rdma
In-Reply-To: <5dd9d9aeada48bc5db0eb0394ed4e3ce38ee41bc.1550773362.git.swise@opengridcomputing.com>
On Thu, Feb 21, 2019 at 08:19:12AM -0800, Steve Wise wrote:
> Add new 'link' subcommand 'add' and 'delete' to allow binding a soft-rdma
> device to a netdev interface.
>
> EG:
>
> rdma link add rxe_eth0 type rxe netdev eth0
> rdma link delete rxe_eth0
This is great that we finally got here!
Jason
^ permalink raw reply
* Re: [PATCH net-next v3 0/3] net: stmmac: Performance improvements in Multi-Queue
From: David Miller @ 2019-02-21 23:42 UTC (permalink / raw)
To: jose.abreu
Cc: netdev, linux-kernel, f.fainelli, joao.pinto, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <cover.1550569066.git.joabreu@synopsys.com>
From: Jose Abreu <jose.abreu@synopsys.com>
Date: Tue, 19 Feb 2019 10:38:46 +0100
> Tested in XGMAC2 and GMAC5.
Series applied, thanks Jose.
^ permalink raw reply
* Re: [PATCH net-next 00/10] mlxsw: Support for shared buffers in Spectrum-2
From: David Miller @ 2019-02-21 23:58 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, mlxsw
In-Reply-To: <20190220193141.16498-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Wed, 20 Feb 2019 19:32:10 +0000
> Petr says:
>
> Spectrum-2 will be configured with a different set of pools than
> Spectrum-1, their sizes will be larger, and the individual quotas will
> be different as well. It is therefore necessary to make the shared
> buffer module aware of this dependence on chip type, and adjust the
> individual tables.
>
> In patch #1, introduce a structure for keeping per-chip immutable and
> default values.
>
> In patch #2, structures for keeping current values of SBPM and SBPR
> (pool configuration and port-pool quota) are allocated dynamically to
> support varying pool counts.
>
> In patches #3 to #7, uses of individual shared buffer configuration
> tables are migrated from global definitions to fields in struct
> mlxsw_sp_sb_vals, which was introduced above.
>
> Up until this point, the actual configuration is still the one suitable
> for Spectrum-1. In patch #8 Spectrum-2 configuration is added.
>
> In patch #9, port headroom configuration is changed to take into account
> current recommended value for a 100-Gbps port, and the split factor.
>
> In patch #10, requests for overlarge headroom are rejected. This avoids
> potential chip freeze should such overlarge requests be made.
Series applied, thanks.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox