p4-projects@freebsd.org
[Top] [All Lists]

PERFORCE change 133444 for review

Subject: PERFORCE change 133444 for review
From: Andre Oppermann
Date: Thu, 17 Jan 2008 00:57:29 GMT
http://perforce.freebsd.org/chv.cgi?CH=133444

Change 133444 by andre@andre_flirtbox on 2008/01/17 00:56:27

        Fix KASSERTs to match inverse test logic.
        
        Remove bogus KASSERT.
        
        TAILQ_FOREACH_SAFE doesn't initialize the temp variable on the first
        loop, do it manually.  The code depends on it being tracked correctly.
        Maybe this should be fixed in the macro itself.
        
        Tighten segment to block match tests.

Affected files ...

.. //depot/projects/tcp_reass/netinet/tcp_reass.c#8 edit

Differences ...

==== //depot/projects/tcp_reass/netinet/tcp_reass.c#8 (text+ko) ====

@@ -163,11 +163,22 @@
 
        KASSERT(*tlenp > 0,
            ("%s: segment doesn't contain any data", __func__));
-       KASSERT(SEQ_GT(tp->rcv_nxt, th->th_seq),
+       KASSERT(SEQ_LEQ(tp->rcv_nxt, th->th_seq),
            ("%s: sequence number below rcv_nxt", __func__));
        KASSERT(!(tp->rcv_nxt == th->th_seq) || !(TAILQ_EMPTY(&tp->t_trq)),
            ("%s: got missing segment but queue is empty", __func__));
 
+#ifdef INVARIANTS
+       TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) {
+               tqen = TAILQ_NEXT(tqe, trq_q);
+               KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt),
+                   ("%s: trq_seq < rcv_nxt", __func__));
+               KASSERT(tqen == NULL ||
+                   SEQ_LT(tqe->trq_seq + tqe->trq_len, tqen->trq_seq),
+                   ("%s: overlapping blocks", __func__));
+       }
+#endif
+
        /*
         * Limit the number of segments in the reassembly queue to prevent
         * holding on to too many segments (and thus running out of mbufs).
@@ -236,7 +247,7 @@
        /* Check if this is the missing segment. */
        if (tp->rcv_nxt == th->th_seq) {
                tqe = TAILQ_FIRST(&tp->t_trq);
-               KASSERT(SEQ_LEQ(tqe->trq_seq, th->th_seq),
+               KASSERT(SEQ_GT(tqe->trq_seq, th->th_seq),
                    ("%s: first block starts below missing segment", __func__));
                /* Check if segment prepends first block. */
                if (SEQ_LEQ(tqe->trq_seq, th->th_seq + *tlenp)) {
@@ -269,19 +280,15 @@
        tcpstat.tcps_rcvoobyte += *tlenp;
 
        /* See where it fits. */
-       TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
-#if 1
+       TAILQ_FOREACH(tqe, &tp->t_trq, trq_q) {
+               tqen = TAILQ_NEXT(tqe, trq_q);
                /* Segment is after this blocks coverage. */
                if (SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq))
                        continue;
-#endif
                /* Segment is after the previous one but before this one. */
                if (SEQ_GT(tqe->trq_seq, th->th_seq + *tlenp))
                        break;          /* Insert as new block. */
 
-               KASSERT(SEQ_GT(tqe->trq_seq, th->th_seq + *tlenp),
-                   ("%s: iterated past insert point", __func__));
-
                /* Segment is already fully covered. */
                if (SEQ_LEQ(tqe->trq_seq, th->th_seq) &&
                    SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) {
@@ -293,7 +300,7 @@
                }
 
                /* Segment covers and extends on both ends. */
-               if (SEQ_GEQ(tqe->trq_seq, th->th_seq) &&
+               if (SEQ_GT(tqe->trq_seq, th->th_seq) &&
                    SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) {
                        /* Replace block content. */
                        tp->t_trqmcnt -= tqe->trq_mcnt;
@@ -313,6 +320,7 @@
 
                /* Segment prepends to this block. */
                if (SEQ_GT(tqe->trq_seq, th->th_seq) &&
+                   SEQ_LEQ(tqe->trq_seq, th->th_seq + *tlenp) &&
                    SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) {
                        /* Trim tail of segment. */
                        if ((i = SEQ_DELTA(tqe->trq_seq, th->th_seq + *tlenp))) 
{
@@ -337,8 +345,9 @@
                }
 
                /* Segment appends to this block. */
-               if (SEQ_LEQ(tqe->trq_seq + tqe->trq_len, th->th_seq) &&
-                   SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp)) {
+               if (SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq + *tlenp) &&
+                   SEQ_LEQ(tqe->trq_seq, th->th_seq) &&
+                   SEQ_GEQ(tqe->trq_seq + tqe->trq_len, th->th_seq)) {
                        /* Trim head of segment. */
                        if ((i = SEQ_DELTA(tqe->trq_seq + tqe->trq_len, 
th->th_seq))) {
                                m_adj(m, i);
@@ -379,7 +388,7 @@
 
        /* Where to insert. */
        if (tqe != NULL && SEQ_LT(tqe->trq_seq + tqe->trq_len, th->th_seq))
-               TAILQ_INSERT_TAIL(&tp->t_trq, tqen, trq_q);
+               TAILQ_INSERT_AFTER(&tp->t_trq, tqe, tqen, trq_q);
        else if (tqe != NULL)
                TAILQ_INSERT_BEFORE(tqe, tqen, trq_q);
        else {
@@ -396,10 +405,16 @@
         * Present data to user, advancing rcv_nxt through
         * completed sequence space.
         */
+       KASSERT(!TAILQ_EMPTY(&tp->t_trq),
+           ("%s: queue empty at present", __func__));
        SOCKBUF_LOCK(&so->so_rcv);
+       tqen = TAILQ_NEXT(TAILQ_FIRST(&tp->t_trq), trq_q);
        TAILQ_FOREACH_SAFE(tqe, &tp->t_trq, trq_q, tqen) {
                KASSERT(SEQ_GEQ(tqe->trq_seq, tp->rcv_nxt),
                    ("%s: trq_seq < rcv_nxt", __func__));
+               KASSERT(tqen == NULL ||
+                   SEQ_LEQ(tqe->trq_seq + tqe->trq_len, tqen->trq_seq),
+                   ("%s: block overlaps into next one", __func__));
                if (tqe->trq_seq != tp->rcv_nxt)
                        break;
 #if 1
_______________________________________________
p4-projects@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/p4-projects
To unsubscribe, send any mail to "p4-projects-unsubscribe@xxxxxxxxxxx"

<Prev in Thread] Current Thread [Next in Thread>
  • PERFORCE change 133444 for review, Andre Oppermann <=