[PATCH 4.4 0/9] net/ncsi: Backport fixes to dev-4.4

[PATCH 4.4 0/9] net/ncsi: Backport fixes to dev-4.4

[Gavin Shan]

This backports patches that has been merged to upstream or Dave's
git repo (branch "net") that should arrive upstream shortly. The
first patch is exceptional because the changes included in it has
been in dev-4.7 and upstream from beginning.

Joel, please help merging it to dev-4.4 if the series looks sane
to you.

Gavin Shan (9):
  net/ncsi: Remove unused parameters for ncsi_free_req()
  net/ncsi: Introduce NCSI_RESERVED_CHANNEL
  net/ncsi: Don't probe on the reserved channel ID (0x1f)
  net/ncsi: Rework request index allocation
  net/ncsi: Allow to extend NCSI request properties
  net/ncsi: Avoid if statements in ncsi_suspend_dev()
  net/ncsi: Fix stale link state of inactive channels on failover
  net/ncsi: Choose hot channel as active one if necessary
  net/ncsi: Improve HNCDSC AEN handler

 net/ncsi/internal.h    |  12 +++-
 net/ncsi/ncsi-aen.c    |   6 +-
 net/ncsi/ncsi-cmd.c    |   6 +-
 net/ncsi/ncsi-manage.c | 182 +++++++++++++++++++++++++++++++++----------------
 net/ncsi/ncsi-rsp.c    |   2 +-
 5 files changed, 142 insertions(+), 66 deletions(-)

-- 
2.1.0

[PATCH 4.4 1/9] net/ncsi: Remove unused parameters for ncsi_free_req()

[Gavin Shan]

There are two parameters (@check and @timeout) in ncsi_free_req()
except @nr. @timeout isn't used inside the function. @check should
have been always set to true though it's not case, meaning we
needn't @check.

This removes the unused parameters (@check and @timeout). Eventually,
we have same parameters for this function among 4.4, 4.7 and upstream
code.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/internal.h    | 2 +-
 net/ncsi/ncsi-cmd.c    | 4 ++--
 net/ncsi/ncsi-manage.c | 6 +++---
 net/ncsi/ncsi-rsp.c    | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 2525229..e97906c 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -139,7 +139,7 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
 				   struct ncsi_package **np,
 				   struct ncsi_channel **nc);
 struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp);
-void ncsi_free_req(struct ncsi_req *nr, bool check, bool timeout);
+void ncsi_free_req(struct ncsi_req *nr);
 struct ncsi_dev *ncsi_find_dev(struct net_device *dev);
 int ncsi_config_dev(struct ncsi_dev *nd);
 
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 9dbdcff..feedb66 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -296,7 +296,7 @@ static struct ncsi_req *ncsi_alloc_cmd_req(struct ncsi_cmd_arg *nca)
 	/* Allocate skb */
 	skb = alloc_skb(len, GFP_ATOMIC);
 	if (!skb) {
-		ncsi_free_req(nr, false, false);
+		ncsi_free_req(nr);
 		return NULL;
 	}
 
@@ -366,6 +366,6 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 
 	return 0;
 out:
-	ncsi_free_req(nr, false, false);
+	ncsi_free_req(nr);
 	return ret;
 }
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5080f72..b32aa39 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -334,7 +334,7 @@ struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp)
 	return nr;
 }
 
-void ncsi_free_req(struct ncsi_req *nr, bool check, bool timeout)
+void ncsi_free_req(struct ncsi_req *nr)
 {
 	struct ncsi_dev_priv *ndp = nr->nr_ndp;
 	struct sk_buff *cmd, *rsp;
@@ -353,7 +353,7 @@ void ncsi_free_req(struct ncsi_req *nr, bool check, bool timeout)
 	nr->nr_used = false;
 	spin_unlock_irqrestore(&ndp->ndp_req_lock, flags);
 
-	if (check && cmd && atomic_dec_return(&ndp->ndp_pending_reqs) == 0)
+	if (cmd && atomic_dec_return(&ndp->ndp_pending_reqs) == 0)
 		schedule_work(&ndp->ndp_work);
 	/* Release command and response */
 	consume_skb(cmd);
@@ -802,7 +802,7 @@ static void ncsi_req_timeout(unsigned long data)
 	spin_unlock_irqrestore(&ndp->ndp_req_lock, flags);
 
 	/* Release the request */
-	ncsi_free_req(nr, true, true);
+	ncsi_free_req(nr);
 }
 
 struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index c34998d..c24a1e1 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -1166,6 +1166,6 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 
 out:
 	spin_unlock_irqrestore(&ndp->ndp_req_lock, flags);
-	ncsi_free_req(nr, true, false);
+	ncsi_free_req(nr);
 	return ret;
 }
-- 
2.1.0

[PATCH 4.4 2/9] net/ncsi: Introduce NCSI_RESERVED_CHANNEL

[Gavin Shan]

This defines NCSI_RESERVED_CHANNEL as the reserved NCSI channel
ID (0x1f). No logical changes introduced.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 net/ncsi/internal.h    |  1 +
 net/ncsi/ncsi-manage.c | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index e97906c..339177a 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -5,6 +5,7 @@ struct ncsi_dev_priv;
 struct ncsi_package;
 
 #define NCSI_PACKAGE_INDEX(c)	(((c) >> 5) & 0x7)
+#define NCSI_RESERVED_CHANNEL	0x1f
 #define NCSI_CHANNEL_INDEX(c)	((c) & 0x1ffff)
 #define NCSI_TO_CHANNEL(p, c)	((((p) & 0x7) << 5) | ((c) & 0x1ffff))
 
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b32aa39..3d1d775 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -397,7 +397,7 @@ static void ncsi_dev_config(struct ncsi_dev_priv *ndp)
 		nca.nca_type = NCSI_PKT_CMD_SP;
 		nca.nca_bytes[0] = 1;
 		nca.nca_package = np->np_id;
-		nca.nca_channel = 0x1f;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		ret = ncsi_xmit_cmd(&nca);
 		if (ret)
 			goto error;
@@ -533,7 +533,7 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 
 		/* Deselect all possible packages */
 		nca.nca_type = NCSI_PKT_CMD_DP;
-		nca.nca_channel = 0x1f;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		for (index = 0; index < 8; index++) {
 			nca.nca_package = index;
 			ret = ncsi_xmit_cmd(&nca);
@@ -549,7 +549,7 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 		/* Select all possible packages */
 		nca.nca_type = NCSI_PKT_CMD_SP;
 		nca.nca_bytes[0] = 1;
-		nca.nca_channel = 0x1f;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		for (index = 0; index < 8; index++) {
 			nca.nca_package = index;
 			ret = ncsi_xmit_cmd(&nca);
@@ -608,7 +608,7 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 		nca.nca_type = NCSI_PKT_CMD_SP;
 		nca.nca_bytes[0] = 1;
 		nca.nca_package = ndp->ndp_active_package->np_id;
-		nca.nca_channel = 0x1f;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		ret = ncsi_xmit_cmd(&nca);
 		if (ret)
 			goto error;
@@ -666,7 +666,7 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 		/* Deselect the active package */
 		nca.nca_type = NCSI_PKT_CMD_DP;
 		nca.nca_package = ndp->ndp_active_package->np_id;
-		nca.nca_channel = 0x1f;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		ret = ncsi_xmit_cmd(&nca);
 		if (ret)
 			goto error;
@@ -716,7 +716,7 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 		nca.nca_package = np->np_id;
 		if (nd->nd_state == ncsi_dev_state_suspend_select) {
 			nca.nca_type = NCSI_PKT_CMD_SP;
-			nca.nca_channel = 0x1f;
+			nca.nca_channel = NCSI_RESERVED_CHANNEL;
 			nca.nca_bytes[0] = 1;
 			nd->nd_state = ncsi_dev_state_suspend_dcnt;
 		} else if (nd->nd_state == ncsi_dev_state_suspend_dcnt) {
@@ -730,7 +730,7 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 			nd->nd_state = ncsi_dev_state_suspend_deselect;
 		} else if (nd->nd_state == ncsi_dev_state_suspend_deselect) {
 			nca.nca_type = NCSI_PKT_CMD_DP;
-			nca.nca_channel = 0x1f;
+			nca.nca_channel = NCSI_RESERVED_CHANNEL;
 			nd->nd_state = ncsi_dev_state_suspend_done;
 		}
 
-- 
2.1.0

[PATCH 4.4 3/9] net/ncsi: Don't probe on the reserved channel ID (0x1f)

[Gavin Shan]

We needn't send CIS (Clear Initial State) command to the NCSI
reserved channel (0x1f) in the enumeration. We shouldn't receive
a valid response from CIS on NCSI channel 0x1f.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 net/ncsi/ncsi-manage.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 3d1d775..b728614 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -616,12 +616,12 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 		nd->nd_state = ncsi_dev_state_probe_cis;
 		break;
 	case ncsi_dev_state_probe_cis:
-		atomic_set(&ndp->ndp_pending_reqs, 0x20);
+		atomic_set(&ndp->ndp_pending_reqs, NCSI_RESERVED_CHANNEL);
 
 		/* Clear initial state */
 		nca.nca_type = NCSI_PKT_CMD_CIS;
 		nca.nca_package = ndp->ndp_active_package->np_id;
-		for (index = 0; index < 0x20; index++) {
+		for (index = 0; index < NCSI_RESERVED_CHANNEL; index++) {
 			nca.nca_channel = index;
 			ret = ncsi_xmit_cmd(&nca);
 			if (ret)
-- 
2.1.0

[PATCH 4.4 4/9] net/ncsi: Rework request index allocation

[Gavin Shan]

The NCSI request index (struct ncsi_req::nr_id) is put into instance
ID (IID) field while sending NCSI command packet. It was designed the
available IDs are given in round-robin fashion. @ndp->request_id was
introduced to represent the next available ID, but it has been used
as number of successively allocated IDs. It breaks the round-robin
design. Besides, we shouldn't put 0 to NCSI command packet's IID
field, meaning ID#0 should be reserved according section 6.3.1.1
in NCSI spec (v1.1.0).

This fixes above two issues. With it applied, the available IDs will
be assigned in round-robin fashion and ID#0 won't be assigned.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 net/ncsi/internal.h    |  1 +
 net/ncsi/ncsi-manage.c | 23 +++++++++++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 339177a..212d77b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -87,6 +87,7 @@ struct ncsi_dev_priv {
 	struct list_head	ndp_packages;
 	atomic_t		ndp_pending_reqs;
 	atomic_t		ndp_last_req_idx;
+#define NCSI_REQ_START_IDX	1
 	spinlock_t		ndp_req_lock;
 	struct ncsi_req		ndp_reqs[256];
 	struct work_struct	ndp_work;
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index b728614..44a740f 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -306,30 +306,29 @@ struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp)
 	spin_lock_irqsave(&ndp->ndp_req_lock, flags);
 
 	/* Check if there is one available request until the ceiling */
-	for (idx = atomic_read(&ndp->ndp_last_req_idx);
-	     !nr && idx < limit; idx++) {
+	for (idx = atomic_read(&ndp->ndp_last_req_idx); idx < limit; idx++) {
 		if (ndp->ndp_reqs[idx].nr_used)
 			continue;
 
-		ndp->ndp_reqs[idx].nr_used = true;
 		nr = &ndp->ndp_reqs[idx];
-		atomic_inc(&ndp->ndp_last_req_idx);
-		if (atomic_read(&ndp->ndp_last_req_idx) >= limit)
-			atomic_set(&ndp->ndp_last_req_idx, 0);
+		nr->nr_used = true;
+		atomic_set(&ndp->ndp_last_req_idx, idx + 1);
+		goto found;
 	}
 
 	/* Fail back to check from the starting cursor */
-	for (idx = 0; !nr && idx < atomic_read(&ndp->ndp_last_req_idx); idx++) {
+	for (idx = NCSI_REQ_START_IDX;
+	     idx < atomic_read(&ndp->ndp_last_req_idx); idx++) {
 		if (ndp->ndp_reqs[idx].nr_used)
 			continue;
 
-		ndp->ndp_reqs[idx].nr_used = true;
 		nr = &ndp->ndp_reqs[idx];
-		atomic_inc(&ndp->ndp_last_req_idx);
-		if (atomic_read(&ndp->ndp_last_req_idx) >= limit)
-			atomic_set(&ndp->ndp_last_req_idx, 0);
+		nr->nr_used = true;
+		atomic_set(&ndp->ndp_last_req_idx, idx + 1);
+		goto found;
 	}
 
+found:
 	spin_unlock_irqrestore(&ndp->ndp_req_lock, flags);
 	return nr;
 }
@@ -834,7 +833,7 @@ struct ncsi_dev *ncsi_register_dev(struct net_device *dev,
 	INIT_LIST_HEAD(&ndp->ndp_packages);
 	INIT_WORK(&ndp->ndp_work, ncsi_dev_work);
 	spin_lock_init(&ndp->ndp_req_lock);
-	atomic_set(&ndp->ndp_last_req_idx, 0);
+	atomic_set(&ndp->ndp_last_req_idx, NCSI_REQ_START_IDX);
 	for (idx = 0; idx < 256; idx++) {
 		ndp->ndp_reqs[idx].nr_id = idx;
 		ndp->ndp_reqs[idx].nr_ndp = ndp;
-- 
2.1.0

[PATCH 4.4 5/9] net/ncsi: Allow to extend NCSI request properties

[Gavin Shan]

There is only one NCSI request property for now: the response for
the sent command need drive the workqueue or not. So we had one
field (@driven) for the purpose. We lost the flexibility to extend
NCSI request properties.

This replaces @driven with @nr_flags and @nca_req_flags in NCSI
request and NCSI command argument struct. Each bit of the newly
introduced field can be used for one property. No functional
changes introduced.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
 net/ncsi/internal.h    |  6 +++++-
 net/ncsi/ncsi-cmd.c    |  2 +-
 net/ncsi/ncsi-manage.c | 12 ++++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 212d77b..18a477b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -41,6 +41,8 @@ struct ncsi_package {
 struct ncsi_req {
 	unsigned char		nr_id;
 	bool			nr_used;
+	unsigned int		nr_flags;
+#define NCSI_REQ_FLAG_EVENT_DRIVEN	1
 	struct ncsi_dev_priv	*nr_ndp;
 	struct sk_buff		*nr_cmd;
 	struct sk_buff		*nr_rsp;
@@ -103,6 +105,7 @@ struct ncsi_cmd_arg {
 	unsigned char		nca_channel;
 	unsigned short		nca_payload;
 	unsigned int		nca_portid;
+	unsigned int		nca_req_flags;
 	union {
 		unsigned char	nca_bytes[16];
 		unsigned short	nca_words[8];
@@ -140,7 +143,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
 				   unsigned char id,
 				   struct ncsi_package **np,
 				   struct ncsi_channel **nc);
-struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp);
+struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp,
+				unsigned int req_flags);
 void ncsi_free_req(struct ncsi_req *nr);
 struct ncsi_dev *ncsi_find_dev(struct net_device *dev);
 int ncsi_config_dev(struct ncsi_dev *nd);
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index feedb66..9b0ddb7 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -279,7 +279,7 @@ static struct ncsi_req *ncsi_alloc_cmd_req(struct ncsi_cmd_arg *nca)
 	struct sk_buff *skb;
 	struct ncsi_req *nr;
 
-	nr = ncsi_alloc_req(ndp);
+	nr = ncsi_alloc_req(ndp, nca->nca_req_flags);
 	if (!nr)
 		return NULL;
 
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 44a740f..e3fe191 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -297,7 +297,8 @@ void ncsi_find_package_and_channel(struct ncsi_dev_priv *ndp,
  * same. Otherwise, the bogus response might be replied. So the
  * available IDs are allocated in round-robin fasion.
  */
-struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp)
+struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp,
+				unsigned int req_flags)
 {
 	struct ncsi_req *nr = NULL;
 	int idx, limit = 256;
@@ -312,6 +313,7 @@ struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp)
 
 		nr = &ndp->ndp_reqs[idx];
 		nr->nr_used = true;
+		nr->nr_flags = req_flags;
 		atomic_set(&ndp->ndp_last_req_idx, idx + 1);
 		goto found;
 	}
@@ -324,6 +326,7 @@ struct ncsi_req *ncsi_alloc_req(struct ncsi_dev_priv *ndp)
 
 		nr = &ndp->ndp_reqs[idx];
 		nr->nr_used = true;
+		nr->nr_flags = req_flags;
 		atomic_set(&ndp->ndp_last_req_idx, idx + 1);
 		goto found;
 	}
@@ -338,6 +341,7 @@ void ncsi_free_req(struct ncsi_req *nr)
 	struct ncsi_dev_priv *ndp = nr->nr_ndp;
 	struct sk_buff *cmd, *rsp;
 	unsigned long flags;
+	bool driven;
 
 	if (nr->nr_timer_enabled) {
 		nr->nr_timer_enabled = false;
@@ -350,9 +354,10 @@ void ncsi_free_req(struct ncsi_req *nr)
 	nr->nr_cmd = NULL;
 	nr->nr_rsp = NULL;
 	nr->nr_used = false;
+	driven = !!(nr->nr_flags & NCSI_REQ_FLAG_EVENT_DRIVEN);
 	spin_unlock_irqrestore(&ndp->ndp_req_lock, flags);
 
-	if (cmd && atomic_dec_return(&ndp->ndp_pending_reqs) == 0)
+	if (driven && cmd && atomic_dec_return(&ndp->ndp_pending_reqs) == 0)
 		schedule_work(&ndp->ndp_work);
 	/* Release command and response */
 	consume_skb(cmd);
@@ -382,6 +387,7 @@ static void ncsi_dev_config(struct ncsi_dev_priv *ndp)
 	int ret;
 
 	nca.nca_ndp = ndp;
+	nca.nca_req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
 
 	/* When we're reconfiguring the active channel, the active package
 	 * should be selected and the old setting on the active channel
@@ -523,6 +529,7 @@ static void ncsi_dev_probe(struct ncsi_dev_priv *ndp)
 	int ret;
 
 	nca.nca_ndp = ndp;
+	nca.nca_req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
 	switch (nd->nd_state) {
 	case ncsi_dev_state_probe:
 		nd->nd_state = ncsi_dev_state_probe_deselect;
@@ -694,6 +701,7 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 	int ret;
 
 	nca.nca_ndp = ndp;
+	nca.nca_req_flags = NCSI_REQ_FLAG_EVENT_DRIVEN;
 	switch (nd->nd_state) {
 	case ncsi_dev_state_suspend:
 		/* If there're no active channel, we're done */
-- 
2.1.0

[PATCH 4.4 6/9] net/ncsi: Avoid if statements in ncsi_suspend_dev()

[Gavin Shan]

There are several if/else statements in the state machine implemented
by switch/case in ncsi_suspend_dev() to avoid duplicated code. It
makes the code a bit hard to be understood.

This drops if/else statements in ncsi_suspend_dev() to improve the
code readability as Joel Stanley suggested. Also, it becomes easy to
add more states in the state machine without affecting current code.
No logical changes introduced by this.

Comparing to the original patch, this also drops the tag "done" in
ncsi_suspend_dev(). The code path covered by the tag is replaced
by newly introduced tag "error".

Suggested-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-manage.c | 83 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 32 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index e3fe191..99045ed 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -695,8 +695,8 @@ error:
 static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 {
 	struct ncsi_dev *nd = &ndp->ndp_ndev;
-	struct ncsi_package *np;
-	struct ncsi_channel *nc;
+	struct ncsi_package *np = ndp->ndp_active_package;
+	struct ncsi_channel *nc = ndp->ndp_active_channel;
 	struct ncsi_cmd_arg nca;
 	int ret;
 
@@ -705,51 +705,66 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 	switch (nd->nd_state) {
 	case ncsi_dev_state_suspend:
 		/* If there're no active channel, we're done */
-		if (!ndp->ndp_active_channel) {
-			nd->nd_state = ncsi_dev_state_suspend_done;
-			goto done;
-		}
+		if (!ndp->ndp_active_channel)
+			goto error;
 
 		nd->nd_state = ncsi_dev_state_suspend_select;
 		/* Fall through */
 	case ncsi_dev_state_suspend_select:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_SP;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
+		nca.nca_bytes[0] = 1;
+
+		nd->nd_state = ncsi_dev_state_suspend_dcnt;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_dcnt:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_DCNT;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = nc->nc_id;
+
+		nd->nd_state = ncsi_dev_state_suspend_dc;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_dc:
+		atomic_set(&ndp->ndp_pending_reqs, 1);
+
+		nca.nca_type = NCSI_PKT_CMD_DC;
+		nca.nca_package = np->np_id;
+		nca.nca_channel = nc->nc_id;
+		nca.nca_bytes[0] = 1;
+
+		nd->nd_state = ncsi_dev_state_suspend_deselect;
+		ret = ncsi_xmit_cmd(&nca);
+		if (ret)
+			goto error;
+
+		break;
 	case ncsi_dev_state_suspend_deselect:
 		atomic_set(&ndp->ndp_pending_reqs, 1);
 
-		np = ndp->ndp_active_package;
-		nc = ndp->ndp_active_channel;
+		nca.nca_type = NCSI_PKT_CMD_DP;
 		nca.nca_package = np->np_id;
-		if (nd->nd_state == ncsi_dev_state_suspend_select) {
-			nca.nca_type = NCSI_PKT_CMD_SP;
-			nca.nca_channel = NCSI_RESERVED_CHANNEL;
-			nca.nca_bytes[0] = 1;
-			nd->nd_state = ncsi_dev_state_suspend_dcnt;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_dcnt) {
-			nca.nca_type = NCSI_PKT_CMD_DCNT;
-			nca.nca_channel = nc->nc_id;
-			nd->nd_state = ncsi_dev_state_suspend_dc;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_dc) {
-			nca.nca_type = NCSI_PKT_CMD_DC;
-			nca.nca_channel = nc->nc_id;
-			nca.nca_bytes[0] = 1;
-			nd->nd_state = ncsi_dev_state_suspend_deselect;
-		} else if (nd->nd_state == ncsi_dev_state_suspend_deselect) {
-			nca.nca_type = NCSI_PKT_CMD_DP;
-			nca.nca_channel = NCSI_RESERVED_CHANNEL;
-			nd->nd_state = ncsi_dev_state_suspend_done;
-		}
+		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 
+		nd->nd_state = ncsi_dev_state_suspend_done;
 		ret = ncsi_xmit_cmd(&nca);
-		if (ret) {
-			nd->nd_state = ncsi_dev_state_suspend_done;
-			goto done;
-		}
+		if (ret)
+			goto error;
 
 		break;
 	case ncsi_dev_state_suspend_done:
-done:
 		if (ndp->ndp_flags & NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE)
 			ncsi_choose_active_channel(ndp);
 
@@ -767,6 +782,10 @@ done:
 		pr_warn("%s: Unsupported NCSI dev state 0x%x\n",
 			__func__, nd->nd_state);
 	}
+
+	return;
+error:
+	nd->nd_state = ncsi_dev_state_functional;
 }
 
 static void ncsi_dev_work(struct work_struct *work)
-- 
2.1.0

[PATCH 4.4 7/9] net/ncsi: Fix stale link state of inactive channels on failover

[Gavin Shan]

The issue was found on BCM5718 which has two NCSI channels in one
package: C0 and C1. Both of them are connected to different LANs,
means they are in link-up state and C0 is chosen as the active one
until resetting BCM5718 happens as below.

Resetting BCM5718 results in LSC (Link State Change) AEN packet
received on C0, meaning LSC AEN is missed on C1. When LSC AEN packet
received on C0 to report link-down, it fails over to C1 because C1
is in link-up state as software can see. However, C1 is in link-down
state in hardware. It means the link state is out of synchronization
between hardware and software, resulting in inappropriate channel (C1)
selected as active one.

This resolves the issue by sending separate GLS (Get Link Status)
commands to all channels in the package before trying to do failover.
The last link states of all channels in the package are retrieved.
With it, C0 (not C1) is selected as active one as expected.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/internal.h    |  1 +
 net/ncsi/ncsi-manage.c | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 18a477b..1696b1b 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -71,6 +71,7 @@ enum {
 	ncsi_dev_state_config_gls,
 	ncsi_dev_state_config_done,
 	ncsi_dev_state_suspend_select	= 0x0401,
+	ncsi_dev_state_suspend_gls,
 	ncsi_dev_state_suspend_dcnt,
 	ncsi_dev_state_suspend_dc,
 	ncsi_dev_state_suspend_deselect,
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 99045ed..a3bae7b 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -718,12 +718,39 @@ static void ncsi_dev_suspend(struct ncsi_dev_priv *ndp)
 		nca.nca_channel = NCSI_RESERVED_CHANNEL;
 		nca.nca_bytes[0] = 1;
 
-		nd->nd_state = ncsi_dev_state_suspend_dcnt;
+		/* To retrieve the last link states of channels in current
+		 * package when current active channel needs fail over to
+		 * another one. It means we will possibly select another
+		 * channel as next active one. The link states of channels
+		 * are most important factor of the selection. So we need
+		 * accurate link states. Unfortunately, the link states on
+		 * inactive channels can't be updated with LSC AEN in time.
+		 */
+		if (ndp->ndp_flags & NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE)
+			nd->nd_state = ncsi_dev_state_suspend_gls;
+		else
+			nd->nd_state = ncsi_dev_state_suspend_dcnt;
 		ret = ncsi_xmit_cmd(&nca);
 		if (ret)
 			goto error;
 
 		break;
+	case ncsi_dev_state_suspend_gls:
+		atomic_set(&ndp->ndp_pending_reqs,
+			   atomic_read(&np->np_channel_num));
+
+		nca.nca_type = NCSI_PKT_CMD_GLS;
+		nca.nca_package = np->np_id;
+
+		nd->nd_state = ncsi_dev_state_suspend_dcnt;
+		NCSI_FOR_EACH_CHANNEL(np, nc) {
+			nca.nca_channel = nc->nc_id;
+			ret = ncsi_xmit_cmd(&nca);
+			if (ret)
+				goto error;
+		}
+
+		break;
 	case ncsi_dev_state_suspend_dcnt:
 		atomic_set(&ndp->ndp_pending_reqs, 1);
 
-- 
2.1.0

[PATCH 4.4 8/9] net/ncsi: Choose hot channel as active one if necessary

[Gavin Shan]

The issue was found on BCM5718 which has two NCSI channels in one
package: C0 and C1. C0 is in link-up state while C1 is in link-down
state. C0 is chosen as active channel until unplugging and plugging
C0's cable:  On unplugging C0's cable, LSC (Link State Change) AEN
packet received on C0 to report link-down event. After that, C1 is
chosen as active channel. LSC AEN for link-up event is lost on C0
when plugging C0's cable back. We lose the network even C0 is usable.

This resolves the issue by recording the (hot) channel that was ever
chosen as active one. The hot channel is chosen to be active one
if none of available channels in link-up state. With this, C0 is still
the active one after unplugging C0's cable. LSC AEN packet received
on C0 when plugging its cable back.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/internal.h    |  1 +
 net/ncsi/ncsi-manage.c | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1696b1b..9476652 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -88,6 +88,7 @@ struct ncsi_dev_priv {
 	atomic_t		ndp_package_num;
 	spinlock_t		ndp_package_lock;
 	struct list_head	ndp_packages;
+	struct ncsi_channel	*ndp_hot_channel;
 	atomic_t		ndp_pending_reqs;
 	atomic_t		ndp_last_req_idx;
 #define NCSI_REQ_START_IDX	1
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bae7b..be767a8 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -382,6 +382,7 @@ static void ncsi_dev_config(struct ncsi_dev_priv *ndp)
 	struct net_device *dev = nd->nd_dev;
 	struct ncsi_package *np = ndp->ndp_active_package;
 	struct ncsi_channel *nc = ndp->ndp_active_channel;
+	struct ncsi_channel *hot_nc = NULL;
 	struct ncsi_cmd_arg nca;
 	unsigned char index;
 	int ret;
@@ -472,8 +473,15 @@ static void ncsi_dev_config(struct ncsi_dev_priv *ndp)
 	case ncsi_dev_state_config_done:
 		nd->nd_state = ncsi_dev_state_functional;
 		nd->nd_link_up = 0;
-		if (nc->nc_modes[NCSI_MODE_LINK].ncm_data[2] & 0x1)
+		if (nc->nc_modes[NCSI_MODE_LINK].ncm_data[2] & 0x1) {
 			nd->nd_link_up = 1;
+			hot_nc = nc;
+		} else {
+			hot_nc = NULL;
+		}
+
+		/* Update the hot channel */
+		ndp->ndp_hot_channel = hot_nc;
 
 		nd->nd_handler(nd);
 		ndp->ndp_flags &= ~NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
@@ -497,11 +505,13 @@ error:
 static void ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
 {
 	struct ncsi_package *np;
-	struct ncsi_channel *nc;
+	struct ncsi_channel *nc, *hot_nc;
 	struct ncsi_channel_mode *ncm;
 
 	ndp->ndp_active_package = NULL;
 	ndp->ndp_active_channel = NULL;
+
+	hot_nc = ndp->ndp_hot_channel;
 	NCSI_FOR_EACH_PACKAGE(ndp, np) {
 		NCSI_FOR_EACH_CHANNEL(np, nc) {
 			if (!ndp->ndp_active_channel) {
@@ -509,6 +519,11 @@ static void ncsi_choose_active_channel(struct ncsi_dev_priv *ndp)
 				ndp->ndp_active_channel = nc;
 			}
 
+			if (nc == hot_nc) {
+				ndp->ndp_active_package = np;
+				ndp->ndp_active_channel = nc;
+			}
+
 			ncm = &nc->nc_modes[NCSI_MODE_LINK];
 			if (ncm->ncm_data[2] & 0x1) {
 				ndp->ndp_active_package = np;
-- 
2.1.0

[PATCH 4.4 9/9] net/ncsi: Improve HNCDSC AEN handler

[Gavin Shan]

This improves AEN handler for Host Network Controller Driver Status
Change (HNCDSC):

   * The channel's lock should be hold when accessing its state.
   * Do failover when host driver isn't ready.
   * Configure channel when host driver becomes ready.

NOTE: The first one isn't applied to the code in dev-4.4.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-aen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 5bc0873..26ac93d 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -142,8 +142,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
 	ncm = &nc->nc_modes[NCSI_MODE_LINK];
 	hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
 	ncm->ncm_data[3] = ntohl(hncdsc->status);
-	if (ndp->ndp_active_channel != nc ||
-	    ncm->ncm_data[3] & 0x1)
+	if (ndp->ndp_active_channel != nc)
 		return 0;
 
 	/* If this channel is the active one and the link doesn't
@@ -151,7 +150,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
 	 * The logic here is exactly similar to what we do when link
 	 * is down on the active channel.
 	 */
-	ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
+	if (!(ncm->ncm_data[3] & 0x1))
+		ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
 	ncsi_suspend_dev(nd);
 
 	return 0;
-- 
2.1.0

Re: [PATCH 4.4 9/9] net/ncsi: Improve HNCDSC AEN handler

[Joel Stanley]

Hi Gavin,

On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> This improves AEN handler for Host Network Controller Driver Status
> Change (HNCDSC):
>
>    * The channel's lock should be hold when accessing its state.
>    * Do failover when host driver isn't ready.
>    * Configure channel when host driver becomes ready.
>
> NOTE: The first one isn't applied to the code in dev-4.4.

Can you clarify what you mean here?

Cheers,

Joel

>
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> ---
>  net/ncsi/ncsi-aen.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
> index 5bc0873..26ac93d 100644
> --- a/net/ncsi/ncsi-aen.c
> +++ b/net/ncsi/ncsi-aen.c
> @@ -142,8 +142,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>         ncm = &nc->nc_modes[NCSI_MODE_LINK];
>         hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
>         ncm->ncm_data[3] = ntohl(hncdsc->status);
> -       if (ndp->ndp_active_channel != nc ||
> -           ncm->ncm_data[3] & 0x1)
> +       if (ndp->ndp_active_channel != nc)
>                 return 0;
>
>         /* If this channel is the active one and the link doesn't
> @@ -151,7 +150,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>          * The logic here is exactly similar to what we do when link
>          * is down on the active channel.
>          */
> -       ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
> +       if (!(ncm->ncm_data[3] & 0x1))
> +               ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>         ncsi_suspend_dev(nd);
>
>         return 0;
> --
> 2.1.0
>

Re: [PATCH 4.4 9/9] net/ncsi: Improve HNCDSC AEN handler

[Gavin Shan]

On Fri, Oct 21, 2016 at 12:09:37PM +1030, Joel Stanley wrote:
>Hi Gavin,
>
>On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>> This improves AEN handler for Host Network Controller Driver Status
>> Change (HNCDSC):
>>
>>    * The channel's lock should be hold when accessing its state.
>>    * Do failover when host driver isn't ready.
>>    * Configure channel when host driver becomes ready.
>>
>> NOTE: The first one isn't applied to the code in dev-4.4.
>
>Can you clarify what you mean here?
>

Joel, dev-4.4 is using old NCSI old where we don't have a spinlock
for NCSI channel. dev-4.7 and upstream code has one to ensure the
consistent read/update on NCSI channel'state. The commit log was
picked from the patch merged to linux.net (branch: "net"), meaning
the commit log corresponds to the code changes for upstream code.

This piece of changes isn't applied to dev-4.4. So I put a note
to clarify it. I'm too lazy and it could be more meaningful actually :)

Thanks,
Gavin

>Cheers,
>
>Joel
>
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  net/ncsi/ncsi-aen.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
>> index 5bc0873..26ac93d 100644
>> --- a/net/ncsi/ncsi-aen.c
>> +++ b/net/ncsi/ncsi-aen.c
>> @@ -142,8 +142,7 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>>         ncm = &nc->nc_modes[NCSI_MODE_LINK];
>>         hncdsc = (struct ncsi_aen_hncdsc_pkt *)h;
>>         ncm->ncm_data[3] = ntohl(hncdsc->status);
>> -       if (ndp->ndp_active_channel != nc ||
>> -           ncm->ncm_data[3] & 0x1)
>> +       if (ndp->ndp_active_channel != nc)
>>                 return 0;
>>
>>         /* If this channel is the active one and the link doesn't
>> @@ -151,7 +150,8 @@ static int ncsi_aen_handler_hncdsc(struct ncsi_dev_priv *ndp,
>>          * The logic here is exactly similar to what we do when link
>>          * is down on the active channel.
>>          */
>> -       ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>> +       if (!(ncm->ncm_data[3] & 0x1))
>> +               ndp->ndp_flags |= NCSI_DEV_PRIV_FLAG_CHANGE_ACTIVE;
>>         ncsi_suspend_dev(nd);
>>
>>         return 0;
>> --
>> 2.1.0
>>
>

Re: [PATCH 4.4 9/9] net/ncsi: Improve HNCDSC AEN handler

[Joel Stanley]

On Fri, Oct 21, 2016 at 1:11 PM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
> On Fri, Oct 21, 2016 at 12:09:37PM +1030, Joel Stanley wrote:
>>Hi Gavin,
>>
>>On Fri, Oct 21, 2016 at 11:17 AM, Gavin Shan <gwshan@linux.vnet.ibm.com> wrote:
>>> This improves AEN handler for Host Network Controller Driver Status
>>> Change (HNCDSC):
>>>
>>>    * The channel's lock should be hold when accessing its state.
>>>    * Do failover when host driver isn't ready.
>>>    * Configure channel when host driver becomes ready.
>>>
>>> NOTE: The first one isn't applied to the code in dev-4.4.
>>
>>Can you clarify what you mean here?
>>
>
> Joel, dev-4.4 is using old NCSI old where we don't have a spinlock
> for NCSI channel. dev-4.7 and upstream code has one to ensure the
> consistent read/update on NCSI channel'state. The commit log was
> picked from the patch merged to linux.net (branch: "net"), meaning
> the commit log corresponds to the code changes for upstream code.
>
> This piece of changes isn't applied to dev-4.4. So I put a note
> to clarify it. I'm too lazy and it could be more meaningful actually :)

Okay, thanks for the explanation.

Cheers,

Joel