[PATCH net-next 2/2] sctp: shrink sctp_tsnmap some more by removing gabs array

Previous thread: none

Next thread: pull request: wireless-2.6 2008-10-08 by John W. Linville on Wednesday, October 8, 2008 - 1:53 pm. (2 messages)
From: Vlad Yasevich
Date: Wednesday, October 8, 2008 - 1:27 pm

The following two patches improve the TSN map implementation in sctp.
The TSN map is used to track which TSNs (transmission sequence numbers) we
received.  The way it's currently written, we have a 4K array of bytes
where each byte represents the TSN.  When we receive a TSN we mark the
corresponding byte and then run through an algorithm to update the
ack points.

This is grossely inefficient in both memory and code.  The only time it
makes sense to mark the TSN in the map is if it's out of order.  In-order
tsns can simply move the ack point and be done with it.  Also, since this
is a simple a 1 or 0 (we have the TSN or we do not), we can do all this
with bitmaps and shrink our map by a factor of 8.  Further more,
it is hightly unlikely that we are every going to see 2 out of order
number with a difference/gap of 4096.  So, we can start our map at
the optimum size of a long word (32 or 64 bits depending on architecure).
We can then grow the map in the unlikely event that we receive a gap
that will push us outside of the map.  On a low loss network, fast
retransmis will prevent that before the map grows to much.

Running netperf shows a tiny improvement, but it's very stable, ie.
it's the same 3% across 10.  It also shows about 1% reduction is
CPU utilization.

The second patch gets rid of the rather useless member of the tsn_map
and was done after the bulk of the work above.  It became more obvious
that the structure member was useless after all the clean up.

Please consider for 2.6.28.

Thanks
-vlad
--

From: Vlad Yasevich
Date: Wednesday, October 8, 2008 - 1:27 pm

The tsn map currently use is 4K large and is stuck inside
the sctp_association structure making memory references REALLY
expensive.  What we really need is at most 4K worth of bits
so the biggest map we would have is 512 bytes.   Also, the
map is only really usefull when we have gaps to store and
report.  As such, starting with minimal map of say 32 TSNs (bits)
should be enough for normal low-loss operations.  We can grow
the map by some multiple of 32 along with some extra room any
time we receive the TSN which would put us outside of the map
boundry.  As we close gaps, we can shift the map to rebase
it on the latest TSN we've seen.  This saves 4088 bytes per
association just in the map alone along savings from the now
unnecessary structure members.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/constants.h |    4 +-
 include/net/sctp/structs.h   |    1 -
 include/net/sctp/tsnmap.h    |   39 +----
 net/sctp/associola.c         |    9 +-
 net/sctp/sm_make_chunk.c     |    5 +-
 net/sctp/sm_sideeffect.c     |    3 +-
 net/sctp/tsnmap.c            |  331 +++++++++++++++++++-----------------------
 net/sctp/ulpevent.c          |   10 +-
 8 files changed, 178 insertions(+), 224 deletions(-)

diff --git a/include/net/sctp/constants.h b/include/net/sctp/constants.h
index c32ddf0..b05b055 100644
--- a/include/net/sctp/constants.h
+++ b/include/net/sctp/constants.h
@@ -261,7 +261,9 @@ enum { SCTP_ARBITRARY_COOKIE_ECHO_LEN = 200 };
  * must be less than 65535 (2^16 - 1), or we will have overflow
  * problems creating SACK's.
  */
-#define SCTP_TSN_MAP_SIZE 2048
+#define SCTP_TSN_MAP_INITIAL BITS_PER_LONG
+#define SCTP_TSN_MAP_INCREMENT SCTP_TSN_MAP_INITIAL
+#define SCTP_TSN_MAP_SIZE 4096
 #define SCTP_TSN_MAX_GAP  65535
 
 /* We will not record more than this many duplicate TSNs between two
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 94c62e4..9661d7b 100644
--- a/include/net/sctp/structs.h
+++ ...
From: David Miller
Date: Wednesday, October 8, 2008 - 2:19 pm

From: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.
--

From: Vlad Yasevich
Date: Wednesday, October 8, 2008 - 1:27 pm

The gabs array in the sctp_tsnmap structure is only used
in one place, sctp_make_sack().  As such, carrying the
array around in the sctp_tsnmap and thus directly in
the sctp_association is rather pointless since most
of the time it's just taking up space.  Now, let
sctp_make_sack create and populate it and then throw
it away when it's done.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
---
 include/net/sctp/tsnmap.h |   14 +++-----------
 net/sctp/sm_make_chunk.c  |    6 ++++--
 net/sctp/tsnmap.c         |   15 ++++++++-------
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/net/sctp/tsnmap.h b/include/net/sctp/tsnmap.h
index 6dabbee..4aabc5a 100644
--- a/include/net/sctp/tsnmap.h
+++ b/include/net/sctp/tsnmap.h
@@ -94,11 +94,8 @@ struct sctp_tsnmap {
 	 * every SACK.  Store up to SCTP_MAX_DUP_TSNS worth of
 	 * information.
 	 */
-	__be32 dup_tsns[SCTP_MAX_DUP_TSNS];
 	__u16 num_dup_tsns;
-
-	/* Record gap ack block information here.  */
-	struct sctp_gap_ack_block gabs[SCTP_MAX_GABS];
+	__be32 dup_tsns[SCTP_MAX_DUP_TSNS];
 };
 
 struct sctp_tsnmap_iter {
@@ -151,17 +148,12 @@ static inline __be32 *sctp_tsnmap_get_dups(struct sctp_tsnmap *map)
 }
 
 /* How many gap ack blocks do we have recorded? */
-__u16 sctp_tsnmap_num_gabs(struct sctp_tsnmap *map);
+__u16 sctp_tsnmap_num_gabs(struct sctp_tsnmap *map,
+			   struct sctp_gap_ack_block *gabs);
 
 /* Refresh the count on pending data. */
 __u16 sctp_tsnmap_pending(struct sctp_tsnmap *map);
 
-/* Return pointer to gap ack blocks as needed by SACK. */
-static inline struct sctp_gap_ack_block *sctp_tsnmap_get_gabs(struct sctp_tsnmap *map)
-{
-	return map->gabs;
-}
-
 /* Is there a gap in the TSN map?  */
 static inline int sctp_tsnmap_has_gap(const struct sctp_tsnmap *map)
 {
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 6dd9b3e..fd8acb4 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -702,12 +702,14 @@ struct sctp_chunk ...
From: David Miller
Date: Wednesday, October 8, 2008 - 2:19 pm

From: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied.
--

Previous thread: none

Next thread: pull request: wireless-2.6 2008-10-08 by John W. Linville on Wednesday, October 8, 2008 - 1:53 pm. (2 messages)