netdev - [PATCH] sctp: optimize searching the active path for tsns

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Mar 2013 13:29:42 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	linux-sctp@...r.kernel-org
Cc:	Neil Horman <nhorman@...driver.com>,
	Xufeng Zhang <xufengzhang.main@...il.com>, vyasevich@...il.com,
	davem@...emloft.net, netdev@...r.kernel.org
Subject: [PATCH] sctp: optimize searching the active path for tsns

SCTP currently attempts to optimize the search for tsns on a transport by first
checking the active_path, then searching alternate transports.  This operation
however is a bit convoluted, as we explicitly search the active path, then serch
all other transports, skipping the active path, when its detected.  Lets
optimize this by preforming a move to front on the transport_addr_list every
time the active_path is changed.  The active_path changes occur in relatively
non-critical paths, and doing so allows us to just search the
transport_addr_list in order, avoiding an extra conditional check in the
relatively hot tsn lookup path.  This also happens to fix a bug where we break
out of the for loop early in the tsn lookup.

CC: Xufeng Zhang <xufengzhang.main@...il.com>
CC: vyasevich@...il.com
CC: davem@...emloft.net
CC: netdev@...r.kernel.org
---
 net/sctp/associola.c | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 43cd0dd..7af96b3 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -513,8 +513,11 @@ void sctp_assoc_set_primary(struct sctp_association *asoc,
 	 * user wants to use this new path.
 	 */
 	if ((transport->state == SCTP_ACTIVE) ||
-	    (transport->state == SCTP_UNKNOWN))
+	    (transport->state == SCTP_UNKNOWN)) {
+		list_del_rcu(&transport->transports);
+		list_add_rcu(&transport->transports, &asoc->peer.transport_addr_list);
 		asoc->peer.active_path = transport;
+	}
 
 	/*
 	 * SFR-CACC algorithm:
@@ -964,6 +967,10 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 	}
 
 	/* Set the active and retran transports.  */
+	if (asoc->peer.active_path != first) {
+		list_del_rcu(first);
+		list_add_rcu(first, &asoc->peer.transport_addr_list);
+	}
 	asoc->peer.active_path = first;
 	asoc->peer.retran_path = second;
 }
@@ -1040,7 +1047,6 @@ struct sctp_chunk *sctp_get_ecne_prepend(struct sctp_association *asoc)
 struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
 					     __u32 tsn)
 {
-	struct sctp_transport *active;
 	struct sctp_transport *match;
 	struct sctp_transport *transport;
 	struct sctp_chunk *chunk;
@@ -1057,29 +1063,16 @@ struct sctp_transport *sctp_assoc_lookup_tsn(struct sctp_association *asoc,
 	 * The general strategy is to search each transport's transmitted
 	 * list.   Return which transport this TSN lives on.
 	 *
-	 * Let's be hopeful and check the active_path first.
-	 * Another optimization would be to know if there is only one
-	 * outbound path and not have to look for the TSN at all.
+	 * Note, that sctp_assoc_set_primary does a move to front operation
+	 * on the active_path transport, so this code implicitly checks
+	 * the active_path first, as we most commonly expect to find our TSN
+	 * there.
 	 *
 	 */
 
-	active = asoc->peer.active_path;
-
-	list_for_each_entry(chunk, &active->transmitted,
-			transmitted_list) {
-
-		if (key == chunk->subh.data_hdr->tsn) {
-			match = active;
-			goto out;
-		}
-	}
-
-	/* If not found, go search all the other transports. */
 	list_for_each_entry(transport, &asoc->peer.transport_addr_list,
 			transports) {
 
-		if (transport == active)
-			break;
 		list_for_each_entry(chunk, &transport->transmitted,
 				transmitted_list) {
 			if (key == chunk->subh.data_hdr->tsn) {
-- 
1.7.11.7

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html