lib: Refactor FRCT

This is a small refactor of FRCT because I found some things a bit
hard to read. I tried to refactor frcti_rcv to always queue the
packet, but that causes unnecessarily retaking the lock when calling
queued_pdu and thus returning idx is a tiny bit faster.

Signed-off-by: Dimitri Staessens <[email protected]>
Signed-off-by: Sander Vrijders <[email protected]>

2 files changed, 59 insertions, 72 deletions

diff --git a/src/lib/dev.c b/src/lib/dev.c
index e8989a48..efd08146 100644
--- a/src/lib/dev.c
+++ b/src/lib/dev.c
@@ -1102,29 +1102,25 @@ ssize_t flow_read(int    fd,
         idx = flow->part_idx;
         if (idx < 0) {
                 idx = frcti_queued_pdu(flow->frcti);
-                if (idx < 0) {
-                        do {
-                                idx = noblock ? shm_rbuff_read(rb) :
-                                        shm_rbuff_read_b(rb, abstime);
-                                if (idx < 0)
-                                        return idx;
-
-                                sdb = shm_rdrbuff_get(ai.rdrb, idx);
-                                if (flow->qs.ber == 0 && chk_crc(sdb) != 0) {
-                                        shm_rdrbuff_remove(ai.rdrb, idx);
-                                        continue;
-                                }
-
-                                pthread_rwlock_wrlock(&ai.lock);
-                                if (flow->qs.cypher_s > 0)
-                                        if (crypt_decrypt(flow, sdb) < 0) {
-                                                pthread_rwlock_unlock(&ai.lock);
-                                                shm_rdrbuff_remove(ai.rdrb,
-                                                                   idx);
-                                                return -ENOMEM;
-                                        }
-                                pthread_rwlock_unlock(&ai.lock);
-                        } while (frcti_rcv(flow->frcti, sdb) != 0);
+                while (idx < 0) {
+                        idx = noblock ? shm_rbuff_read(rb) :
+                                shm_rbuff_read_b(rb, abstime);
+                        if (idx < 0)
+                                return idx;
+
+                        sdb = shm_rdrbuff_get(ai.rdrb, idx);
+                        if (flow->qs.ber == 0 && chk_crc(sdb) != 0) {
+                                shm_rdrbuff_remove(ai.rdrb, idx);
+                                continue;
+                        }
+
+                        if (flow->qs.cypher_s > 0
+                            && crypt_decrypt(flow, sdb) < 0) {
+                                shm_rdrbuff_remove(ai.rdrb, idx);
+                                return -ENOMEM;
+                        }
+
+                        idx = frcti_rcv(flow->frcti, sdb);
                 }
         }
 
@@ -1528,22 +1524,19 @@ int ipcp_flow_read(int                   fd,
 
         pthread_rwlock_unlock(&ai.lock);
 
-        if (flow->frcti != NULL) {
-                idx = frcti_queued_pdu(flow->frcti);
-                if (idx >= 0) {
-                        *sdb = shm_rdrbuff_get(ai.rdrb, idx);
-                        return 0;
-                }
-        }
-
-        do {
+        idx = frcti_queued_pdu(flow->frcti);
+        while (idx < 0) {
                 idx = shm_rbuff_read(rb);
                 if (idx < 0)
                         return idx;
                 *sdb = shm_rdrbuff_get(ai.rdrb, idx);
                 if (flow->qs.ber == 0 && chk_crc(*sdb) != 0)
                         continue;
-        } while (frcti_rcv(flow->frcti, *sdb) != 0);
+
+                idx = frcti_rcv(flow->frcti, *sdb);
+        }
+
+        *sdb = shm_rdrbuff_get(ai.rdrb, idx);
 
         return 0;
 }
diff --git a/src/lib/frct.c b/src/lib/frct.c
index 3c180128..2322a039 100644
--- a/src/lib/frct.c
+++ b/src/lib/frct.c
@@ -68,7 +68,7 @@ enum frct_flags {
         FRCT_DRF  = 0x02, /* Data run flag    */
         FRCT_ACK  = 0x04, /* ACK field valid  */
         FRCT_FC   = 0x08, /* FC window valid  */
-        FRCT_RDVZ = 0x10, /* Rendez-vous      */
+        FRCT_RDVS = 0x10, /* Rendez-vous      */
         FRCT_FFGM = 0x20, /* First Fragment   */
         FRCT_MFGM = 0x40, /* More fragments   */
 };
@@ -181,8 +181,8 @@ static uint16_t frcti_getconf(struct frcti * frcti)
 #define frcti_snd(frcti, sdb) \
         (frcti == NULL ? 0 : __frcti_snd(frcti, sdb))
 
-#define frcti_rcv(frcti, sdb) \
-        (frcti == NULL ? 0 : __frcti_rcv(frcti, sdb))
+#define frcti_rcv(frcti, sdb)                           \
+        (frcti == NULL ? idx : __frcti_rcv(frcti, sdb))
 
 static ssize_t __frcti_queued_pdu(struct frcti * frcti)
 {
@@ -195,6 +195,7 @@ static ssize_t __frcti_queued_pdu(struct frcti * frcti)
         pthread_rwlock_wrlock(&frcti->lock);
 
         pos = frcti->rcv_cr.lwe & (RQ_SIZE - 1);
+
         idx = frcti->rq[pos];
         if (idx != -1) {
                 ++frcti->rcv_cr.lwe;
@@ -319,9 +320,9 @@ static void rtt_estimator(struct frcti * frcti,
         frcti->rto         = MAX(RTO_MIN, srtt + (rttvar >> 2));
 }
 
-/* Returns 0 when idx contains a packet for the application. */
-static int __frcti_rcv(struct frcti *       frcti,
-                       struct shm_du_buff * sdb)
+/* Always queues the packet on the RQ for the application. */
+static ssize_t __frcti_rcv(struct frcti *       frcti,
+                           struct shm_du_buff * sdb)
 {
         ssize_t           idx;
         struct frct_pci * pci;
@@ -329,39 +330,48 @@ static int __frcti_rcv(struct frcti *       frcti,
         struct frct_cr *  snd_cr;
         struct frct_cr *  rcv_cr;
         uint32_t          seqno;
-        int               ret = 0;
 
         assert(frcti);
 
         rcv_cr = &frcti->rcv_cr;
         snd_cr = &frcti->snd_cr;
 
-        pci = (struct frct_pci *) shm_du_buff_head_release(sdb, FRCT_PCILEN);
-
         clock_gettime(CLOCK_REALTIME, &now);
 
-        pthread_rwlock_wrlock(&frcti->lock);
+        pci = (struct frct_pci *) shm_du_buff_head_release(sdb, FRCT_PCILEN);
 
         idx = shm_du_buff_get_idx(sdb);
-
         seqno = ntoh32(pci->seqno);
 
-        /* Check if receiver inactivity is true. */
+        pthread_rwlock_wrlock(&frcti->lock);
+
         if (now.tv_sec - rcv_cr->act > rcv_cr->inact) {
-                /* Inactive receiver, check for DRF. */
                 if (pci->flags & FRCT_DRF) /* New run. */
                         rcv_cr->lwe = seqno;
                 else
                         goto drop_packet;
         }
 
-        if (seqno == rcv_cr->lwe) {
-                ++rcv_cr->lwe;
-        } else { /* Out of order. */
-                if (before(seqno, rcv_cr->lwe) )
-                        goto drop_packet;
+        if (before(seqno, rcv_cr->lwe))
+                goto drop_packet;
+
+        if (rcv_cr->cflags & FRCTFRTX) {
+                if (pci->flags & FRCT_ACK) {
+                        uint32_t ackno = ntoh32(pci->ackno);
+                        /* Check for duplicate (old) acks. */
+                        if (after(ackno, snd_cr->lwe))
+                                snd_cr->lwe = ackno;
+
+                        if (frcti->probe && after(ackno, frcti->rttseq)) {
+                                rtt_estimator(frcti, ts_diff_us(&frcti->t_probe,
+                                                                &now));
+                                frcti->probe = false;
+                        }
+                }
 
-                if (rcv_cr->cflags & FRCTFRTX) {
+                if (seqno == rcv_cr->lwe) {
+                        ++frcti->rcv_cr.lwe;
+                } else {
                         size_t pos = seqno & (RQ_SIZE - 1);
                         if ((seqno - rcv_cr->lwe) >= RQ_SIZE)
                                 goto drop_packet; /* Out of rq. */
@@ -369,37 +379,21 @@ static int __frcti_rcv(struct frcti *       frcti,
                         if (frcti->rq[pos] != -1)
                                 goto drop_packet; /* Duplicate in rq */
 
-                        /* Queue. */
                         frcti->rq[pos] = idx;
-                        ret = -EAGAIN;
-                } else {
-                        rcv_cr->lwe = seqno + 1;
-                }
-        }
-
-        if (rcv_cr->cflags & FRCTFRTX && pci->flags & FRCT_ACK) {
-                uint32_t ackno = ntoh32(pci->ackno);
-                /* Check for duplicate (old) acks. */
-                if ((int32_t)(ackno - snd_cr->lwe) > 0)
-                        snd_cr->lwe = ackno;
-
-                if (frcti->probe && after(ackno, frcti->rttseq)) {
-                        rtt_estimator(frcti, ts_diff_us(&frcti->t_probe, &now));
-                        frcti->probe = false;
+                        idx = -EAGAIN;
                 }
+        } else {
+                rcv_cr->lwe = seqno + 1;
         }
 
         rcv_cr->act = now.tv_sec;
 
         pthread_rwlock_unlock(&frcti->lock);
 
-        if (ret == 0 && !(pci->flags & FRCT_DATA))
-                shm_rdrbuff_remove(ai.rdrb, idx);
-
         if (frcti->rw != NULL)
                 rxmwheel_move(frcti->rw);
 
-        return ret;
+        return idx;
 
  drop_packet:
         pthread_rwlock_unlock(&frcti->lock);