Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH v8 4/6] mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
From: Michal Hocko @ 2015-08-28 14:31 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Andrew Morton, Vlastimil Babka, Jonathan Corbet,
	Kirill A. Shutemov, linux-alpha, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, sparclinux, linux-xtensa, linux-arch,
	linux-api, linux-mm
In-Reply-To: <1440613465-30393-5-git-send-email-emunson@akamai.com>

On Wed 26-08-15 14:24:23, Eric B Munson wrote:
> The previous patch introduced a flag that specified pages in a VMA
> should be placed on the unevictable LRU, but they should not be made
> present when the area is created.  This patch adds the ability to set
> this state via the new mlock system calls.
> 
> We add MLOCK_ONFAULT for mlock2 and MCL_ONFAULT for mlockall.
> MLOCK_ONFAULT will set the VM_LOCKONFAULT modifier for VM_LOCKED.
> MCL_ONFAULT should be used as a modifier to the two other mlockall
> flags.  When used with MCL_CURRENT, all current mappings will be marked
> with VM_LOCKED | VM_LOCKONFAULT.  When used with MCL_FUTURE, the
> mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.  When used
> with both MCL_CURRENT and MCL_FUTURE, all current mappings and
> mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.
> 
> Prior to this patch, mlockall() will unconditionally clear the
> mm->def_flags any time it is called without MCL_FUTURE.  This behavior
> is maintained after adding MCL_ONFAULT.  If a call to
> mlockall(MCL_FUTURE) is followed by mlockall(MCL_CURRENT), the
> mm->def_flags will be cleared and new VMAs will be unlocked.  This
> remains true with or without MCL_ONFAULT in either mlockall()
> invocation.
> 
> munlock() will unconditionally clear both vma flags.  munlockall()
> unconditionally clears for VMA flags on all VMAs and in the
> mm->def_flags field.
> 
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-xtensa@linux-xtensa.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-api@vger.kernel.org
> Cc: linux-mm@kvack.org

I haven't checked the arch specific bits but the core part looks good to
me.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  arch/alpha/include/uapi/asm/mman.h     |  3 ++
>  arch/mips/include/uapi/asm/mman.h      |  6 ++++
>  arch/parisc/include/uapi/asm/mman.h    |  3 ++
>  arch/powerpc/include/uapi/asm/mman.h   |  1 +
>  arch/sparc/include/uapi/asm/mman.h     |  1 +
>  arch/tile/include/uapi/asm/mman.h      |  1 +
>  arch/xtensa/include/uapi/asm/mman.h    |  6 ++++
>  include/uapi/asm-generic/mman-common.h |  5 ++++
>  include/uapi/asm-generic/mman.h        |  1 +
>  mm/mlock.c                             | 52 +++++++++++++++++++++++++---------
>  10 files changed, 66 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
> index 0086b47..f2f9496 100644
> --- a/arch/alpha/include/uapi/asm/mman.h
> +++ b/arch/alpha/include/uapi/asm/mman.h
> @@ -37,6 +37,9 @@
>  
>  #define MCL_CURRENT	 8192		/* lock all currently mapped pages */
>  #define MCL_FUTURE	16384		/* lock all additions to address space */
> +#define MCL_ONFAULT	32768		/* lock all pages that are faulted in */
> +
> +#define MLOCK_ONFAULT	0x01		/* Lock pages in range after they are faulted in, do not prefault */
>  
>  #define MADV_NORMAL	0		/* no further special treatment */
>  #define MADV_RANDOM	1		/* expect random page references */
> diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
> index cfcb876..97c03f4 100644
> --- a/arch/mips/include/uapi/asm/mman.h
> +++ b/arch/mips/include/uapi/asm/mman.h
> @@ -61,6 +61,12 @@
>   */
>  #define MCL_CURRENT	1		/* lock all current mappings */
>  #define MCL_FUTURE	2		/* lock all future mappings */
> +#define MCL_ONFAULT	4		/* lock all pages that are faulted in */
> +
> +/*
> + * Flags for mlock
> + */
> +#define MLOCK_ONFAULT	0x01		/* Lock pages in range after they are faulted in, do not prefault */
>  
>  #define MADV_NORMAL	0		/* no further special treatment */
>  #define MADV_RANDOM	1		/* expect random page references */
> diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
> index 294d251..ecc3ae1 100644
> --- a/arch/parisc/include/uapi/asm/mman.h
> +++ b/arch/parisc/include/uapi/asm/mman.h
> @@ -31,6 +31,9 @@
>  
>  #define MCL_CURRENT	1		/* lock all current mappings */
>  #define MCL_FUTURE	2		/* lock all future mappings */
> +#define MCL_ONFAULT	4		/* lock all pages that are faulted in */
> +
> +#define MLOCK_ONFAULT	0x01		/* Lock pages in range after they are faulted in, do not prefault */
>  
>  #define MADV_NORMAL     0               /* no further special treatment */
>  #define MADV_RANDOM     1               /* expect random page references */
> diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
> index 6ea26df..03c06ba 100644
> --- a/arch/powerpc/include/uapi/asm/mman.h
> +++ b/arch/powerpc/include/uapi/asm/mman.h
> @@ -22,6 +22,7 @@
>  
>  #define MCL_CURRENT     0x2000          /* lock all currently mapped pages */
>  #define MCL_FUTURE      0x4000          /* lock all additions to address space */
> +#define MCL_ONFAULT	0x8000		/* lock all pages that are faulted in */
>  
>  #define MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
>  #define MAP_NONBLOCK	0x10000		/* do not block on IO */
> diff --git a/arch/sparc/include/uapi/asm/mman.h b/arch/sparc/include/uapi/asm/mman.h
> index 0b14df3..9765896 100644
> --- a/arch/sparc/include/uapi/asm/mman.h
> +++ b/arch/sparc/include/uapi/asm/mman.h
> @@ -17,6 +17,7 @@
>  
>  #define MCL_CURRENT     0x2000          /* lock all currently mapped pages */
>  #define MCL_FUTURE      0x4000          /* lock all additions to address space */
> +#define MCL_ONFAULT	0x8000		/* lock all pages that are faulted in */
>  
>  #define MAP_POPULATE	0x8000		/* populate (prefault) pagetables */
>  #define MAP_NONBLOCK	0x10000		/* do not block on IO */
> diff --git a/arch/tile/include/uapi/asm/mman.h b/arch/tile/include/uapi/asm/mman.h
> index 81b8fc3..63ee13f 100644
> --- a/arch/tile/include/uapi/asm/mman.h
> +++ b/arch/tile/include/uapi/asm/mman.h
> @@ -36,6 +36,7 @@
>   */
>  #define MCL_CURRENT	1		/* lock all current mappings */
>  #define MCL_FUTURE	2		/* lock all future mappings */
> +#define MCL_ONFAULT	4		/* lock all pages that are faulted in */
>  
>  
>  #endif /* _ASM_TILE_MMAN_H */
> diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
> index 201aec0..360944e 100644
> --- a/arch/xtensa/include/uapi/asm/mman.h
> +++ b/arch/xtensa/include/uapi/asm/mman.h
> @@ -74,6 +74,12 @@
>   */
>  #define MCL_CURRENT	1		/* lock all current mappings */
>  #define MCL_FUTURE	2		/* lock all future mappings */
> +#define MCL_ONFAULT	4		/* lock all pages that are faulted in */
> +
> +/*
> + * Flags for mlock
> + */
> +#define MLOCK_ONFAULT	0x01		/* Lock pages in range after they are faulted in, do not prefault */
>  
>  #define MADV_NORMAL	0		/* no further special treatment */
>  #define MADV_RANDOM	1		/* expect random page references */
> diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
> index ddc3b36..a74dd84 100644
> --- a/include/uapi/asm-generic/mman-common.h
> +++ b/include/uapi/asm-generic/mman-common.h
> @@ -25,6 +25,11 @@
>  # define MAP_UNINITIALIZED 0x0		/* Don't support this flag */
>  #endif
>  
> +/*
> + * Flags for mlock
> + */
> +#define MLOCK_ONFAULT	0x01		/* Lock pages in range after they are faulted in, do not prefault */
> +
>  #define MS_ASYNC	1		/* sync memory asynchronously */
>  #define MS_INVALIDATE	2		/* invalidate the caches */
>  #define MS_SYNC		4		/* synchronous memory sync */
> diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
> index e9fe6fd..7162cd4 100644
> --- a/include/uapi/asm-generic/mman.h
> +++ b/include/uapi/asm-generic/mman.h
> @@ -17,5 +17,6 @@
>  
>  #define MCL_CURRENT	1		/* lock all current mappings */
>  #define MCL_FUTURE	2		/* lock all future mappings */
> +#define MCL_ONFAULT	4		/* lock all pages that are faulted in */
>  
>  #endif /* __ASM_GENERIC_MMAN_H */
> diff --git a/mm/mlock.c b/mm/mlock.c
> index 7efe27d..0747663 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -506,7 +506,8 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev,
>  
>  	if (newflags == vma->vm_flags || (vma->vm_flags & VM_SPECIAL) ||
>  	    is_vm_hugetlb_page(vma) || vma == get_gate_vma(current->mm))
> -		goto out;	/* don't set VM_LOCKED,  don't count */
> +		/* don't set VM_LOCKED or VM_LOCKONFAULT and don't count */
> +		goto out;
>  
>  	pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT);
>  	*prev = vma_merge(mm, *prev, start, end, newflags, vma->anon_vma,
> @@ -576,7 +577,7 @@ static int apply_vma_lock_flags(unsigned long start, size_t len,
>  		prev = vma;
>  
>  	for (nstart = start ; ; ) {
> -		vm_flags_t newflags = vma->vm_flags & ~VM_LOCKED;
> +		vm_flags_t newflags = vma->vm_flags & VM_LOCKED_CLEAR_MASK;
>  
>  		newflags |= flags;
>  
> @@ -645,10 +646,15 @@ SYSCALL_DEFINE2(mlock, unsigned long, start, size_t, len)
>  
>  SYSCALL_DEFINE3(mlock2, unsigned long, start, size_t, len, int, flags)
>  {
> -	if (flags)
> +	vm_flags_t vm_flags = VM_LOCKED;
> +
> +	if (flags & ~MLOCK_ONFAULT)
>  		return -EINVAL;
>  
> -	return do_mlock(start, len, VM_LOCKED);
> +	if (flags & MLOCK_ONFAULT)
> +		vm_flags |= VM_LOCKONFAULT;
> +
> +	return do_mlock(start, len, vm_flags);
>  }
>  
>  SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
> @@ -665,24 +671,43 @@ SYSCALL_DEFINE2(munlock, unsigned long, start, size_t, len)
>  	return ret;
>  }
>  
> +/*
> + * Take the MCL_* flags passed into mlockall (or 0 if called from munlockall)
> + * and translate into the appropriate modifications to mm->def_flags and/or the
> + * flags for all current VMAs.
> + *
> + * There are a couple of subtleties with this.  If mlockall() is called multiple
> + * times with different flags, the values do not necessarily stack.  If mlockall
> + * is called once including the MCL_FUTURE flag and then a second time without
> + * it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_flags.
> + */
>  static int apply_mlockall_flags(int flags)
>  {
>  	struct vm_area_struct * vma, * prev = NULL;
> +	vm_flags_t to_add = 0;
>  
> -	if (flags & MCL_FUTURE)
> +	current->mm->def_flags &= VM_LOCKED_CLEAR_MASK;
> +	if (flags & MCL_FUTURE) {
>  		current->mm->def_flags |= VM_LOCKED;
> -	else
> -		current->mm->def_flags &= ~VM_LOCKED;
>  
> -	if (flags == MCL_FUTURE)
> -		goto out;
> +		if (flags & MCL_ONFAULT)
> +			current->mm->def_flags |= VM_LOCKONFAULT;
> +
> +		if (!(flags & MCL_CURRENT))
> +			goto out;
> +	}
> +
> +	if (flags & MCL_CURRENT) {
> +		to_add |= VM_LOCKED;
> +		if (flags & MCL_ONFAULT)
> +			to_add |= VM_LOCKONFAULT;
> +	}
>  
>  	for (vma = current->mm->mmap; vma ; vma = prev->vm_next) {
>  		vm_flags_t newflags;
>  
> -		newflags = vma->vm_flags & ~VM_LOCKED;
> -		if (flags & MCL_CURRENT)
> -			newflags |= VM_LOCKED;
> +		newflags = vma->vm_flags & VM_LOCKED_CLEAR_MASK;
> +		newflags |= to_add;
>  
>  		/* Ignore errors */
>  		mlock_fixup(vma, &prev, vma->vm_start, vma->vm_end, newflags);
> @@ -697,7 +722,8 @@ SYSCALL_DEFINE1(mlockall, int, flags)
>  	unsigned long lock_limit;
>  	int ret = -EINVAL;
>  
> -	if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE)))
> +	if (!flags || (flags & ~(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT)) ||
> +	    flags == MCL_ONFAULT)
>  		goto out;
>  
>  	ret = -EPERM;
> -- 
> 1.9.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v8 4/6] mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
From: Michal Hocko @ 2015-08-28 14:32 UTC (permalink / raw)
  To: Eric B Munson
  Cc: Andrew Morton, Vlastimil Babka, Jonathan Corbet,
	Kirill A. Shutemov, linux-alpha, linux-kernel, linux-mips,
	linux-parisc, linuxppc-dev, sparclinux, linux-xtensa, linux-arch,
	linux-api, linux-mm
In-Reply-To: <20150828143130.GE5301@dhcp22.suse.cz>

On Fri 28-08-15 16:31:30, Michal Hocko wrote:
> On Wed 26-08-15 14:24:23, Eric B Munson wrote:
> > The previous patch introduced a flag that specified pages in a VMA
> > should be placed on the unevictable LRU, but they should not be made
> > present when the area is created.  This patch adds the ability to set
> > this state via the new mlock system calls.
> > 
> > We add MLOCK_ONFAULT for mlock2 and MCL_ONFAULT for mlockall.
> > MLOCK_ONFAULT will set the VM_LOCKONFAULT modifier for VM_LOCKED.
> > MCL_ONFAULT should be used as a modifier to the two other mlockall
> > flags.  When used with MCL_CURRENT, all current mappings will be marked
> > with VM_LOCKED | VM_LOCKONFAULT.  When used with MCL_FUTURE, the
> > mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.  When used
> > with both MCL_CURRENT and MCL_FUTURE, all current mappings and
> > mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.
> > 
> > Prior to this patch, mlockall() will unconditionally clear the
> > mm->def_flags any time it is called without MCL_FUTURE.  This behavior
> > is maintained after adding MCL_ONFAULT.  If a call to
> > mlockall(MCL_FUTURE) is followed by mlockall(MCL_CURRENT), the
> > mm->def_flags will be cleared and new VMAs will be unlocked.  This
> > remains true with or without MCL_ONFAULT in either mlockall()
> > invocation.

Btw. I think we really want a man page for this new mlock call.
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT
From: Vlastimil Babka @ 2015-08-28 14:54 UTC (permalink / raw)
  To: Eric B Munson, Andrew Morton
  Cc: Michal Hocko, Jonathan Corbet, Kirill A. Shutemov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1440613465-30393-4-git-send-email-emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>

On 08/26/2015 08:24 PM, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings.  If only portions of the mapping will be
> used this can incur a high penalty for locking.
>
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well).  For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
>
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in.  The VM_LOCKONFAULT flag will be used together with
> VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> be added to the unevictable LRU when they are faulted or if they are
> already present, but will not cause any missing pages to be faulted in.
>
> Exposing this new lock state means that we cannot overload the meaning
> of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> to mean that the VMA for a fault was locked.  This means we need the
> new FOLL_MLOCK flag to communicate the locked state of a VMA.
> FOLL_POPULATE will now only control if the VMA should be populated and
> in the case of VM_LOCKONFAULT, it will not be set.
>
> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> Cc: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
> Cc: Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>
> Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> Cc: "Kirill A. Shutemov" <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Acked-by: Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>

I just wonder if the call to populate_vma_page_range from mprotect_fixup 
is just an potentially expensive no-op for VM_LOCKONFAULT vma's? It 
might find many cow candidates but faultin_page() won't do anything. And 
it shouldn't find any existing pages to put on the unevictable list from 
this context.

But it's a corner case and preventing it would mean putting in another 
VM_LOCKONFAULT check so maybe we can leave it like this.
-

^ permalink raw reply

* [PATCH v6.1 3/6] task_isolation: support PR_TASK_ISOLATION_STRICT mode
From: Chris Metcalf @ 2015-08-28 15:31 UTC (permalink / raw)
  To: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon, linux-doc, linux-api,
	linux-kernel
  Cc: Chris Metcalf
In-Reply-To: <20150826103651.GA30466@arm.com>

With task_isolation mode, the task is in principle guaranteed not to
be interrupted by the kernel, but only if it behaves.  In particular,
if it enters the kernel via system call, page fault, or any of a
number of other synchronous traps, it may be unexpectedly exposed
to long latencies.  Add a simple flag that puts the process into
a state where any such kernel entry is fatal.

To allow the state to be entered and exited, we ignore the prctl()
syscall so that we can clear the bit again later, and we ignore
exit/exit_group to allow exiting the task without a pointless signal
killing you as you try to do so.

This change adds the syscall-detection hooks only for x86, arm64,
and tile.  For arm64 we use an explict #ifdef CONFIG_TASK_ISOLATION
so we can both achieve no overhead for !TASK_ISOLATION, but also
achieve low latency (test TIF_NOHZ first) for TASK_ISOLATION.

The signature of context_tracking_exit() changes to report whether
we, in fact, are exiting back to user space, so that we can track
user exceptions properly separately from other kernel entries.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
This "v6.1" is just a tweak to the existing v6 series to reflect
Will Deacon's suggestions about the arm64 syscall entry code.
I've updated the git tree with this updated patch in the series.
A more disruptive change would be to capture the thread flags
up front like x86 and tile, which allows the test itself to be
optimized away if the task_isolation call becomes a no-op.

 arch/arm64/kernel/ptrace.c       |  6 ++++++
 arch/tile/kernel/ptrace.c        |  5 ++++-
 arch/x86/kernel/ptrace.c         |  2 ++
 include/linux/context_tracking.h | 11 ++++++++---
 include/linux/isolation.h        | 16 ++++++++++++++++
 include/uapi/linux/prctl.h       |  1 +
 kernel/context_tracking.c        |  9 ++++++---
 kernel/isolation.c               | 38 ++++++++++++++++++++++++++++++++++++++
 8 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index d882b833dbdb..5d4284445f70 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -37,6 +37,7 @@
 #include <linux/regset.h>
 #include <linux/tracehook.h>
 #include <linux/elf.h>
+#include <linux/isolation.h>
 
 #include <asm/compat.h>
 #include <asm/debug-monitors.h>
@@ -1154,6 +1155,11 @@ asmlinkage int syscall_trace_enter(struct pt_regs *regs)
 	if (secure_computing() == -1)
 		return -1;
 
+#ifdef CONFIG_TASK_ISOLATION
+	if (test_thread_flag(TIF_NOHZ) && task_isolation_strict())
+		task_isolation_syscall(regs->syscallno);
+#endif
+
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER);
 
diff --git a/arch/tile/kernel/ptrace.c b/arch/tile/kernel/ptrace.c
index f84eed8243da..c327cb918a44 100644
--- a/arch/tile/kernel/ptrace.c
+++ b/arch/tile/kernel/ptrace.c
@@ -259,8 +259,11 @@ int do_syscall_trace_enter(struct pt_regs *regs)
 	 * If TIF_NOHZ is set, we are required to call user_exit() before
 	 * doing anything that could touch RCU.
 	 */
-	if (work & _TIF_NOHZ)
+	if (work & _TIF_NOHZ) {
 		user_exit();
+		if (task_isolation_strict())
+			task_isolation_syscall(regs->regs[TREG_SYSCALL_NR]);
+	}
 
 	if (work & _TIF_SYSCALL_TRACE) {
 		if (tracehook_report_syscall_entry(regs))
diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
index 9be72bc3613f..2f9ce9466daf 100644
--- a/arch/x86/kernel/ptrace.c
+++ b/arch/x86/kernel/ptrace.c
@@ -1479,6 +1479,8 @@ unsigned long syscall_trace_enter_phase1(struct pt_regs *regs, u32 arch)
 	if (work & _TIF_NOHZ) {
 		user_exit();
 		work &= ~_TIF_NOHZ;
+		if (task_isolation_strict())
+			task_isolation_syscall(regs->orig_ax);
 	}
 
 #ifdef CONFIG_SECCOMP
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
index b96bd299966f..e0ac0228fea1 100644
--- a/include/linux/context_tracking.h
+++ b/include/linux/context_tracking.h
@@ -3,6 +3,7 @@
 
 #include <linux/sched.h>
 #include <linux/vtime.h>
+#include <linux/isolation.h>
 #include <linux/context_tracking_state.h>
 #include <asm/ptrace.h>
 
@@ -11,7 +12,7 @@
 extern void context_tracking_cpu_set(int cpu);
 
 extern void context_tracking_enter(enum ctx_state state);
-extern void context_tracking_exit(enum ctx_state state);
+extern bool context_tracking_exit(enum ctx_state state);
 extern void context_tracking_user_enter(void);
 extern void context_tracking_user_exit(void);
 
@@ -35,8 +36,12 @@ static inline enum ctx_state exception_enter(void)
 		return 0;
 
 	prev_ctx = this_cpu_read(context_tracking.state);
-	if (prev_ctx != CONTEXT_KERNEL)
-		context_tracking_exit(prev_ctx);
+	if (prev_ctx != CONTEXT_KERNEL) {
+		if (context_tracking_exit(prev_ctx)) {
+			if (task_isolation_strict())
+				task_isolation_exception();
+		}
+	}
 
 	return prev_ctx;
 }
diff --git a/include/linux/isolation.h b/include/linux/isolation.h
index fd04011b1c1e..27a4469831c1 100644
--- a/include/linux/isolation.h
+++ b/include/linux/isolation.h
@@ -15,10 +15,26 @@ static inline bool task_isolation_enabled(void)
 }
 
 extern void task_isolation_enter(void);
+extern void task_isolation_syscall(int nr);
+extern void task_isolation_exception(void);
 extern void task_isolation_wait(void);
 #else
 static inline bool task_isolation_enabled(void) { return false; }
 static inline void task_isolation_enter(void) { }
+static inline void task_isolation_syscall(int nr) { }
+static inline void task_isolation_exception(void) { }
 #endif
 
+static inline bool task_isolation_strict(void)
+{
+#ifdef CONFIG_TASK_ISOLATION
+	if (tick_nohz_full_cpu(smp_processor_id()) &&
+	    (current->task_isolation_flags &
+	     (PR_TASK_ISOLATION_ENABLE | PR_TASK_ISOLATION_STRICT)) ==
+	    (PR_TASK_ISOLATION_ENABLE | PR_TASK_ISOLATION_STRICT))
+		return true;
+#endif
+	return false;
+}
+
 #endif
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index 79da784fe17a..e16e13911e8a 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -194,5 +194,6 @@ struct prctl_mm_map {
 #define PR_SET_TASK_ISOLATION		47
 #define PR_GET_TASK_ISOLATION		48
 # define PR_TASK_ISOLATION_ENABLE	(1 << 0)
+# define PR_TASK_ISOLATION_STRICT	(1 << 1)
 
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index c57c99f5c4d7..17a71f7b66b8 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -147,15 +147,16 @@ NOKPROBE_SYMBOL(context_tracking_user_enter);
  * This call supports re-entrancy. This way it can be called from any exception
  * handler without needing to know if we came from userspace or not.
  */
-void context_tracking_exit(enum ctx_state state)
+bool context_tracking_exit(enum ctx_state state)
 {
 	unsigned long flags;
+	bool from_user = false;
 
 	if (!context_tracking_is_enabled())
-		return;
+		return false;
 
 	if (in_interrupt())
-		return;
+		return false;
 
 	local_irq_save(flags);
 	if (!context_tracking_recursion_enter())
@@ -169,6 +170,7 @@ void context_tracking_exit(enum ctx_state state)
 			 */
 			rcu_user_exit();
 			if (state == CONTEXT_USER) {
+				from_user = true;
 				vtime_user_exit(current);
 				trace_user_exit(0);
 			}
@@ -178,6 +180,7 @@ void context_tracking_exit(enum ctx_state state)
 	context_tracking_recursion_exit();
 out_irq_restore:
 	local_irq_restore(flags);
+	return from_user;
 }
 NOKPROBE_SYMBOL(context_tracking_exit);
 EXPORT_SYMBOL_GPL(context_tracking_exit);
diff --git a/kernel/isolation.c b/kernel/isolation.c
index d4618cd9e23d..a89a6e9adfb4 100644
--- a/kernel/isolation.c
+++ b/kernel/isolation.c
@@ -10,6 +10,7 @@
 #include <linux/swap.h>
 #include <linux/vmstat.h>
 #include <linux/isolation.h>
+#include <asm/unistd.h>
 #include "time/tick-sched.h"
 
 /*
@@ -73,3 +74,40 @@ void task_isolation_enter(void)
 		dump_stack();
 	}
 }
+
+static void kill_task_isolation_strict_task(void)
+{
+	dump_stack();
+	current->task_isolation_flags &= ~PR_TASK_ISOLATION_ENABLE;
+	send_sig(SIGKILL, current, 1);
+}
+
+/*
+ * This routine is called from syscall entry (with the syscall number
+ * passed in) if the _STRICT flag is set.
+ */
+void task_isolation_syscall(int syscall)
+{
+	/* Ignore prctl() syscalls or any task exit. */
+	switch (syscall) {
+	case __NR_prctl:
+	case __NR_exit:
+	case __NR_exit_group:
+		return;
+	}
+
+	pr_warn("%s/%d: task_isolation strict mode violated by syscall %d\n",
+		current->comm, current->pid, syscall);
+	kill_task_isolation_strict_task();
+}
+
+/*
+ * This routine is called from any userspace exception if the _STRICT
+ * flag is set.
+ */
+void task_isolation_exception(void)
+{
+	pr_warn("%s/%d: task_isolation strict mode violated by exception\n",
+		current->comm, current->pid);
+	kill_task_isolation_strict_task();
+}
-- 
2.1.2


^ permalink raw reply related

* Re: [PATCH v6 4/6] task_isolation: provide strict mode configurable signal
From: Andy Lutomirski @ 2015-08-28 19:22 UTC (permalink / raw)
  To: Chris Metcalf
  Cc: Gilad Ben Yossef, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
	Andrew Morton, Rik van Riel, Tejun Heo, Frederic Weisbecker,
	Thomas Gleixner, Paul E. McKenney, Christoph Lameter,
	Viresh Kumar, Catalin Marinas, Will Deacon,
	linux-doc@vger.kernel.org, Linux API,
	linux-kernel@vger.kernel.org
In-Reply-To: <1440532555-15492-5-git-send-email-cmetcalf@ezchip.com>

On Tue, Aug 25, 2015 at 12:55 PM, Chris Metcalf <cmetcalf@ezchip.com> wrote:
> Allow userspace to override the default SIGKILL delivered
> when a task_isolation process in STRICT mode does a syscall
> or otherwise synchronously enters the kernel.
>
> In addition to being able to set the signal, we now also
> pass whether or not the interruption was from a syscall in
> the si_code field of the siginfo.
>
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
> ---
>  include/uapi/linux/prctl.h |  2 ++
>  kernel/isolation.c         | 17 +++++++++++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
> index e16e13911e8a..2a4ddc890e22 100644
> --- a/include/uapi/linux/prctl.h
> +++ b/include/uapi/linux/prctl.h
> @@ -195,5 +195,7 @@ struct prctl_mm_map {
>  #define PR_GET_TASK_ISOLATION          48
>  # define PR_TASK_ISOLATION_ENABLE      (1 << 0)
>  # define PR_TASK_ISOLATION_STRICT      (1 << 1)
> +# define PR_TASK_ISOLATION_SET_SIG(sig)        (((sig) & 0x7f) << 8)
> +# define PR_TASK_ISOLATION_GET_SIG(bits) (((bits) >> 8) & 0x7f)
>
>  #endif /* _LINUX_PRCTL_H */
> diff --git a/kernel/isolation.c b/kernel/isolation.c
> index a89a6e9adfb4..b776aa632c8f 100644
> --- a/kernel/isolation.c
> +++ b/kernel/isolation.c
> @@ -75,11 +75,20 @@ void task_isolation_enter(void)
>         }
>  }
>
> -static void kill_task_isolation_strict_task(void)
> +static void kill_task_isolation_strict_task(int is_syscall)
>  {
> +       siginfo_t info = {};
> +       int sig;
> +
>         dump_stack();
>         current->task_isolation_flags &= ~PR_TASK_ISOLATION_ENABLE;
> -       send_sig(SIGKILL, current, 1);
> +
> +       sig = PR_TASK_ISOLATION_GET_SIG(current->task_isolation_flags);
> +       if (sig == 0)
> +               sig = SIGKILL;
> +       info.si_signo = sig;
> +       info.si_code = is_syscall;
> +       send_sig_info(sig, &info, current);

The stuff you're doing here is sufficiently nasty that I think you
should add something like:

rcu_lockdep_assert(rcu_is_watching(), "some message here");

Because as it stands this is just asking for trouble.

For the record, I am *extremely* unhappy with the state of the context
tracking hooks.

--Andy

^ permalink raw reply

* Re: [PATCH v8 3/6] mm: Introduce VM_LOCKONFAULT
From: Eric B Munson @ 2015-08-28 19:34 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, Vlastimil Babka, Jonathan Corbet,
	Kirill A. Shutemov, linux-kernel, linux-mm, linux-api
In-Reply-To: <20150828141829.GD5301@dhcp22.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 5386 bytes --]

On Fri, 28 Aug 2015, Michal Hocko wrote:

> On Wed 26-08-15 14:24:22, Eric B Munson wrote:
> > The cost of faulting in all memory to be locked can be very high when
> > working with large mappings.  If only portions of the mapping will be
> > used this can incur a high penalty for locking.
> > 
> > For the example of a large file, this is the usage pattern for a large
> > statical language model (probably applies to other statical or graphical
> > models as well).  For the security example, any application transacting
> > in data that cannot be swapped out (credit card data, medical records,
> > etc).
> > 
> > This patch introduces the ability to request that pages are not
> > pre-faulted, but are placed on the unevictable LRU when they are finally
> > faulted in.  The VM_LOCKONFAULT flag will be used together with
> > VM_LOCKED and has no effect when set without VM_LOCKED.  Setting the
> > VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> > be added to the unevictable LRU when they are faulted or if they are
> > already present, but will not cause any missing pages to be faulted in.
> 
> OK, I can live with this. Thank you for removing the part which exports
> the flag to the userspace.
>  
> > Exposing this new lock state means that we cannot overload the meaning
> > of the FOLL_POPULATE flag any longer.  Prior to this patch it was used
> > to mean that the VMA for a fault was locked.  This means we need the
> > new FOLL_MLOCK flag to communicate the locked state of a VMA.
> > FOLL_POPULATE will now only control if the VMA should be populated and
> > in the case of VM_LOCKONFAULT, it will not be set.
> 
> I thinking that this part is really unnecessary. populate_vma_page_range
> could have simply returned without calling gup for VM_LOCKONFAULT
> vmas. You would save the pte walk and the currently mapped pages would
> be still protected from the reclaim. The side effect would be that they
> would litter the regular LRUs and mlock/unevictable counters wouldn't be
> updated until those pages are encountered during the reclaim and culled
> to unevictable list.
> 
> I would expect that mlock with this flag would be typically called
> on mostly unpopulated mappings so the side effects would be barely
> noticeable while the lack of pte walk would be really nice (especially
> for the large mappings).
> 
> This would be a nice optimization and minor code reduction but I am not
> going to insist on it. I will leave the decision to you.

If I am understanding you correctly, this is how the lock on fault set
started.  Jon Corbet pointed out that this would leave pages which were
present when mlock2(MLOCK_ONFAULT) was called in an unlocked state, only
locking them after they were reclaimed and then refaulted.

Even if this was never the case, we scan the entire range for a call to
mlock() and will lock the pages which are present.  Why would we pay the
cost of getting the accounting right on the present pages for mlock, but
not lock on fault?

> 
> > Signed-off-by: Eric B Munson <emunson@akamai.com>
> > Cc: Michal Hocko <mhocko@suse.cz>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-mm@kvack.org
> > Cc: linux-api@vger.kernel.org
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 
> One note below:
> 
> > ---
> > Changes from v7:
> > *Drop entries in smaps and dri code to avoid exposing VM_LOCKONFAULT to
> >  userspace.  VM_LOCKONFAULT is still exposed via mm/debug.c
> > *Create VM_LOCKED_CLEAR_MASK to be used anywhere we want to clear all
> >  flags relating to locked VMAs
> > 
> >  include/linux/mm.h |  5 +++++
> >  kernel/fork.c      |  2 +-
> >  mm/debug.c         |  1 +
> >  mm/gup.c           | 10 ++++++++--
> >  mm/huge_memory.c   |  2 +-
> >  mm/hugetlb.c       |  4 ++--
> >  mm/mlock.c         |  2 +-
> >  mm/mmap.c          |  2 +-
> >  mm/rmap.c          |  6 ++++--
> >  9 files changed, 24 insertions(+), 10 deletions(-)
> [...]
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index 171b687..14ce002 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -744,7 +744,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> >  
> >  		if (vma->vm_flags & VM_LOCKED) {
> >  			spin_unlock(ptl);
> > -			pra->vm_flags |= VM_LOCKED;
> > +			pra->vm_flags |=
> > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> >  			return SWAP_FAIL; /* To break the loop */
> >  		}
> >  
> > @@ -765,7 +766,8 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
> >  
> >  		if (vma->vm_flags & VM_LOCKED) {
> >  			pte_unmap_unlock(pte, ptl);
> > -			pra->vm_flags |= VM_LOCKED;
> > +			pra->vm_flags |=
> > +				(vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT));
> >  			return SWAP_FAIL; /* To break the loop */
> >  		}
> 
> Why do we need to export this? Neither of the consumers care and should
> care. VM_LOCKONFAULT should never be set without VM_LOCKED which is the
> only thing that we should care about.

I exported VM_LOCKONFAULT because this is an internal interface and I
saw no harm in doing so.  I do not have a use case for it at the moment,
so I would be fine dropping this hunk.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [PATCH v2 net-next 0/3] ipv4: Hash-based multipath routing
From: pch @ 2015-08-28 20:00 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, linux-api, Roopa Prabhu,
	Scott Feldman, Eric W. Biederman, Nicolas Dichtel, Thomas Graf,
	Jiri Benc, Peter Nørlund

When the routing cache was removed in 3.6, the IPv4 multipath algorithm changed
from more or less being destination-based into being quasi-random per-packet
scheduling. This increases the risk of out-of-order packets and makes it
impossible to use multipath together with anycast services.

This patch series seeks to extend the multipath system to support both L3 and
L4-based multipath while still supporting per-packet multipath.

The multipath algorithm is set as a per-route attribute (RTA_MP_ALGO) with some
degree of binary compatibility with the old implementation (2.6.12 - 2.6.22),
but without source level compatibility since attributes have different names:

RT_MP_ALG_L3_HASH:
L3 hash-based distribution. This was IP_MP_ALG_NONE, which with the route
cached behaved somewhat like L3-based distribution. This is the new default.

RT_MP_ALG_PER_PACKET:
Per-packet distribution. Was IP_MP_ALG_RR. Uses round-robin.

RT_MP_ALG_DRR, RT_MP_ALG_RANDOM, RT_MP_ALG_WRANDOM:
Unsupported values, but reserved because they existed in 2.6.12 - 2.6.22.

RT_MP_ALG_L4_HASH:
L4 hash-based distribution. This is new.

The traditional modulo approach is replaced by a threshold-based approach,
described in RFC 2992. This reduces disruption in case of link failures or
route changes.

To better support anycast environments where PMTU usually breaks with
multipath, certain ICMP packets are hashed using the IP addresses within the
ICMP payload when using L3 hashing. This ensures that ICMP packets are routed
over the same path as the flow they belong to. It is not enabled with L4
hashing, since we can only consistently rely on L4 information, when PMTU is
used, and PMTU may be used in one direction while not being used in the other.

As a side effect, the multipath spinlock was removed and the code got faster.
I measured ip_mkroute_input (excl. __mkroute_input) on a Xeon X3350 (4 cores,
2.66GHz) with two paths:

Old per-packet: ~393.9 cycles(tsc)
New per-packet:  ~75.2 cycles(tsc)
New L3:         ~107.9 cycles(tsc)
New L4:         ~129.1 cycles(tsc)

The timings are approximately the same with a single core, except for the old
per-packet which gets faster (~199.8 cycles) most likely because there is no
contention on the spinlock.

If this patch is accepted, a follow-up patch to iproute2 will also be
submitted.

Changes in v2:
- Replaced 8-bit xor hash with 31-bit jenkins hash
- Don't scale weights (since 31-bit)
- Avoided unnecesary renaming of variables
- Rely on DF-bit instead of fragment offset when checking for fragmentation
- upper_bound is now inclusive to avoid overflow
- Use a callback to postpone extracting flow information until necessary
- Skipped ICMP inspection entirely with L4 hashing
- Handle newly added sysctl ignore_routes_with_linkdown

Best Regards,
 Peter Nørlund

Peter Nørlund (3):
      ipv4: Lock-less per-packet multipath
      ipv4: L3 and L4 hash-based multipath routing
      ipv4: ICMP packet inspection for L3 multipath

 include/net/ip_fib.h           |  26 ++++++-
 include/net/route.h            |  12 ++-
 include/uapi/linux/rtnetlink.h |  14 +++-
 net/ipv4/Kconfig               |   1 +
 net/ipv4/fib_frontend.c        |   4 +
 net/ipv4/fib_semantics.c       | 168 ++++++++++++++++++++++++++---------------
 net/ipv4/icmp.c                |  34 ++++++++-
 net/ipv4/route.c               | 112 +++++++++++++++++++++++++--
 8 files changed, 298 insertions(+), 73 deletions(-)

^ permalink raw reply

* [PATCH v2 net-next 1/3] ipv4: Lock-less per-packet multipath
From: pch-chEQUL3jiZBWk0Htik3J/w @ 2015-08-28 20:00 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Roopa Prabhu, Scott Feldman,
	Eric W. Biederman, Nicolas Dichtel, Thomas Graf, Jiri Benc,
	Peter Nørlund
In-Reply-To: <1440792050-2109-1-git-send-email-pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>

From: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>

The current multipath attempted to be quasi random, but in most cases it
behaved just like a round robin balancing. This patch refactors the
algorithm to be exactly that and in doing so, avoids the spin lock.

The new design paves the way for hash-based multipath, replacing the
modulo with thresholds, minimizing disruption in case of failing paths or
route replacements.

Signed-off-by: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
---
 include/net/ip_fib.h     |   4 +-
 net/ipv4/Kconfig         |   1 +
 net/ipv4/fib_semantics.c | 129 ++++++++++++++++++++++++++---------------------
 3 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index a37d043..18a3c7f 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -79,7 +79,7 @@ struct fib_nh {
 	unsigned char		nh_scope;
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	int			nh_weight;
-	int			nh_power;
+	atomic_t		nh_upper_bound;
 #endif
 #ifdef CONFIG_IP_ROUTE_CLASSID
 	__u32			nh_tclassid;
@@ -118,7 +118,7 @@ struct fib_info {
 #define fib_advmss fib_metrics[RTAX_ADVMSS-1]
 	int			fib_nhs;
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
-	int			fib_power;
+	int			fib_weight;
 #endif
 	struct rcu_head		rcu;
 	struct fib_nh		fib_nh[0];
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 416dfa0..23d8b2b 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -81,6 +81,7 @@ config IP_MULTIPLE_TABLES
 config IP_ROUTE_MULTIPATH
 	bool "IP: equal cost multipath"
 	depends on IP_ADVANCED_ROUTER
+	select BITREVERSE
 	help
 	  Normally, the routing tables specify a single action to be taken in
 	  a deterministic manner for a given packet. If you say Y here
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 1b2d011..becb63f 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -15,6 +15,7 @@
 
 #include <asm/uaccess.h>
 #include <linux/bitops.h>
+#include <linux/bitrev.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/jiffies.h>
@@ -58,7 +59,7 @@ static struct hlist_head fib_info_devhash[DEVINDEX_HASHSIZE];
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 
-static DEFINE_SPINLOCK(fib_multipath_lock);
+static DEFINE_PER_CPU(int, fib_multipath_rr_counter);
 
 #define for_nexthops(fi) {						\
 	int nhsel; const struct fib_nh *nh;				\
@@ -468,6 +469,52 @@ static int fib_count_nexthops(struct rtnexthop *rtnh, int remaining)
 	return remaining > 0 ? 0 : nhs;
 }
 
+static void fib_rebalance(struct fib_info *fi)
+{
+	int total;
+	int w;
+	struct in_device *in_dev;
+
+	if (fi->fib_nhs < 2)
+		return;
+
+	total = 0;
+	for_nexthops(fi) {
+		if (nh->nh_flags & RTNH_F_DEAD)
+			continue;
+
+		in_dev = __in_dev_get_rcu(nh->nh_dev);
+
+		if (in_dev &&
+		    IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+		    nh->nh_flags & RTNH_F_LINKDOWN)
+			continue;
+
+		total += nh->nh_weight;
+	} endfor_nexthops(fi);
+
+	w = 0;
+	change_nexthops(fi) {
+		int upper_bound;
+
+		in_dev = __in_dev_get_rcu(nexthop_nh->nh_dev);
+
+		if (nexthop_nh->nh_flags & RTNH_F_DEAD) {
+			upper_bound = -1;
+		} else if (in_dev &&
+			   IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
+			   nexthop_nh->nh_flags & RTNH_F_LINKDOWN) {
+			upper_bound = -1;
+		} else {
+			w += nexthop_nh->nh_weight;
+			upper_bound = DIV_ROUND_CLOSEST(2147483648LL * w,
+							total) - 1;
+		}
+
+		atomic_set(&nexthop_nh->nh_upper_bound, upper_bound);
+	} endfor_nexthops(fi);
+}
+
 static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
 		       int remaining, struct fib_config *cfg)
 {
@@ -1077,8 +1124,15 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
 
 	change_nexthops(fi) {
 		fib_info_update_nh_saddr(net, nexthop_nh);
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+		fi->fib_weight += nexthop_nh->nh_weight;
+#endif
 	} endfor_nexthops(fi)
 
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+	fib_rebalance(fi);
+#endif
+
 link_it:
 	ofi = fib_find_info(fi);
 	if (ofi) {
@@ -1300,12 +1354,6 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event)
 					nexthop_nh->nh_flags |= RTNH_F_LINKDOWN;
 					break;
 				}
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
-				spin_lock_bh(&fib_multipath_lock);
-				fi->fib_power -= nexthop_nh->nh_power;
-				nexthop_nh->nh_power = 0;
-				spin_unlock_bh(&fib_multipath_lock);
-#endif
 				dead++;
 			}
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
@@ -1328,6 +1376,10 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event)
 			}
 			ret++;
 		}
+
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+		fib_rebalance(fi);
+#endif
 	}
 
 	return ret;
@@ -1450,20 +1502,17 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
 			    !__in_dev_get_rtnl(dev))
 				continue;
 			alive++;
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
-			spin_lock_bh(&fib_multipath_lock);
-			nexthop_nh->nh_power = 0;
-			nexthop_nh->nh_flags &= ~nh_flags;
-			spin_unlock_bh(&fib_multipath_lock);
-#else
 			nexthop_nh->nh_flags &= ~nh_flags;
-#endif
 		} endfor_nexthops(fi)
 
 		if (alive > 0) {
 			fi->fib_flags &= ~nh_flags;
 			ret++;
 		}
+
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+		fib_rebalance(fi);
+#endif
 	}
 
 	return ret;
@@ -1478,55 +1527,19 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
 void fib_select_multipath(struct fib_result *res)
 {
 	struct fib_info *fi = res->fi;
-	struct in_device *in_dev;
-	int w;
-
-	spin_lock_bh(&fib_multipath_lock);
-	if (fi->fib_power <= 0) {
-		int power = 0;
-		change_nexthops(fi) {
-			in_dev = __in_dev_get_rcu(nexthop_nh->nh_dev);
-			if (nexthop_nh->nh_flags & RTNH_F_DEAD)
-				continue;
-			if (in_dev &&
-			    IN_DEV_IGNORE_ROUTES_WITH_LINKDOWN(in_dev) &&
-			    nexthop_nh->nh_flags & RTNH_F_LINKDOWN)
-				continue;
-			power += nexthop_nh->nh_weight;
-			nexthop_nh->nh_power = nexthop_nh->nh_weight;
-		} endfor_nexthops(fi);
-		fi->fib_power = power;
-		if (power <= 0) {
-			spin_unlock_bh(&fib_multipath_lock);
-			/* Race condition: route has just become dead. */
-			res->nh_sel = 0;
-			return;
-		}
-	}
+	int h;
 
+	h = bitrev32(this_cpu_inc_return(fib_multipath_rr_counter)) >> 1;
 
-	/* w should be random number [0..fi->fib_power-1],
-	 * it is pretty bad approximation.
-	 */
-
-	w = jiffies % fi->fib_power;
+	for_nexthops(fi) {
+		if (h > atomic_read(&nh->nh_upper_bound))
+			continue;
 
-	change_nexthops(fi) {
-		if (!(nexthop_nh->nh_flags & RTNH_F_DEAD) &&
-		    nexthop_nh->nh_power) {
-			w -= nexthop_nh->nh_power;
-			if (w <= 0) {
-				nexthop_nh->nh_power--;
-				fi->fib_power--;
-				res->nh_sel = nhsel;
-				spin_unlock_bh(&fib_multipath_lock);
-				return;
-			}
-		}
-	} endfor_nexthops(fi);
+		res->nh_sel = nhsel;
+		return;
+	} endfor_nexthops(fi)
 
 	/* Race condition: route has just become dead. */
 	res->nh_sel = 0;
-	spin_unlock_bh(&fib_multipath_lock);
 }
 #endif
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 2/3] ipv4: L3 and L4 hash-based multipath routing
From: pch-chEQUL3jiZBWk0Htik3J/w @ 2015-08-28 20:00 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy,
	linux-api-u79uwXL29TY76Z2rM5mHXA, Roopa Prabhu, Scott Feldman,
	Eric W. Biederman, Nicolas Dichtel, Thomas Graf, Jiri Benc,
	Peter Nørlund
In-Reply-To: <1440792050-2109-1-git-send-email-pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>

From: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>

This patch adds L3 and L4 hash-based multipath routing, selectable on a
per-route basis with the reintroduced RTA_MP_ALGO attribute. The default is
now RT_MP_ALG_L3_HASH.

Signed-off-by: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
---
 include/net/ip_fib.h           | 22 ++++++++++++++++-
 include/uapi/linux/rtnetlink.h | 14 ++++++++++-
 net/ipv4/fib_frontend.c        |  4 +++
 net/ipv4/fib_semantics.c       | 43 +++++++++++++++++++++++++++-----
 net/ipv4/route.c               | 56 ++++++++++++++++++++++++++++++++++++++++--
 5 files changed, 129 insertions(+), 10 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 18a3c7f..21e74b5 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -37,6 +37,7 @@ struct fib_config {
 	u32			fc_flags;
 	u32			fc_priority;
 	__be32			fc_prefsrc;
+	int			fc_mp_alg;
 	struct nlattr		*fc_mx;
 	struct rtnexthop	*fc_mp;
 	int			fc_mx_len;
@@ -119,6 +120,7 @@ struct fib_info {
 	int			fib_nhs;
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	int			fib_weight;
+	int			fib_mp_alg;
 #endif
 	struct rcu_head		rcu;
 	struct fib_nh		fib_nh[0];
@@ -312,7 +314,25 @@ int ip_fib_check_default(__be32 gw, struct net_device *dev);
 int fib_sync_down_dev(struct net_device *dev, unsigned long event);
 int fib_sync_down_addr(struct net *net, __be32 local);
 int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
-void fib_select_multipath(struct fib_result *res);
+
+struct multipath_flow4 {
+	__be32 saddr;
+	__be32 daddr;
+	union {
+		__be32 ports;
+		struct {
+			__be16 sport;
+			__be16 dport;
+		};
+	};
+};
+
+typedef void (*multipath_flow4_func_t)(struct multipath_flow4 *flow,
+				       void *ctx);
+
+void fib_select_multipath(struct fib_result *res,
+			  multipath_flow4_func_t flow_func,
+			  void *ctx);
 
 /* Exported by fib_trie.c */
 void fib_trie_init(void);
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 0d3d3cc..2563a96 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -271,6 +271,18 @@ enum rt_scope_t {
 #define RTM_F_EQUALIZE		0x400	/* Multipath equalizer: NI	*/
 #define RTM_F_PREFIX		0x800	/* Prefix addresses		*/
 
+/* Multipath algorithms */
+
+enum rt_mp_alg_t {
+	RT_MP_ALG_L3_HASH,	/* Was IP_MP_ALG_NONE */
+	RT_MP_ALG_PER_PACKET,	/* Was IP_MP_ALG_RR */
+	RT_MP_ALG_DRR,		/* not used */
+	RT_MP_ALG_RANDOM,	/* not used */
+	RT_MP_ALG_WRANDOM,	/* not used */
+	RT_MP_ALG_L4_HASH,
+	__RT_MP_ALG_MAX
+};
+
 /* Reserved table identifiers */
 
 enum rt_class_t {
@@ -301,7 +313,7 @@ enum rtattr_type_t {
 	RTA_FLOW,
 	RTA_CACHEINFO,
 	RTA_SESSION, /* no longer used */
-	RTA_MP_ALGO, /* no longer used */
+	RTA_MP_ALGO,
 	RTA_TABLE,
 	RTA_MARK,
 	RTA_MFC_STATS,
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 7fa2771..5ba4442 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -622,6 +622,7 @@ const struct nla_policy rtm_ipv4_policy[RTA_MAX + 1] = {
 	[RTA_PREFSRC]		= { .type = NLA_U32 },
 	[RTA_METRICS]		= { .type = NLA_NESTED },
 	[RTA_MULTIPATH]		= { .len = sizeof(struct rtnexthop) },
+	[RTA_MP_ALGO]		= { .type = NLA_U32 },
 	[RTA_FLOW]		= { .type = NLA_U32 },
 	[RTA_ENCAP_TYPE]	= { .type = NLA_U16 },
 	[RTA_ENCAP]		= { .type = NLA_NESTED },
@@ -684,6 +685,9 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
 			cfg->fc_mp = nla_data(attr);
 			cfg->fc_mp_len = nla_len(attr);
 			break;
+		case RTA_MP_ALGO:
+			cfg->fc_mp_alg = nla_get_u32(attr);
+			break;
 		case RTA_FLOW:
 			cfg->fc_flow = nla_get_u32(attr);
 			break;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index becb63f..3a80b1a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -259,6 +259,11 @@ static inline int nh_comp(const struct fib_info *fi, const struct fib_info *ofi)
 {
 	const struct fib_nh *onh = ofi->fib_nh;
 
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+	if (fi->fib_mp_alg != ofi->fib_mp_alg)
+		return -1;
+#endif
+
 	for_nexthops(fi) {
 		if (nh->nh_oif != onh->nh_oif ||
 		    nh->nh_gw  != onh->nh_gw ||
@@ -1028,6 +1033,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
 
 	if (cfg->fc_mp) {
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
+		fi->fib_mp_alg = cfg->fc_mp_alg;
 		err = fib_get_nhs(fi, cfg->fc_mp, cfg->fc_mp_len, cfg);
 		if (err != 0)
 			goto failure;
@@ -1245,6 +1251,10 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
 		struct rtnexthop *rtnh;
 		struct nlattr *mp;
 
+		if (fi->fib_mp_alg &&
+		    nla_put_u32(skb, RTA_MP_ALGO, fi->fib_mp_alg))
+			goto nla_put_failure;
+
 		mp = nla_nest_start(skb, RTA_MULTIPATH);
 		if (!mp)
 			goto nla_put_failure;
@@ -1520,16 +1530,37 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags)
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 
-/*
- * The algorithm is suboptimal, but it provides really
- * fair weighted route distribution.
- */
-void fib_select_multipath(struct fib_result *res)
+/* Compute multipath hash based on 3- or 5-tuple */
+static int fib_multipath_hash(const struct fib_result *res,
+			      multipath_flow4_func_t flow_func, void *ctx)
+{
+	struct multipath_flow4 flow;
+
+	flow_func(&flow, ctx);
+
+	if (res->fi->fib_mp_alg == RT_MP_ALG_L4_HASH)
+		return jhash_3words(flow.saddr, flow.daddr, flow.ports, 0) >> 1;
+	else
+		return jhash_2words(flow.saddr, flow.daddr, 0) >> 1;
+}
+
+static int fib_multipath_perpacket(void)
+{
+	return bitrev32(this_cpu_inc_return(fib_multipath_rr_counter)) >> 1;
+}
+
+void fib_select_multipath(struct fib_result *res,
+			  multipath_flow4_func_t flow_func,
+			  void *ctx)
 {
 	struct fib_info *fi = res->fi;
 	int h;
 
-	h = bitrev32(this_cpu_inc_return(fib_multipath_rr_counter)) >> 1;
+	if (res->fi->fib_mp_alg == RT_MP_ALG_PER_PACKET) {
+		h = fib_multipath_perpacket();
+	} else {
+		h = fib_multipath_hash(res, flow_func, ctx);
+	}
 
 	for_nexthops(fi) {
 		if (h > atomic_read(&nh->nh_upper_bound))
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f3087aa..f50f84f 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1643,6 +1643,58 @@ out:
 	return err;
 }
 
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+
+/* Fill multipath flow key data based on socket buffer */
+static void ip_multipath_flow_skb(struct multipath_flow4 *flow, void *ctx)
+{
+	const struct sk_buff *skb = (const struct sk_buff *)ctx;
+	const struct iphdr *iph;
+
+	iph = ip_hdr(skb);
+
+	flow->saddr = iph->saddr;
+	flow->daddr = iph->daddr;
+	flow->ports = 0;
+
+	if (unlikely(!(iph->frag_off & htons(IP_DF))))
+		return;
+
+	if (iph->protocol == IPPROTO_TCP ||
+	    iph->protocol == IPPROTO_UDP ||
+	    iph->protocol == IPPROTO_SCTP) {
+		__be16 _ports[2];
+		const __be16 *ports;
+
+		ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
+					   &_ports);
+		if (ports) {
+			flow->sport = ports[0];
+			flow->dport = ports[1];
+		}
+	}
+}
+
+/* Fill multipath flow key data based on flowi4  */
+static void ip_multipath_flow_fl4(struct multipath_flow4 *flow, void *ctx)
+{
+	const struct flowi4 *fl4 = (const struct flowi4 *)ctx;
+
+	flow->saddr = fl4->saddr;
+	flow->daddr = fl4->daddr;
+
+	if (fl4->flowi4_proto == IPPROTO_TCP ||
+	    fl4->flowi4_proto == IPPROTO_UDP ||
+	    fl4->flowi4_proto == IPPROTO_SCTP) {
+		flow->sport = fl4->fl4_sport;
+		flow->dport = fl4->fl4_dport;
+	} else {
+		flow->ports = 0;
+	}
+}
+
+#endif /* CONFIG_IP_ROUTE_MULTIPATH */
+
 static int ip_mkroute_input(struct sk_buff *skb,
 			    struct fib_result *res,
 			    const struct flowi4 *fl4,
@@ -1651,7 +1703,7 @@ static int ip_mkroute_input(struct sk_buff *skb,
 {
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	if (res->fi && res->fi->fib_nhs > 1)
-		fib_select_multipath(res);
+		fib_select_multipath(res, ip_multipath_flow_skb, skb);
 #endif
 
 	/* create a routing cache entry */
@@ -2197,7 +2249,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 	if (res.fi->fib_nhs > 1 && fl4->flowi4_oif == 0)
-		fib_select_multipath(&res);
+		fib_select_multipath(&res, ip_multipath_flow_fl4, fl4);
 	else
 #endif
 	if (!res.prefixlen &&
-- 
2.1.4

^ permalink raw reply related

* [PATCH v2 net-next 3/3] ipv4: ICMP packet inspection for L3 multipath
From: pch @ 2015-08-28 20:00 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, linux-api, Roopa Prabhu,
	Scott Feldman, Eric W. Biederman, Nicolas Dichtel, Thomas Graf,
	Jiri Benc, Peter Nørlund
In-Reply-To: <1440792050-2109-1-git-send-email-pch@ordbogen.com>

From: Peter Nørlund <pch@ordbogen.com>

When doing L3 based multipath, ICMP packets are inspected to let them route
over the same path as the flow they relate to, allowing anycast
environments to work with ECMP.

Signed-off-by: Peter Nørlund <pch@ordbogen.com>
---
 include/net/ip_fib.h     |  2 +-
 include/net/route.h      | 12 ++++++-
 net/ipv4/fib_semantics.c |  2 +-
 net/ipv4/icmp.c          | 34 +++++++++++++++++++-
 net/ipv4/route.c         | 82 ++++++++++++++++++++++++++++++++++++++----------
 5 files changed, 112 insertions(+), 20 deletions(-)

diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 21e74b5..3e5d4ed 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -328,7 +328,7 @@ struct multipath_flow4 {
 };
 
 typedef void (*multipath_flow4_func_t)(struct multipath_flow4 *flow,
-				       void *ctx);
+				       enum rt_mp_alg_t algo, void *ctx);
 
 void fib_select_multipath(struct fib_result *res,
 			  multipath_flow4_func_t flow_func,
diff --git a/include/net/route.h b/include/net/route.h
index 395d79b..ccb85fc 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -28,6 +28,7 @@
 #include <net/inetpeer.h>
 #include <net/flow.h>
 #include <net/inet_sock.h>
+#include <net/ip_fib.h>
 #include <linux/in_route.h>
 #include <linux/rtnetlink.h>
 #include <linux/rcupdate.h>
@@ -110,7 +111,16 @@ struct in_device;
 int ip_rt_init(void);
 void rt_cache_flush(struct net *net);
 void rt_flush_dev(struct net_device *dev);
-struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp);
+struct rtable *__ip_route_output_key_flow(struct net *, struct flowi4 *flp,
+					  multipath_flow4_func_t flow_func,
+					  void *ctx);
+
+static inline struct rtable *__ip_route_output_key(struct net *net,
+						   struct flowi4 *flp)
+{
+	return __ip_route_output_key_flow(net, flp, NULL, NULL);
+}
+
 struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
 				    struct sock *sk);
 struct dst_entry *ipv4_blackhole_route(struct net *net,
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 3a80b1a..000c535 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1536,7 +1536,7 @@ static int fib_multipath_hash(const struct fib_result *res,
 {
 	struct multipath_flow4 flow;
 
-	flow_func(&flow, ctx);
+	flow_func(&flow, res->fi->fib_mp_alg, ctx);
 
 	if (res->fi->fib_mp_alg == RT_MP_ALG_L4_HASH)
 		return jhash_3words(flow.saddr, flow.daddr, flow.ports, 0) >> 1;
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index f16488e..0e25fe4 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -439,6 +439,38 @@ out_unlock:
 	icmp_xmit_unlock(sk);
 }
 
+/* Source and destination is swapped. See ip_multipath_flow_skb */
+static void icmp_multipath_flow(struct multipath_flow4 *flow,
+				enum rt_mp_alg_t algo, void *ctx)
+{
+	const struct sk_buff *skb = (const struct sk_buff *)ctx;
+	const struct iphdr *iph = ip_hdr(skb);
+
+	flow->saddr = iph->daddr;
+	flow->daddr = iph->saddr;
+	flow->ports = 0;
+
+	if (algo == RT_MP_ALG_L4_HASH)
+		return;
+
+	if (unlikely(!(iph->frag_off & htons(IP_DF))))
+		return;
+
+	if (iph->protocol == IPPROTO_TCP ||
+	    iph->protocol == IPPROTO_UDP ||
+	    iph->protocol == IPPROTO_SCTP) {
+		__be16 _ports[2];
+		const __be16 *ports;
+
+		ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
+					   &_ports);
+		if (ports) {
+			flow->sport = ports[1];
+			flow->dport = ports[0];
+		}
+	}
+}
+
 static struct rtable *icmp_route_lookup(struct net *net,
 					struct flowi4 *fl4,
 					struct sk_buff *skb_in,
@@ -463,7 +495,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
 	fl4->flowi4_oif = vrf_master_ifindex(skb_in->dev) ? : skb_in->dev->ifindex;
 
 	security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
-	rt = __ip_route_output_key(net, fl4);
+	rt = __ip_route_output_key_flow(net, fl4, icmp_multipath_flow, skb_in);
 	if (IS_ERR(rt))
 		return rt;
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f50f84f..edbeb56 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1646,37 +1646,82 @@ out:
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
 
 /* Fill multipath flow key data based on socket buffer */
-static void ip_multipath_flow_skb(struct multipath_flow4 *flow, void *ctx)
+static void ip_multipath_flow_skb(struct multipath_flow4 *flow,
+				  enum rt_mp_alg_t algo, void *ctx)
 {
 	const struct sk_buff *skb = (const struct sk_buff *)ctx;
-	const struct iphdr *iph;
+	struct icmphdr _icmph;
+	struct iphdr _inner_iph;
+	const struct iphdr *outer_iph;
+	const struct icmphdr *icmph;
+	const struct iphdr *inner_iph;
+	unsigned int offset;
 
-	iph = ip_hdr(skb);
+	outer_iph = ip_hdr(skb);
 
-	flow->saddr = iph->saddr;
-	flow->daddr = iph->daddr;
+	flow->saddr = outer_iph->saddr;
+	flow->daddr = outer_iph->daddr;
 	flow->ports = 0;
 
-	if (unlikely(!(iph->frag_off & htons(IP_DF))))
-		return;
+	offset = outer_iph->ihl * 4;
 
-	if (iph->protocol == IPPROTO_TCP ||
-	    iph->protocol == IPPROTO_UDP ||
-	    iph->protocol == IPPROTO_SCTP) {
+	if (algo == RT_MP_ALG_L4_HASH) {
 		__be16 _ports[2];
 		const __be16 *ports;
 
-		ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
+		if (unlikely(!(outer_iph->frag_off & htons(IP_DF))))
+			return;
+
+		if (outer_iph->protocol != IPPROTO_TCP &&
+		    outer_iph->protocol != IPPROTO_UDP &&
+		    outer_iph->protocol != IPPROTO_SCTP) {
+			return;
+		}
+
+		ports = skb_header_pointer(skb, offset, sizeof(_ports),
 					   &_ports);
 		if (ports) {
 			flow->sport = ports[0];
 			flow->dport = ports[1];
 		}
+
+		return;
+	}
+
+	if (outer_iph->protocol != IPPROTO_ICMP)
+		return;
+
+	if (unlikely((outer_iph->frag_off & htons(IP_OFFSET)) != 0))
+		return;
+
+	icmph = skb_header_pointer(skb, offset, sizeof(_icmph), &_icmph);
+	if (!icmph)
+		return;
+
+	if (icmph->type != ICMP_DEST_UNREACH &&
+	    icmph->type != ICMP_SOURCE_QUENCH &&
+	    icmph->type != ICMP_REDIRECT &&
+	    icmph->type != ICMP_TIME_EXCEEDED &&
+	    icmph->type != ICMP_PARAMETERPROB) {
+		return;
 	}
+
+	offset += sizeof(_icmph);
+	inner_iph = skb_header_pointer(skb, offset, sizeof(_inner_iph),
+				       &_inner_iph);
+	if (!inner_iph)
+		return;
+
+	/* Since the ICMP payload contains a packet sent from the current
+	 * recipient, we swap source and destination addresses
+	 */
+	flow->saddr = inner_iph->daddr;
+	flow->daddr = inner_iph->saddr;
 }
 
 /* Fill multipath flow key data based on flowi4  */
-static void ip_multipath_flow_fl4(struct multipath_flow4 *flow, void *ctx)
+static void ip_multipath_flow_fl4(struct multipath_flow4 *flow,
+				  enum rt_mp_alg_t algo, void *ctx)
 {
 	const struct flowi4 *fl4 = (const struct flowi4 *)ctx;
 
@@ -2086,7 +2131,9 @@ add:
  * Major route resolver routine.
  */
 
-struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
+struct rtable *__ip_route_output_key_flow(struct net *net, struct flowi4 *fl4,
+					  multipath_flow4_func_t flow_func,
+					  void *ctx)
 {
 	struct net_device *dev_out = NULL;
 	__u8 tos = RT_FL_TOS(fl4);
@@ -2248,9 +2295,12 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
 	}
 
 #ifdef CONFIG_IP_ROUTE_MULTIPATH
-	if (res.fi->fib_nhs > 1 && fl4->flowi4_oif == 0)
-		fib_select_multipath(&res, ip_multipath_flow_fl4, fl4);
-	else
+	if (res.fi->fib_nhs > 1 && fl4->flowi4_oif == 0) {
+		if (flow_func)
+			fib_select_multipath(&res, flow_func, ctx);
+		else
+			fib_select_multipath(&res, ip_multipath_flow_fl4, fl4);
+	} else
 #endif
 	if (!res.prefixlen &&
 	    res.table->tb_num_default > 1 &&
-- 
2.1.4

^ permalink raw reply related

* Re: [RFC v6 01/40] vfs: Add IS_ACL() and IS_RICHACL() tests
From: J. Bruce Fields @ 2015-08-28 20:34 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-2-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Aug 04, 2015 at 01:52:59PM +0200, Andreas Gruenbacher wrote:
> The vfs does not apply the umask for file systems that support acls. The
> test used for this used to be called IS_POSIXACL(). Switch to a new
> IS_ACL() test to check for either posix acls or richacls instead. Add a new
> MS_RICHACL flag and IS_RICHACL() test for richacls alone. The IS_POSIXACL()
> test is still needed by file systems that specifically support POSIX ACLs,
> like nfsd.

Nit: nfsd's not a filesystem.  Maybe just list the users instead.

--b.

> 
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/Kconfig              |  3 +++
>  fs/namei.c              |  8 ++++----
>  include/linux/fs.h      | 12 ++++++++++++
>  include/uapi/linux/fs.h |  3 ++-
>  4 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 011f433..3e09c06 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -59,6 +59,9 @@ endif # BLOCK
>  config FS_POSIX_ACL
>  	def_bool n
>  
> +config FS_RICHACL
> +	def_bool n
> +
>  config EXPORTFS
>  	tristate
>  
> diff --git a/fs/namei.c b/fs/namei.c
> index fbbcf09..8f3db24 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -2771,7 +2771,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
>  	}
>  
>  	mode = op->mode;
> -	if ((open_flag & O_CREAT) && !IS_POSIXACL(dir))
> +	if ((open_flag & O_CREAT) && !IS_ACL(dir))
>  		mode &= ~current_umask();
>  
>  	excl = (open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT);
> @@ -2955,7 +2955,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
>  	/* Negative dentry, just create the file */
>  	if (!dentry->d_inode && (op->open_flag & O_CREAT)) {
>  		umode_t mode = op->mode;
> -		if (!IS_POSIXACL(dir->d_inode))
> +		if (!IS_ACL(dir->d_inode))
>  			mode &= ~current_umask();
>  		/*
>  		 * This write is needed to ensure that a
> @@ -3526,7 +3526,7 @@ retry:
>  	if (IS_ERR(dentry))
>  		return PTR_ERR(dentry);
>  
> -	if (!IS_POSIXACL(path.dentry->d_inode))
> +	if (!IS_ACL(path.dentry->d_inode))
>  		mode &= ~current_umask();
>  	error = security_path_mknod(&path, dentry, mode, dev);
>  	if (error)
> @@ -3595,7 +3595,7 @@ retry:
>  	if (IS_ERR(dentry))
>  		return PTR_ERR(dentry);
>  
> -	if (!IS_POSIXACL(path.dentry->d_inode))
> +	if (!IS_ACL(path.dentry->d_inode))
>  		mode &= ~current_umask();
>  	error = security_path_mkdir(&path, dentry, mode);
>  	if (!error)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index cc008c3..44e696e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1770,6 +1770,12 @@ struct super_operations {
>  #define IS_IMMUTABLE(inode)	((inode)->i_flags & S_IMMUTABLE)
>  #define IS_POSIXACL(inode)	__IS_FLG(inode, MS_POSIXACL)
>  
> +#ifdef CONFIG_FS_RICHACL
> +#define IS_RICHACL(inode)	__IS_FLG(inode, MS_RICHACL)
> +#else
> +#define IS_RICHACL(inode)	0
> +#endif
> +
>  #define IS_DEADDIR(inode)	((inode)->i_flags & S_DEAD)
>  #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
>  #define IS_SWAPFILE(inode)	((inode)->i_flags & S_SWAPFILE)
> @@ -1783,6 +1789,12 @@ struct super_operations {
>  				 (inode)->i_rdev == WHITEOUT_DEV)
>  
>  /*
> + * IS_ACL() tells the VFS to not apply the umask
> + * and use check_acl for acl permission checks when defined.
> + */
> +#define IS_ACL(inode)		__IS_FLG(inode, MS_POSIXACL | MS_RICHACL)
> +
> +/*
>   * Inode state bits.  Protected by inode->i_lock
>   *
>   * Three bits determine the dirty state of the inode, I_DIRTY_SYNC,
> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 9b964a5..6ac6bc9 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -81,7 +81,7 @@ struct inodes_stat_t {
>  #define MS_VERBOSE	32768	/* War is peace. Verbosity is silence.
>  				   MS_VERBOSE is deprecated. */
>  #define MS_SILENT	32768
> -#define MS_POSIXACL	(1<<16)	/* VFS does not apply the umask */
> +#define MS_POSIXACL	(1<<16)	/* Supports POSIX ACLs */
>  #define MS_UNBINDABLE	(1<<17)	/* change to unbindable */
>  #define MS_PRIVATE	(1<<18)	/* change to private */
>  #define MS_SLAVE	(1<<19)	/* change to slave */
> @@ -91,6 +91,7 @@ struct inodes_stat_t {
>  #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
>  #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
>  #define MS_LAZYTIME	(1<<25) /* Update the on-disk [acm]times lazily */
> +#define MS_RICHACL	(1<<26) /* Supports richacls */
>  
>  /* These sb flags are internal to the kernel */
>  #define MS_NOSEC	(1<<28)
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: J. Bruce Fields @ 2015-08-28 20:44 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel, linux-fsdevel, linux-nfs, linux-api, linux-cifs,
	linux-security-module, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-4-git-send-email-agruenba@redhat.com>

On Tue, Aug 04, 2015 at 01:53:01PM +0200, Andreas Gruenbacher wrote:
> Normally, deleting a file requires write and execute access to the parent
> directory.  With Richacls, a process with MAY_DELETE_SELF access to a file
> may delete the file even without write access to the parent directory.
> 
> To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
> when checking for delete access inside a directory, and MAY_DELETE_SELF
> when checking for delete access to a file itelf.
> 
> The MAY_DELETE_SELF permission does not override the sticky directory
> check.  It probably should.

I guess it would basically just let the file owner delegate permission
to delete your file to a non-owner?  Makes sense to me to allow that.

--b.

> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/namei.c         | 15 +++++++++++----
>  include/linux/fs.h |  2 ++
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 3504d36..2ac759c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -454,7 +454,7 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>   * changing the "normal" UIDs which are used for other things.
>   *
>   * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> - * MAY_WRITE must also be set in @mask.
> + * MAY_DELETE_CHILD, MAY_DELETE_SELF, MAY_WRITE must also be set in @mask.
>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2527,7 +2527,7 @@ static int may_delete(struct inode *dir, struct dentry *victim,
>  		      bool isdir, bool replace)
>  {
>  	struct inode *inode = d_backing_inode(victim);
> -	int error, mask = MAY_WRITE | MAY_EXEC;
> +	int error, mask = MAY_EXEC;
>  
>  	if (d_is_negative(victim))
>  		return -ENOENT;
> @@ -2537,8 +2537,15 @@ static int may_delete(struct inode *dir, struct dentry *victim,
>  	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>  
>  	if (replace)
> -		mask |= isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
> -	error = inode_permission(dir, mask);
> +		mask |= MAY_WRITE | (isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE);
> +	error = inode_permission(dir, mask | MAY_WRITE | MAY_DELETE_CHILD);
> +	if (error && IS_RICHACL(inode)) {
> +		/* Deleting is also permitted with MAY_EXEC on the directory
> +		 * and MAY_DELETE_SELF on the inode.  */
> +		if (!inode_permission(inode, MAY_DELETE_SELF) &&
> +		    !inode_permission(dir, mask))
> +			error = 0;
> +	}
>  	if (error)
>  		return error;
>  	if (IS_APPEND(dir))
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 9c44f27..abf5b0e 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -83,6 +83,8 @@ typedef void (dax_iodone_t)(struct buffer_head *bh_map, int uptodate);
>  #define MAY_NOT_BLOCK		0x00000080
>  #define MAY_CREATE_FILE		0x00000100
>  #define MAY_CREATE_DIR		0x00000200
> +#define MAY_DELETE_CHILD	0x00000400
> +#define MAY_DELETE_SELF		0x00000800
>  
>  /*
>   * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: Andreas Gruenbacher @ 2015-08-28 21:08 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Andreas Gruenbacher, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150828204413.GB23326-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>

2015-08-28 22:44 GMT+02:00 J. Bruce Fields <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>:
>> The MAY_DELETE_SELF permission does not override the sticky
>> directory check.  It probably should.
>
> I guess it would basically just let the file owner delegate permission
> to delete your file to a non-owner?  Makes sense to me to allow that.

Yes, independent of whether or not the process has MAY_DELETE_CHILD
access on the directory but not independent of the sticky directory
check, which is a bit of a weird combination.

Andreas

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: Andy Lutomirski @ 2015-08-28 21:36 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux FS Devel, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Linux API,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, LSM List, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-4-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Aug 4, 2015 at 4:53 AM, Andreas Gruenbacher
<andreas.gruenbacher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Normally, deleting a file requires write and execute access to the parent
> directory.  With Richacls, a process with MAY_DELETE_SELF access to a file
> may delete the file even without write access to the parent directory.
>
> To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
> when checking for delete access inside a directory, and MAY_DELETE_SELF
> when checking for delete access to a file itelf.
>
> The MAY_DELETE_SELF permission does not override the sticky directory
> check.  It probably should.

Silly question from the peanut gallery: is there any such thing as
opening an fd pointing at a file such that the "open file description"
(i.e. the struct file) captures the right to delete the file?

IOW do we need FMODE_DELETE_SELF?

--Andy

^ permalink raw reply

* Re: [RFC v6 10/40] richacl: Permission check algorithm
From: J. Bruce Fields @ 2015-08-28 21:49 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA, Andreas Gruenbacher
In-Reply-To: <1438689218-6921-11-git-send-email-agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Tue, Aug 04, 2015 at 01:53:08PM +0200, Andreas Gruenbacher wrote:
> A richacl roughly grants a requested access if the NFSv4 acl in the
> richacl grants the requested permissions according to the NFSv4
> permission check algorithm and the file mask that applies to the process
> includes the requested permissions.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/Makefile             |   2 +-
>  fs/richacl_inode.c      | 146 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/richacl.h |   3 +
>  3 files changed, 150 insertions(+), 1 deletion(-)
>  create mode 100644 fs/richacl_inode.c
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index ddc43d8..1305047 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -48,7 +48,7 @@ obj-$(CONFIG_SYSCTL)		+= drop_caches.o
>  
>  obj-$(CONFIG_FHANDLE)		+= fhandle.o
>  obj-$(CONFIG_FS_RICHACL)	+= richacl.o
> -richacl-y			:= richacl_base.o
> +richacl-y			:= richacl_base.o richacl_inode.o
>  
>  obj-y				+= quota/
>  
> diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
> new file mode 100644
> index 0000000..6d94e064
> --- /dev/null
> +++ b/fs/richacl_inode.c
> @@ -0,0 +1,146 @@
> +/*
> + * Copyright (C) 2010  Novell, Inc.
> + * Copyright (C) 2015  Red Hat, Inc.
> + * Written by Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2, or (at your option) any
> + * later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * General Public License for more details.
> + */
> +
> +#include <linux/sched.h>
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/richacl.h>
> +
> +/**
> + * richacl_permission  -  richacl permission check algorithm
> + * @inode:	inode to check
> + * @acl:	rich acl of the inode
> + * @want:	requested access (MAY_* flags)
> + *
> + * Checks if the current process is granted @mask flags in @acl.
> + */
> +int
> +richacl_permission(struct inode *inode, const struct richacl *acl,
> +		   int want)
> +{
> +	const struct richace *ace;
> +	unsigned int mask = richacl_want_to_mask(want);
> +	unsigned int requested = mask, denied = 0;
> +	int in_owning_group = in_group_p(inode->i_gid);
> +	int in_owner_or_group_class = in_owning_group;
> +
> +	/*
> +	 * A process is
> +	 *   - in the owner file class if it owns the file,
> +	 *   - in the group file class if it is in the file's owning group or
> +	 *     it matches any of the user or group entries, and
> +	 *   - in the other file class otherwise.
> +	 * The file class is only relevant for determining which file mask to
> +	 * apply, which only happens for masked acls.
> +	 */
> +	if (acl->a_flags & RICHACL_MASKED) {
> +		if ((acl->a_flags & RICHACL_WRITE_THROUGH) &&
> +		    uid_eq(current_fsuid(), inode->i_uid)) {
> +			denied = requested & ~acl->a_owner_mask;
> +			goto out;
> +		}
> +	} else {
> +		/*
> +		 * We don't care which class the process is in when the acl is
> +		 * not masked.
> +		 */
> +		in_owner_or_group_class = 1;

So why bother setting it at all, then?  Oh, I see, it lets you break out
of the loop below earlier.  OK.

Patch makes sense to me, ACK.

--b.

> +	}
> +
> +	/*
> +	 * Check if the acl grants the requested access and determine which
> +	 * file class the process is in.
> +	 */
> +	richacl_for_each_entry(ace, acl) {
> +		unsigned int ace_mask = ace->e_mask;
> +
> +		if (richace_is_inherit_only(ace))
> +			continue;
> +		if (richace_is_owner(ace)) {
> +			if (!uid_eq(current_fsuid(), inode->i_uid))
> +				continue;
> +			goto entry_matches_owner;
> +		} else if (richace_is_group(ace)) {
> +			if (!in_owning_group)
> +				continue;
> +		} else if (richace_is_unix_user(ace)) {
> +			if (!uid_eq(current_fsuid(), ace->e_id.uid))
> +				continue;
> +		} else if (richace_is_unix_group(ace)) {
> +			if (!in_group_p(ace->e_id.gid))
> +				continue;
> +		} else
> +			goto entry_matches_everyone;
> +
> +		/*
> +		 * Apply the group file mask to entries other than owner@ and
> +		 * everyone@ or user entries matching the owner.  This ensures
> +		 * that we grant the same permissions as the acl computed by
> +		 * richacl_apply_masks().
> +		 *
> +		 * Without this restriction, the following richacl would grant
> +		 * rw access to processes which are both the owner and in the
> +		 * owning group, but not to other users in the owning group,
> +		 * which could not be represented without masks:
> +		 *
> +		 *  owner:rw::mask
> +		 *  group@:rw::allow
> +		 */
> +		if ((acl->a_flags & RICHACL_MASKED) && richace_is_allow(ace))
> +			ace_mask &= acl->a_group_mask;
> +
> +entry_matches_owner:
> +		/* The process is in the owner or group file class. */
> +		in_owner_or_group_class = 1;
> +
> +entry_matches_everyone:
> +		/* Check which mask flags the ACE allows or denies. */
> +		if (richace_is_deny(ace))
> +			denied |= ace_mask & mask;
> +		mask &= ~ace_mask;
> +
> +		/*
> +		 * Keep going until we know which file class
> +		 * the process is in.
> +		 */
> +		if (!mask && in_owner_or_group_class)
> +			break;
> +	}
> +	denied |= mask;
> +
> +	if (acl->a_flags & RICHACL_MASKED) {
> +		/*
> +		 * The file class a process is in determines which file mask
> +		 * applies.  Check if that file mask also grants the requested
> +		 * access.
> +		 */
> +		if (uid_eq(current_fsuid(), inode->i_uid))
> +			denied |= requested & ~acl->a_owner_mask;
> +		else if (in_owner_or_group_class)
> +			denied |= requested & ~acl->a_group_mask;
> +		else {
> +			if (acl->a_flags & RICHACL_WRITE_THROUGH)
> +				denied = requested & ~acl->a_other_mask;
> +			else
> +				denied |= requested & ~acl->a_other_mask;
> +		}
> +	}
> +
> +out:
> +	return denied ? -EACCES : 0;
> +}
> +EXPORT_SYMBOL_GPL(richacl_permission);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 8df6d93..756b711 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -306,4 +306,7 @@ extern unsigned int richacl_want_to_mask(unsigned int);
>  extern void richacl_compute_max_masks(struct richacl *, kuid_t);
>  extern struct richacl *richacl_chmod(struct richacl *, mode_t);
>  
> +/* richacl_inode.c */
> +extern int richacl_permission(struct inode *, const struct richacl *, int);
> +
>  #endif /* __RICHACL_H */
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: Andreas Grünbacher @ 2015-08-28 21:54 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux FS Devel, Linux NFS Mailing List, Linux API,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, LSM List, Andreas Gruenbacher
In-Reply-To: <CALCETrUoBnPyEGExpoDzHOCgnHh5=a1ROALmb63LLJZG+L=aQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2015-08-28 23:36 GMT+02:00 Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org>:
> Silly question from the peanut gallery: is there any such thing as
> opening an fd pointing at a file such that the "open file description"
> (i.e. the struct file) captures the right to delete the file?
>
> IOW do we need FMODE_DELETE_SELF?

When would that permission be checked, what syscall would you use to
unlink an open file descriptor?

Thanks,
Andreas

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: J. Bruce Fields @ 2015-08-28 21:57 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Andreas Gruenbacher,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Linux FS Devel, linux-nfs-u79uwXL29TY76Z2rM5mHXA, Linux API,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA, LSM List, Andreas Gruenbacher
In-Reply-To: <CALCETrUoBnPyEGExpoDzHOCgnHh5=a1ROALmb63LLJZG+L=aQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Aug 28, 2015 at 02:36:15PM -0700, Andy Lutomirski wrote:
> On Tue, Aug 4, 2015 at 4:53 AM, Andreas Gruenbacher
> <andreas.gruenbacher-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Normally, deleting a file requires write and execute access to the parent
> > directory.  With Richacls, a process with MAY_DELETE_SELF access to a file
> > may delete the file even without write access to the parent directory.
> >
> > To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
> > when checking for delete access inside a directory, and MAY_DELETE_SELF
> > when checking for delete access to a file itelf.
> >
> > The MAY_DELETE_SELF permission does not override the sticky directory
> > check.  It probably should.
> 
> Silly question from the peanut gallery: is there any such thing as
> opening an fd pointing at a file such that the "open file description"
> (i.e. the struct file) captures the right to delete the file?
> 
> IOW do we need FMODE_DELETE_SELF?

I guess FMODE_READ and _WRITE make sense because we pass file
descriptors to read() and write().  But we don't have a way to pass a
file descriptor to an operation that deletes a file.

(I think Windows may be different in both respects, it might be
interesting to compare, but I really don't understand how it works...).

--b.

^ permalink raw reply

* Re: [RFC v6 10/40] richacl: Permission check algorithm
From: Andreas Grünbacher @ 2015-08-28 22:06 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
	Linux NFS Mailing List, Linux API Mailing List, linux-cifs,
	LSM List, Andreas Gruenbacher
In-Reply-To: <20150828214917.GC23326@fieldses.org>

2015-08-28 23:49 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> On Tue, Aug 04, 2015 at 01:53:08PM +0200, Andreas Gruenbacher wrote:
>> +             /*
>> +              * We don't care which class the process is in when the acl is
>> +              * not masked.
>> +              */
>> +             in_owner_or_group_class = 1;
>
> So why bother setting it at all, then?  Oh, I see, it lets you break out
> of the loop below earlier.  OK.

Comment changed to:

                /*
                 * When the acl is not masked, there is no need to determine if
                 * the process is in the group class and we can earlier break
                 * out of the loop below.
                 */

> Patch makes sense to me, ACK.

Thanks,
Andreas

^ permalink raw reply

* Re: [RFC v6 03/40] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
From: Andy Lutomirski @ 2015-08-29  1:04 UTC (permalink / raw)
  To: Andreas Gruenbacher
  Cc: linux-cifs, linux-nfs, LSM List, Linux API, Andreas Gruenbacher,
	Linux FS Devel, linux-kernel@vger.kernel.org
In-Reply-To: <CAHpGcM+i887PJ07_CNZ38aK0DhRWgmyYbAGrDN2i1xXkH+1rUg@mail.gmail.com>

On Aug 28, 2015 2:54 PM, "Andreas Grünbacher"
<andreas.gruenbacher@gmail.com> wrote:
>
> 2015-08-28 23:36 GMT+02:00 Andy Lutomirski <luto@amacapital.net>:
> > Silly question from the peanut gallery: is there any such thing as
> > opening an fd pointing at a file such that the "open file description"
> > (i.e. the struct file) captures the right to delete the file?
> >
> > IOW do we need FMODE_DELETE_SELF?
>
> When would that permission be checked, what syscall would you use to
> unlink an open file descriptor?

Good point.  It's remotely plausible that there's some trick with bind
mounts, it's likely possible to unlink a directory by fd (using
unlinkat), and you can *link* a file (with linkat or /proc), but
unlinkat doesn't appear to allow you to unlink a file by fd.

--Andy

>
> Thanks,
> Andreas

^ permalink raw reply

* Re: Persistent Reservation API V3
From: Jeremy Linton @ 2015-08-29  1:33 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe
  Cc: linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1440608214-14497-1-git-send-email-hch-jcswGhMUV9g@public.gmane.org>

Hello,
	So, looking at this, I don't see how it supports the algorithm I've been using
for years. For that algorithm to successfully migrate PRs across multiple paths
on a single machine without affecting other possible users (who may legitimately
have PR'ed the same device) I need PR_IN SA 1, READ RESERVATIONS to assure the
current node owns the reservation before attempting to preempt it on another
path. This can also assure that the device hasn't been reserved with a legacy
reservation.

	So, this leads me to two more general questions. The first is why isn't the PR
API simply exported to filesystems as a general reserve/release so that the PR
happens during mount/dismount. Then DM and friends can be setup to transparently
migrate or share the reservation, rather than depending on userspace to handle
these operations...
	Also, it seems to me the use of CLEAR is extremely dangerous in any environment
where actual arbitration or sharing of the resource is taking place.


	thanks,

On 8/26/2015 11:56 AM, Christoph Hellwig wrote:
> This series adds support for a simplified Persistent Reservation API
> to the block layer.  The intent is that both in-kernel and userspace
> consumers can use the API instead of having to hand craft SCSI or NVMe
> command through the various pass through interfaces.  It also adds
> DM support as getting reservations through dm-multipath is a major
> pain with the current scheme.
> 
> NVMe support currently isn't included as I don't have a multihost
> NVMe setup to test on, but Keith offered to test it and I'll have
> a patch for it shortly.
> 
> The ioctl API is documented in Documentation/block/pr.txt, but to
> fully understand the concept you'll have to read up the SPC spec,
> PRs are too complicated that trying to rephrase them into different
> terminology is just going to create confusion.
> 
> Note that Mike wants to include the DM patches so through the DM
> tree, so they are only included for reference.
> 
> I also have a set of simple test tools available at:
> 
> 	git://git.infradead.org/users/hch/pr-tests.git
> 
> Changes since V2:
>   - added an ignore flag to the reserve opertion as well, and redid
>     the ioctl API to have general flags fields
>   - rebased on top of the latest block layer tree updates
> Changes since V1:
>   - rename DM ->ioctl to ->prepare_ioctl
>   - rename dm_get_ioctl_table to dm_get_live_table_for_ioctl
>   - merge two DM patches into one
>   - various spelling fixes
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

^ permalink raw reply

* Re: Persistent Reservation API V3
From: Christoph Hellwig @ 2015-08-29 13:52 UTC (permalink / raw)
  To: Jeremy Linton
  Cc: Christoph Hellwig, Jens Axboe, linux-scsi, linux-nvme, dm-devel,
	linux-api, linux-kernel
In-Reply-To: <55E10BE4.6080400@tributary.com>

On Fri, Aug 28, 2015 at 08:33:24PM -0500, Jeremy Linton wrote:
> Hello,
> 	So, looking at this, I don't see how it supports the algorithm I've been using
> for years. For that algorithm to successfully migrate PRs across multiple paths
> on a single machine without affecting other possible users (who may legitimately
> have PR'ed the same device) I need PR_IN SA 1, READ RESERVATIONS to assure the
> current node owns the reservation before attempting to preempt it on another
> path. This can also assure that the device hasn't been reserved with a legacy
> reservation.

Do you have any code describing this in more detail?  We could add the
read side as well if there is strong interest.

> 	So, this leads me to two more general questions. The first is why isn't the PR
> API simply exported to filesystems as a general reserve/release so that the PR
> happens during mount/dismount. Then DM and friends can be setup to transparently
> migrate or share the reservation, rather than depending on userspace to handle
> these operations...

The API can be used by file systems, and my upcoming NFS SCSI layout
support was the main reason to write this.

> 	Also, it seems to me the use of CLEAR is extremely dangerous in any environment
> where actual arbitration or sharing of the resource is taking place.

Yes, but having it as a specific API isn't any less dangerous than
having it issued using SG_IO.  Reservations really only make sense if
you assume every user of a LU is actually cooperating in some way
and not actively hostile.

^ permalink raw reply

* [PATCH 0/1] Corsair Vengeance K90 driver
From: Clément Vuchener @ 2015-08-29 14:03 UTC (permalink / raw)
  To: jkosina-IBi9RG/b67k
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA

This patch implements a HID driver for the Corsair Vengeance K90 keyboard.

This keyboard has a backlight, macro keys (G1 to G18) and some other 
special keys (macro recording, profile switching, changing the light 
level or disabling the super/meta/"windows logo" keys). The macro 
playback can be switched between hardware or software (from the driver 
not a key). Using the generic HID driver, the macro keys and special 
keys send wrong key code or no key code at all (because of the HID usage 
code used by the keyboard).
The purpose of this driver is to fix the behaviour of the keys using 
incorrect HID usage codes and expose the other features to the user space.

This is my first Linux driver and I would like to get feedback for both 
my code and my choices for the user space interface.

For the keyboard to work in software mode, a user program needs to be 
able to get key events. I am not sure about which key codes should be 
used for that. I choose the BTN_TRIGGER_HAPPY* as they seem to be used 
for devices with extra buttons.

The other special keys does send any key codes. They are used to change 
the hardware state and user space should not need them directly.

Playback mode and current profile can be read and written through simple 
sysfs attributes.

The backlight is managed through a LED class device.

Hardware macro profiles (there are three of them) are managed thanks to 
devices of the new k90_profile class. Data is sent through binary 
attributes in the same format sent to the hardware. I choose to do so to 
keep the driver simple. Should it use a more abstract ABI? Those 
attributes are write-only as is does not look like there is way to read 
them back (the Windows driver does not do it).

The macro recording is not actually done from hardware and this driver 
does not implement it. The recording LED has its device, but having it 
on or pressing the button does not record anything.

Clément Vuchener (1):
   Corsair Vengeance K90 driver

  Documentation/ABI/testing/sysfs-class-k90_profile  |  55 ++
  .../ABI/testing/sysfs-driver-hid-corsair-k90       |  15 +
  drivers/hid/Kconfig                                |  10 +
  drivers/hid/Makefile                               |   1 +
  drivers/hid/hid-core.c                             |   1 +
  drivers/hid/hid-corsair-k90.c                      | 690 
+++++++++++++++++++++
  drivers/hid/hid-ids.h                              |   3 +
  7 files changed, 775 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-k90_profile
  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
  create mode 100644 drivers/hid/hid-corsair-k90.c

-- 
2.4.3

^ permalink raw reply

* [PATCH 1/1] Corsair Vengeance K90 driver
From: Clément Vuchener @ 2015-08-29 14:03 UTC (permalink / raw)
  To: jkosina-IBi9RG/b67k
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1440853951.git.clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
  Documentation/ABI/testing/sysfs-class-k90_profile  |  55 ++
  .../ABI/testing/sysfs-driver-hid-corsair-k90       |  15 +
  drivers/hid/Kconfig                                |  10 +
  drivers/hid/Makefile                               |   1 +
  drivers/hid/hid-core.c                             |   1 +
  drivers/hid/hid-corsair-k90.c                      | 690 
+++++++++++++++++++++
  drivers/hid/hid-ids.h                              |   3 +
  7 files changed, 775 insertions(+)
  create mode 100644 Documentation/ABI/testing/sysfs-class-k90_profile
  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
  create mode 100644 drivers/hid/hid-corsair-k90.c

diff --git a/Documentation/ABI/testing/sysfs-class-k90_profile 
b/Documentation/ABI/testing/sysfs-class-k90_profile
new file mode 100644
index 0000000..3275c16
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-k90_profile
@@ -0,0 +1,55 @@
+What:		/sys/class/k90_profile/<profile>/profile_number
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Get the number of the profile.
+
+What:		/sys/class/k90_profile/<profile>/bindings
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Write bindings to the keyboard on-board profile.
+		The data structure is:
+		 - number of bindings in this structure (1 byte)
+		 - size of this data structure (2 bytes big endian)
+		 - size of the macro data written to
+		   /sys/class/k90_profile/<profile>/data (2 bytes big endian)
+		 - array of bindings referencing the data from
+		   /sys/class/k90_profile/<profile>/data, each containing:
+		   * 0x10 for a key usage, 0x20 for a macro (1 byte)
+		   * offset of the key usage/macro data (2 bytes big endian)
+		   * length of the key usage/macro data (2 bytes big endian)
+
+What:		/sys/class/k90_profile/<profile>/keys
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Write the list of the keys used by the bindings and how
+		the macros are repeated.
+		The data struct is:
+		 - number of keys in this structure (1 byte)
+		 - array of keys and repeat mode:
+		   * key usage (G1 to G16 are 0xD0 to 0xDF, G17 and
+		     G18 are 0xE8 and 0xE9) (1 byte)
+		   * repeat mode (1 byte):
+		       1: play when pressed
+		       2: repeat while key is pressed
+		       3: repeat until the key is pressed again
+
+What:		/sys/class/k90_profile/<profile>/data
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Write the key usage and macros used by
+		/sys/class/k90_profile/<profile>/bindings
+		Macro items are:
+		 - Key event:
+		   * item type: 0x84 (1 byte)
+		   * HID usage (1 byte)
+		   * new state: 0 released, 1 pressed (1 byte)
+		 - Delay
+		   * item type: 0x87 (1 byte)
+		   * delay in milliseconds (2 bytes big endian)
+		 - Macro end
+		   * item type: 0x86 (1 byte)
+		   * repeat count (2 bytes big endian)
diff --git a/Documentation/ABI/testing/sysfs-driver-hid-corsair-k90 
b/Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
new file mode 100644
index 0000000..16d50ae
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
@@ -0,0 +1,15 @@
+What:		/sys/bus/drivers/k90/<dev>/macro_mode
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Get/set the current playback mode. "SW" for software mode
+		where G-keys triggers their regular key codes. "HW" for
+		hardware playback mode where the G-keys play their macro
+		from the on-board memory.
+
+
+What:		/sys/bus/drivers/k90/<dev>/current_profile
+Date:		August 2015
+KernelVersion:	4.2
+Contact:	Clement Vuchener <clement.vuchener-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+Description:	Get/set the current selected profile. Values are from 1 to 3.
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index cc4c664..db6d8a5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -171,6 +171,16 @@ config HID_CHICONY
  	---help---
  	Support for Chicony Tactical pad.
  +config HID_CORSAIR
+	tristate "Corsair devices"
+	depends on HID
+	---help---
+	Support for Corsair devices that are not fully compliant with the
+	HID standard.
+
+	Supported devices:
+	- Vengeance K90
+
  config HID_PRODIKEYS
  	tristate "Prodikeys PC-MIDI Keyboard support"
  	depends on HID && SND
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 2f8a41d..e94a0a5 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_HID_BELKIN)	+= hid-belkin.o
  obj-$(CONFIG_HID_BETOP_FF)	+= hid-betopff.o
  obj-$(CONFIG_HID_CHERRY)	+= hid-cherry.o
  obj-$(CONFIG_HID_CHICONY)	+= hid-chicony.o
+obj-$(CONFIG_HID_CORSAIR)	+= hid-corsair-k90.o
  obj-$(CONFIG_HID_CP2112)	+= hid-cp2112.o
  obj-$(CONFIG_HID_CYPRESS)	+= hid-cypress.o
  obj-$(CONFIG_HID_DRAGONRISE)	+= hid-dr.o
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e6fce23..f0d9125 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1807,6 +1807,7 @@ static const struct hid_device_id 
hid_have_special_driver[] = {
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, 
USB_DEVICE_ID_CHICONY_WIRELESS) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, 
USB_DEVICE_ID_CHICONY_WIRELESS2) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, 
USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_CP2112) },
  	{ HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS, 
USB_DEVICE_ID_CYPRESS_BARCODE_1) },
diff --git a/drivers/hid/hid-corsair-k90.c b/drivers/hid/hid-corsair-k90.c
new file mode 100644
index 0000000..67c1095
--- /dev/null
+++ b/drivers/hid/hid-corsair-k90.c
@@ -0,0 +1,690 @@
+/*
+ * HID driver for Corsair Vengeance K90 Keyboard
+ * Copyright (c) 2015 Clement Vuchener
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by 
the Free
+ * Software Foundation; either version 2 of the License, or (at your 
option)
+ * any later version.
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/leds.h>
+
+#include "hid-ids.h"
+
+struct k90_led {
+	struct led_classdev cdev;
+	int brightness;
+	struct work_struct work;
+};
+
+struct k90_profile {
+	struct device *dev;
+	int profile;
+};
+
+struct k90_drvdata {
+	int current_profile;
+	int macro_mode;
+	int meta_locked;
+	struct k90_led backlight;
+	struct k90_led record_led;
+	struct k90_profile profile[3];
+};
+
+#define K90_GKEY_COUNT	18
+
+static int k90_usage_to_gkey(unsigned int usage)
+{
+	/* G1 (0xd0) to G16 (0xdf) */
+	if (usage >= 0xd0 && usage <= 0xdf)
+		return usage - 0xd0 + 1;
+	/* G17 (0xe8) to G18 (0xe9) */
+	if (usage >= 0xe8 && usage <= 0xe9)
+		return usage - 0xe8 + 17;
+	return 0;
+}
+
+static unsigned short k90_gkey_map[K90_GKEY_COUNT] = {
+	BTN_TRIGGER_HAPPY1,
+	BTN_TRIGGER_HAPPY2,
+	BTN_TRIGGER_HAPPY3,
+	BTN_TRIGGER_HAPPY4,
+	BTN_TRIGGER_HAPPY5,
+	BTN_TRIGGER_HAPPY6,
+	BTN_TRIGGER_HAPPY7,
+	BTN_TRIGGER_HAPPY8,
+	BTN_TRIGGER_HAPPY9,
+	BTN_TRIGGER_HAPPY10,
+	BTN_TRIGGER_HAPPY11,
+	BTN_TRIGGER_HAPPY12,
+	BTN_TRIGGER_HAPPY13,
+	BTN_TRIGGER_HAPPY14,
+	BTN_TRIGGER_HAPPY15,
+	BTN_TRIGGER_HAPPY16,
+	BTN_TRIGGER_HAPPY17,
+	BTN_TRIGGER_HAPPY18,
+};
+
+module_param_array_named(gkey_codes, k90_gkey_map, ushort, NULL, S_IRUGO);
+
+#define K90_USAGE_SPECIAL_MIN 0xf0
+#define K90_USAGE_SPECIAL_MAX 0xff
+
+#define K90_USAGE_MACRO_RECORD_START 0xf6
+#define K90_USAGE_MACRO_RECORD_STOP 0xf7
+
+#define K90_USAGE_PROFILE 0xf1
+#define K90_USAGE_M1 0xf1
+#define K90_USAGE_M2 0xf2
+#define K90_USAGE_M3 0xf3
+#define K90_USAGE_PROFILE_MAX 0xf3
+
+#define K90_USAGE_META_OFF 0xf4
+#define K90_USAGE_META_ON  0xf5
+
+#define K90_USAGE_LIGHT 0xfa
+#define K90_USAGE_LIGHT_OFF 0xfa
+#define K90_USAGE_LIGHT_DIM 0xfb
+#define K90_USAGE_LIGHT_MEDIUM 0xfc
+#define K90_USAGE_LIGHT_BRIGHT 0xfd
+#define K90_USAGE_LIGHT_MAX 0xfd
+
+/* USB control protocol */
+
+#define K90_REQUEST_BRIGHTNESS 49
+#define K90_REQUEST_MACRO_MODE 2
+#define K90_REQUEST_STATUS 4
+#define K90_REQUEST_GET_MODE 5
+#define K90_REQUEST_PROFILE 20
+
+#define K90_REQUEST_PROFILE_BINDINGS 16
+#define K90_REQUEST_PROFILE_KEYS 22
+#define K90_REQUEST_PROFILE_DATA 18
+
+#define K90_BINDINGS_MAX_LENGTH 128
+#define K90_KEYS_MAX_LENGTH 64
+/* K90_DATA_MAX_LENGTH may be higher but that is the maximum I tested */
+#define K90_DATA_MAX_LENGTH 4096
+
+#define K90_MACRO_MODE_SW 0x0030
+#define K90_MACRO_MODE_HW 0x0001
+
+#define K90_MACRO_LED_ON  0x0020
+#define K90_MACRO_LED_OFF 0x0040
+
+/*
+ * LED class devices
+ */
+
+#define K90_BACKLIGHT_LED_SUFFIX ":blue:backlight"
+#define K90_RECORD_LED_SUFFIX ":red:record"
+
+static enum led_brightness k90_brightness_get(struct led_classdev 
*led_cdev)
+{
+	struct k90_led *led = container_of(led_cdev, struct k90_led, cdev);
+
+	return led->brightness;
+}
+
+static void k90_brightness_set(struct led_classdev *led_cdev,
+			       enum led_brightness brightness)
+{
+	struct k90_led *led = container_of(led_cdev, struct k90_led, cdev);
+
+	led->brightness = brightness;
+	schedule_work(&led->work);
+}
+
+static void k90_backlight_work(struct work_struct *work)
+{
+	int ret;
+	struct k90_led *led = container_of(work, struct k90_led, work);
+	struct device *dev = led->cdev.dev->parent;
+	struct usb_interface *usbif = to_usb_interface(dev->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_BRIGHTNESS,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, led->brightness, 0,
+			      NULL, 0, USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		dev_warn(dev, "Failed to set backlight brightness (error: %d).\n",
+			 ret);
+}
+
+static void k90_record_led_work(struct work_struct *work)
+{
+	int ret;
+	struct k90_led *led = container_of(work, struct k90_led, work);
+	struct device *dev = led->cdev.dev->parent;
+	struct usb_interface *usbif = to_usb_interface(dev->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	int value;
+
+	if (led->brightness > 0)
+		value = K90_MACRO_LED_ON;
+	else
+		value = K90_MACRO_LED_OFF;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_MACRO_MODE,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, value, 0, NULL, 0,
+			      USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		dev_warn(dev, "Failed to set record LED state (error: %d).\n",
+			 ret);
+}
+
+/*
+ * Keyboard attributes
+ */
+
+static ssize_t k90_show_macro_mode(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct k90_drvdata *drvdata = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+			(drvdata->macro_mode ? "HW" : "SW"));
+}
+
+static ssize_t k90_store_macro_mode(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	int ret;
+	struct usb_interface *usbif = to_usb_interface(dev->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	struct k90_drvdata *drvdata = dev_get_drvdata(dev);
+	__u16 value;
+
+	if (strncmp(buf, "SW", 2) == 0)
+		value = K90_MACRO_MODE_SW;
+	else if (strncmp(buf, "HW", 2) == 0)
+		value = K90_MACRO_MODE_HW;
+	else
+		return -EINVAL;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_MACRO_MODE,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, value, 0, NULL, 0,
+			      USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		return ret;
+
+	drvdata->macro_mode = (value == K90_MACRO_MODE_HW);
+
+	return count;
+}
+
+static ssize_t k90_show_current_profile(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct k90_drvdata *drvdata = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", drvdata->current_profile);
+}
+
+static ssize_t k90_store_current_profile(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	int ret;
+	struct usb_interface *usbif = to_usb_interface(dev->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	struct k90_drvdata *drvdata = dev_get_drvdata(dev);
+	int profile;
+
+	if (kstrtoint(buf, 10, &profile))
+		return -EINVAL;
+	if (profile < 1 || profile > 3)
+		return -EINVAL;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_PROFILE,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, profile, 0, NULL, 0,
+			      USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		return ret;
+
+	drvdata->current_profile = profile;
+
+	return count;
+}
+
+static DEVICE_ATTR(macro_mode, 0644, k90_show_macro_mode, 
k90_store_macro_mode);
+static DEVICE_ATTR(current_profile, 0644, k90_show_current_profile,
+		   k90_store_current_profile);
+
+static struct attribute *k90_attrs[] = {
+	&dev_attr_macro_mode.attr,
+	&dev_attr_current_profile.attr,
+	NULL
+};
+
+static const struct attribute_group k90_attr_group = {
+	.attrs = k90_attrs,
+};
+
+/*
+ * Profile device class and attributes
+ */
+
+static struct class *k90_profile_class;
+
+static ssize_t k90_profile_show_profile_number(struct device *dev,
+					       struct device_attribute *attr,
+					       char *buf)
+{
+	struct k90_profile *data = dev_get_drvdata(dev);
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", data->profile);
+}
+
+static ssize_t k90_profile_write_bindings(struct file *fp, struct 
kobject *kobj,
+					  struct bin_attribute *attr, char *buf,
+					  loff_t off, size_t count)
+{
+	int ret;
+	struct device *pdev = container_of(kobj, struct device, kobj);
+	struct k90_profile *data = dev_get_drvdata(pdev);
+	struct usb_interface *usbif = to_usb_interface(pdev->parent->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+
+	if (count > K90_BINDINGS_MAX_LENGTH)
+		return -EMSGSIZE;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_PROFILE_BINDINGS,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, 0, data->profile, buf,
+			      count, USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t k90_profile_write_keys(struct file *fp, struct kobject 
*kobj,
+				      struct bin_attribute *attr, char *buf,
+				      loff_t off, size_t count)
+{
+	int ret;
+	struct device *pdev = container_of(kobj, struct device, kobj);
+	struct k90_profile *data = dev_get_drvdata(pdev);
+	struct usb_interface *usbif = to_usb_interface(pdev->parent->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+
+	if (count > K90_KEYS_MAX_LENGTH)
+		return -EMSGSIZE;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_PROFILE_KEYS,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, 0, data->profile, buf,
+			      count, USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		return ret;
+
+	return count;
+}
+
+static ssize_t k90_profile_write_data(struct file *fp, struct kobject 
*kobj,
+				      struct bin_attribute *attr, char *buf,
+				      loff_t off, size_t count)
+{
+	int ret;
+	struct device *pdev = container_of(kobj, struct device, kobj);
+	struct k90_profile *data = dev_get_drvdata(pdev);
+	struct usb_interface *usbif = to_usb_interface(pdev->parent->parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+
+	if (count > K90_DATA_MAX_LENGTH)
+		return -EMSGSIZE;
+
+	ret = usb_control_msg(usbdev, usb_sndctrlpipe(usbdev, 0),
+			      K90_REQUEST_PROFILE_DATA,
+			      USB_DIR_OUT | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, 0, data->profile, buf,
+			      count, USB_CTRL_SET_TIMEOUT);
+	if (ret != 0)
+		return ret;
+
+	return count;
+}
+
+static DEVICE_ATTR(profile_number, 0444, 
k90_profile_show_profile_number, NULL);
+static BIN_ATTR(bindings, 0200, NULL, k90_profile_write_bindings, 0);
+static BIN_ATTR(keys, 0200, NULL, k90_profile_write_keys, 0);
+static BIN_ATTR(data, 0200, NULL, k90_profile_write_data, 0);
+
+static struct attribute *k90_profile_attrs[] = {
+	&dev_attr_profile_number.attr,
+	NULL
+};
+
+static struct bin_attribute *k90_profile_bin_attrs[] = {
+	&bin_attr_bindings,
+	&bin_attr_keys,
+	&bin_attr_data,
+	NULL
+};
+
+static const struct attribute_group k90_profile_attr_group = {
+	.attrs = k90_profile_attrs,
+	.bin_attrs = k90_profile_bin_attrs,
+};
+
+static const struct attribute_group *k90_profile_attr_groups[] = {
+	&k90_profile_attr_group,
+	NULL
+};
+
+/*
+ * Driver functions
+ */
+
+static int k90_init_special_functions(struct hid_device *dev)
+{
+	int ret, i;
+	struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
+	struct usb_device *usbdev = interface_to_usbdev(usbif);
+	char data[8];
+	struct k90_drvdata *drvdata =
+	    kzalloc(sizeof(struct k90_drvdata), GFP_KERNEL);
+	size_t name_sz;
+	char *name;
+	struct k90_led *led;
+
+	if (!drvdata) {
+		ret = -ENOMEM;
+		goto fail_drvdata;
+	}
+	hid_set_drvdata(dev, drvdata);
+
+	/* Get current status */
+	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
+			      K90_REQUEST_STATUS,
+			      USB_DIR_IN | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, 0, 0, data, 8,
+			      USB_CTRL_SET_TIMEOUT);
+	if (ret < 0) {
+		hid_warn(dev, "Failed to get K90 initial state (error %d).\n",
+			 ret);
+		drvdata->backlight.brightness = 0;
+		drvdata->current_profile = 1;
+	} else {
+		drvdata->backlight.brightness = data[4];
+		drvdata->current_profile = data[7];
+	}
+	/* Get current mode */
+	ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
+			      K90_REQUEST_GET_MODE,
+			      USB_DIR_IN | USB_TYPE_VENDOR |
+			      USB_RECIP_DEVICE, 0, 0, data, 2,
+			      USB_CTRL_SET_TIMEOUT);
+	if (ret < 0)
+		hid_warn(dev, "Failed to get K90 initial mode (error %d).\n",
+			 ret);
+	else {
+		switch (data[0]) {
+		case K90_MACRO_MODE_HW:
+			drvdata->macro_mode = 1;
+			break;
+		case K90_MACRO_MODE_SW:
+			drvdata->macro_mode = 0;
+			break;
+		default:
+			hid_warn(dev, "K90 in unknown mode: %02x.\n",
+				 data[0]);
+		}
+	}
+
+	/* Init LED device for backlight */
+	name_sz =
+	    strlen(dev_name(&dev->dev)) + sizeof(K90_BACKLIGHT_LED_SUFFIX);
+	name = devm_kzalloc(&dev->dev, name_sz, GFP_KERNEL);
+	if (!name) {
+		ret = -ENOMEM;
+		goto fail_backlight;
+	}
+	snprintf(name, name_sz, "%s" K90_BACKLIGHT_LED_SUFFIX,
+		 dev_name(&dev->dev));
+	led = &drvdata->backlight;
+	led->cdev.name = name;
+	led->cdev.max_brightness = 3;
+	led->cdev.brightness_set = k90_brightness_set;
+	led->cdev.brightness_get = k90_brightness_get;
+	INIT_WORK(&led->work, k90_backlight_work);
+	ret = led_classdev_register(&dev->dev, &led->cdev);
+	if (ret != 0)
+		goto fail_backlight;
+
+	/* Init LED device for record LED */
+	name_sz = strlen(dev_name(&dev->dev)) + sizeof(K90_RECORD_LED_SUFFIX);
+	name = devm_kzalloc(&dev->dev, name_sz, GFP_KERNEL);
+	if (!name) {
+		ret = -ENOMEM;
+		goto fail_record_led;
+	}
+	snprintf(name, name_sz, "%s" K90_RECORD_LED_SUFFIX,
+		 dev_name(&dev->dev));
+	led = &drvdata->record_led;
+	led->cdev.name = name;
+	led->cdev.max_brightness = 1;
+	led->cdev.brightness_set = k90_brightness_set;
+	led->cdev.brightness_get = k90_brightness_get;
+	INIT_WORK(&led->work, k90_record_led_work);
+	ret = led_classdev_register(&dev->dev, &led->cdev);
+	if (ret != 0)
+		goto fail_record_led;
+
+	/* Create profile devices */
+	for (i = 0; i < 3; ++i) {
+		drvdata->profile[i].profile = i + 1;
+		drvdata->profile[i].dev =
+		    device_create_with_groups(k90_profile_class, &dev->dev, 0,
+					      &drvdata->profile[i],
+					      k90_profile_attr_groups,
+					      "%s:profile%d",
+					      dev_name(&dev->dev), i + 1);
+		if (IS_ERR(drvdata->profile[i].dev)) {
+			ret = PTR_ERR(drvdata->profile[i].dev);
+			for (i = i - 1; i >= 0; --i)
+				device_unregister(drvdata->profile[i].dev);
+			goto fail_profile;
+		}
+	}
+
+	/* Init attributes */
+	ret = sysfs_create_group(&dev->dev.kobj, &k90_attr_group);
+	if (ret != 0)
+		goto fail_sysfs;
+
+	return 0;
+
+fail_sysfs:
+	for (i = 0; i < 3; ++i)
+		device_unregister(drvdata->profile[i].dev);
+fail_profile:
+	led_classdev_unregister(&drvdata->record_led.cdev);
+	flush_work(&drvdata->record_led.work);
+fail_record_led:
+	led_classdev_unregister(&drvdata->backlight.cdev);
+	flush_work(&drvdata->backlight.work);
+fail_backlight:
+	kfree(drvdata);
+fail_drvdata:
+	hid_set_drvdata(dev, NULL);
+	return ret;
+}
+
+static void k90_cleanup_special_functions(struct hid_device *dev)
+{
+	int i;
+	struct k90_drvdata *drvdata = hid_get_drvdata(dev);
+
+	if (drvdata) {
+		sysfs_remove_group(&dev->dev.kobj, &k90_attr_group);
+		for (i = 0; i < 3; ++i)
+			device_unregister(drvdata->profile[i].dev);
+		led_classdev_unregister(&drvdata->record_led.cdev);
+		led_classdev_unregister(&drvdata->backlight.cdev);
+		flush_work(&drvdata->record_led.work);
+		flush_work(&drvdata->backlight.work);
+		kfree(drvdata);
+	}
+}
+
+static int k90_probe(struct hid_device *dev, const struct hid_device_id 
*id)
+{
+	int ret;
+	struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
+
+	ret = hid_parse(dev);
+	if (ret != 0) {
+		hid_err(dev, "parse failed\n");
+		return ret;
+	}
+	ret = hid_hw_start(dev, HID_CONNECT_DEFAULT);
+	if (ret != 0) {
+		hid_err(dev, "hw start failed\n");
+		return ret;
+	}
+
+	if (usbif->cur_altsetting->desc.bInterfaceNumber == 0) {
+		ret = k90_init_special_functions(dev);
+		if (ret != 0)
+			hid_warn(dev, "Failed to initialize K90 special functions.\n");
+	} else
+		hid_set_drvdata(dev, NULL);
+
+	return 0;
+}
+
+static void k90_remove(struct hid_device *dev)
+{
+	struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
+
+	if (usbif->cur_altsetting->desc.bInterfaceNumber == 0)
+		k90_cleanup_special_functions(dev);
+
+	hid_hw_stop(dev);
+}
+
+static int k90_event(struct hid_device *dev, struct hid_field *field,
+		     struct hid_usage *usage, __s32 value)
+{
+	struct k90_drvdata *drvdata = hid_get_drvdata(dev);
+
+	if (!drvdata)
+		return 0;
+
+	switch (usage->hid & HID_USAGE) {
+	case K90_USAGE_MACRO_RECORD_START:
+		drvdata->record_led.brightness = 1;
+		break;
+	case K90_USAGE_MACRO_RECORD_STOP:
+		drvdata->record_led.brightness = 0;
+		break;
+	case K90_USAGE_M1:
+	case K90_USAGE_M2:
+	case K90_USAGE_M3:
+		drvdata->current_profile =
+		    (usage->hid & HID_USAGE) - K90_USAGE_PROFILE + 1;
+		break;
+	case K90_USAGE_META_OFF:
+		drvdata->meta_locked = 0;
+		break;
+	case K90_USAGE_META_ON:
+		drvdata->meta_locked = 1;
+		break;
+	case K90_USAGE_LIGHT_OFF:
+	case K90_USAGE_LIGHT_DIM:
+	case K90_USAGE_LIGHT_MEDIUM:
+	case K90_USAGE_LIGHT_BRIGHT:
+		drvdata->backlight.brightness = (usage->hid & HID_USAGE) -
+		    K90_USAGE_LIGHT;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int k90_input_mapping(struct hid_device *dev, struct hid_input 
*input,
+			     struct hid_field *field, struct hid_usage *usage,
+			     unsigned long **bit, int *max)
+{
+	int gkey;
+
+	gkey = k90_usage_to_gkey(usage->hid & HID_USAGE);
+	if (gkey != 0) {
+		hid_map_usage_clear(input, usage, bit, max, EV_KEY,
+				    k90_gkey_map[gkey - 1]);
+		return 1;
+	}
+	if ((usage->hid & HID_USAGE) >= K90_USAGE_SPECIAL_MIN &&
+	    (usage->hid & HID_USAGE) <= K90_USAGE_SPECIAL_MAX)
+		return -1;
+
+	return 0;
+}
+
+static const struct hid_device_id k90_devices[] = {
+	{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
+	{}
+};
+
+MODULE_DEVICE_TABLE(hid, k90_devices);
+
+static struct hid_driver k90_driver = {
+	.name = "k90",
+	.id_table = k90_devices,
+	.probe = k90_probe,
+	.event = k90_event,
+	.remove = k90_remove,
+	.input_mapping = k90_input_mapping,
+};
+
+static int __init k90_init(void)
+{
+	int ret;
+
+	k90_profile_class = class_create(THIS_MODULE, "k90_profile");
+	if (IS_ERR(k90_profile_class))
+		return PTR_ERR(k90_profile_class);
+
+	ret = hid_register_driver(&k90_driver);
+	if (ret != 0) {
+		class_destroy(k90_profile_class);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void k90_exit(void)
+{
+	hid_unregister_driver(&k90_driver);
+	class_destroy(k90_profile_class);
+}
+
+module_init(k90_init);
+module_exit(k90_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Clement Vuchener");
+MODULE_DESCRIPTION("HID driver for Corsair Vengeance K90 Keyboard");
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b3b225b..f23b9ac 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -246,6 +246,9 @@
  #define USB_DEVICE_ID_CODEMERCS_IOW_FIRST	0x1500
  #define USB_DEVICE_ID_CODEMERCS_IOW_LAST	0x15ff
  +#define USB_VENDOR_ID_CORSAIR		0x1b1c
+#define USB_DEVICE_ID_CORSAIR_K90	0x1b02
+
  #define USB_VENDOR_ID_CREATIVELABS	0x041e
  #define USB_DEVICE_ID_PRODIKEYS_PCMIDI	0x2801
  -- 2.4.3

^ permalink raw reply related

* Re: [RFC] watchdog: Add watchdog device control through sysfs attributes
From: Guenter Roeck @ 2015-08-29 16:51 UTC (permalink / raw)
  To: Pratyush Anand
  Cc: linux-watchdog-u79uwXL29TY76Z2rM5mHXA,
	dyoung-H+wXaHxf7aLQT0dZR+AlfA, dzickus-H+wXaHxf7aLQT0dZR+AlfA,
	open list:ABI/API, open list, Wim Van Sebroeck
In-Reply-To: <d9ac417a3bc1c6846f1ddfcd3cd07e5a92865c19.1440179209.git.panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Fri, Aug 21, 2015 at 11:18:12PM +0530, Pratyush Anand wrote:
> This patch adds following attributes to watchdog device's sysfs interface.
> 
> * start - writes non zero value to start and zero to stop

I would suggest to drop this attribute, as well as 'keepalive'.
Both will make keeping the internal state very difficult, especially
when we add support for heartbeats triggered by the watchdog core.

> * state - reads whether device is active or not(1 for active and 0 for
> inactive)

How about reporting the state as text ?

> * identity - reads Watchdog device's identity string.
> * keepalive - writes to ping a watchdog device
> * timeout - reads current timeout and writes to program a new timeout.
> * timeleft - reads timeleft before watchdog generates a reset
> * bootstatus - reads status of the watchdog device at boot
> * status - reads watchdog device's  internal status bits
> * nowayout - reads whether nowayout feature was set or not
> 
> Testing with iTCO_wdt:
>  # cd /sys/class/watchdog/watchdog1/
>  # ls
> bootstatus  dev  device  identity  keepalive  nowayout  power  start  state
> status  subsystem  timeleft  timeout  uevent
>  # cat identity
> iTCO_wdt
>  # cat timeout
> 30
>  # echo 1 > start
>  # cat timeleft
> 26
>  # echo 120 > timeout
>  # cat timeleft
> 116
>  # echo > keepalive
>  # cat timeleft
> 118
>  # cat state
> 1
>  # echo 0 > start
>  # cat state
> 0
>  # cat bootstatus
> 0
>  # cat nowayout
> 0
>  # cat status
> cat: status: Operation not supported
> 
Unsupported attributes should not appear in the first place.
Please use is_visible to determine if an attribute should
be there or not.

> Signed-off-by: Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-class-watchdog |  74 +++++++++
>  drivers/watchdog/watchdog_core.c               |   6 +-
>  drivers/watchdog/watchdog_core.h               |   2 +
>  drivers/watchdog/watchdog_dev.c                | 206 +++++++++++++++++++++++++
>  4 files changed, 284 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-watchdog
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-watchdog b/Documentation/ABI/testing/sysfs-class-watchdog
> new file mode 100644
> index 000000000000..31e7be53edf8
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-watchdog
> @@ -0,0 +1,74 @@
> +What:		/sys/class/watchdog/watchdogn/bootstatus
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Who is normally listed here ? Shouldn't it be the maintainer ?

> +Description:
> +		It is a read only file. It contains status of the watchdog
> +		device at boot. It is equivalent to WDIOC_GETBOOTSTATUS of
> +		ioctl interface.
> +
> +What:		/sys/class/watchdog/watchdogn/identity
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read only file. It contains identity string of
> +		watchdog device.
> +
> +What:		/sys/class/watchdog/watchdogn/keepalive
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a write only file. It is written to ping a watchdog
> +		device to keep it alive. It is equivalent to
> +		WDIOC_KEEPALIVE of ioctl interface.
> +
> +What:		/sys/class/watchdog/watchdogn/nowayout
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read only file. While reading, it gives '1' if that
> +		device supports nowayout feature else, it gives '0'.
> +
> +What:		/sys/class/watchdog/watchdogn/start
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a write only file. Writing '1' will trigger that
> +		watchdog device and writing '0' will stop it. These are
> +		equivalent to WDIOS_ENABLECARD and WDIOS_DISABLECARD of ioctl
> +		interface. If a device has nowayout programmed, then that
> +		can not be stopped. Therefore, it is recommended to read
> +		state file to insure that whether watchdog device was
> +		stopped or not after writing '0'.
> +
> +What:		/sys/class/watchdog/watchdogn/state
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read only file. If it is read as '1' then a watchdog
> +		device is active. If it is read as '0' then a watchdog
> +		device is inactive.
> +
> +What:		/sys/class/watchdog/watchdogn/status
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read only file. It contains watchdog device's
> +		internal status bits. It is equivalent to WDIOC_GETSTATUS
> +		of ioctl interface.
> +
> +What:		/sys/class/watchdog/watchdogn/timeleft
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read only file. It contains value of time left for
> +		reset generation. It is equivalent to WDIOC_GETTIMELEFT of
> +		ioctl interface.
> +
> +What:		/sys/class/watchdog/watchdogn/timeout
> +Date:		August 2015
> +Contact:	Pratyush Anand <panand-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> +Description:
> +		It is a read/write file. It is read to know about current
> +		timeout and written to program a new timeout value. These
> +		are equivalent to WDIOC_SETTIMEOUT and WDIOC_GETTIMEOUT of
> +		ioctl interface.
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 1a8059455413..703ff7b23f31 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -139,7 +139,7 @@ EXPORT_SYMBOL_GPL(watchdog_init_timeout);
>  
>  static int __watchdog_register_device(struct watchdog_device *wdd)
>  {
> -	int ret, id, devno;
> +	int ret, id;
>  
>  	if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL)
>  		return -EINVAL;
> @@ -181,9 +181,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>  		}
>  	}
>  
> -	devno = wdd->cdev.dev;
> -	wdd->dev = device_create(watchdog_class, wdd->parent, devno,
> -					NULL, "watchdog%d", wdd->id);
> +	wdd->dev = watchdog_device_create(watchdog_class, wdd);

Can we do this in watchdog_dev_register() ?
Seems to make more sense than adding another callback into watchdog_dev.c.

>  	if (IS_ERR(wdd->dev)) {
>  		watchdog_dev_unregister(wdd);
>  		ida_simple_remove(&watchdog_ida, id);
> diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
> index 6c951418fca7..36abd15ffcea 100644
> --- a/drivers/watchdog/watchdog_core.h
> +++ b/drivers/watchdog/watchdog_core.h
> @@ -31,6 +31,8 @@
>  /*
>   *	Functions/procedures to be called by the core
>   */
> +struct device * watchdog_device_create(struct class *,
> +				struct watchdog_device *);
>  extern int watchdog_dev_register(struct watchdog_device *);
>  extern int watchdog_dev_unregister(struct watchdog_device *);
>  extern int __init watchdog_dev_init(void);
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 6aaefbad303e..1186b5a918e9 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -248,6 +248,212 @@ out_timeleft:
>  	return err;
>  }
>  
> +static ssize_t nowayout_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	bool nowayout = false;
> +
> +	mutex_lock(&wdd->lock);
> +	if (test_bit(WDOG_NO_WAY_OUT, &wdd->status))
> +		nowayout = true;
> +	mutex_unlock(&wdd->lock);
> +
> +	return sprintf(buf, "%d\n", nowayout);

	return sprintf(buf, "%d\n", !!test_bit(WDOG_NO_WAY_OUT, &wdd->status));

should do it, and the lock doesn't seem to be very helpful,
as it doesn't make a difference when the flag is evaluated.

> +}
> +static DEVICE_ATTR_RO(nowayout);
> +
> +static ssize_t status_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)

Please align continuation lines with '('.

> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status;
> +	unsigned int val;
> +
> +	status = watchdog_get_status(wdd, &val);
> +	if (!status)
> +		status = sprintf(buf, "%u\n", val);
> +

This attribute should only be visible if supported.

> +	return status;
> +}
> +static DEVICE_ATTR_RO(status);
> +
> +static ssize_t bootstatus_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status;
> +
> +	mutex_lock(&wdd->lock);
> +	status = sprintf(buf, "%u\n", wdd->bootstatus);
> +	mutex_unlock(&wdd->lock);
> +
Useless lock.

> +	return status;
> +}
> +static DEVICE_ATTR_RO(bootstatus);
> +
> +static ssize_t timeleft_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status;
> +	unsigned int val;
> +
> +	status = watchdog_get_timeleft(wdd, &val);
> +
> +	if (!status)
> +		status = sprintf(buf, "%u\n", val);
> +
This attribute should only be visible if supported.

> +	return status;
> +}
> +static DEVICE_ATTR_RO(timeleft);
> +
> +static ssize_t timeout_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	unsigned int val;
> +	ssize_t status = 0;

Unnecessary initialization.

> +
> +	status = kstrtouint(buf, 0, &val);
> +	if (!status) {
> +		status = watchdog_set_timeout(wdd, val);
> +		if (status >= 0)
> +			status = watchdog_ping(wdd);
> +	}
> +
> +	if (status < 0)
> +		return status;
> +	else

Unnecessary else.

> +		return size;
> +}
> +
> +static ssize_t timeout_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status;
> +
> +	mutex_lock(&wdd->lock);
> +	if (wdd->timeout == 0)
> +		status = -EOPNOTSUPP;

Why ?

> +	else
> +		status = sprintf(buf, "%u\n", wdd->timeout);
> +	mutex_unlock(&wdd->lock);
> +
Unnecessary lock.

> +	return status;
> +}
> +static DEVICE_ATTR_RW(timeout);
> +
> +static ssize_t keepalive_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status = 0;
> +
> +	mutex_lock(&wdd->lock);
> +	if (!(wdd->info->options & WDIOF_KEEPALIVEPING))
> +		status = -EOPNOTSUPP;

In that case the attribute should not be there to start with.

Anyway, I would prefer if this attribute would not be there.
Otherwise you'd at least have to check the current state to see
if the watchdog is running in the first place, and you would have
to define what to do if it isn't. Lots of complexity for no clear gain.

> +	mutex_unlock(&wdd->lock);
> +
The lock is unnecessary.

> +	if (!status)
> +		status = watchdog_ping(wdd);
> +
> +	if (status < 0)
> +		return status;
> +	else
> +		return size;
> +}
> +static DEVICE_ATTR_WO(keepalive);
> +
> +static ssize_t identity_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status;
> +
> +	mutex_lock(&wdd->lock);
> +	status = sprintf(buf, "%s\n", wdd->info->identity);
> +	mutex_unlock(&wdd->lock);
> +
Unnecessary lock.

> +	return status;
> +}
> +static DEVICE_ATTR_RO(identity);
> +
> +static ssize_t state_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	bool active;
> +
> +	mutex_lock(&wdd->lock);
> +	active = watchdog_active(wdd);
> +	mutex_unlock(&wdd->lock);
> +
Unnecessary lock.

> +	return sprintf(buf, "%d\n", active);
> +}
> +static DEVICE_ATTR_RO(state);
> +
> +static ssize_t start_store(struct device *dev,
> +		struct device_attribute *attr, const char *buf, size_t size)
> +{

I would prefer not to have this attribute.

> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
> +	ssize_t status = 0;
> +	unsigned int val;
> +
> +	status = kstrtouint(buf, 0, &val);
> +	if (status)
> +		return status;
> +	if (val)
> +		status = watchdog_start(wdd);
> +	else
> +		status = watchdog_stop(wdd);
> +
> +	if (status < 0)
> +		return status;
> +	else
> +		return size;
> +}
> +static DEVICE_ATTR_WO(start);
> +
> +static struct attribute *wdt_attrs[] = {
> +	&dev_attr_start.attr,
> +	&dev_attr_state.attr,
> +	&dev_attr_identity.attr,
> +	&dev_attr_keepalive.attr,
> +	&dev_attr_timeout.attr,
> +	&dev_attr_timeleft.attr,
> +	&dev_attr_bootstatus.attr,
> +	&dev_attr_status.attr,
> +	&dev_attr_nowayout.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group wdt_group = {
> +	.attrs = wdt_attrs,
> +};
> +
> +static const struct attribute_group *wdt_groups[] = {
> +	&wdt_group,
> +	NULL
> +};

You should be able to use ATTRIBUTE_GROUPS or __ATTRIBUTE_GROUPS.

> +
> +/*
> + *	watchdog_device_create: create a struct device for a given watchdog
> + *	device
> + *	@wdc: watchdog class with which created device is associated
> + *	@wdd: watchdog device
> + *
> + *	Creates a struct device corresponding to given watchdog device and
> + *	associates a device attribute_group with it.
> + */
> +struct device * watchdog_device_create(struct class *wdc,
> +					struct watchdog_device *wdd)
> +{
> +	return device_create_with_groups(wdc, wdd->parent, wdd->cdev.dev,
> +			wdd, wdt_groups, "watchdog%d", wdd->id);
> +}
> +
>  /*
>   *	watchdog_ioctl_op: call the watchdog drivers ioctl op if defined
>   *	@wddev: the watchdog device to do the ioctl on

^ permalink raw reply

* Re: [PATCH v2 net-next 0/3] ipv4: Hash-based multipath routing
From: David Miller @ 2015-08-29 20:14 UTC (permalink / raw)
  To: pch-chEQUL3jiZBWk0Htik3J/w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, kuznet-v/Mj1YrvjDBInbfyfbPRSQ,
	jmorris-gx6/JNMH7DfYtjvyW6yDsg, yoshfuji-VfPWfsRibaP+Ru+s062T9g,
	kaber-dcUjhNyLwpNeoWH0uzbU5w, linux-api-u79uwXL29TY76Z2rM5mHXA,
	roopa-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR,
	sfeldma-Re5JQEeQqe8AvxtiuMwx3w, ebiederm-aS9lmoZGLiVWk0Htik3J/w,
	nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, tgraf-G/eBtMaohhA,
	jbenc-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <1440792050-2109-1-git-send-email-pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>

From: pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org
Date: Fri, 28 Aug 2015 22:00:47 +0200

> When the routing cache was removed in 3.6, the IPv4 multipath algorithm changed
> from more or less being destination-based into being quasi-random per-packet
> scheduling. This increases the risk of out-of-order packets and makes it
> impossible to use multipath together with anycast services.

Don't even try to be fancy.

Simply kill the round-robin stuff off completely, and make hash based
routing the one and only mode, no special configuration stuff
necessary.

This puts ipv4 in line with ipv6, and nobody cares about the existing
round robin behavior at all.

Thanks.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox