ipcpd: Revise PCI manipulation in normal

The pci function will store offsets to avoid having to recalculate
them every time. TTL handling is removed from the fmgr, the TTL is
automatically decreased when deserializing the PCI (or set to 1 if
there is no TTL present so the PDU will be forwarded). The
deserialisation function now takes a pointer to a struct pci as input
to avoid memory allocation and release each time a PDU is
processed. Some checks have been replaced with assert() and return
values replaced with void where it makes sense.

3 files changed, 92 insertions, 134 deletions

diff --git a/src/ipcpd/normal/fmgr.c b/src/ipcpd/normal/fmgr.c
index 6b21cdbf..b79d20b4 100644
--- a/src/ipcpd/normal/fmgr.c
+++ b/src/ipcpd/normal/fmgr.c
@@ -129,13 +129,15 @@ void * fmgr_nm1_sdu_reader(void * o)
 {
         struct timespec      timeout = {0, FD_UPDATE_TIMEOUT};
         struct shm_du_buff * sdb;
-        struct pci *         pci;
+        struct pci           pci;
         int                  fd;
         int                  i = 0;
         int                  ret;
 
         (void) o;
 
+        memset(&pci, 0, sizeof(pci));
+
         while (true) {
                 /* FIXME: replace with scheduling policy call */
                 i = (i + 1) % QOS_CUBE_MAX;
@@ -157,30 +159,15 @@ void * fmgr_nm1_sdu_reader(void * o)
                                 continue;
                         }
 
-                        pci = shm_pci_des(sdb);
-                        if (pci == NULL) {
-                                LOG_ERR("Failed to get PCI.");
-                                ipcp_flow_del(sdb);
-                                continue;
-                        }
+                        shm_pci_des(sdb, &pci);
 
-                        if (pci->dst_addr != ipcpi.address) {
+                        if (pci.dst_addr != ipcpi.address) {
                                 LOG_DBG("PDU needs to be forwarded.");
 
-                                if (pci->has_ttl) {
-                                        if (pci->ttl == 0) {
-                                                LOG_DBG("TTL was zero.");
-                                                ipcp_flow_del(sdb);
-                                                free(pci);
-                                                continue;
-                                        }
-
-                                        if (shm_pci_dec_ttl(sdb)) {
-                                                LOG_ERR("Failed to dec TTL.");
-                                                ipcp_flow_del(sdb);
-                                                free(pci);
-                                                continue;
-                                        }
+                                if (pci.ttl == 0) {
+                                        LOG_DBG("TTL was zero.");
+                                        ipcp_flow_del(sdb);
+                                        continue;
                                 }
 
                                 /*
@@ -188,21 +175,14 @@ void * fmgr_nm1_sdu_reader(void * o)
                                  * we don't have a PFF yet
                                  */
                                 ipcp_flow_del(sdb);
-                                free(pci);
                                 continue;
                         }
 
-                        if (shm_pci_shrink(sdb)) {
-                                LOG_ERR("Failed to shrink PDU.");
-                                ipcp_flow_del(sdb);
-                                free(pci);
-                                continue;
-                        }
+                        shm_pci_shrink(sdb);
 
-                        if (frct_nm1_post_sdu(pci, sdb)) {
+                        if (frct_nm1_post_sdu(&pci, sdb)) {
                                 LOG_ERR("Failed to hand PDU to FRCT.");
                                 ipcp_flow_del(sdb);
-                                free(pci);
                                 continue;
                         }
                 }
diff --git a/src/ipcpd/normal/shm_pci.c b/src/ipcpd/normal/shm_pci.c
index 735bdee1..1c2cee54 100644
--- a/src/ipcpd/normal/shm_pci.c
+++ b/src/ipcpd/normal/shm_pci.c
@@ -20,10 +20,7 @@
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
 
-#define OUROBOROS_PREFIX "shm-pci"
-
 #include <ouroboros/config.h>
-#include <ouroboros/logs.h>
 #include <ouroboros/errno.h>
 #include <ouroboros/crc32.h>
 #include <ouroboros/rib.h>
@@ -33,6 +30,7 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 
 #define PDU_TYPE_SIZE 1
 #define QOS_ID_SIZE 1
@@ -46,33 +44,44 @@ struct {
         struct dt_const dtc;
         size_t head_size;
         size_t tail_size;
+
+        /* offsets */
+        size_t dst_addr_o;
+        size_t src_addr_o;
+        size_t dst_cep_id_o;
+        size_t src_cep_id_o;
+        size_t pdu_length_o;
+        size_t seqno_o;
+        size_t qos_id_o;
+        size_t ttl_o;
 } pci_info;
 
 
 static void ser_pci_head(uint8_t *    head,
                          struct pci * pci)
 {
-        int offset = 0;
         uint8_t ttl = DEFAULT_TTL;
 
+        assert(head);
+        assert(pci);
+
+        /* FIXME: Add check and operations for Big Endian machines */
         memcpy(head, &pci->pdu_type, PDU_TYPE_SIZE);
-        offset += PDU_TYPE_SIZE;
-        memcpy(head + offset, &pci->dst_addr, pci_info.dtc.addr_size);
-        offset += pci_info.dtc.addr_size;
-        memcpy(head + offset, &pci->src_addr, pci_info.dtc.addr_size);
-        offset += pci_info.dtc.addr_size;
-        memcpy(head + offset, &pci->dst_cep_id, pci_info.dtc.cep_id_size);
-        offset += pci_info.dtc.cep_id_size;
-        memcpy(head + offset, &pci->src_cep_id, pci_info.dtc.cep_id_size);
-        offset += pci_info.dtc.cep_id_size;
-        memcpy(head + offset, &pci->pdu_length, pci_info.dtc.pdu_length_size);
-        offset += pci_info.dtc.pdu_length_size;
-        memcpy(head + offset, &pci->seqno, pci_info.dtc.seqno_size);
-        offset += pci_info.dtc.seqno_size;
-        memcpy(head + offset, &pci->qos_id, QOS_ID_SIZE);
-        offset += QOS_ID_SIZE;
+        memcpy(head + pci_info.dst_addr_o, &pci->dst_addr,
+               pci_info.dtc.addr_size);
+        memcpy(head + pci_info.src_addr_o, &pci->src_addr,
+               pci_info.dtc.addr_size);
+        memcpy(head + pci_info.dst_cep_id_o, &pci->dst_cep_id,
+               pci_info.dtc.cep_id_size);
+        memcpy(head + pci_info.src_cep_id_o, &pci->src_cep_id,
+               pci_info.dtc.cep_id_size);
+        memcpy(head + pci_info.pdu_length_o, &pci->pdu_length,
+               pci_info.dtc.pdu_length_size);
+        memcpy(head + pci_info.seqno_o, &pci->seqno,
+               pci_info.dtc.seqno_size);
+        memcpy(head + pci_info.qos_id_o, &pci->qos_id, QOS_ID_SIZE);
         if (pci_info.dtc.has_ttl)
-                memcpy(head + offset, &ttl, TTL_SIZE);
+                memcpy(head + pci_info.ttl_o, &ttl, TTL_SIZE);
 }
 
 int shm_pci_init(void)
@@ -101,9 +110,19 @@ int shm_pci_init(void)
                       sizeof(pci_info.dtc.max_pdu_size)))
                 return -1;
 
-        pci_info.head_size = PDU_TYPE_SIZE + pci_info.dtc.addr_size * 2 +
-                pci_info.dtc.cep_id_size * 2 + pci_info.dtc.pdu_length_size +
-                pci_info.dtc.seqno_size + QOS_ID_SIZE;
+        pci_info.dst_addr_o = PDU_TYPE_SIZE;
+        pci_info.src_addr_o = pci_info.dst_addr_o + pci_info.dtc.addr_size;
+        pci_info.dst_cep_id_o = pci_info.dst_addr_o + pci_info.dtc.addr_size;
+        pci_info.dst_cep_id_o = pci_info.src_addr_o + pci_info.dtc.addr_size;
+        pci_info.src_cep_id_o = pci_info.dst_cep_id_o
+                + pci_info.dtc.cep_id_size;
+        pci_info.pdu_length_o = pci_info.src_cep_id_o
+                + pci_info.dtc.cep_id_size;
+        pci_info.seqno_o = pci_info.pdu_length_o + pci_info.dtc.pdu_length_size;
+        pci_info.qos_id_o = pci_info.seqno_o + pci_info.dtc.seqno_size;
+        pci_info.ttl_o = pci_info.qos_id_o + QOS_ID_SIZE;
+
+        pci_info.head_size = pci_info.ttl_o;
 
         if (pci_info.dtc.has_ttl)
                 pci_info.head_size += TTL_SIZE;
@@ -118,14 +137,17 @@ void shm_pci_fini(void) {
 }
 
 int shm_pci_ser(struct shm_du_buff * sdb,
-                struct pci * pci)
+                struct pci *         pci)
 {
         uint8_t * head;
         uint8_t * tail;
 
+        assert(sdb);
+        assert(pci);
+
         head = shm_du_buff_head_alloc(sdb, pci_info.head_size);
         if (head == NULL)
-                return -1;
+                return -EPERM;
 
         ser_pci_head(head, pci);
 
@@ -133,7 +155,7 @@ int shm_pci_ser(struct shm_du_buff * sdb,
                 tail = shm_du_buff_tail_alloc(sdb, pci_info.tail_size);
                 if (tail == NULL) {
                         shm_du_buff_head_release(sdb, pci_info.head_size);
-                        return -1;
+                        return -EPERM;
                 }
 
                 crc32((uint32_t *) tail, head, tail - head);
@@ -147,8 +169,8 @@ buffer_t * shm_pci_ser_buf(buffer_t *   buf,
 {
         buffer_t * buffer;
 
-        if (buf == NULL || pci == NULL)
-                return NULL;
+        assert(buf);
+        assert(pci);
 
         buffer = malloc(sizeof(*buffer));
         if (buffer == NULL)
@@ -178,85 +200,44 @@ buffer_t * shm_pci_ser_buf(buffer_t *   buf,
         return buffer;
 }
 
-struct pci * shm_pci_des(struct shm_du_buff * sdb)
+void shm_pci_des(struct shm_du_buff * sdb,
+                 struct pci *         pci)
 {
         uint8_t * head;
-        struct pci * pci;
-        int offset = 0;
 
-        if (sdb == NULL)
-                return NULL;
+        assert(sdb);
+        assert(pci);
 
         head = shm_du_buff_head(sdb);
 
-        pci = malloc(sizeof(*pci));
-        if (pci == NULL)
-                return NULL;
-
+        /* FIXME: Add check and operations for Big Endian machines */
         memcpy(&pci->pdu_type, head, PDU_TYPE_SIZE);
-        offset += PDU_TYPE_SIZE;
-        memcpy(&pci->dst_addr, head + offset, pci_info.dtc.addr_size);
-        offset += pci_info.dtc.addr_size;
-        memcpy(&pci->src_addr, head + offset, pci_info.dtc.addr_size);
-        offset += pci_info.dtc.addr_size;
-        memcpy(&pci->dst_cep_id, head + offset, pci_info.dtc.cep_id_size);
-        offset += pci_info.dtc.cep_id_size;
-        memcpy(&pci->src_cep_id, head + offset, pci_info.dtc.cep_id_size);
-        offset += pci_info.dtc.cep_id_size;
-        memcpy(&pci->pdu_length, head + offset, pci_info.dtc.pdu_length_size);
-        offset += pci_info.dtc.pdu_length_size;
-        memcpy(&pci->seqno, head + offset, pci_info.dtc.seqno_size);
-        offset += pci_info.dtc.seqno_size;
-        memcpy(&pci->qos_id, head + offset, QOS_ID_SIZE);
-        offset += QOS_ID_SIZE;
-        pci->has_ttl = pci_info.dtc.has_ttl;
-        if (pci_info.dtc.has_ttl)
-                memcpy(&pci->ttl, head + offset, TTL_SIZE);
-
-        return pci;
-}
-
-int shm_pci_shrink(struct shm_du_buff * sdb)
-{
-        if (sdb == NULL)
-                return -1;
-
-        if (shm_du_buff_head_release(sdb, pci_info.head_size)) {
-                LOG_ERR("Failed to shrink head.");
-                return -1;
-        }
-
-        if (shm_du_buff_tail_release(sdb, pci_info.tail_size)) {
-                LOG_ERR("Failed to shrink tail.");
-                return -1;
+        memcpy(&pci->dst_addr, head + pci_info.dst_addr_o,
+               pci_info.dtc.addr_size);
+        memcpy(&pci->src_addr, head + pci_info.src_addr_o,
+               pci_info.dtc.addr_size);
+        memcpy(&pci->dst_cep_id, head + pci_info.dst_cep_id_o,
+               pci_info.dtc.cep_id_size);
+        memcpy(&pci->src_cep_id, head + pci_info.src_cep_id_o,
+               pci_info.dtc.cep_id_size);
+        memcpy(&pci->pdu_length, head + pci_info.pdu_length_o,
+               pci_info.dtc.pdu_length_size);
+        memcpy(&pci->seqno, head + pci_info.seqno_o,
+               pci_info.dtc.seqno_size);
+        memcpy(&pci->qos_id, head + pci_info.qos_id_o, QOS_ID_SIZE);
+
+        if (pci_info.dtc.has_ttl) {
+                --*(head + pci_info.ttl_o); /* decrease TTL */
+                memcpy(&pci->ttl, head + pci_info.ttl_o, TTL_SIZE);
+        } else {
+                pci->ttl = 1;
         }
-
-        return 0;
 }
 
-int shm_pci_dec_ttl(struct shm_du_buff * sdb)
+void shm_pci_shrink(struct shm_du_buff * sdb)
 {
-        uint8_t * head;
-        uint8_t * tail;
-
-        if (pci_info.dtc.has_ttl == false)
-                return 0;
+        assert(sdb);
 
-        head = shm_du_buff_head(sdb);
-        if (head == NULL)
-                return -1;
-
-        head[pci_info.head_size - TTL_SIZE]--;
-
-        if (pci_info.dtc.has_chk) {
-                tail = shm_du_buff_tail(sdb);
-                if (tail == NULL)
-                        return -1;
-
-                tail -= CHK_SIZE;
-
-                crc32((uint32_t *) tail, head, tail - head);
-        }
-
-        return 0;
+        shm_du_buff_head_release(sdb, pci_info.head_size);
+        shm_du_buff_tail_release(sdb, pci_info.tail_size);
 }
diff --git a/src/ipcpd/normal/shm_pci.h b/src/ipcpd/normal/shm_pci.h
index 9fe3d9aa..17ce5cdd 100644
--- a/src/ipcpd/normal/shm_pci.h
+++ b/src/ipcpd/normal/shm_pci.h
@@ -44,8 +44,6 @@ struct pci {
         uint32_t pdu_length;
         uint64_t seqno;
         uint8_t  ttl;
-        /* FIXME: Deprecate this and the dec_ttl call */
-        int      has_ttl;
 };
 
 int          shm_pci_init(void);
@@ -58,10 +56,9 @@ int          shm_pci_ser(struct shm_du_buff * sdb,
 buffer_t *   shm_pci_ser_buf(buffer_t *   buf,
                              struct pci * pci);
 
-struct pci * shm_pci_des(struct shm_du_buff * sdb);
-
-int          shm_pci_shrink(struct shm_du_buff * sdb);
+void         shm_pci_des(struct shm_du_buff * sdb,
+                         struct pci *         pci);
 
-int          shm_pci_dec_ttl(struct shm_du_buff * sdb);
+void         shm_pci_shrink(struct shm_du_buff * sdb);
 
 #endif /* OUROBOROS_IPCPD_NORMAL_SHM_PCI_H */