* [PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive
2017-10-19 2:43 [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length Samuel Mendoza-Jonas
@ 2017-10-19 2:43 ` Samuel Mendoza-Jonas
2017-10-21 0:57 ` David Miller
2017-10-19 2:43 ` [PATCH net 3/5] net/ncsi: Disable HWA mode when no channels are found Samuel Mendoza-Jonas
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-10-19 2:43 UTC (permalink / raw)
To: netdev; +Cc: Samuel Mendoza-Jonas, David S . Miller, linux-kernel
ncsi_channel_monitor() misses stopping the channel monitor in several
places that it should, causing a WARN_ON_ONCE() to trigger when the
monitor is re-started later, eg:
[ 459.040000] WARNING: CPU: 0 PID: 1093 at net/ncsi/ncsi-manage.c:269 ncsi_start_channel_monitor+0x7c/0x90
[ 459.040000] CPU: 0 PID: 1093 Comm: kworker/0:3 Not tainted 4.10.17-gaca2fdd #140
[ 459.040000] Hardware name: ASpeed SoC
[ 459.040000] Workqueue: events ncsi_dev_work
[ 459.040000] [<80010094>] (unwind_backtrace) from [<8000d950>] (show_stack+0x20/0x24)
[ 459.040000] [<8000d950>] (show_stack) from [<801dbf70>] (dump_stack+0x20/0x28)
[ 459.040000] [<801dbf70>] (dump_stack) from [<80018d7c>] (__warn+0xe0/0x108)
[ 459.040000] [<80018d7c>] (__warn) from [<80018e70>] (warn_slowpath_null+0x30/0x38)
[ 459.040000] [<80018e70>] (warn_slowpath_null) from [<803f6a08>] (ncsi_start_channel_monitor+0x7c/0x90)
[ 459.040000] [<803f6a08>] (ncsi_start_channel_monitor) from [<803f7664>] (ncsi_configure_channel+0xdc/0x5fc)
[ 459.040000] [<803f7664>] (ncsi_configure_channel) from [<803f8160>] (ncsi_dev_work+0xac/0x474)
[ 459.040000] [<803f8160>] (ncsi_dev_work) from [<8002d244>] (process_one_work+0x1e0/0x450)
[ 459.040000] [<8002d244>] (process_one_work) from [<8002d510>] (worker_thread+0x5c/0x570)
[ 459.040000] [<8002d510>] (worker_thread) from [<80033614>] (kthread+0x124/0x164)
[ 459.040000] [<80033614>] (kthread) from [<8000a5e8>] (ret_from_fork+0x14/0x2c)
This also updates the monitor instead of just returning if
ncsi_xmit_cmd() fails to send the get-link-status command so that the
monitor properly times out.
Fixes: e6f44ed6d04d3 "net/ncsi: Package and channel management"
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-manage.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b6a449aa9d4b..b022deb39d31 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -202,11 +202,15 @@ static void ncsi_channel_monitor(unsigned long data)
monitor_state = nc->monitor.state;
spin_unlock_irqrestore(&nc->lock, flags);
- if (!enabled || chained)
+ if (!enabled || chained) {
+ ncsi_stop_channel_monitor(nc);
return;
+ }
if (state != NCSI_CHANNEL_INACTIVE &&
- state != NCSI_CHANNEL_ACTIVE)
+ state != NCSI_CHANNEL_ACTIVE) {
+ ncsi_stop_channel_monitor(nc);
return;
+ }
switch (monitor_state) {
case NCSI_CHANNEL_MONITOR_START:
@@ -217,12 +221,9 @@ static void ncsi_channel_monitor(unsigned long data)
nca.type = NCSI_PKT_CMD_GLS;
nca.req_flags = 0;
ret = ncsi_xmit_cmd(&nca);
- if (ret) {
+ if (ret)
netdev_err(ndp->ndev.dev, "Error %d sending GLS\n",
ret);
- return;
- }
-
break;
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
@@ -233,6 +234,8 @@ static void ncsi_channel_monitor(unsigned long data)
ndp->flags |= NCSI_DEV_RESHUFFLE;
}
+ ncsi_stop_channel_monitor(nc);
+
spin_lock_irqsave(&nc->lock, flags);
nc->state = NCSI_CHANNEL_INVISIBLE;
spin_unlock_irqrestore(&nc->lock, flags);
--
2.14.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive
2017-10-19 2:43 ` [PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive Samuel Mendoza-Jonas
@ 2017-10-21 0:57 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-10-21 0:57 UTC (permalink / raw)
To: sam; +Cc: netdev, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Thu, 19 Oct 2017 13:43:06 +1100
> ncsi_channel_monitor() misses stopping the channel monitor in several
> places that it should, causing a WARN_ON_ONCE() to trigger when the
> monitor is re-started later, eg:
>
> [ 459.040000] WARNING: CPU: 0 PID: 1093 at net/ncsi/ncsi-manage.c:269 ncsi_start_channel_monitor+0x7c/0x90
> [ 459.040000] CPU: 0 PID: 1093 Comm: kworker/0:3 Not tainted 4.10.17-gaca2fdd #140
> [ 459.040000] Hardware name: ASpeed SoC
> [ 459.040000] Workqueue: events ncsi_dev_work
> [ 459.040000] [<80010094>] (unwind_backtrace) from [<8000d950>] (show_stack+0x20/0x24)
> [ 459.040000] [<8000d950>] (show_stack) from [<801dbf70>] (dump_stack+0x20/0x28)
> [ 459.040000] [<801dbf70>] (dump_stack) from [<80018d7c>] (__warn+0xe0/0x108)
> [ 459.040000] [<80018d7c>] (__warn) from [<80018e70>] (warn_slowpath_null+0x30/0x38)
> [ 459.040000] [<80018e70>] (warn_slowpath_null) from [<803f6a08>] (ncsi_start_channel_monitor+0x7c/0x90)
> [ 459.040000] [<803f6a08>] (ncsi_start_channel_monitor) from [<803f7664>] (ncsi_configure_channel+0xdc/0x5fc)
> [ 459.040000] [<803f7664>] (ncsi_configure_channel) from [<803f8160>] (ncsi_dev_work+0xac/0x474)
> [ 459.040000] [<803f8160>] (ncsi_dev_work) from [<8002d244>] (process_one_work+0x1e0/0x450)
> [ 459.040000] [<8002d244>] (process_one_work) from [<8002d510>] (worker_thread+0x5c/0x570)
> [ 459.040000] [<8002d510>] (worker_thread) from [<80033614>] (kthread+0x124/0x164)
> [ 459.040000] [<80033614>] (kthread) from [<8000a5e8>] (ret_from_fork+0x14/0x2c)
>
> This also updates the monitor instead of just returning if
> ncsi_xmit_cmd() fails to send the get-link-status command so that the
> monitor properly times out.
>
> Fixes: e6f44ed6d04d3 "net/ncsi: Package and channel management"
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 3/5] net/ncsi: Disable HWA mode when no channels are found
2017-10-19 2:43 [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length Samuel Mendoza-Jonas
2017-10-19 2:43 ` [PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive Samuel Mendoza-Jonas
@ 2017-10-19 2:43 ` Samuel Mendoza-Jonas
2017-10-21 0:57 ` David Miller
2017-10-19 2:43 ` [PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout Samuel Mendoza-Jonas
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-10-19 2:43 UTC (permalink / raw)
To: netdev; +Cc: Gavin Shan, David S . Miller, linux-kernel, Samuel Mendoza-Jonas
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
When there are no NCSI channels probed, HWA (Hardware Arbitration)
mode is enabled. It's not correct because HWA depends on the fact:
NCSI channels exist and all of them support HWA mode. This disables
HWA when no channels are probed.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-manage.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b022deb39d31..0966eff48ce7 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -1005,12 +1005,15 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
struct ncsi_package *np;
struct ncsi_channel *nc;
unsigned int cap;
+ bool has_channel = false;
/* The hardware arbitration is disabled if any one channel
* doesn't support explicitly.
*/
NCSI_FOR_EACH_PACKAGE(ndp, np) {
NCSI_FOR_EACH_CHANNEL(np, nc) {
+ has_channel = true;
+
cap = nc->caps[NCSI_CAP_GENERIC].cap;
if (!(cap & NCSI_CAP_GENERIC_HWA) ||
(cap & NCSI_CAP_GENERIC_HWA_MASK) !=
@@ -1021,8 +1024,13 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
}
}
- ndp->flags |= NCSI_DEV_HWA;
- return true;
+ if (has_channel) {
+ ndp->flags |= NCSI_DEV_HWA;
+ return true;
+ }
+
+ ndp->flags &= ~NCSI_DEV_HWA;
+ return false;
}
static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
--
2.14.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout
2017-10-19 2:43 [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length Samuel Mendoza-Jonas
2017-10-19 2:43 ` [PATCH net 2/5] net/ncsi: Stop monitor if channel times out or is inactive Samuel Mendoza-Jonas
2017-10-19 2:43 ` [PATCH net 3/5] net/ncsi: Disable HWA mode when no channels are found Samuel Mendoza-Jonas
@ 2017-10-19 2:43 ` Samuel Mendoza-Jonas
2017-10-21 0:57 ` David Miller
2017-10-19 2:43 ` [PATCH net 5/5] net/ncsi: Fix length of GVI response packet Samuel Mendoza-Jonas
2017-10-21 0:56 ` [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length David Miller
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-10-19 2:43 UTC (permalink / raw)
To: netdev; +Cc: Gavin Shan, David S . Miller, linux-kernel, Samuel Mendoza-Jonas
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
The NCSI channel has been configured to provide service if its link
monitor timer is enabled, regardless of its state (inactive or active).
So the timeout event on the link monitor indicates the out-of-service
on that channel, for which a failover is needed.
This sets NCSI_DEV_RESHUFFLE flag to enforce failover on link monitor
timeout, regardless the channel's original state (inactive or active).
Also, the link is put into "down" state to give the failing channel
lowest priority when selecting for the active channel. The state of
failing channel should be set to active in order for deinitialization
and failover to be done.
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-manage.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 0966eff48ce7..28c42b22b748 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -189,6 +189,7 @@ static void ncsi_channel_monitor(unsigned long data)
struct ncsi_channel *nc = (struct ncsi_channel *)data;
struct ncsi_package *np = nc->package;
struct ncsi_dev_priv *ndp = np->ndp;
+ struct ncsi_channel_mode *ncm;
struct ncsi_cmd_arg nca;
bool enabled, chained;
unsigned int monitor_state;
@@ -228,20 +229,21 @@ static void ncsi_channel_monitor(unsigned long data)
case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
break;
default:
- if (!(ndp->flags & NCSI_DEV_HWA) &&
- state == NCSI_CHANNEL_ACTIVE) {
+ if (!(ndp->flags & NCSI_DEV_HWA)) {
ncsi_report_link(ndp, true);
ndp->flags |= NCSI_DEV_RESHUFFLE;
}
ncsi_stop_channel_monitor(nc);
+ ncm = &nc->modes[NCSI_MODE_LINK];
spin_lock_irqsave(&nc->lock, flags);
nc->state = NCSI_CHANNEL_INVISIBLE;
+ ncm->data[2] &= ~0x1;
spin_unlock_irqrestore(&nc->lock, flags);
spin_lock_irqsave(&ndp->lock, flags);
- nc->state = NCSI_CHANNEL_INACTIVE;
+ nc->state = NCSI_CHANNEL_ACTIVE;
list_add_tail_rcu(&nc->link, &ndp->channel_queue);
spin_unlock_irqrestore(&ndp->lock, flags);
ncsi_process_next_channel(ndp);
--
2.14.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout
2017-10-19 2:43 ` [PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout Samuel Mendoza-Jonas
@ 2017-10-21 0:57 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-10-21 0:57 UTC (permalink / raw)
To: sam; +Cc: netdev, gwshan, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Thu, 19 Oct 2017 13:43:08 +1100
> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
> The NCSI channel has been configured to provide service if its link
> monitor timer is enabled, regardless of its state (inactive or active).
> So the timeout event on the link monitor indicates the out-of-service
> on that channel, for which a failover is needed.
>
> This sets NCSI_DEV_RESHUFFLE flag to enforce failover on link monitor
> timeout, regardless the channel's original state (inactive or active).
> Also, the link is put into "down" state to give the failing channel
> lowest priority when selecting for the active channel. The state of
> failing channel should be set to active in order for deinitialization
> and failover to be done.
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net 5/5] net/ncsi: Fix length of GVI response packet
2017-10-19 2:43 [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length Samuel Mendoza-Jonas
` (2 preceding siblings ...)
2017-10-19 2:43 ` [PATCH net 4/5] net/ncsi: Enforce failover on link monitor timeout Samuel Mendoza-Jonas
@ 2017-10-19 2:43 ` Samuel Mendoza-Jonas
2017-10-21 0:57 ` David Miller
2017-10-21 0:56 ` [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length David Miller
4 siblings, 1 reply; 10+ messages in thread
From: Samuel Mendoza-Jonas @ 2017-10-19 2:43 UTC (permalink / raw)
To: netdev; +Cc: Gavin Shan, David S . Miller, linux-kernel, Samuel Mendoza-Jonas
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
The length of GVI (GetVersionInfo) response packet should be 40 instead
of 36. This issue was found from /sys/kernel/debug/ncsi/eth0/stats.
# ethtool --ncsi eth0 swstats
:
RESPONSE OK TIMEOUT ERROR
=======================================
GVI 0 0 2
With this applied, no error reported on GVI response packets:
# ethtool --ncsi eth0 swstats
:
RESPONSE OK TIMEOUT ERROR
=======================================
GVI 2 0 0
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
---
net/ncsi/ncsi-rsp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 265b9a892d41..927dad4759d1 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -959,7 +959,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_EGMF, 4, ncsi_rsp_handler_egmf },
{ NCSI_PKT_RSP_DGMF, 4, ncsi_rsp_handler_dgmf },
{ NCSI_PKT_RSP_SNFC, 4, ncsi_rsp_handler_snfc },
- { NCSI_PKT_RSP_GVI, 36, ncsi_rsp_handler_gvi },
+ { NCSI_PKT_RSP_GVI, 40, ncsi_rsp_handler_gvi },
{ NCSI_PKT_RSP_GC, 32, ncsi_rsp_handler_gc },
{ NCSI_PKT_RSP_GP, -1, ncsi_rsp_handler_gp },
{ NCSI_PKT_RSP_GCPS, 172, ncsi_rsp_handler_gcps },
--
2.14.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net 5/5] net/ncsi: Fix length of GVI response packet
2017-10-19 2:43 ` [PATCH net 5/5] net/ncsi: Fix length of GVI response packet Samuel Mendoza-Jonas
@ 2017-10-21 0:57 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-10-21 0:57 UTC (permalink / raw)
To: sam; +Cc: netdev, gwshan, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Thu, 19 Oct 2017 13:43:09 +1100
> From: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
> The length of GVI (GetVersionInfo) response packet should be 40 instead
> of 36. This issue was found from /sys/kernel/debug/ncsi/eth0/stats.
>
> # ethtool --ncsi eth0 swstats
> :
> RESPONSE OK TIMEOUT ERROR
> =======================================
> GVI 0 0 2
>
> With this applied, no error reported on GVI response packets:
>
> # ethtool --ncsi eth0 swstats
> :
> RESPONSE OK TIMEOUT ERROR
> =======================================
> GVI 2 0 0
>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length
2017-10-19 2:43 [PATCH net 1/5] net/ncsi: Fix AEN HNCDSC packet length Samuel Mendoza-Jonas
` (3 preceding siblings ...)
2017-10-19 2:43 ` [PATCH net 5/5] net/ncsi: Fix length of GVI response packet Samuel Mendoza-Jonas
@ 2017-10-21 0:56 ` David Miller
4 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2017-10-21 0:56 UTC (permalink / raw)
To: sam; +Cc: netdev, linux-kernel
From: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Date: Thu, 19 Oct 2017 13:43:05 +1100
> Correct the value of the HNCDSC AEN packet.
> Fixes: 7a82ecf4cfb85 "net/ncsi: NCSI AEN packet handler"
>
> Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread