stream/sack: turn SACK record list into rbtree

Convert to rbtree from linked list. These ranges, of which there can
be multiple per packet, are fully controlled by an attacked. The
attacker could craft a stream of packet in such a way that the list
would grow very large. This would make inserts/removals very expensive,
as well as the list walk that is done and size calculation and pruning
operations.

The RBTREE makes inserts/removals much cheaper, at a slight overhead
for 'normal' operations and slightly higher per record memory use.

diff --git a/src/stream-tcp-private.h b/src/stream-tcp-private.h
index 554e0f57d9ba28847f7431333d7c282bbdd3f077..82ff33a007e1e96bbe8160d13cb26266a5c61b10 100644 (file)
--- a/src/stream-tcp-private.h
+++ b/src/stream-tcp-private.h
@@ -46,12 +46,18 @@ typedef struct TcpStateQueue_ {
     struct TcpStateQueue_ *next;
 } TcpStateQueue;
 
-typedef struct StreamTcpSackRecord_ {
+typedef struct StreamTcpSackRecord {
     uint32_t le;    /**< left edge, host order */
     uint32_t re;    /**< right edge, host order */
-    struct StreamTcpSackRecord_ *next;
+    RB_ENTRY(StreamTcpSackRecord) rb;
 } StreamTcpSackRecord;
 
+int TcpSackCompare(struct StreamTcpSackRecord *a, struct StreamTcpSackRecord *b);
+
+/* red-black tree prototype for SACK records */
+RB_HEAD(TCPSACK, StreamTcpSackRecord);
+RB_PROTOTYPE(TCPSACK, StreamTcpSackRecord, rb, TcpSackCompare);
+
 typedef struct TcpSegment {
     PoolThreadReserved res;
     uint16_t payload_len;       /**< actual size of the payload */
@@ -105,8 +111,7 @@ typedef struct TcpStream_ {
     StreamingBuffer sb;
     struct TCPSEG seg_tree;         /**< red black tree of TCP segments. Data is stored in TcpStream::sb */
 
-    StreamTcpSackRecord *sack_head; /**< head of list of SACK records */
-    StreamTcpSackRecord *sack_tail; /**< tail of list of SACK records */
+    struct TCPSACK sack_tree;       /**< red back tree of TCP SACK records. */
 } TcpStream;
 
 #define STREAM_BASE_OFFSET(stream)  ((stream)->sb.stream_offset)

diff --git a/src/stream-tcp-sack.c b/src/stream-tcp-sack.c
index d3f710273609f7ddd6e4a2def49fc112fabf1ae2..0e877608c10a79a26b6b4ee4a5215435a1a9706d 100644 (file)
--- a/src/stream-tcp-sack.c
+++ b/src/stream-tcp-sack.c
@@ -29,17 +29,34 @@
 #include "stream-tcp-sack.h"
 #include "util-unittest.h"
 
+RB_GENERATE(TCPSACK, StreamTcpSackRecord, rb, TcpSackCompare);
+
+int TcpSackCompare(struct StreamTcpSackRecord *a, struct StreamTcpSackRecord *b)
+{
+    if (SEQ_GT(a->le, b->le))
+        return 1;
+    else if (SEQ_LT(a->le, b->le))
+        return -1;
+    else {
+        if (SEQ_EQ(a->re, b->re))
+            return 0;
+        else if (SEQ_GT(a->re, b->re))
+            return 1;
+        else
+            return -1;
+    }
+}
 #ifdef DEBUG
 static void StreamTcpSackPrintList(TcpStream *stream)
 {
-    StreamTcpSackRecord *rec = stream->sack_head;
-    for (; rec != NULL; rec = rec->next) {
+    StreamTcpSackRecord *rec = NULL;
+    RB_FOREACH(rec, TCPSACK, &stream->sack_tree) {
         SCLogDebug("record %8u - %8u", rec->le, rec->re);
     }
 }
 #endif /* DEBUG */
 
-static StreamTcpSackRecord *StreamTcpSackRecordAlloc(void)
+static inline StreamTcpSackRecord *StreamTcpSackRecordAlloc(void)
 {
     if (StreamTcpCheckMemcap((uint32_t)sizeof(StreamTcpSackRecord)) == 0)
         return NULL;
@@ -52,12 +69,124 @@ static StreamTcpSackRecord *StreamTcpSackRecordAlloc(void)
     return rec;
 }
 
-static void StreamTcpSackRecordFree(StreamTcpSackRecord *rec)
+static inline void StreamTcpSackRecordFree(StreamTcpSackRecord *rec)
 {
     SCFree(rec);
     StreamTcpDecrMemuse((uint64_t)sizeof(*rec));
 }
 
+static inline void ConsolidateFwd(struct TCPSACK *tree, struct StreamTcpSackRecord *sa)
+{
+    struct StreamTcpSackRecord *tr, *s = sa;
+    RB_FOREACH_FROM(tr, TCPSACK, s) {
+        if (sa == tr)
+            continue;
+        SCLogDebug("-> (fwd) tr %p %u/%u", tr, tr->le, tr->re);
+
+        if (SEQ_LT(sa->re, tr->le))
+            break; // entirely before
+
+        if (SEQ_GEQ(sa->le, tr->le) && SEQ_LEQ(sa->re, tr->re)) {
+            sa->re = tr->re;
+            sa->le = tr->le;
+            SCLogDebug("-> (fwd) tr %p %u/%u REMOVED ECLIPSED2", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        /*
+            sa: [         ]
+            tr: [         ]
+            sa: [         ]
+            tr:    [      ]
+            sa: [         ]
+            tr:    [   ]
+        */
+        } else if (SEQ_LEQ(sa->le, tr->le) && SEQ_GEQ(sa->re, tr->re)) {
+            SCLogDebug("-> (fwd) tr %p %u/%u REMOVED ECLIPSED", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        /*
+            sa: [         ]
+            tr:      [         ]
+            sa: [       ]
+            tr:         [       ]
+        */
+        } else if (SEQ_LT(sa->le, tr->le) && // starts before
+                   SEQ_GEQ(sa->re, tr->le) && SEQ_LT(sa->re, tr->re) // ends inside
+            ) {
+            // merge
+            sa->re = tr->re;
+            SCLogDebug("-> (fwd) tr %p %u/%u REMOVED MERGED", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        }
+    }
+}
+
+static inline void ConsolidateBackward(struct TCPSACK *tree, struct StreamTcpSackRecord *sa)
+{
+    struct StreamTcpSackRecord *tr, *s = sa;
+    RB_FOREACH_REVERSE_FROM(tr, TCPSACK, s) {
+        if (sa == tr)
+            continue;
+        SCLogDebug("-> (bwd) tr %p %u/%u", tr, tr->le, tr->re);
+
+        if (SEQ_GT(sa->le, tr->re))
+            break; // entirely after
+        if (SEQ_GEQ(sa->le, tr->le) && SEQ_LEQ(sa->re, tr->re)) {
+            sa->re = tr->re;
+            sa->le = tr->le;
+            SCLogDebug("-> (bwd) tr %p %u/%u REMOVED ECLIPSED2", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        /*
+            sa: [         ]
+            tr: [         ]
+            sa:    [      ]
+            tr: [         ]
+            sa:    [   ]
+            tr: [         ]
+        */
+        } else if (SEQ_LEQ(sa->le, tr->le) && SEQ_GEQ(sa->re, tr->re)) {
+            SCLogDebug("-> (bwd) tr %p %u/%u REMOVED ECLIPSED", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        /*
+            sa:     [   ]
+            tr: [   ]
+            sa:    [    ]
+            tr: [   ]
+        */
+        } else if (SEQ_GT(sa->le, tr->le) && SEQ_GT(sa->re, tr->re) && SEQ_LEQ(sa->le,tr->re)) {
+            // merge
+            sa->le = tr->le;
+            SCLogDebug("-> (bwd) tr %p %u/%u REMOVED MERGED", tr, tr->le, tr->re);
+            TCPSACK_RB_REMOVE(tree, tr);
+            StreamTcpSackRecordFree(tr);
+        }
+    }
+}
+
+static int Insert(struct TCPSACK *tree, uint32_t le, uint32_t re)
+{
+    SCLogDebug("* inserting: %u/%u\n", le, re);
+
+    struct StreamTcpSackRecord *sa = StreamTcpSackRecordAlloc();
+    if (unlikely(sa == NULL))
+        return -1;
+    sa->le = le;
+    sa->re = re;
+    struct StreamTcpSackRecord *res = TCPSACK_RB_INSERT(tree, sa);
+    if (res) {
+        // exact overlap
+        SCLogDebug("* insert failed: exact match in tree with %p %u/%u", res, res->le, res->re);
+        StreamTcpSackRecordFree(sa);
+        return 0;
+    }
+    ConsolidateBackward(tree, sa);
+    ConsolidateFwd(tree, sa);
+    return 0;
+}
+
 /**
  *  \brief insert a SACK range
  *
@@ -77,167 +206,17 @@ static int StreamTcpSackInsertRange(TcpStream *stream, uint32_t le, uint32_t re)
     /* if to the left of last_ack then ignore */
     if (SEQ_LT(re, stream->last_ack)) {
         SCLogDebug("too far left. discarding");
-        goto end;
+        SCReturnInt(0);
     }
     /* if to the right of the tcp window then ignore */
     if (SEQ_GT(le, (stream->last_ack + stream->window))) {
         SCLogDebug("too far right. discarding");
-        goto end;
+        SCReturnInt(0);
     }
-    if (stream->sack_head != NULL) {
-        StreamTcpSackRecord *rec;
-
-        for (rec = stream->sack_head; rec != NULL; rec = rec->next) {
-            SCLogDebug("rec %p, le %u, re %u", rec, rec->le, rec->re);
-
-            if (SEQ_LT(le, rec->le)) {
-                SCLogDebug("SEQ_LT(le, rec->le)");
-                if (SEQ_LT(re, rec->le)) {
-                    SCLogDebug("SEQ_LT(re, rec->le)");
-                    // entirely before, prepend
-                    StreamTcpSackRecord *stsr = StreamTcpSackRecordAlloc();
-                    if (unlikely(stsr == NULL)) {
-                        SCReturnInt(-1);
-                    }
-                    stsr->le = le;
-                    stsr->re = re;
-
-                    stsr->next = stream->sack_head;
-                    stream->sack_head = stsr;
-                    goto end;
-                } else if (SEQ_EQ(re, rec->le)) {
-                    SCLogDebug("SEQ_EQ(re, rec->le)");
-                    // starts before, ends on rec->le, expand
-                    rec->le = le;
-                } else if (SEQ_GT(re, rec->le)) {
-                    SCLogDebug("SEQ_GT(re, rec->le)");
-                    // starts before, ends beyond rec->le
-                    if (SEQ_LEQ(re, rec->re)) {
-                        SCLogDebug("SEQ_LEQ(re, rec->re)");
-                        // ends before rec->re, expand
-                        rec->le = le;
-                    } else { // implied if (re > rec->re)
-                        SCLogDebug("implied if (re > rec->re), le set to %u", rec->re);
-                        le = rec->re;
-                        continue;
-                    }
-                }
-            } else if (SEQ_EQ(le, rec->le)) {
-                SCLogDebug("SEQ_EQ(le, rec->le)");
-                if (SEQ_LEQ(re, rec->re)) {
-                    SCLogDebug("SEQ_LEQ(re, rec->re)");
-                    // new record fully overlapped
-                    SCReturnInt(0);
-                } else { // implied re > rec->re
-                    SCLogDebug("implied re > rec->re");
-                    if (rec->next != NULL) {
-                        if (SEQ_LEQ(re, rec->next->le)) {
-                            rec->re = re;
-                            goto end;
-                        } else {
-                            rec->re = rec->next->le;
-                            le = rec->next->le;
-                            SCLogDebug("le is now %u", le);
-                            continue;
-                        }
-                    } else {
-                        rec->re = re;
-                        goto end;
-                    }
-                }
-            } else { // implied (le > rec->le)
-                SCLogDebug("implied (le > rec->le)");
-                if (SEQ_LT(le, rec->re)) {
-                    SCLogDebug("SEQ_LT(le, rec->re))");
-                    // new record fully overlapped
-                    if (SEQ_GT(re, rec->re)) {
-                        SCLogDebug("SEQ_GT(re, rec->re)");
-
-                        if (rec->next != NULL) {
-                            if (SEQ_LEQ(re, rec->next->le)) {
-                                rec->re = re;
-                                goto end;
-                            } else {
-                                rec->re = rec->next->le;
-                                le = rec->next->le;
-                                continue;
-                            }
-                        } else {
-                            rec->re = re;
-                            goto end;
-                        }
-                    }
-
-                    SCLogDebug("new range fully overlapped");
-                    SCReturnInt(0);
-                } else if (SEQ_EQ(le, rec->re)) {
-                    SCLogDebug("here");
-                    // new record fully overlapped
-                    //int r = StreamTcpSackInsertRange(stream, rec->re+1, re);
-                    //SCReturnInt(r);
-                    le = rec->re;
-                    continue;
-                } else { /* implied le > rec->re */
-                    SCLogDebug("implied le > rec->re");
-                    if (rec->next == NULL) {
-                        SCLogDebug("rec->next == NULL");
-                        StreamTcpSackRecord *stsr = StreamTcpSackRecordAlloc();
-                        if (unlikely(stsr == NULL)) {
-                            SCReturnInt(-1);
-                        }
-                        stsr->le = le;
-                        stsr->re = re;
-                        stsr->next = NULL;
-
-                        stream->sack_tail->next = stsr;
-                        stream->sack_tail = stsr;
-                        goto end;
-                    } else {
-                        SCLogDebug("implied rec->next != NULL");
-                        if (SEQ_LT(le, rec->next->le) && SEQ_LT(re, rec->next->le)) {
-                            SCLogDebug("SEQ_LT(le, rec->next->le) && SEQ_LT(re, rec->next->le)");
-                            StreamTcpSackRecord *stsr = StreamTcpSackRecordAlloc();
-                            if (unlikely(stsr == NULL)) {
-                                SCReturnInt(-1);
-                            }
-                            stsr->le = le;
-                            stsr->re = re;
-                            stsr->next = rec->next;
-                            rec->next = stsr;
-
-                        } else if (SEQ_LT(le, rec->next->le) && SEQ_GEQ(re, rec->next->le)) {
-                            SCLogDebug("SEQ_LT(le, rec->next->le) && SEQ_GEQ(re, rec->next->le)");
-                            StreamTcpSackRecord *stsr = StreamTcpSackRecordAlloc();
-                            if (unlikely(stsr == NULL)) {
-                                SCReturnInt(-1);
-                            }
-                            stsr->le = le;
-                            stsr->re = rec->next->le;
-                            stsr->next = rec->next;
-                            rec->next = stsr;
-
-                            le = rec->next->le;
-                        }
-                    }
-                }
-            }
-        }
-    } else {
-        SCLogDebug("implied empty list");
-        StreamTcpSackRecord *stsr = StreamTcpSackRecordAlloc();
-        if (unlikely(stsr == NULL)) {
-            SCReturnInt(-1);
-        }
-        stsr->le = le;
-        stsr->re = re;
-        stsr->next = NULL;
 
-        stream->sack_head = stsr;
-        stream->sack_tail = stsr;
-    }
+    if (Insert(&stream->sack_tree, le, re) < 0)
+        SCReturnInt(-1);
 
-    StreamTcpSackPruneList(stream);
-end:
     SCReturnInt(0);
 }
 
@@ -252,8 +231,7 @@ end:
  */
 int StreamTcpSackUpdatePacket(TcpStream *stream, Packet *p)
 {
-    int records = TCP_GET_SACK_CNT(p);
-    int record = 0;
+    const int records = TCP_GET_SACK_CNT(p);
     const uint8_t *data = TCP_GET_SACK_PTR(p);
 
     if (records == 0 || data == NULL)
@@ -262,35 +240,37 @@ int StreamTcpSackUpdatePacket(TcpStream *stream, Packet *p)
     TCPOptSackRecord rec[records], *sack_rec = rec;
     memcpy(&rec, data, sizeof(TCPOptSackRecord) * records);
 
-    for (record = 0; record < records; record++) {
+    for (int record = 0; record < records; record++) {
+        const uint32_t le = SCNtohl(sack_rec->le);
+        const uint32_t re = SCNtohl(sack_rec->re);
+
         SCLogDebug("%p last_ack %u, left edge %u, right edge %u", sack_rec,
-            stream->last_ack, SCNtohl(sack_rec->le), SCNtohl(sack_rec->re));
+            stream->last_ack, le, re);
 
-        if (SEQ_LEQ(SCNtohl(sack_rec->re), stream->last_ack)) {
+        if (SEQ_LEQ(re, stream->last_ack)) {
             SCLogDebug("record before last_ack");
             goto next;
         }
 
-        if (SEQ_GT(SCNtohl(sack_rec->re), stream->next_win)) {
+        if (SEQ_GT(re, stream->next_win)) {
             SCLogDebug("record %u:%u beyond next_win %u",
-                    SCNtohl(sack_rec->le), SCNtohl(sack_rec->re), stream->next_win);
+                    le, re, stream->next_win);
             goto next;
         }
 
-        if (SEQ_GEQ(SCNtohl(sack_rec->le), SCNtohl(sack_rec->re))) {
+        if (SEQ_GEQ(le, re)) {
             SCLogDebug("invalid record: le >= re");
             goto next;
         }
 
-        if (StreamTcpSackInsertRange(stream, SCNtohl(sack_rec->le),
-                    SCNtohl(sack_rec->re)) == -1)
-        {
+        if (StreamTcpSackInsertRange(stream, le, re) == -1) {
             SCReturnInt(-1);
         }
 
     next:
         sack_rec++;
     }
+    StreamTcpSackPruneList(stream);
 #ifdef DEBUG
     StreamTcpSackPrintList(stream);
 #endif
@@ -301,23 +281,13 @@ void StreamTcpSackPruneList(TcpStream *stream)
 {
     SCEnter();
 
-    StreamTcpSackRecord *rec = stream->sack_head;
-
-    while (rec != NULL) {
+    StreamTcpSackRecord *rec = NULL, *safe = NULL;
+    RB_FOREACH_SAFE(rec, TCPSACK, &stream->sack_tree, safe) {
         if (SEQ_LT(rec->re, stream->last_ack)) {
             SCLogDebug("removing le %u re %u", rec->le, rec->re);
+            TCPSACK_RB_REMOVE(&stream->sack_tree, rec);
+            StreamTcpSackRecordFree(rec);
 
-            if (rec->next != NULL) {
-                stream->sack_head = rec->next;
-                StreamTcpSackRecordFree(rec);
-                rec = stream->sack_head;
-                continue;
-            } else {
-                stream->sack_head = NULL;
-                stream->sack_tail = NULL;
-                StreamTcpSackRecordFree(rec);
-                break;
-            }
         } else if (SEQ_LT(rec->le, stream->last_ack)) {
             SCLogDebug("adjusting record to le %u re %u", rec->le, rec->re);
             /* last ack inside this record, update */
@@ -335,7 +305,7 @@ void StreamTcpSackPruneList(TcpStream *stream)
 }
 
 /**
- *  \brief Free SACK list from a stream
+ *  \brief Free SACK tree from a stream
  *
  *  \param stream Stream to cleanup
  */
@@ -343,17 +313,12 @@ void StreamTcpSackFreeList(TcpStream *stream)
 {
     SCEnter();
 
-    StreamTcpSackRecord *rec = stream->sack_head;
-    StreamTcpSackRecord *next = NULL;
-
-    while (rec != NULL) {
-        next = rec->next;
+    StreamTcpSackRecord *rec = NULL, *safe = NULL;
+    RB_FOREACH_SAFE(rec, TCPSACK, &stream->sack_tree, safe) {
+        TCPSACK_RB_REMOVE(&stream->sack_tree, rec);
         StreamTcpSackRecordFree(rec);
-        rec = next;
     }
 
-    stream->sack_head = NULL;
-    stream->sack_tail = NULL;
     SCReturn;
 }
 
@@ -369,8 +334,6 @@ void StreamTcpSackFreeList(TcpStream *stream)
 static int StreamTcpSackTest01 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -382,21 +345,15 @@ static int StreamTcpSackTest01 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 1 || stream.sack_head->re != 20) {
-        printf("list in weird state, head le %u, re %u: ",
-                stream.sack_head->le, stream.sack_head->re);
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 19) {
-        printf("size should be 19, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 1);
+    FAIL_IF(rec->re != 20);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 19);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -408,8 +365,6 @@ end:
 static int StreamTcpSackTest02 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -419,21 +374,15 @@ static int StreamTcpSackTest02 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 1 || stream.sack_head->re != 20) {
-        printf("list in weird state, head le %u, re %u: ",
-                stream.sack_head->le, stream.sack_head->re);
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 19) {
-        printf("size should be 19, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 1);
+    FAIL_IF(rec->re != 20);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 19);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -445,8 +394,6 @@ end:
 static int StreamTcpSackTest03 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -460,19 +407,15 @@ static int StreamTcpSackTest03 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 5) {
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 20) {
-        printf("size should be 20, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 5);
+    FAIL_IF(rec->re != 25);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 20);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -484,8 +427,6 @@ end:
 static int StreamTcpSackTest04 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -496,19 +437,15 @@ static int StreamTcpSackTest04 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 45) {
-        printf("size should be 45, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 25);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 45);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -520,8 +457,6 @@ end:
 static int StreamTcpSackTest05 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -532,19 +467,15 @@ static int StreamTcpSackTest05 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 50) {
-        printf("size should be 50, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 50);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 50);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -556,8 +487,6 @@ end:
 static int StreamTcpSackTest06 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -570,19 +499,15 @@ static int StreamTcpSackTest06 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
 
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 40);
 
-    retval = 1;
-end:
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -594,8 +519,6 @@ end:
 static int StreamTcpSackTest07 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -608,28 +531,18 @@ static int StreamTcpSackTest07 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 40);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
     stream.last_ack = 10;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 30);
 
-    if (StreamTcpSackedSize(&stream) != 30) {
-        printf("size should be 30, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -641,8 +554,6 @@ end:
 static int StreamTcpSackTest08 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -655,28 +566,18 @@ static int StreamTcpSackTest08 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 40);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
     stream.last_ack = 41;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 0);
 
-    if (StreamTcpSackedSize(&stream) != 0) {
-        printf("size should be 0, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -688,8 +589,6 @@ end:
 static int StreamTcpSackTest09 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 100;
 
@@ -703,28 +602,18 @@ static int StreamTcpSackTest09 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 0) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 0);
+    FAIL_IF(rec->re != 40);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
     stream.last_ack = 39;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 1);
 
-    if (StreamTcpSackedSize(&stream) != 1) {
-        printf("size should be 1, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -736,8 +625,6 @@ end:
 static int StreamTcpSackTest10 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 1000;
 
@@ -750,28 +637,18 @@ static int StreamTcpSackTest10 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 100) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 100);
+    FAIL_IF(rec->re != 140);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
     stream.last_ack = 99;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -783,8 +660,6 @@ end:
 static int StreamTcpSackTest11 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 1000;
 
@@ -797,28 +672,18 @@ static int StreamTcpSackTest11 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 100) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 100);
+    FAIL_IF(rec->re != 140);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
     stream.last_ack = 99;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 40);
 
-    if (StreamTcpSackedSize(&stream) != 40) {
-        printf("size should be 40, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -830,8 +695,6 @@ end:
 static int StreamTcpSackTest12 (void)
 {
     TcpStream stream;
-    int retval = 0;
-
     memset(&stream, 0, sizeof(stream));
     stream.window = 2000;
 
@@ -844,35 +707,21 @@ static int StreamTcpSackTest12 (void)
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (stream.sack_head->le != 100) {
-        goto end;
-    }
-
-    if (StreamTcpSackedSize(&stream) != 900) {
-        printf("size should be 900, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    StreamTcpSackRecord *rec = RB_MIN(TCPSACK, &stream.sack_tree);
+    FAIL_IF_NULL(rec);
+    FAIL_IF(rec->le != 100);
+    FAIL_IF(rec->re != 1000);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 900);
 
     StreamTcpSackInsertRange(&stream, 0, 1000);
-
-    if (StreamTcpSackedSize(&stream) != 1000) {
-        printf("size should be 1000, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(StreamTcpSackedSize(&stream) != 1000);
 
     stream.last_ack = 500;
-
     StreamTcpSackPruneList(&stream);
+    FAIL_IF(StreamTcpSackedSize(&stream) != 500);
 
-    if (StreamTcpSackedSize(&stream) != 500) {
-        printf("size should be 500, is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
-
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -883,29 +732,21 @@ end:
 
 static int StreamTcpSackTest13 (void) {
     TcpStream stream;
-    int retval = 0;
-    int i;
-
     memset(&stream, 0, sizeof(stream));
     stream.last_ack = 10000;
     stream.window = 2000;
 
-    for (i = 0; i < 10; i++) {
+    for (int i = 0; i < 10; i++) {
         StreamTcpSackInsertRange(&stream, 100+(20*i), 110+(20*i));
     }
 #ifdef DEBUG
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (StreamTcpSackedSize(&stream) != 0) {
-        printf("Sacked size is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(StreamTcpSackedSize(&stream) != 0);
 
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 /**
@@ -916,29 +757,21 @@ end:
 
 static int StreamTcpSackTest14 (void) {
     TcpStream stream;
-    int retval = 0;
-    int i;
-
     memset(&stream, 0, sizeof(stream));
     stream.last_ack = 1000;
     stream.window = 2000;
 
-    for (i = 0; i < 10; i++) {
+    for (int i = 0; i < 10; i++) {
         StreamTcpSackInsertRange(&stream, 4000+(20*i), 4010+(20*i));
     }
 #ifdef DEBUG
     StreamTcpSackPrintList(&stream);
 #endif /* DEBUG */
 
-    if (StreamTcpSackedSize(&stream) != 0) {
-        printf("Sacked size is %u: ", StreamTcpSackedSize(&stream));
-        goto end;
-    }
+    FAIL_IF(StreamTcpSackedSize(&stream) != 0);
 
-    retval = 1;
-end:
     StreamTcpSackFreeList(&stream);
-    SCReturnInt(retval);
+    PASS;
 }
 
 #endif /* UNITTESTS */

diff --git a/src/stream-tcp-sack.h b/src/stream-tcp-sack.h
index 632fa14dbb8c2479de7ccea76933f5f075bc564a..6411e2b5a46338d1d3965dad4915369cd74089f5 100644 (file)
--- a/src/stream-tcp-sack.h
+++ b/src/stream-tcp-sack.h
@@ -40,14 +40,13 @@
  */
 static inline uint32_t StreamTcpSackedSize(TcpStream *stream)
 {
-    if (likely(stream->sack_head == NULL)) {
+    if (likely(RB_EMPTY(&stream->sack_tree))) {
         SCReturnUInt(0U);
     } else {
         uint32_t size = 0;
 
         StreamTcpSackRecord *rec = NULL;
-
-        for (rec = stream->sack_head; rec != NULL; rec = rec->next) {
+        RB_FOREACH(rec, TCPSACK, &stream->sack_tree) {
             size += (rec->re - rec->le);
         }