Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] mm: memcontrol: consolidate cgroup socket tracking
From: kbuild test robot @ 2016-09-14 21:12 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: kbuild-all, Andrew Morton, Tejun Heo, David S. Miller,
	Michal Hocko, Vladimir Davydov, linux-mm, cgroups, netdev,
	linux-kernel, kernel-team
In-Reply-To: <20160914194846.11153-3-hannes@cmpxchg.org>

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

Hi Johannes,

[auto build test ERROR on net/master]
[also build test ERROR on v4.8-rc6 next-20160914]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Johannes-Weiner/mm-memcontrol-make-per-cpu-charge-cache-IRQ-safe-for-socket-accounting/20160915-035634
config: parisc-b180_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=parisc 

All errors (new ones prefixed by >>):

   net/built-in.o: In function `sk_alloc':
   (.text.sk_alloc+0xb4): undefined reference to `mem_cgroup_sk_alloc'
   net/built-in.o: In function `__sk_destruct':
>> net/core/.tmp_sock.o:(.text.__sk_destruct+0xdc): undefined reference to `mem_cgroup_sk_free'
   net/built-in.o: In function `sk_clone_lock':
   (.text.sk_clone_lock+0x184): undefined reference to `mem_cgroup_sk_alloc'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 12799 bytes --]

^ permalink raw reply

* Re: [PATCH 7/9] net: ethernet: ti: cpts: calc mult and shift from refclk freq
From: Grygorii Strashko @ 2016-09-14 21:14 UTC (permalink / raw)
  To: Richard Cochran
  Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
	linux-omap, WingMan Kwok, John Stultz
In-Reply-To: <20160914210332.GG12195@netboy>

On 09/15/2016 12:03 AM, Richard Cochran wrote:
> On Wed, Sep 14, 2016 at 11:47:46PM +0300, Grygorii Strashko wrote:
>> As I understand (and tested), clocks_calc_mult_shift() will
>> return max possible mult which can be used without overflow.
>
> Yes, BUT the returned values depends on the @maxsec input.  As the
> kerneldec says,
>
>  * Larger ranges may reduce the conversion accuracy by chosing smaller
>  * mult and shift factors.
>
> In addition to that, frequency tuning by calculating a percentage of
> 'mult', and if 'mult' is small, then the tuning resolution is poor.
>
> So we don't want @maxsec as large as possible, but as small as
> possible.
>

Ok. So, will it work if maxsec will be ~= (maxsec / 2) + 1?
+ 1 to cover potential delays of overflow check work execution.

[...]


-- 
regards,
-grygorii

^ permalink raw reply

* Re: [RFC v3 14/22] bpf/cgroup: Make cgroup_bpf_update() return an error code
From: Alexei Starovoitov @ 2016-09-14 21:16 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module
In-Reply-To: <20160914072415.26021-15-mic@digikod.net>

On Wed, Sep 14, 2016 at 09:24:07AM +0200, Mickaël Salaün wrote:
> This will be useful to support Landlock for the next commits.
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Tejun Heo <tj@kernel.org>

I think this is good to do regardless. Sooner or later cgroup_bpf_update
will start rejecting the prog attach. Like we discussed to have a flag
that would dissallow processeses lower in the cgroup hierarchy to install
their own bpf programs.
It will be minimal change to bpf_prog_attach() error handling,
but will greatly help Mickael to build stuff on top.
DanielM can you refactor your patch to do that from the start ?

Thanks!

> ---
>  include/linux/bpf-cgroup.h |  4 ++--
>  kernel/bpf/cgroup.c        |  3 ++-
>  kernel/bpf/syscall.c       | 10 ++++++----
>  kernel/cgroup.c            |  6 ++++--
>  4 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
> index 2234042d7f61..6cca7924ee17 100644
> --- a/include/linux/bpf-cgroup.h
> +++ b/include/linux/bpf-cgroup.h
> @@ -31,13 +31,13 @@ struct cgroup_bpf {
>  void cgroup_bpf_put(struct cgroup *cgrp);
>  void cgroup_bpf_inherit(struct cgroup *cgrp, struct cgroup *parent);
>  
> -void __cgroup_bpf_update(struct cgroup *cgrp,
> +int __cgroup_bpf_update(struct cgroup *cgrp,
>  			 struct cgroup *parent,
>  			 struct bpf_prog *prog,
>  			 enum bpf_attach_type type);
>  
>  /* Wrapper for __cgroup_bpf_update() protected by cgroup_mutex */
> -void cgroup_bpf_update(struct cgroup *cgrp,
> +int cgroup_bpf_update(struct cgroup *cgrp,
>  		       struct bpf_prog *prog,
>  		       enum bpf_attach_type type);
>  
> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
> index 782878ec4f2d..7b75fa692617 100644
> --- a/kernel/bpf/cgroup.c
> +++ b/kernel/bpf/cgroup.c
> @@ -83,7 +83,7 @@ void cgroup_bpf_inherit(struct cgroup *cgrp, struct cgroup *parent)
>   *
>   * Must be called with cgroup_mutex held.
>   */
> -void __cgroup_bpf_update(struct cgroup *cgrp,
> +int __cgroup_bpf_update(struct cgroup *cgrp,
>  			 struct cgroup *parent,
>  			 struct bpf_prog *prog,
>  			 enum bpf_attach_type type)
> @@ -117,6 +117,7 @@ void __cgroup_bpf_update(struct cgroup *cgrp,
>  		bpf_prog_put(old_pinned.prog);
>  		static_branch_dec(&cgroup_bpf_enabled_key);
>  	}
> +	return 0;
>  }
>  
>  /**
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 45a91d511119..c978f2d9a1b3 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -831,6 +831,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>  {
>  	struct bpf_prog *prog;
>  	struct cgroup *cgrp;
> +	int result;
>  
>  	if (!capable(CAP_NET_ADMIN))
>  		return -EPERM;
> @@ -858,10 +859,10 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>  		return PTR_ERR(cgrp);
>  	}
>  
> -	cgroup_bpf_update(cgrp, prog, attr->attach_type);
> +	result = cgroup_bpf_update(cgrp, prog, attr->attach_type);
>  	cgroup_put(cgrp);
>  
> -	return 0;
> +	return result;
>  }
>  
>  #define BPF_PROG_DETACH_LAST_FIELD attach_type
> @@ -869,6 +870,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
>  static int bpf_prog_detach(const union bpf_attr *attr)
>  {
>  	struct cgroup *cgrp;
> +	int result = 0;
>  
>  	if (!capable(CAP_NET_ADMIN))
>  		return -EPERM;
> @@ -883,7 +885,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>  		if (IS_ERR(cgrp))
>  			return PTR_ERR(cgrp);
>  
> -		cgroup_bpf_update(cgrp, NULL, attr->attach_type);
> +		result = cgroup_bpf_update(cgrp, NULL, attr->attach_type);
>  		cgroup_put(cgrp);
>  		break;
>  
> @@ -891,7 +893,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
>  		return -EINVAL;
>  	}
>  
> -	return 0;
> +	return result;
>  }
>  #endif /* CONFIG_CGROUP_BPF */
>  
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 87324ce481b1..48b650a640a9 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -6450,15 +6450,17 @@ static __init int cgroup_namespaces_init(void)
>  subsys_initcall(cgroup_namespaces_init);
>  
>  #ifdef CONFIG_CGROUP_BPF
> -void cgroup_bpf_update(struct cgroup *cgrp,
> +int cgroup_bpf_update(struct cgroup *cgrp,
>  		       struct bpf_prog *prog,
>  		       enum bpf_attach_type type)
>  {
>  	struct cgroup *parent = cgroup_parent(cgrp);
> +	int result;
>  
>  	mutex_lock(&cgroup_mutex);
> -	__cgroup_bpf_update(cgrp, parent, prog, type);
> +	result = __cgroup_bpf_update(cgrp, parent, prog, type);
>  	mutex_unlock(&cgroup_mutex);
> +	return result;
>  }
>  #endif /* CONFIG_CGROUP_BPF */
>  
> -- 
> 2.9.3
> 

^ permalink raw reply

* Re: [RFC v3 21/22] bpf,landlock: Add optional skb pointer in the Landlock context
From: Alexei Starovoitov @ 2016-09-14 21:20 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module
In-Reply-To: <20160914072415.26021-22-mic@digikod.net>

On Wed, Sep 14, 2016 at 09:24:14AM +0200, Mickaël Salaün wrote:
> This is a proof of concept to expose optional values that could depend
> of the process access rights.
> 
> There is two dedicated flags: LANDLOCK_FLAG_ACCESS_SKB_READ and
> LANDLOCK_FLAG_ACCESS_SKB_WRITE. Each of them can be activated to access
> eBPF functions manipulating a skb in a read or write way.
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
...
>  /* Handle check flags */
>  #define LANDLOCK_FLAG_FS_DENTRY		(1 << 0)
> @@ -619,12 +621,15 @@ struct landlock_handle {
>   * @args: LSM hook arguments, see include/linux/lsm_hooks.h for there
>   *        description and the LANDLOCK_HOOK* definitions from
>   *        security/landlock/lsm.c for their types.
> + * @opt_skb: optional skb pointer, accessible with the
> + *           LANDLOCK_FLAG_ACCESS_SKB_* flags for network-related hooks.
>   */
>  struct landlock_data {
>  	__u32 hook; /* enum landlock_hook_id */
>  	__u16 origin; /* LANDLOCK_FLAG_ORIGIN_* */
>  	__u16 cookie; /* seccomp RET_LANDLOCK */
>  	__u64 args[6];
> +	__u64 opt_skb;
>  };

missing something here.
This patch doesn't make use of it.
That's something for the future?
How that field will be populated?
Why make it different vs the rest or args[6] ?

^ permalink raw reply

* Re: [RFC v3 22/22] samples/landlock: Add sandbox example
From: Alexei Starovoitov @ 2016-09-14 21:24 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module
In-Reply-To: <20160914072415.26021-23-mic@digikod.net>

On Wed, Sep 14, 2016 at 09:24:15AM +0200, Mickaël Salaün wrote:
> Add a basic sandbox tool to create a process isolated from some part of
> the system. This can depend of the current cgroup.
> 
> Example with the current process hierarchy (seccomp):
> 
>   $ ls /home
>   user1
>   $ LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
>       ./samples/landlock/sandbox /bin/sh -i
>   Launching a new sandboxed process.
>   $ ls /home
>   ls: cannot open directory '/home': Permission denied
> 
> Example with a cgroup:
> 
>   $ mkdir /sys/fs/cgroup/sandboxed
>   $ ls /home
>   user1
>   $ LANDLOCK_CGROUPS='/sys/fs/cgroup/sandboxed' \
>       LANDLOCK_ALLOWED='/bin:/lib:/usr:/tmp:/proc/self/fd/0' \
>       ./samples/landlock/sandbox
>   Ready to sandbox with cgroups.
>   $ ls /home
>   user1
>   $ echo $$ > /sys/fs/cgroup/sandboxed/cgroup.procs
>   $ ls /home
>   ls: cannot open directory '/home': Permission denied
> 
> Changes since v2:
> * use BPF_PROG_ATTACH for cgroup handling
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
...
> +	struct bpf_insn hook_path[] = {
> +		/* specify an option, if any */
> +		BPF_MOV32_IMM(BPF_REG_1, 0),
> +		/* handles to compare with */
> +		BPF_LD_MAP_FD(BPF_REG_2, map_fs),
> +		BPF_MOV64_IMM(BPF_REG_3, BPF_MAP_ARRAY_OP_OR),
> +		/* hook argument (struct file) */
> +		BPF_LDX_MEM(BPF_DW, BPF_REG_4, BPF_REG_6, offsetof(struct
> +					landlock_data, args[0])),
> +		/* checker function */
> +		BPF_EMIT_CALL(BPF_FUNC_landlock_cmp_fs_beneath_with_struct_file),

the example is sweet!
Since only that helper is used, could you skip the other one
from the patches (for now) ?

^ permalink raw reply

* [RFC 0/3] Add support for led triggers on phy link state change
From: Zach Brown @ 2016-09-14 21:55 UTC (permalink / raw)
  To: f.fainelli
  Cc: devel, florian.c.schilhabel, netdev, linux-kernel, Larry.Finger,
	mlindner

Some drivers that include phy.h defined LED_OFF which conflicts with
definition in leds.h. phy led support uses leds.h so the two namespaces are no
longer isolated.
The first two patches fix the two net drivers that declared enum constants that
conflict with enum constants in linux/leds.h.

The final patch adds support for led triggers on phy link state changes by
adding a config option. When set the config option will create a set of led
triggers for each phy device. Users can use the led triggers to represent link
state changes on the phy.
The changes assumes that there are 5 speed options
10Mb,100Mb,1Gb,2.5Gb,10Gb
The assumption makes mapping a phy_device's current speed to a trigger easy,
but means there are triggers made that aren't used if the phy doesn't support
the corresponding speeds.
Thoughts on how to better manage the triggers created would be appreciated
if is important to do so.

Josh Cartwright (1):
  phy,leds: add support for led triggers on phy link state change

Zach Brown (2):
  skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid
    conflicts with leds namespace
  staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid
    conflicts with LED namespace

 drivers/net/ethernet/marvell/skge.c   |   4 +-
 drivers/net/ethernet/marvell/skge.h   |   2 +-
 drivers/net/phy/Kconfig               |  12 +++
 drivers/net/phy/Makefile              |   1 +
 drivers/net/phy/phy.c                 |   8 ++
 drivers/net/phy/phy_device.c          |   4 +
 drivers/net/phy/phy_led_triggers.c    | 109 +++++++++++++++++++
 drivers/staging/rtl8712/rtl8712_led.c | 192 +++++++++++++++++-----------------
 include/linux/phy.h                   |   9 ++
 include/linux/phy_led_triggers.h      |  42 ++++++++
 10 files changed, 284 insertions(+), 99 deletions(-)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

--
2.7.4

^ permalink raw reply

* [RFC 1/3] skge: Change LED_OFF to LED_REG_OFF in marvel skge driver to avoid conflicts with leds namespace
From: Zach Brown @ 2016-09-14 21:55 UTC (permalink / raw)
  To: f.fainelli
  Cc: devel, florian.c.schilhabel, netdev, linux-kernel, Larry.Finger,
	mlindner
In-Reply-To: <1473890149-2066-1-git-send-email-zach.brown@ni.com>

Adding led support for phy causes namespace conflicts for some
phy drivers.

The marvel skge driver declared an enum for representing the states of
Link LED Register. The enum contained constant LED_OFF which conflicted
with declartation found in linux/leds.h.
LED_OFF changed to LED_REG_OFF

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/net/ethernet/marvell/skge.c | 4 ++--
 drivers/net/ethernet/marvell/skge.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/marvell/skge.c b/drivers/net/ethernet/marvell/skge.c
index 7173836..ff5a087 100644
--- a/drivers/net/ethernet/marvell/skge.c
+++ b/drivers/net/ethernet/marvell/skge.c
@@ -1062,7 +1062,7 @@ static void skge_link_up(struct skge_port *skge)
 
 static void skge_link_down(struct skge_port *skge)
 {
-	skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+	skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
 	netif_carrier_off(skge->netdev);
 	netif_stop_queue(skge->netdev);
 
@@ -2668,7 +2668,7 @@ static int skge_down(struct net_device *dev)
 	if (hw->ports == 1)
 		free_irq(hw->pdev->irq, hw);
 
-	skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_OFF);
+	skge_write8(skge->hw, SK_REG(skge->port, LNK_LED_REG), LED_REG_OFF);
 	if (is_genesis(hw))
 		genesis_stop(skge);
 	else
diff --git a/drivers/net/ethernet/marvell/skge.h b/drivers/net/ethernet/marvell/skge.h
index a2eb341..ec054d3 100644
--- a/drivers/net/ethernet/marvell/skge.h
+++ b/drivers/net/ethernet/marvell/skge.h
@@ -663,7 +663,7 @@ enum {
 	LED_SYNC_ON	= 1<<3,	/* Use Sync Wire to switch LED */
 	LED_SYNC_OFF	= 1<<2,	/* Disable Sync Wire Input */
 	LED_ON	= 1<<1,	/* switch LED on */
-	LED_OFF	= 1<<0,	/* switch LED off */
+	LED_REG_OFF	= 1<<0,	/* switch LED off */
 };
 
 /* Receive GMAC FIFO (YUKON) */
-- 
2.7.4

^ permalink raw reply related

* [RFC 2/3] staging: rtl8712: Change _LED_STATE enum in rtl871x driver to avoid conflicts with LED namespace
From: Zach Brown @ 2016-09-14 21:55 UTC (permalink / raw)
  To: f.fainelli
  Cc: mlindner, stephen, netdev, linux-kernel, devel,
	florian.c.schilhabel, Larry.Finger
In-Reply-To: <1473890149-2066-1-git-send-email-zach.brown@ni.com>

Adding led support for phy causes namespace conflicts for some
phy drivers.

The rtl871 driver declared an enum for representing LED states. The enum
contains constant LED_OFF which conflicted with declartation found in
linux/leds.h. LED_OFF changed to LED_STATE_OFF

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/staging/rtl8712/rtl8712_led.c | 192 +++++++++++++++++-----------------
 1 file changed, 96 insertions(+), 96 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl8712_led.c b/drivers/staging/rtl8712/rtl8712_led.c
index 9055827..d53ad76 100644
--- a/drivers/staging/rtl8712/rtl8712_led.c
+++ b/drivers/staging/rtl8712/rtl8712_led.c
@@ -52,7 +52,7 @@
 enum _LED_STATE_871x {
 	LED_UNKNOWN = 0,
 	LED_ON = 1,
-	LED_OFF = 2,
+	LED_STATE_OFF = 2,
 	LED_BLINK_NORMAL = 3,
 	LED_BLINK_SLOWLY = 4,
 	LED_POWER_ON_BLINK = 5,
@@ -92,7 +92,7 @@ static void InitLed871x(struct _adapter *padapter, struct LED_871x *pLed,
 	nic = padapter->pnetdev;
 	pLed->padapter = padapter;
 	pLed->LedPin = LedPin;
-	pLed->CurrLedState = LED_OFF;
+	pLed->CurrLedState = LED_STATE_OFF;
 	pLed->bLedOn = false;
 	pLed->bLedBlinkInProgress = false;
 	pLed->BlinkTimes = 0;
@@ -249,7 +249,7 @@ static void SwLedBlink(struct LED_871x *pLed)
 	} else {
 		/* Assign LED state to toggle. */
 		if (pLed->BlinkingLedState == LED_ON)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 
@@ -312,7 +312,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 	switch (pLed->CurrLedState) {
 	case LED_BLINK_SLOWLY:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -320,7 +320,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_NORMAL:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -335,7 +335,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->bLedLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_NORMAL;
 				if (pLed->bLedOn)
-					pLed->BlinkingLedState = LED_OFF;
+					pLed->BlinkingLedState = LED_STATE_OFF;
 				else
 					pLed->BlinkingLedState = LED_ON;
 				mod_timer(&pLed->BlinkTimer, jiffies +
@@ -344,7 +344,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
 				if (pLed->bLedOn)
-					pLed->BlinkingLedState = LED_OFF;
+					pLed->BlinkingLedState = LED_STATE_OFF;
 				else
 					pLed->BlinkingLedState = LED_ON;
 				mod_timer(&pLed->BlinkTimer, jiffies +
@@ -353,7 +353,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
 			 if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -369,7 +369,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->bLedLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_NORMAL;
 				if (pLed->bLedOn)
-					pLed->BlinkingLedState = LED_OFF;
+					pLed->BlinkingLedState = LED_STATE_OFF;
 				else
 					pLed->BlinkingLedState = LED_ON;
 				mod_timer(&pLed->BlinkTimer, jiffies +
@@ -378,7 +378,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 				pLed->bLedNoLinkBlinkInProgress = true;
 				pLed->CurrLedState = LED_BLINK_SLOWLY;
 				if (pLed->bLedOn)
-					pLed->BlinkingLedState = LED_OFF;
+					pLed->BlinkingLedState = LED_STATE_OFF;
 				else
 					pLed->BlinkingLedState = LED_ON;
 				mod_timer(&pLed->BlinkTimer, jiffies +
@@ -388,7 +388,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			 if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -397,7 +397,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -405,7 +405,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS_STOP:	/* WPS success */
 		if (pLed->BlinkingLedState == LED_ON) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA));
 			bStopBlinking = false;
@@ -416,7 +416,7 @@ static void SwLedBlink1(struct LED_871x *pLed)
 			pLed->bLedLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_NORMAL;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -451,14 +451,14 @@ static void SwLedBlink2(struct LED_871x *pLed)
 				pLed->BlinkingLedState = LED_ON;
 				SwLedOn(padapter, pLed);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
-				pLed->CurrLedState = LED_OFF;
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->CurrLedState = LED_STATE_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				SwLedOff(padapter, pLed);
 			}
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
 			 if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -475,14 +475,14 @@ static void SwLedBlink2(struct LED_871x *pLed)
 				pLed->BlinkingLedState = LED_ON;
 				SwLedOn(padapter, pLed);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
-				pLed->CurrLedState = LED_OFF;
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->CurrLedState = LED_STATE_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				SwLedOff(padapter, pLed);
 			}
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -518,15 +518,15 @@ static void SwLedBlink3(struct LED_871x *pLed)
 				if (!pLed->bLedOn)
 					SwLedOn(padapter, pLed);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
-				pLed->CurrLedState = LED_OFF;
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->CurrLedState = LED_STATE_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				if (pLed->bLedOn)
 					SwLedOff(padapter, pLed);
 			}
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -544,15 +544,15 @@ static void SwLedBlink3(struct LED_871x *pLed)
 				if (!pLed->bLedOn)
 					SwLedOn(padapter, pLed);
 			} else if (!check_fwstate(pmlmepriv, _FW_LINKED)) {
-				pLed->CurrLedState = LED_OFF;
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->CurrLedState = LED_STATE_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				if (pLed->bLedOn)
 					SwLedOff(padapter, pLed);
 			}
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -561,7 +561,7 @@ static void SwLedBlink3(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -569,7 +569,7 @@ static void SwLedBlink3(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS_STOP:	/*WPS success*/
 		if (pLed->BlinkingLedState == LED_ON) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA));
 			bStopBlinking = false;
@@ -602,14 +602,14 @@ static void SwLedBlink4(struct LED_871x *pLed)
 		SwLedOff(padapter, pLed);
 	if (!pLed1->bLedWPSBlinkInProgress &&
 	    pLed1->BlinkingLedState == LED_UNKNOWN) {
-		pLed1->BlinkingLedState = LED_OFF;
-		pLed1->CurrLedState = LED_OFF;
+		pLed1->BlinkingLedState = LED_STATE_OFF;
+		pLed1->CurrLedState = LED_STATE_OFF;
 		SwLedOff(padapter, pLed1);
 	}
 	switch (pLed->CurrLedState) {
 	case LED_BLINK_SLOWLY:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -617,7 +617,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_StartToBlink:
 		if (pLed->bLedOn) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_SLOWLY_INTERVAL));
 		} else {
@@ -634,7 +634,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 			pLed->bLedNoLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_SLOWLY;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -642,7 +642,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -657,7 +657,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 			pLed->bLedNoLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_SLOWLY;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -665,7 +665,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			 if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -674,7 +674,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS:
 		if (pLed->bLedOn) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_SLOWLY_INTERVAL));
 		} else {
@@ -685,7 +685,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS_STOP:	/*WPS authentication fail*/
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -706,7 +706,7 @@ static void SwLedBlink4(struct LED_871x *pLed)
 				  msecs_to_jiffies(LED_BLINK_LINK_INTERVAL_ALPHA));
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -742,7 +742,7 @@ static void SwLedBlink5(struct LED_871x *pLed)
 			pLed->bLedScanBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -762,7 +762,7 @@ static void SwLedBlink5(struct LED_871x *pLed)
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			 if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -797,7 +797,7 @@ static void SwLedBlink6(struct LED_871x *pLed)
 			pLed->bLedBlinkInProgress = false;
 		} else {
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -806,7 +806,7 @@ static void SwLedBlink6(struct LED_871x *pLed)
 		break;
 	case LED_BLINK_WPS:
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -908,7 +908,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->bLedNoLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_SLOWLY;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -931,7 +931,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->bLedLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_NORMAL;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -961,7 +961,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->CurrLedState = LED_SCAN_BLINK;
 			pLed->BlinkTimes = 24;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -986,7 +986,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1016,7 +1016,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->bLedWPSBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_WPS;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1046,7 +1046,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			pLed->bLedWPSBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_WPS_STOP;
 		if (pLed->bLedOn) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA));
 		} else {
@@ -1063,15 +1063,15 @@ static void SwLedControlMode1(struct _adapter *padapter,
 		pLed->bLedNoLinkBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_SLOWLY;
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
 			  msecs_to_jiffies(LED_BLINK_NO_LINK_INTERVAL_ALPHA));
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedNoLinkBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedNoLinkBlinkInProgress = false;
@@ -1123,7 +1123,7 @@ static void SwLedControlMode2(struct _adapter *padapter,
 			pLed->CurrLedState = LED_SCAN_BLINK;
 			pLed->BlinkTimes = 24;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1142,7 +1142,7 @@ static void SwLedControlMode2(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1195,8 +1195,8 @@ static void SwLedControlMode2(struct _adapter *padapter,
 
 	case LED_CTL_STOP_WPS_FAIL:
 		pLed->bLedWPSBlinkInProgress = false;
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		mod_timer(&pLed->BlinkTimer,
 			  jiffies + msecs_to_jiffies(0));
 		break;
@@ -1204,15 +1204,15 @@ static void SwLedControlMode2(struct _adapter *padapter,
 	case LED_CTL_START_TO_LINK:
 	case LED_CTL_NO_LINK:
 		if (!IS_LED_BLINKING(pLed)) {
-			pLed->CurrLedState = LED_OFF;
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->CurrLedState = LED_STATE_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer,
 				  jiffies + msecs_to_jiffies(0));
 		}
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedBlinkInProgress = false;
@@ -1255,7 +1255,7 @@ static void SwLedControlMode3(struct _adapter *padapter,
 			pLed->CurrLedState = LED_SCAN_BLINK;
 			pLed->BlinkTimes = 24;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1273,7 +1273,7 @@ static void SwLedControlMode3(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1310,7 +1310,7 @@ static void SwLedControlMode3(struct _adapter *padapter,
 			pLed->bLedWPSBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_WPS;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1326,7 +1326,7 @@ static void SwLedControlMode3(struct _adapter *padapter,
 		}
 		pLed->CurrLedState = LED_BLINK_WPS_STOP;
 		if (pLed->bLedOn) {
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer, jiffies +
 				  msecs_to_jiffies(LED_BLINK_WPS_SUCESS_INTERVAL_ALPHA));
 		} else {
@@ -1340,23 +1340,23 @@ static void SwLedControlMode3(struct _adapter *padapter,
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedWPSBlinkInProgress = false;
 		}
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		mod_timer(&pLed->BlinkTimer,
 			  jiffies + msecs_to_jiffies(0));
 		break;
 	case LED_CTL_START_TO_LINK:
 	case LED_CTL_NO_LINK:
 		if (!IS_LED_BLINKING(pLed)) {
-			pLed->CurrLedState = LED_OFF;
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->CurrLedState = LED_STATE_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 			mod_timer(&pLed->BlinkTimer,
 				  jiffies + msecs_to_jiffies(0));
 		}
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedBlinkInProgress = false;
@@ -1390,8 +1390,8 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		if (pLed1->bLedWPSBlinkInProgress) {
 			pLed1->bLedWPSBlinkInProgress = false;
 			del_timer(&pLed1->BlinkTimer);
-			pLed1->BlinkingLedState = LED_OFF;
-			pLed1->CurrLedState = LED_OFF;
+			pLed1->BlinkingLedState = LED_STATE_OFF;
+			pLed1->CurrLedState = LED_STATE_OFF;
 			if (pLed1->bLedOn)
 				mod_timer(&pLed->BlinkTimer,
 					  jiffies + msecs_to_jiffies(0));
@@ -1411,7 +1411,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed->bLedStartToLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_StartToBlink;
 			if (pLed->bLedOn) {
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				mod_timer(&pLed->BlinkTimer, jiffies +
 					  msecs_to_jiffies(LED_BLINK_SLOWLY_INTERVAL));
 			} else {
@@ -1428,8 +1428,8 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			if (pLed1->bLedWPSBlinkInProgress) {
 				pLed1->bLedWPSBlinkInProgress = false;
 				del_timer(&pLed1->BlinkTimer);
-				pLed1->BlinkingLedState = LED_OFF;
-				pLed1->CurrLedState = LED_OFF;
+				pLed1->BlinkingLedState = LED_STATE_OFF;
+				pLed1->CurrLedState = LED_STATE_OFF;
 				if (pLed1->bLedOn)
 					mod_timer(&pLed->BlinkTimer,
 						  jiffies + msecs_to_jiffies(0));
@@ -1446,7 +1446,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed->bLedNoLinkBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_SLOWLY;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1472,7 +1472,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed->CurrLedState = LED_SCAN_BLINK;
 			pLed->BlinkTimes = 24;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1493,7 +1493,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1505,8 +1505,8 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		if (pLed1->bLedWPSBlinkInProgress) {
 			pLed1->bLedWPSBlinkInProgress = false;
 			del_timer(&pLed1->BlinkTimer);
-			pLed1->BlinkingLedState = LED_OFF;
-			pLed1->CurrLedState = LED_OFF;
+			pLed1->BlinkingLedState = LED_STATE_OFF;
+			pLed1->CurrLedState = LED_STATE_OFF;
 			if (pLed1->bLedOn)
 				mod_timer(&pLed->BlinkTimer,
 					  jiffies + msecs_to_jiffies(0));
@@ -1527,7 +1527,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed->bLedWPSBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_WPS;
 			if (pLed->bLedOn) {
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 				mod_timer(&pLed->BlinkTimer, jiffies +
 					  msecs_to_jiffies(LED_BLINK_SLOWLY_INTERVAL));
 			} else {
@@ -1545,7 +1545,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		pLed->bLedNoLinkBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_SLOWLY;
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1559,7 +1559,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		pLed->bLedNoLinkBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_SLOWLY;
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1571,7 +1571,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 			pLed1->bLedWPSBlinkInProgress = true;
 		pLed1->CurrLedState = LED_BLINK_WPS_STOP;
 		if (pLed1->bLedOn)
-			pLed1->BlinkingLedState = LED_OFF;
+			pLed1->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed1->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1585,7 +1585,7 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		pLed->bLedNoLinkBlinkInProgress = true;
 		pLed->CurrLedState = LED_BLINK_SLOWLY;
 		if (pLed->bLedOn)
-			pLed->BlinkingLedState = LED_OFF;
+			pLed->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1598,15 +1598,15 @@ static void SwLedControlMode4(struct _adapter *padapter,
 		pLed1->CurrLedState = LED_BLINK_WPS_STOP_OVERLAP;
 		pLed1->BlinkTimes = 10;
 		if (pLed1->bLedOn)
-			pLed1->BlinkingLedState = LED_OFF;
+			pLed1->BlinkingLedState = LED_STATE_OFF;
 		else
 			pLed1->BlinkingLedState = LED_ON;
 		mod_timer(&pLed->BlinkTimer, jiffies +
 			  msecs_to_jiffies(LED_BLINK_NORMAL_INTERVAL));
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedNoLinkBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedNoLinkBlinkInProgress = false;
@@ -1679,7 +1679,7 @@ static void SwLedControlMode5(struct _adapter *padapter,
 			pLed->CurrLedState = LED_SCAN_BLINK;
 			pLed->BlinkTimes = 24;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1695,7 +1695,7 @@ static void SwLedControlMode5(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1703,8 +1703,8 @@ static void SwLedControlMode5(struct _adapter *padapter,
 		}
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedBlinkInProgress = false;
@@ -1746,7 +1746,7 @@ static void SwLedControlMode6(struct _adapter *padapter,
 			pLed->CurrLedState = LED_TXRX_BLINK;
 			pLed->BlinkTimes = 2;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1763,7 +1763,7 @@ static void SwLedControlMode6(struct _adapter *padapter,
 			pLed->bLedWPSBlinkInProgress = true;
 			pLed->CurrLedState = LED_BLINK_WPS;
 			if (pLed->bLedOn)
-				pLed->BlinkingLedState = LED_OFF;
+				pLed->BlinkingLedState = LED_STATE_OFF;
 			else
 				pLed->BlinkingLedState = LED_ON;
 			mod_timer(&pLed->BlinkTimer, jiffies +
@@ -1782,8 +1782,8 @@ static void SwLedControlMode6(struct _adapter *padapter,
 			  jiffies + msecs_to_jiffies(0));
 		break;
 	case LED_CTL_POWER_OFF:
-		pLed->CurrLedState = LED_OFF;
-		pLed->BlinkingLedState = LED_OFF;
+		pLed->CurrLedState = LED_STATE_OFF;
+		pLed->BlinkingLedState = LED_STATE_OFF;
 		if (pLed->bLedBlinkInProgress) {
 			del_timer(&pLed->BlinkTimer);
 			pLed->bLedBlinkInProgress = false;
-- 
2.7.4

^ permalink raw reply related

* [RFC 3/3] phy,leds: add support for led triggers on phy link state change
From: Zach Brown @ 2016-09-14 21:55 UTC (permalink / raw)
  To: f.fainelli
  Cc: mlindner, stephen, netdev, linux-kernel, devel,
	florian.c.schilhabel, Larry.Finger
In-Reply-To: <1473890149-2066-1-git-send-email-zach.brown@ni.com>

From: Josh Cartwright <josh.cartwright@ni.com>

Create an option CONFIG_LED_TRIGGER_PHY (default n), which will
create a set of led triggers for each instantiated PHY device.  There is
one LED trigger per link-speed, per-phy.

This allows for a user to configure their system to allow a set of LEDs
to represent link state changes on the phy.

Signed-off-by: Josh Cartwright <josh.cartwright@ni.com>
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/net/phy/Kconfig            |  12 ++++
 drivers/net/phy/Makefile           |   1 +
 drivers/net/phy/phy.c              |   8 +++
 drivers/net/phy/phy_device.c       |   4 ++
 drivers/net/phy/phy_led_triggers.c | 109 +++++++++++++++++++++++++++++++++++++
 include/linux/phy.h                |   9 +++
 include/linux/phy_led_triggers.h   |  42 ++++++++++++++
 7 files changed, 185 insertions(+)
 create mode 100644 drivers/net/phy/phy_led_triggers.c
 create mode 100644 include/linux/phy_led_triggers.h

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1c3e07c..4aeaba4 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -15,6 +15,18 @@ if PHYLIB
 config SWPHY
 	bool
 
+config LED_TRIGGER_PHY
+	bool "Support LED triggers for tracking link state"
+	depends on LEDS_TRIGGERS
+	---help---
+	  Adds support for a set of LED trigger events per-PHY.  Link
+	  state change will trigger the events, for consumption by an
+	  LED class driver.  There are triggers for each link speed,
+	  and are of the form:
+	       <mii bus id>:<phy>:<speed>
+
+	  Where speed is one of: 10Mb, 100Mb, Gb, 2.5Gb, or 10GbE.
+
 comment "MDIO bus device drivers"
 
 config MDIO_BCM_IPROC
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index e58667d..86d12cd 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -2,6 +2,7 @@
 
 libphy-y			:= phy.o phy_device.o mdio_bus.o mdio_device.o
 libphy-$(CONFIG_SWPHY)		+= swphy.o
+libphy-$(CONFIG_LED_TRIGGER_PHY)	+= phy_led_triggers.o
 
 obj-$(CONFIG_PHYLIB)		+= libphy.o
 
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index c6f6683..3345767 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -936,6 +936,7 @@ void phy_state_machine(struct work_struct *work)
 			phydev->state = PHY_NOLINK;
 			netif_carrier_off(phydev->attached_dev);
 			phydev->adjust_link(phydev->attached_dev);
+			phy_led_trigger_change_speed(phydev);
 			break;
 		}
 
@@ -949,6 +950,7 @@ void phy_state_machine(struct work_struct *work)
 			phydev->state = PHY_RUNNING;
 			netif_carrier_on(phydev->attached_dev);
 			phydev->adjust_link(phydev->attached_dev);
+			phy_led_trigger_change_speed(phydev);
 
 		} else if (0 == phydev->link_timeout--)
 			needs_aneg = true;
@@ -976,6 +978,7 @@ void phy_state_machine(struct work_struct *work)
 			phydev->state = PHY_RUNNING;
 			netif_carrier_on(phydev->attached_dev);
 			phydev->adjust_link(phydev->attached_dev);
+			phy_led_trigger_change_speed(phydev);
 		}
 		break;
 	case PHY_FORCING:
@@ -992,6 +995,7 @@ void phy_state_machine(struct work_struct *work)
 		}
 
 		phydev->adjust_link(phydev->attached_dev);
+		phy_led_trigger_change_speed(phydev);
 		break;
 	case PHY_RUNNING:
 		/* Only register a CHANGE if we are polling and link changed
@@ -1021,6 +1025,7 @@ void phy_state_machine(struct work_struct *work)
 		}
 
 		phydev->adjust_link(phydev->attached_dev);
+		phy_led_trigger_change_speed(phydev);
 
 		if (phy_interrupt_is_valid(phydev))
 			err = phy_config_interrupt(phydev,
@@ -1031,6 +1036,7 @@ void phy_state_machine(struct work_struct *work)
 			phydev->link = 0;
 			netif_carrier_off(phydev->attached_dev);
 			phydev->adjust_link(phydev->attached_dev);
+			phy_led_trigger_change_speed(phydev);
 			do_suspend = true;
 		}
 		break;
@@ -1055,6 +1061,7 @@ void phy_state_machine(struct work_struct *work)
 					phydev->state = PHY_NOLINK;
 				}
 				phydev->adjust_link(phydev->attached_dev);
+				phy_led_trigger_change_speed(phydev);
 			} else {
 				phydev->state = PHY_AN;
 				phydev->link_timeout = PHY_AN_TIMEOUT;
@@ -1071,6 +1078,7 @@ void phy_state_machine(struct work_struct *work)
 				phydev->state = PHY_NOLINK;
 			}
 			phydev->adjust_link(phydev->attached_dev);
+			phy_led_trigger_change_speed(phydev);
 		}
 		break;
 	}
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e977ba9..4671c13 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -30,6 +30,7 @@
 #include <linux/mii.h>
 #include <linux/ethtool.h>
 #include <linux/phy.h>
+#include <linux/phy_led_triggers.h>
 #include <linux/mdio.h>
 #include <linux/io.h>
 #include <linux/uaccess.h>
@@ -57,6 +58,7 @@ static void phy_mdio_device_free(struct mdio_device *mdiodev)
 
 static void phy_device_release(struct device *dev)
 {
+	phy_led_triggers_unregister(to_phy_device(dev));
 	kfree(to_phy_device(dev));
 }
 
@@ -345,6 +347,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
 
 	dev->state = PHY_DOWN;
 
+	phy_led_triggers_register(dev);
+
 	mutex_init(&dev->lock);
 	INIT_DELAYED_WORK(&dev->state_queue, phy_state_machine);
 	INIT_WORK(&dev->phy_queue, phy_change);
diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c
new file mode 100644
index 0000000..6c40414
--- /dev/null
+++ b/drivers/net/phy/phy_led_triggers.c
@@ -0,0 +1,109 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * 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.
+ *
+ * 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/leds.h>
+#include <linux/phy.h>
+
+void phy_led_trigger_change_speed(struct phy_device *phy)
+{
+	struct phy_led_trigger *plt;
+
+	if (!phy->link) {
+		if (phy->last_triggered) {
+			led_trigger_event(&phy->last_triggered->trigger,
+					  LED_OFF);
+			phy->last_triggered = NULL;
+		}
+		return;
+	}
+
+	switch (phy->speed) {
+	case SPEED_10:
+		plt = &phy->phy_led_trigger[0];
+		break;
+	case SPEED_100:
+		plt = &phy->phy_led_trigger[1];
+		break;
+	case SPEED_1000:
+		plt = &phy->phy_led_trigger[2];
+		break;
+	case SPEED_2500:
+		plt = &phy->phy_led_trigger[3];
+		break;
+	case SPEED_10000:
+		plt = &phy->phy_led_trigger[4];
+		break;
+	default:
+		plt = NULL;
+		break;
+	}
+
+	if (plt != phy->last_triggered) {
+		led_trigger_event(&phy->last_triggered->trigger, LED_OFF);
+		led_trigger_event(&plt->trigger, LED_FULL);
+		phy->last_triggered = plt;
+	}
+}
+EXPORT_SYMBOL_GPL(phy_led_trigger_change_speed);
+
+static int phy_led_trigger_register(struct phy_device *phy,
+				    struct phy_led_trigger *plt, int i)
+{
+	static const char * const name_suffix[] = {
+		"10Mb",
+		"100Mb",
+		"Gb",
+		"2.5Gb",
+		"10GbE",
+	};
+	snprintf(plt->name, sizeof(plt->name), PHY_ID_FMT ":%s",
+		 phy->mdio.bus->id, phy->mdio.addr, name_suffix[i]);
+	plt->trigger.name = plt->name;
+	return led_trigger_register(&plt->trigger);
+}
+
+static void phy_led_trigger_unregister(struct phy_led_trigger *plt)
+{
+	led_trigger_unregister(&plt->trigger);
+}
+
+int phy_led_triggers_register(struct phy_device *phy)
+{
+	int i, err;
+
+	for (i = 0; i < ARRAY_SIZE(phy->phy_led_trigger); i++) {
+		err = phy_led_trigger_register(phy, &phy->phy_led_trigger[i],
+					       i);
+		if (err)
+			goto out_unreg;
+	}
+
+	phy->last_triggered = NULL;
+	phy_led_trigger_change_speed(phy);
+
+	return 0;
+
+out_unreg:
+	while (i--)
+		phy_led_trigger_unregister(&phy->phy_led_trigger[i]);
+	return err;
+}
+EXPORT_SYMBOL_GPL(phy_led_triggers_register);
+
+void phy_led_triggers_unregister(struct phy_device *phy)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(phy->phy_led_trigger); i++)
+		phy_led_trigger_unregister(&phy->phy_led_trigger[i]);
+}
+EXPORT_SYMBOL_GPL(phy_led_triggers_unregister);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 2d24b28..315dd06 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -25,6 +25,7 @@
 #include <linux/timer.h>
 #include <linux/workqueue.h>
 #include <linux/mod_devicetable.h>
+#include <linux/phy_led_triggers.h>
 
 #include <linux/atomic.h>
 
@@ -402,6 +403,14 @@ struct phy_device {
 
 	int link_timeout;
 
+#ifdef CONFIG_LED_TRIGGER_PHY
+	/*
+	 * A led_trigger per SPEED_*
+	 */
+	struct phy_led_trigger phy_led_trigger[5];
+	struct phy_led_trigger *last_triggered;
+#endif
+
 	/*
 	 * Interrupt number for this PHY
 	 * -1 means no interrupt
diff --git a/include/linux/phy_led_triggers.h b/include/linux/phy_led_triggers.h
new file mode 100644
index 0000000..1fb6506
--- /dev/null
+++ b/include/linux/phy_led_triggers.h
@@ -0,0 +1,42 @@
+/* Copyright (C) 2016 National Instruments Corp.
+ *
+ * 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.
+ *
+ * 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.
+ */
+#ifndef __PHY_LED_TRIGGERS
+#define __PHY_LED_TRIGGERS
+
+struct phy_device;
+
+#ifdef CONFIG_LED_TRIGGER_PHY
+
+#include <linux/leds.h>
+
+struct phy_led_trigger {
+	struct led_trigger trigger;
+	char name[64];
+};
+
+extern int phy_led_triggers_register(struct phy_device *phy);
+extern void phy_led_triggers_unregister(struct phy_device *phy);
+extern void phy_led_trigger_change_speed(struct phy_device *phy);
+
+#else
+
+static inline int phy_led_triggers_register(struct phy_device *phy)
+{
+	return 0;
+}
+static inline void phy_led_triggers_unregister(struct phy_device *phy) { }
+static inline void phy_led_trigger_change_speed(struct phy_device *phy) { }
+
+#endif
+
+#endif
-- 
2.7.4

^ permalink raw reply related

* RE: [PATCH v4 00/16] Add Paravirtual RDMA Driver
From: Woodruff, Robert J @ 2016-09-14 22:03 UTC (permalink / raw)
  To: Adit Ranadive, Jason Gunthorpe
  Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, pv-drivers,
	netdev@vger.kernel.org, linux-pci@vger.kernel.org,
	Jorgen S. Hansen, Aditya Sarwade, George Zhang, Bryan Tan
In-Reply-To: <DM2PR0501MB844473AE0FCAD0FD748D43DC5F10@DM2PR0501MB844.namprd05.prod.outlook.com>

Adit wrote,
>Thanks. So does this mean that the libraries distributed via OFED (openfabrics.org) will be now from the rdma-plumbing git tree?
>Or is the switch to happen only when distros start shipping with the 4.9 kernel by default?

We are still discussing this within the OFA EWG for the OFED releases. I suspect that if all of the maintainers of the user-space 
packages agree to merge into and support this new merged repo-model, then OFED would eventually base their OFED user-space packages
on that repo, rather than the individual repo's that are used today. The question is, when. Work has now just started on OFED-4.8,
that is based on the kernel.org 4.8 kernel. If this new scheme works with kernel.org 4.8, then it is possible that it could go into that OFED-4.8
Release, but again, we are still looking at the new scheme and evaluating how it affects the community OFED.

Woody

^ permalink raw reply

* Re: [RFC v3 17/22] cgroup: Add access check for cgroup_get_from_fd()
From: Mickaël Salaün @ 2016-09-14 22:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module, netdev, cgroups
In-Reply-To: <20160914072415.26021-18-mic@digikod.net>


[-- Attachment #1.1: Type: text/plain, Size: 2103 bytes --]


On 14/09/2016 09:24, Mickaël Salaün wrote:
> Add security access check for cgroup backed FD. The "cgroup.procs" file
> of the corresponding cgroup must be readable to identify the cgroup, and
> writable to prove that the current process can manage this cgroup (e.g.
> through delegation). This is similar to the check done by
> cgroup_procs_write_permission().
> 
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Tejun Heo <tj@kernel.org>
> ---
>  include/linux/cgroup.h |  2 +-
>  kernel/bpf/arraymap.c  |  2 +-
>  kernel/bpf/syscall.c   |  6 +++---
>  kernel/cgroup.c        | 16 +++++++++++++++-
>  4 files changed, 20 insertions(+), 6 deletions(-)
...
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 48b650a640a9..3bbaf3f02ed2 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -6241,17 +6241,20 @@ EXPORT_SYMBOL_GPL(cgroup_get_from_path);
>  /**
>   * cgroup_get_from_fd - get a cgroup pointer from a fd
>   * @fd: fd obtained by open(cgroup2_dir)
> + * @access_mask: contains the permission mask
>   *
>   * Find the cgroup from a fd which should be obtained
>   * by opening a cgroup directory.  Returns a pointer to the
>   * cgroup on success. ERR_PTR is returned if the cgroup
>   * cannot be found.
>   */
> -struct cgroup *cgroup_get_from_fd(int fd)
> +struct cgroup *cgroup_get_from_fd(int fd, int access_mask)
>  {
>  	struct cgroup_subsys_state *css;
>  	struct cgroup *cgrp;
>  	struct file *f;
> +	struct inode *inode;
> +	int ret;
>  
>  	f = fget_raw(fd);
>  	if (!f)
> @@ -6268,6 +6271,17 @@ struct cgroup *cgroup_get_from_fd(int fd)
>  		return ERR_PTR(-EBADF);
>  	}
>  
> +	ret = -ENOMEM;
> +	inode = kernfs_get_inode(f->f_path.dentry->d_sb, cgrp->procs_file.kn);

I forgot to properly move fput(f) after this line… This will be fixed.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [RFC v3 18/22] cgroup,landlock: Add CGRP_NO_NEW_PRIVS to handle unprivileged hooks
From: Mickaël Salaün @ 2016-09-14 22:11 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry,
	kernel-hardening@lists.openwall.com, Linux API
In-Reply-To: <CALCETrXBDVe9AzHnD1B5r=GGVNsU5gsC92iqD0S94mQBZOzOBQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 2025 bytes --]


On 14/09/2016 20:27, Andy Lutomirski wrote:
> On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün <mic@digikod.net> wrote:
>> Add a new flag CGRP_NO_NEW_PRIVS for each cgroup. This flag is initially
>> set for all cgroup except the root. The flag is clear when a new process
>> without the no_new_privs flags is attached to the cgroup.
>>
>> If a cgroup is landlocked, then any new attempt, from an unprivileged
>> process, to attach a process without no_new_privs to this cgroup will
>> be denied.
> 
> Until and unless everyone can agree on a way to properly namespace,
> delegate, etc cgroups, I think that trying to add unprivileged
> semantics to cgroups is nuts.  Given the big thread about cgroup v2,
> no-internal-tasks, etc, I just don't see how this approach can be
> viable.

As far as I can tell, the no_new_privs flag of at task is not related to
namespaces. The CGRP_NO_NEW_PRIVS flag is only a cache to quickly access
the no_new_privs property of *tasks* in a cgroup. The semantic is unchanged.

Using cgroup is optional, any task could use the seccomp-based
landlocking instead. However, for those that want/need to manage a
security policy in a more dynamic way, using cgroups may make sense.

I though cgroup delegation was OK in the v2, isn't it the case? Do you
have some links?

> 
> Can we try to make landlock work completely independently of cgroups
> so that it doesn't get stuck and so that programs can use it without
> worrying about cgroup v1 vs v2, interactions with cgroup managers,
> cgroup managers that (supposedly?) will start migrating processes
> around piecemeal and almost certainly blowing up landlock in the
> process, etc?

This RFC handle both cgroup and seccomp approaches in a similar way. I
don't see why building on top of cgroup v2 is a problem. Is there
security issues with delegation?

> 
> I have no problem with looking at prototypes for how landlock +
> cgroups would work, but I can't imagine the result being mergeable.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [RFC v3 19/22] landlock: Add interrupted origin
From: Mickaël Salaün @ 2016-09-14 22:14 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry,
	kernel-hardening@lists.openwall.com, Linux API
In-Reply-To: <CALCETrUyqZEcJansuN=NX04CXv4ROtNqB25YZw7mBGuu43DFew@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 891 bytes --]


On 14/09/2016 20:29, Andy Lutomirski wrote:
> On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün <mic@digikod.net> wrote:
>> This third origin of hook call should cover all possible trigger paths
>> (e.g. page fault). Landlock eBPF programs can then take decisions
>> accordingly.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Kees Cook <keescook@chromium.org>
>> ---
> 
> 
>>
>> +       if (unlikely(in_interrupt())) {
> 
> IMO security hooks have no business being called from interrupts.
> Aren't they all synchronous things done by tasks?  Interrupts are
> driver things.
> 
> Are you trying to check for page faults and such?

Yes, that was the idea you did put in my mind. Not sure how to deal with
this.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] mm: memcontrol: consolidate cgroup socket tracking
From: Andrew Morton @ 2016-09-14 22:17 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Johannes Weiner, kbuild-all, Tejun Heo, David S. Miller,
	Michal Hocko, Vladimir Davydov, linux-mm, cgroups, netdev,
	linux-kernel, kernel-team
In-Reply-To: <201609151357.bgs2EcXM%fengguang.wu@intel.com>

On Thu, 15 Sep 2016 13:34:24 +0800 kbuild test robot <lkp@intel.com> wrote:

> Hi Johannes,
> 
> [auto build test ERROR on net/master]
> [also build test ERROR on v4.8-rc6 next-20160914]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> [Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
> [Check https://git-scm.com/docs/git-format-patch for more information]
> 
> url:    https://github.com/0day-ci/linux/commits/Johannes-Weiner/mm-memcontrol-make-per-cpu-charge-cache-IRQ-safe-for-socket-accounting/20160915-035634
> config: m68k-sun3_defconfig (attached as .config)
> compiler: m68k-linux-gcc (GCC) 4.9.0
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=m68k 
> 
> All errors (new ones prefixed by >>):
> 
>    net/built-in.o: In function `sk_alloc':
> >> (.text+0x4076): undefined reference to `mem_cgroup_sk_alloc'
>    net/built-in.o: In function `__sk_destruct':
> >> sock.c:(.text+0x457e): undefined reference to `mem_cgroup_sk_free'
>    net/built-in.o: In function `sk_clone_lock':
>    (.text+0x4f1c): undefined reference to `mem_cgroup_sk_alloc'

This?

--- a/mm/memcontrol.c~mm-memcontrol-consolidate-cgroup-socket-tracking-fix
+++ a/mm/memcontrol.c
@@ -5655,9 +5655,6 @@ void mem_cgroup_sk_alloc(struct sock *sk
 {
 	struct mem_cgroup *memcg;
 
-	if (!mem_cgroup_sockets_enabled)
-		return;
-
 	/*
 	 * Socket cloning can throw us here with sk_memcg already
 	 * filled. It won't however, necessarily happen from
--- a/net/core/sock.c~mm-memcontrol-consolidate-cgroup-socket-tracking-fix
+++ a/net/core/sock.c
@@ -1385,7 +1385,8 @@ static void sk_prot_free(struct proto *p
 	slab = prot->slab;
 
 	cgroup_sk_free(&sk->sk_cgrp_data);
-	mem_cgroup_sk_free(sk);
+	if (mem_cgroup_sockets_enabled)
+		mem_cgroup_sk_free(sk);
 	security_sk_free(sk);
 	if (slab != NULL)
 		kmem_cache_free(slab, sk);
@@ -1422,7 +1423,8 @@ struct sock *sk_alloc(struct net *net, i
 		sock_net_set(sk, net);
 		atomic_set(&sk->sk_wmem_alloc, 1);
 
-		mem_cgroup_sk_alloc(sk);
+		if (mem_cgroup_sockets_enabled)
+			mem_cgroup_sk_alloc(sk);
 		cgroup_sk_alloc(&sk->sk_cgrp_data);
 		sock_update_classid(&sk->sk_cgrp_data);
 		sock_update_netprioidx(&sk->sk_cgrp_data);
@@ -1569,7 +1571,8 @@ struct sock *sk_clone_lock(const struct
 		newsk->sk_incoming_cpu = raw_smp_processor_id();
 		atomic64_set(&newsk->sk_cookie, 0);
 
-		mem_cgroup_sk_alloc(newsk);
+		if (mem_cgroup_sockets_enabled)
+			mem_cgroup_sk_alloc(newsk);
 		cgroup_sk_alloc(&newsk->sk_cgrp_data);
 
 		/*
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* RE: [PATCH v4 00/16] Add Paravirtual RDMA Driver
From: Woodruff, Robert J @ 2016-09-14 22:20 UTC (permalink / raw)
  To: Woodruff, Robert J, Adit Ranadive, Jason Gunthorpe
  Cc: dledford@redhat.com, linux-rdma@vger.kernel.org, pv-drivers,
	netdev@vger.kernel.org, linux-pci@vger.kernel.org,
	Jorgen S. Hansen, Aditya Sarwade, George Zhang, Bryan Tan
In-Reply-To: <9C6B67F36DCAFC479B1CF6A967258A8C7DE14CE8@ORSMSX115.amr.corp.intel.com>

Woody Wrote,
>We are still discussing this within the OFA EWG for the OFED releases. I suspect that if all of the maintainers of the user-space packages agree to merge into >and support this new merged repo-model, then OFED would eventually base their OFED user-space packages on that repo, rather than the individual repo's >that are used today. The question is, when. Work has now just started on OFED-4.8, that is based on the kernel.org 4.8 kernel. If this new scheme works with >kernel.org 4.8, then it is possible that it could go into that OFED-4.8 Release, but again, we are still looking at the new scheme and evaluating how it affects >the community OFED.

One other question.  Jason, the OFA (and its members) want to maintain the dual-license (BSD/GPL) for the code, as is the case for all the code that was in the OFA git trees on the OFA server that you pulled from. Will you guys be ensuring that this new user-space package 
follows that licensing model for accepting any new code into that combined repo ?

^ permalink raw reply

* Re: [RFC v3 11/22] seccomp,landlock: Handle Landlock hooks per process hierarchy
From: Mickaël Salaün @ 2016-09-14 22:34 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: linux-kernel@vger.kernel.org, Alexei Starovoitov, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry,
	kernel-hardening@lists.openwall.com, Linux API
In-Reply-To: <CALCETrXm18nNDY-a+toBEcjKKh+zOCVG0=t3ufkpROOHdqfqJQ@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1920 bytes --]


On 14/09/2016 20:43, Andy Lutomirski wrote:
> On Wed, Sep 14, 2016 at 12:24 AM, Mickaël Salaün <mic@digikod.net> wrote:
>> A Landlock program will be triggered according to its subtype/origin
>> bitfield. The LANDLOCK_FLAG_ORIGIN_SECCOMP value will trigger the
>> Landlock program when a seccomp filter will return RET_LANDLOCK.
>> Moreover, it is possible to return a 16-bit cookie which will be
>> readable by the Landlock programs in its context.
> 
> Are you envisioning that the filters will return RET_LANDLOCK most of
> the time or rarely?  If it's most of the time, then maybe this could
> be simplified a bit by unconditionally calling the landlock filter and
> letting the landlock filter access a struct seccomp_data if needed.

Exposing seccomp_data in a Landlock context may be a good idea. The main
implication is that Landlock programs may then be architecture specific
(if dealing with data) as seccomp filters are. Another point is that it
remove any direct binding between seccomp filters and Landlock programs.
I will try this (more simple) approach.

> 
>>
>> Only seccomp filters loaded from the same thread and before a Landlock
>> program can trigger it through LANDLOCK_FLAG_ORIGIN_SECCOMP. Multiple
>> Landlock programs can be triggered by one or more seccomp filters. This
>> way, each RET_LANDLOCK (with specific cookie) will trigger all the
>> allowed Landlock programs once.
> 
> This interface seems somewhat awkward.  Should we not have a way to
> atomicaly install a whole pile of landlock filters and associated
> seccomp filter all at once?

I can change the seccomp(2) use in this way: instead of loading a
Landlock program, (atomically) load an array of Landlock programs.

However, exposing seccomp_data to Landlock programs looks like a better
way to deal with it. This does not needs to manage an array of Landlock
programs.

 Mickaël


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [RFC v3 07/22] landlock: Handle file comparisons
From: Mickaël Salaün @ 2016-09-14 22:39 UTC (permalink / raw)
  To: Jann Horn
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module, netdev
In-Reply-To: <20160914190723.GB5617@pc.thejh.net>


[-- Attachment #1.1: Type: text/plain, Size: 3266 bytes --]


On 14/09/2016 21:07, Jann Horn wrote:
> On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
>> Add eBPF functions to compare file system access with a Landlock file
>> system handle:
>> * bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
>>   This function allows to compare the dentry, inode, device or mount
>>   point of the currently accessed file, with a reference handle.
>> * bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
>>   This function allows an eBPF program to check if the current accessed
>>   file is the same or in the hierarchy of a reference handle.
> [...]
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index 94256597eacd..edaab4c87292 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -603,6 +605,9 @@ static void landlock_put_handle(struct map_landlock_handle *handle)
>>  	enum bpf_map_handle_type handle_type = handle->type;
>>  
>>  	switch (handle_type) {
>> +	case BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD:
>> +		path_put(&handle->path);
>> +		break;
>>  	case BPF_MAP_HANDLE_TYPE_UNSPEC:
>>  	default:
>>  		WARN_ON(1);
> [...]
>> diff --git a/security/landlock/checker_fs.c b/security/landlock/checker_fs.c
>> new file mode 100644
>> index 000000000000..39eb85dc7d18
>> --- /dev/null
>> +++ b/security/landlock/checker_fs.c
> [...]
>> +static inline u64 bpf_landlock_cmp_fs_prop_with_struct_file(u64 r1_property,
>> +		u64 r2_map, u64 r3_map_op, u64 r4_file, u64 r5)
>> +{
>> +	u8 property = (u8) r1_property;
>> +	struct bpf_map *map = (struct bpf_map *) (unsigned long) r2_map;
>> +	enum bpf_map_array_op map_op = r3_map_op;
>> +	struct file *file = (struct file *) (unsigned long) r4_file;
>> +	struct bpf_array *array = container_of(map, struct bpf_array, map);
>> +	struct path *p1, *p2;
>> +	struct map_landlock_handle *handle;
>> +	int i;
> 
> Please don't use int when iterating over an array, use size_t.

OK, I will use size_t.

> 
> 
>> +	/* for now, only handle OP_OR */
> 
> Is "OP_OR" an appropriate name for something that ANDs the success of
> checks?
> 
> 
> [...]
>> +	synchronize_rcu();
> 
> Can you put a comment here that explains what's going on?

Hum, this should not be here.

> 
> 
>> +	for (i = 0; i < array->n_entries; i++) {
>> +		bool result_dentry = !(property & LANDLOCK_FLAG_FS_DENTRY);
>> +		bool result_inode = !(property & LANDLOCK_FLAG_FS_INODE);
>> +		bool result_device = !(property & LANDLOCK_FLAG_FS_DEVICE);
>> +		bool result_mount = !(property & LANDLOCK_FLAG_FS_MOUNT);
>> +
>> +		handle = (struct map_landlock_handle *)
>> +				(array->value + array->elem_size * i);
>> +
>> +		if (handle->type != BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) {
>> +			WARN_ON(1);
>> +			return -EFAULT;
>> +		}
>> +		p1 = &handle->path;
>> +
>> +		if (!result_dentry && p1->dentry == p2->dentry)
>> +			result_dentry = true;
> 
> Why is this safe? As far as I can tell, this is not in an RCU read-side
> critical section (synchronize_rcu() was just called), and no lock has been
> taken. What prevents someone from removing the arraymap entry while we're
> looking at it? Am I missing something?

I will try to properly deal with RCU.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [RFC v3 21/22] bpf,landlock: Add optional skb pointer in the Landlock context
From: Mickaël Salaün @ 2016-09-14 22:46 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module, netdev
In-Reply-To: <20160914212054.GC57174@ast-mbp.thefacebook.com>


[-- Attachment #1.1: Type: text/plain, Size: 1626 bytes --]


On 14/09/2016 23:20, Alexei Starovoitov wrote:
> On Wed, Sep 14, 2016 at 09:24:14AM +0200, Mickaël Salaün wrote:
>> This is a proof of concept to expose optional values that could depend
>> of the process access rights.
>>
>> There is two dedicated flags: LANDLOCK_FLAG_ACCESS_SKB_READ and
>> LANDLOCK_FLAG_ACCESS_SKB_WRITE. Each of them can be activated to access
>> eBPF functions manipulating a skb in a read or write way.
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ...
>>  /* Handle check flags */
>>  #define LANDLOCK_FLAG_FS_DENTRY		(1 << 0)
>> @@ -619,12 +621,15 @@ struct landlock_handle {
>>   * @args: LSM hook arguments, see include/linux/lsm_hooks.h for there
>>   *        description and the LANDLOCK_HOOK* definitions from
>>   *        security/landlock/lsm.c for their types.
>> + * @opt_skb: optional skb pointer, accessible with the
>> + *           LANDLOCK_FLAG_ACCESS_SKB_* flags for network-related hooks.
>>   */
>>  struct landlock_data {
>>  	__u32 hook; /* enum landlock_hook_id */
>>  	__u16 origin; /* LANDLOCK_FLAG_ORIGIN_* */
>>  	__u16 cookie; /* seccomp RET_LANDLOCK */
>>  	__u64 args[6];
>> +	__u64 opt_skb;
>>  };
> 
> missing something here.
> This patch doesn't make use of it.
> That's something for the future?
> How that field will be populated?
> Why make it different vs the rest or args[6] ?
> 
> 

I don't use this code, it's only purpose is to show how to deal with
fine-grained privileges of Landlock programs (to allow Sargun to add his
custom helpers from Checmate). However, this optional field may be part
of args[6].


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [PATCHv3 net-next 04/15] bpf: don't (ab)use instructions to store state
From: Alexei Starovoitov @ 2016-09-14 22:58 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, ast, daniel, jiri, john.fastabend, kubakici
In-Reply-To: <1473879623-15382-5-git-send-email-jakub.kicinski@netronome.com>

On Wed, Sep 14, 2016 at 08:00:12PM +0100, Jakub Kicinski wrote:
> Storing state in reserved fields of instructions makes
> it impossible to run validator on programs already
> marked as read-only. Allocate and use an array of
> per-instruction state instead.
> 
> While touching the error path rename and move existing
> jump target.
> 
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Nice and clean. Thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH v4 00/16] Add Paravirtual RDMA Driver
From: Jason Gunthorpe @ 2016-09-14 22:59 UTC (permalink / raw)
  To: Woodruff, Robert J
  Cc: Adit Ranadive, dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pv-drivers,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Jorgen S. Hansen, Aditya Sarwade, George Zhang, Bryan Tan
In-Reply-To: <9C6B67F36DCAFC479B1CF6A967258A8C7DE14D3E-8oqHQFITsIFqS6EAlXoojrfspsVTdybXVpNB7YpNyf8@public.gmane.org>

On Wed, Sep 14, 2016 at 10:20:22PM +0000, Woodruff, Robert J wrote:

> >this new scheme works with >kernel.org 4.8, then it is possible
> >that it could go into that OFED-4.8 Release, but again, we are
> >still looking at the new scheme and evaluating how it affects >the
> >community OFED.
> 
> One other question.  Jason, the OFA (and its members) want to
> maintain the dual-license (BSD/GPL) for the code, as is the case for
> all the code that was in the OFA git trees on the OFA server that
> you pulled from.

The code in the OFA trees has a mixture of licenses. It is all GPLv2
compatible for sure, but there are at least three variations of the
'BSD' license. As far as I know, only the 2 clause OpenIB.org BSD
license is approved for use in OFA projects. Someone from the board
could correct me if I am wrong.

This means the 3 projects with the BSD patent clause, hosted on the
OFA servers, were already not conforming.

As is Intels HFI1 driver which uses a three clause BSD license.

> package follows that licensing model for accepting any new code into
> that combined repo ?

As with the kernel we'd discourage 're-licensing' existing files.

However, since this is not a OFA project, I, personally, would not
turn away a GPLv2 compatible contribution, but I am proposing that the
'default' license for the project be OFA compatible.

I think license enforcement of its members falls to the OFA.

Doug may feel differently.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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 v3 07/22] landlock: Handle file comparisons
From: Mickaël Salaün @ 2016-09-14 23:02 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: linux-kernel, Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
	Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
	David S . Miller, Elena Reshetova, Eric W . Biederman,
	James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
	Serge E . Hallyn, Tejun Heo, Will Drewry, kernel-hardening,
	linux-api, linux-security-module, netdev
In-Reply-To: <20160914210649.GA57174@ast-mbp.thefacebook.com>


[-- Attachment #1.1: Type: text/plain, Size: 4653 bytes --]



On 14/09/2016 23:06, Alexei Starovoitov wrote:
> On Wed, Sep 14, 2016 at 09:24:00AM +0200, Mickaël Salaün wrote:
>> Add eBPF functions to compare file system access with a Landlock file
>> system handle:
>> * bpf_landlock_cmp_fs_prop_with_struct_file(prop, map, map_op, file)
>>   This function allows to compare the dentry, inode, device or mount
>>   point of the currently accessed file, with a reference handle.
>> * bpf_landlock_cmp_fs_beneath_with_struct_file(opt, map, map_op, file)
>>   This function allows an eBPF program to check if the current accessed
>>   file is the same or in the hierarchy of a reference handle.
>>
>> The goal of file system handle is to abstract kernel objects such as a
>> struct file or a struct inode. Userland can create this kind of handle
>> thanks to the BPF_MAP_UPDATE_ELEM command. The element is a struct
>> landlock_handle containing the handle type (e.g.
>> BPF_MAP_HANDLE_TYPE_LANDLOCK_FS_FD) and a file descriptor. This could
>> also be any descriptions able to match a struct file or a struct inode
>> (e.g. path or glob string).
>>
>> Changes since v2:
>> * add MNT_INTERNAL check to only add file handle from user-visible FS
>>   (e.g. no anonymous inode)
>> * replace struct file* with struct path* in map_landlock_handle
>> * add BPF protos
>> * fix bpf_landlock_cmp_fs_prop_with_struct_file()
>>
>> Signed-off-by: Mickaël Salaün <mic@digikod.net>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Andy Lutomirski <luto@amacapital.net>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: James Morris <james.l.morris@oracle.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Serge E. Hallyn <serge@hallyn.com>
>> Link: https://lkml.kernel.org/r/CALCETrWwTiz3kZTkEgOW24-DvhQq6LftwEXh77FD2G5o71yD7g@mail.gmail.com
> 
> thanks for keeping the links to the previous discussion.
> Long term it should help, though I worry we already at the point
> where there are too many outstanding issues to resolve before we
> can proceed with reasonable code review.
> 
>> +/*
>> + * bpf_landlock_cmp_fs_prop_with_struct_file
>> + *
>> + * Cf. include/uapi/linux/bpf.h
>> + */
>> +static inline u64 bpf_landlock_cmp_fs_prop_with_struct_file(u64 r1_property,
>> +		u64 r2_map, u64 r3_map_op, u64 r4_file, u64 r5)
>> +{
>> +	u8 property = (u8) r1_property;
>> +	struct bpf_map *map = (struct bpf_map *) (unsigned long) r2_map;
>> +	enum bpf_map_array_op map_op = r3_map_op;
>> +	struct file *file = (struct file *) (unsigned long) r4_file;
> 
> please use just added BPF_CALL_ macros. They will help readability of the above.
> 
>> +	struct bpf_array *array = container_of(map, struct bpf_array, map);
>> +	struct path *p1, *p2;
>> +	struct map_landlock_handle *handle;
>> +	int i;
>> +
>> +	/* ARG_CONST_PTR_TO_LANDLOCK_HANDLE_FS is an arraymap */
>> +	if (unlikely(!map)) {
>> +		WARN_ON(1);
>> +		return -EFAULT;
>> +	}
>> +	if (unlikely(!file))
>> +		return -ENOENT;
>> +	if (unlikely((property | _LANDLOCK_FLAG_FS_MASK) != _LANDLOCK_FLAG_FS_MASK))
>> +		return -EINVAL;
>> +
>> +	/* for now, only handle OP_OR */
>> +	switch (map_op) {
>> +	case BPF_MAP_ARRAY_OP_OR:
>> +		break;
>> +	case BPF_MAP_ARRAY_OP_UNSPEC:
>> +	case BPF_MAP_ARRAY_OP_AND:
>> +	case BPF_MAP_ARRAY_OP_XOR:
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +	p2 = &file->f_path;
>> +
>> +	synchronize_rcu();
> 
> that is completely broken.
> bpf programs are executing under rcu_lock.
> please enable CONFIG_PROVE_RCU and retest everything.

Thanks for the tip. I will fix this.

> 
> I would suggest for the next RFC to do minimal 7 patches up to this point
> with simple example that demonstrates the use case.
> I would avoid all unpriv stuff and all of seccomp for the next RFC as well,
> otherwise I don't think we can realistically make forward progress, since
> there are too many issues raised in the subsequent patches.

I hope we will find a common agreement about seccomp vs cgroup… I think
both approaches have their advantages, can be complementary and nicely
combined.

Unprivileged sandboxing is the main goal of Landlock. This should not be
a problem, even for privileged features, thanks to the new subtype/access.

> 
> The common part that is mergeable is prog's subtype extension to
> the verifier that can be used for better tracing and is the common
> piece of infra needed for both landlock and checmate LSMs
> (which must be one LSM anyway)

Agreed. With this RFC, the Checmate features (i.e. network helpers)
should be able to sit on top of Landlock.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply

* Re: [PATCHv3 net-next 05/15] bpf: enable non-core use of the verfier
From: Alexei Starovoitov @ 2016-09-14 23:05 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, ast, daniel, jiri, john.fastabend, kubakici
In-Reply-To: <1473879623-15382-6-git-send-email-jakub.kicinski@netronome.com>

On Wed, Sep 14, 2016 at 08:00:13PM +0100, Jakub Kicinski wrote:
> Advanced JIT compilers and translators may want to use
> eBPF verifier as a base for parsers or to perform custom
> checks and validations.
> 
> Add ability for external users to invoke the verifier
> and provide callbacks to be invoked for every intruction
> checked.  For now only add most basic callback for
> per-instruction pre-interpretation checks is added.  More
> advanced users may also like to have per-instruction post
> callback and state comparison callback.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
>  include/linux/bpf_parser.h |  89 ++++++++++++++++++++++++++++++
>  kernel/bpf/verifier.c      | 134 +++++++++++++++++++++++----------------------
>  2 files changed, 158 insertions(+), 65 deletions(-)
>  create mode 100644 include/linux/bpf_parser.h
> 
> diff --git a/include/linux/bpf_parser.h b/include/linux/bpf_parser.h
> new file mode 100644
> index 000000000000..daa53b204f4d
> --- /dev/null
> +++ b/include/linux/bpf_parser.h

'bpf parser' is a bit misleading name, since it can be interpreted
as parser written in bpf.
Also the header file containes verifier bits, therefore I think
the better name would be bpf_verifier.h ?

> @@ -0,0 +1,89 @@
> +/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of version 2 of the GNU General Public
> + * License as published by the Free Software Foundation.
> + */
> +#ifndef _LINUX_BPF_PARSER_H
> +#define _LINUX_BPF_PARSER_H 1
> +
> +#include <linux/bpf.h> /* for enum bpf_reg_type */
> +#include <linux/filter.h> /* for MAX_BPF_STACK */
> +
> +struct reg_state {
> +	enum bpf_reg_type type;
> +	union {
> +		/* valid when type == CONST_IMM | PTR_TO_STACK | UNKNOWN_VALUE */
> +		s64 imm;
> +
> +		/* valid when type == PTR_TO_PACKET* */
> +		struct {
> +			u32 id;
> +			u16 off;
> +			u16 range;
> +		};
> +
> +		/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
> +		 *   PTR_TO_MAP_VALUE_OR_NULL
> +		 */
> +		struct bpf_map *map_ptr;
> +	};
> +};
> +
> +enum bpf_stack_slot_type {
> +	STACK_INVALID,    /* nothing was stored in this stack slot */
> +	STACK_SPILL,      /* register spilled into stack */
> +	STACK_MISC	  /* BPF program wrote some data into this slot */
> +};
> +
> +#define BPF_REG_SIZE 8	/* size of eBPF register in bytes */
> +
> +/* state of the program:
> + * type of all registers and stack info
> + */
> +struct verifier_state {
> +	struct reg_state regs[MAX_BPF_REG];
> +	u8 stack_slot_type[MAX_BPF_STACK];
> +	struct reg_state spilled_regs[MAX_BPF_STACK / BPF_REG_SIZE];
> +};
> +
> +/* linked list of verifier states used to prune search */
> +struct verifier_state_list {
> +	struct verifier_state state;
> +	struct verifier_state_list *next;
> +};
> +
> +struct bpf_insn_aux_data {
> +	enum bpf_reg_type ptr_type;	/* pointer type for load/store insns */
> +};
> +
> +#define MAX_USED_MAPS 64 /* max number of maps accessed by one eBPF program */
> +
> +struct verifier_env;
> +struct bpf_ext_parser_ops {
> +	int (*insn_hook)(struct verifier_env *env,
> +			 int insn_idx, int prev_insn_idx);
> +};

How about calling this bpf_ext_analyzer_ops
and main entry bpf_analyzer() ?
I think it will better convey what it's doing.

> +
> +/* single container for all structs
> + * one verifier_env per bpf_check() call
> + */
> +struct verifier_env {
> +	struct bpf_prog *prog;		/* eBPF program being verified */
> +	struct verifier_stack_elem *head; /* stack of verifier states to be processed */
> +	int stack_size;			/* number of states to be processed */
> +	struct verifier_state cur_state; /* current verifier state */
> +	struct verifier_state_list **explored_states; /* search pruning optimization */
> +	const struct bpf_ext_parser_ops *pops; /* external parser ops */
> +	void *ppriv; /* pointer to external parser's private data */

a bit hard to review, since move and addition is in one patch.
I think ppriv and pops are too obscure names.
May be analyzer_ops and analyzer_priv ?

Conceptually looks good.

^ permalink raw reply

* [PATCH net-next 0/4] rxrpc: Support IPv6 [ver #2]
From: David Howells @ 2016-09-14 23:06 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel


Here is a set of patches that add IPv6 support.  They need to be applied on
top of the just-posted miscellaneous fix patches.  They are:

 (1) Make autobinding of an unconnected socket work when sendmsg() is
     called to initiate a client call.

 (2) Don't specify the protocol when creating the client socket, but rather
     take the default instead.

 (3) Use rxrpc_extract_addr_from_skb() in a couple of places that were
     doing the same thing manually.  This allows the IPv6 address
     extraction to be done in fewer places.

 (4) Add IPv6 support.  With this, calls can be made to IPv6 servers from
     userspace AF_RXRPC programs; AFS, however, can't use IPv6 yet as the
     RPC calls need to be upgradeable.

Changes:

 (V2) Made IPv6 support conditional on CONFIG_IPV6.

      Changed a memcpy argument that was taking the address of an array
      struct member (with the '&' operator) and adding 12 to just add 12 to
      the array member.  This otherwise causes a problem on gcc-4.9.

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite

Tagged thusly:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-rewrite-20160913-2

David
---
David Howells (4):
      rxrpc: Create an address for sendmsg() to bind unbound socket with
      rxrpc: Don't specify protocol to when creating transport socket
      rxrpc: Use rxrpc_extract_addr_from_skb() rather than doing this manually
      rxrpc: Add IPv6 support


 net/rxrpc/Kconfig        |    7 +++
 net/rxrpc/af_rxrpc.c     |   32 +++++++++++-
 net/rxrpc/conn_object.c  |   10 ++++
 net/rxrpc/local_event.c  |   13 ++---
 net/rxrpc/local_object.c |   41 +++++++---------
 net/rxrpc/output.c       |   50 +++++++++----------
 net/rxrpc/peer_event.c   |   26 ++++++++++
 net/rxrpc/peer_object.c  |  119 ++++++++++++++++++++++++++++++----------------
 net/rxrpc/proc.c         |   30 +++++-------
 net/rxrpc/utils.c        |    2 +
 10 files changed, 211 insertions(+), 119 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/4] rxrpc: Create an address for sendmsg() to bind unbound socket with [ver #2]
From: David Howells @ 2016-09-14 23:06 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147389436004.2083.13454744739435764689.stgit@warthog.procyon.org.uk>

Create an address for sendmsg() to bind unbound socket with rather than
using a completely blank address otherwise the transport socket creation
will fail because it will try to use address family 0.

We use the address family specified in the protocol argument when the
AF_RXRPC socket was created and SOCK_DGRAM as the default.  For anything
else, bind() must be used.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/af_rxrpc.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 25d00ded24bc..741b0d8d2e8c 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -401,6 +401,18 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
 
 	switch (rx->sk.sk_state) {
 	case RXRPC_UNBOUND:
+		rx->srx.srx_family = AF_RXRPC;
+		rx->srx.srx_service = 0;
+		rx->srx.transport_type = SOCK_DGRAM;
+		rx->srx.transport.family = rx->family;
+		switch (rx->family) {
+		case AF_INET:
+			rx->srx.transport_len = sizeof(struct sockaddr_in);
+			break;
+		default:
+			ret = -EAFNOSUPPORT;
+			goto error_unlock;
+		}
 		local = rxrpc_lookup_local(&rx->srx);
 		if (IS_ERR(local)) {
 			ret = PTR_ERR(local);

^ permalink raw reply related

* [PATCH net-next 2/4] rxrpc: Don't specify protocol to when creating transport socket [ver #2]
From: David Howells @ 2016-09-14 23:06 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147389436004.2083.13454744739435764689.stgit@warthog.procyon.org.uk>

Pass 0 as the protocol argument when creating the transport socket rather
than IPPROTO_UDP.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/local_object.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index 782b9adf67cb..8720be2a6250 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -103,8 +103,8 @@ static int rxrpc_open_socket(struct rxrpc_local *local)
 	_enter("%p{%d}", local, local->srx.transport_type);
 
 	/* create a socket to represent the local endpoint */
-	ret = sock_create_kern(&init_net, PF_INET, local->srx.transport_type,
-			       IPPROTO_UDP, &local->socket);
+	ret = sock_create_kern(&init_net, local->srx.transport.family,
+			       local->srx.transport_type, 0, &local->socket);
 	if (ret < 0) {
 		_leave(" = %d [socket]", ret);
 		return ret;

^ permalink raw reply related


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