[tipc-discussion] Proposed TIPC 1.7.7-rc4 patch to correct bug in earlier "Ignore broadcast acknowledgements ..." patch

[Stephens, A. <all...@wi...>]

A.R. Karthick has recently identified an apparent problem with the recently published TIPC 1.7 patch "Ignore broadcast acknowledgements that are out-of-range". This email contains a supplementary patch that is designed to correct the bug introduced by that patch.

Any TIPC 1.7.7-rc4 user who has incorporated the defective patch and wishes to test this corrective patch can apply it to their system directly; there is no need to remove the previous patch. Feedback on this new patch -- either positive or negative -- is greatly appreciated. Once I've got some assurance that the corrective patch really does what it's supposed to, and doesn't introduce any new issues, I'll make it generally available in the TIPC 1.7 git repository at SourceForge.

Regards,
Al

...............

This patch revises the checks that were added to TIPC's broadcast link by the
patch "Ignore broadcast acknowledgements that are out-of-range" to
prevent a failed logic assertion from triggering a kernel BUG report.

This revision ensures that any unsent messages in the broadcast link's
transmit queue are properly purged when contact with TIPC's only
neighboring node is lost, thereby restoring the clean up capability
that existed before the new broadcast link checks were added. The checks
are now performed only if TIPC is still in contact with the acknowledging
node. Conversely, if contact with the acknowledging node has been lost
the checks are unnecessary and the broadcast link determines which
messages in the transmit queue need to be cleaned up, based on whether
or not the unsent messages (if any) can still be delivered.

Signed-off-by: Allan Stephens <all...@wi...>
---
 net/tipc/tipc_bcast.c |   47 +++++++++++++++++++++++++----------------------
 1 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/net/tipc/tipc_bcast.c b/net/tipc/tipc_bcast.c
index c07fc4c..c53dcfb 100644
--- a/net/tipc/tipc_bcast.c
+++ b/net/tipc/tipc_bcast.c
@@ -218,32 +218,37 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 
 	spin_lock_bh(&bc_lock);
 
-	/*
-	 * Invalid number indicates that all queued messages are acknowledged
-	 * (used when contact with specified node has been lost)
-	 */
-
-	if (acked == INVALID_LINK_SEQ)
-		acked = bcl->fsm_msg_cnt;
-
-	/*
-	 * Bail out if no unacknowledged messages in queue
-	 * (either the node is acknowledging messages it has previously ack'd
-	 * or the ack value is invalid and should be ignored)
-	 */
+	/* Bail out if tx queue is empty (no clean up is required) */
 
 	crs = bcl->first_out;
 	if (!crs)
 		goto exit;
 
-	/*
-	 * Validate sequence number to ensure it corresponds to a message
-	 * that has been sent and not yet acknowledged
-	 */
+	/* Determine which messages need to be acknowledged */
 
-	if (less(acked, buf_seqno(crs)) || less(bcl->fsm_msg_cnt, acked) ||
-	    less_eq(acked, n_ptr->bclink.acked)) {
-		goto exit;
+	if (acked == INVALID_LINK_SEQ) {
+
+		/*
+		 * Contact with specified node has been lost, so need to
+		 * acknowledge sent messages only (if other nodes still exist)
+		 * or both sent and unsent messages (otherwise)
+		 */
+
+		if (bclink->bcast_nodes.count)
+			acked = bcl->fsm_msg_cnt;
+		else
+			acked = bcl->next_out_no;
+	} else {
+
+		/*
+		 * Bail out if specified sequence number does not correspond
+		 * to a message that has been sent and not yet acknowledged
+		 */
+
+		if (less(acked, buf_seqno(crs)) ||
+		    less(bcl->fsm_msg_cnt, acked) ||
+		    less_eq(acked, n_ptr->bclink.acked))
+			goto exit;
 	}
 
 	/* Skip over packets that node has previously acknowledged */
@@ -259,8 +264,6 @@ void tipc_bclink_acknowledge(struct tipc_node *n_ptr, u32 acked)
 
 		if (crs != bcl->next_out)
 			bcbuf_decr_acks(crs);
-		else if (bclink->bcast_nodes.count)
-			break;
 		else {
 			bcbuf_set_acks(crs, 0);
 			bcl->next_out = next;
-- 
1.7.1