refactor(hicn-light): cleanup towards optimizations to UDP socket face

Change-Id: If5bc3c4dfcdde386ac02674f3c03d32d495b2231
Ticket: HICN-771
Signed-off-by: Jordan Augé <jordan.auge+fdio@cisco.com>

10 files changed, 78 insertions, 50 deletions

diff --git a/hicn-light/src/hicn/base/loop.c b/hicn-light/src/hicn/base/loop.c
index 68970ed3a..3407bfc1c 100644
--- a/hicn-light/src/hicn/base/loop.c
+++ b/hicn-light/src/hicn/base/loop.c
@@ -40,6 +40,7 @@ loop_t *MAIN_LOOP = NULL;
 typedef struct {
   void *owner;
   fd_callback_t callback;
+  unsigned id;
   void *data;
 } cb_wrapper_args_t;
 
@@ -111,16 +112,19 @@ int loop_undispatch(loop_t *loop) { return 0; }
 
 void cb_wrapper(evutil_socket_t fd, short what, void *arg) {
   cb_wrapper_args_t *cb_wrapper_args = arg;
-  cb_wrapper_args->callback(cb_wrapper_args->owner, fd, cb_wrapper_args->data);
+  cb_wrapper_args->callback(cb_wrapper_args->owner, fd, cb_wrapper_args->id,
+                            cb_wrapper_args->data);
 }
 
 static inline void _event_create(event_t **event, loop_t *loop,
                                  event_type_t type, void *callback_owner,
-                                 fd_callback_t callback, void *callback_data) {
+                                 fd_callback_t callback, unsigned id,
+                                 void *callback_data) {
   *event = malloc(sizeof(event_t));
   (*event)->callback = (cb_wrapper_args_t){
       .owner = callback_owner,
       .callback = callback,
+      .id = id,
       .data = callback_data,
   };
   (*event)->event_type = type;
@@ -129,8 +133,8 @@ static inline void _event_create(event_t **event, loop_t *loop,
 
 int loop_fd_event_create(event_t **event, loop_t *loop, int fd,
                          void *callback_owner, fd_callback_t callback,
-                         void *callback_data) {
-  _event_create(event, loop, EVTYPE_FD, callback_owner, callback,
+                         unsigned id, void *callback_data) {
+  _event_create(event, loop, EVTYPE_FD, callback_owner, callback, id,
                 callback_data);
 
   evutil_make_socket_nonblocking(fd);
@@ -161,7 +165,7 @@ int loop_event_unregister(event_t *event) {
 
 int loop_timer_create(event_t **timer, loop_t *loop, void *callback_owner,
                       fd_callback_t callback, void *callback_data) {
-  _event_create(timer, loop, EVTYPE_TIMER, callback_owner, callback,
+  _event_create(timer, loop, EVTYPE_TIMER, callback_owner, callback, 0,
                 callback_data);
 
   evtimer_assign(&(*timer)->raw_event, loop->event_base, cb_wrapper,
diff --git a/hicn-light/src/hicn/base/loop.h b/hicn-light/src/hicn/base/loop.h
index f81af333b..0eecb60fe 100644
--- a/hicn-light/src/hicn/base/loop.h
+++ b/hicn-light/src/hicn/base/loop.h
@@ -23,12 +23,13 @@
 
 /* fd & timer callbacks */
 
-typedef int (*fd_callback_t)(void *owner, int fd, void *data);
+typedef int (*fd_callback_t)(void *owner, int fd, unsigned id, void *data);
 
 typedef struct {
   int fd;
   void *owner;
   fd_callback_t callback;
+  unsigned id;
   void *data;
 } fd_callback_data_t;
 
@@ -81,12 +82,14 @@ int loop_undispatch(loop_t *loop);
  * \param [in] callback_owner - Pointer to the owner of the callack (first
  *      parameter of callback function)
  * \param [in] callback - Callback function
+ * \param [in] id - User parameter to pass alongside callback invocation,
+ *      allowing to pass an id without allocating memory.
  * \param [in] callback_data - User data to pass alongside callback invocation
  * \return 0 in case of success, -1 otherwise
  */
 int loop_fd_event_create(event_t **event, loop_t *loop, int fd,
                          void *callback_owner, fd_callback_t callback,
-                         void *callback_data);
+                         unsigned id, void *callback_data);
 
 /**
  * Register event in corresponding event loop.
diff --git a/hicn-light/src/hicn/core/connection.c b/hicn-light/src/hicn/core/connection.c
index 40802368f..a9d632d52 100644
--- a/hicn-light/src/hicn/core/connection.c
+++ b/hicn-light/src/hicn/core/connection.c
@@ -207,7 +207,8 @@ int connection_initialize(connection_t *connection, face_type_t type,
      * the listener.
      */
     loop_fd_event_create(&connection->event_data, MAIN_LOOP, fd, listener,
-                         (fd_callback_t)listener_read_callback, NULL);
+                         (fd_callback_t)listener_read_callback, connection->id,
+                         NULL);
 
     if (!connection->event_data) {
       goto ERR_REGISTER_FD;
diff --git a/hicn-light/src/hicn/core/forwarder.c b/hicn-light/src/hicn/core/forwarder.c
index face35fd6..d0472b520 100644
--- a/hicn-light/src/hicn/core/forwarder.c
+++ b/hicn-light/src/hicn/core/forwarder.c
@@ -1460,43 +1460,38 @@ ssize_t forwarder_receive(forwarder_t *forwarder, listener_t *listener,
   assert(msgbuf_id_is_valid(msgbuf_id));
   assert(pair);
 
+  hicn_name_t name;
   msgbuf_pool_t *msgbuf_pool = forwarder_get_msgbuf_pool(forwarder);
   msgbuf_t *msgbuf = msgbuf_pool_at(msgbuf_pool, msgbuf_id);
   assert(msgbuf);
 
   size_t size = msgbuf_get_len(msgbuf);
 
-  /* Connection lookup */
   const connection_table_t *table =
       forwarder_get_connection_table(listener->forwarder);
-  connection_t *connection = connection_table_get_by_pair(table, pair);
-  unsigned conn_id = connection ? (unsigned)connection_table_get_connection_id(
-                                      table, connection)
-                                : CONNECTION_ID_UNDEFINED;
 
-  assert((conn_id != CONNECTION_ID_UNDEFINED) || listener);
-
-#if 0
-  /*
-   * We have a msgbuf with payload and size, we nee to populate other
-   * information, including packet type etc.
-   */
-  msgbuf_type_t type = get_type_from_packet(msgbuf_get_packet(msgbuf));
+  /* Connection lookup */
+  if (msgbuf_get_connection_id(msgbuf) == CONNECTION_ID_UNDEFINED) {
+    connection_t *connection = connection_table_get_by_pair(table, pair);
+    unsigned conn_id =
+        connection
+            ? (unsigned)connection_table_get_connection_id(table, connection)
+            : CONNECTION_ID_UNDEFINED;
+
+    assert((conn_id != CONNECTION_ID_UNDEFINED) || listener);
+    msgbuf->connection_id = conn_id;
+  }
 
   forwarder->stats.countReceived++;
-  msgbuf->type = type;
-#endif
-  forwarder->stats.countReceived++;
 
   /* Initialize packet buffer stored in msgbuf through libhicn */
   msgbuf_initialize_from_packet(msgbuf);
+
+  /* Detect packet type */
   hicn_packet_analyze(msgbuf_get_pkbuf(msgbuf));
 
-  msgbuf->connection_id = conn_id;
   msgbuf->recv_ts = now;
 
-  hicn_name_t name;
-
 RETRY:
 
   switch (msgbuf_get_type(msgbuf)) {
@@ -1516,7 +1511,6 @@ RETRY:
           goto DROP;
         }
         msgbuf->connection_id = connection_id;
-        connection = connection_table_get_by_id(table, connection_id);
       }
       msgbuf->path_label = 0;  // not used for interest packets
       hicn_interest_get_name(msgbuf_get_pkbuf(msgbuf), &name);
@@ -1546,13 +1540,17 @@ RETRY:
       pkt_cache_log(forwarder->pkt_cache);
       break;
 
+#ifdef WITH_WLDR
     case HICN_PACKET_TYPE_WLDR_NOTIFICATION:
       if (!connection_id_is_valid(msgbuf->connection_id)) {
         ERROR("Invalid connection for WLDR packet");
         goto DROP;
       }
+      connection_t *connection =
+          connection_table_get_by_id(table, msgbuf->connection_id);
       connection_wldr_handle_notification(connection, msgbuf);
       break;
+#endif
 
     case HICN_PACKET_TYPE_MAPME:
       // XXX what about acks ?
@@ -1592,7 +1590,6 @@ RETRY:
           goto DROP;
         }
         msgbuf->connection_id = connection_id;
-        connection = connection_table_get_by_id(table, connection_id);
       }
 
       msg_header_t *msg = (msg_header_t *)msgbuf_get_packet(msgbuf);
@@ -1602,6 +1599,12 @@ RETRY:
         goto DROP;
       }
 
+      /*
+       * We need to retrieve the connection (in case it is useful) before
+       * proceeding through the removal
+       */
+      connection_t *connection =
+          connection_table_get_by_id(table, msgbuf_get_connection_id(msgbuf));
       size = command_process_msgbuf(forwarder, msgbuf);
       if (msgbuf->command.type == COMMAND_TYPE_CONNECTION_REMOVE)
         _forwarder_finalize_connection_if_self(connection, msgbuf);
diff --git a/hicn-light/src/hicn/core/listener.c b/hicn-light/src/hicn/core/listener.c
index 58ba53fe1..d2f547863 100644
--- a/hicn-light/src/hicn/core/listener.c
+++ b/hicn-light/src/hicn/core/listener.c
@@ -111,7 +111,8 @@ int listener_initialize(listener_t *listener, face_type_t type,
   // XXX data should be pre-allocated here
 
   loop_fd_event_create(&listener->event_data, MAIN_LOOP, listener->fd, listener,
-                       (fd_callback_t)listener_read_callback, NULL);
+                       (fd_callback_t)listener_read_callback,
+                       CONNECTION_ID_UNDEFINED, NULL);
 
   if (!listener->event_data) {
     goto ERR_REGISTER_FD;
@@ -267,7 +268,8 @@ int listener_punt(const listener_t *listener, const char *prefix_s) {
   return listener_vft[get_protocol(listener->type)]->punt(listener, prefix_s);
 }
 
-ssize_t listener_read_single(listener_t *listener, int fd) {
+ssize_t listener_read_single(listener_t *listener, int fd,
+                             unsigned connection_id) {
   assert(listener);
 
   msgbuf_pool_t *msgbuf_pool = forwarder_get_msgbuf_pool(listener->forwarder);
@@ -291,6 +293,7 @@ ssize_t listener_read_single(listener_t *listener, int fd) {
   }
 
   msgbuf_pool_acquire(msgbuf);
+  msgbuf_set_connection_id(msgbuf, connection_id);
 
   // Process received packet
   size_t processed_bytes = forwarder_receive(listener->forwarder, listener,
@@ -308,7 +311,8 @@ ssize_t listener_read_single(listener_t *listener, int fd) {
   return processed_bytes;
 }
 
-ssize_t listener_read_batch(listener_t *listener, int fd) {
+ssize_t listener_read_batch(listener_t *listener, int fd,
+                            unsigned connection_id) {
   assert(listener);
 
   size_t total_processed_bytes = 0;
@@ -356,6 +360,7 @@ ssize_t listener_read_batch(listener_t *listener, int fd) {
       }
 
       msgbuf_pool_acquire(msgbufs[i]);
+      msgbuf_set_connection_id(msgbufs[i], connection_id);
       forwarder_acquired_msgbuf_ids_push(forwarder, msgbuf_ids[i]);
     }
 
@@ -392,10 +397,10 @@ ssize_t listener_read_batch(listener_t *listener, int fd) {
  * This might be called for a connection on the listener too. The listener is
  * the entity that owns the buffers used for reading.
  */
-ssize_t listener_read_callback(listener_t *listener, int fd, void *user_data) {
-  // DEBUG("[listener_read_callback]");
-  // XXX make a single callback and arbitrate between read and readbatch
+ssize_t listener_read_callback(listener_t *listener, int fd,
+                               unsigned connection_id, void *user_data) {
   assert(listener);
+  assert(!user_data);
 
   /*
    * As the listener callback is shared between the listener and the different
@@ -404,9 +409,9 @@ ssize_t listener_read_callback(listener_t *listener, int fd, void *user_data) {
   // assert(fd == listener->fd);
 
   if (listener_vft[get_protocol(listener->type)]->read_batch)
-    return listener_read_batch(listener, fd);
+    return listener_read_batch(listener, fd, connection_id);
 
-  return listener_read_single(listener, fd);
+  return listener_read_single(listener, fd, connection_id);
 }
 
 void listener_setup_local(forwarder_t *forwarder, uint16_t port) {
diff --git a/hicn-light/src/hicn/core/listener.h b/hicn-light/src/hicn/core/listener.h
index 5d0384329..76d865c5d 100644
--- a/hicn-light/src/hicn/core/listener.h
+++ b/hicn-light/src/hicn/core/listener.h
@@ -115,8 +115,10 @@ void listener_setup_local(struct forwarder_s *forwarder, uint16_t port);
 void listener_process_packet(const listener_t *listener, const uint8_t *packet,
                              size_t size);
 
-ssize_t listener_read_single(listener_t *listener, int fd);
-ssize_t listener_read_batch(listener_t *listener, int fd);
+ssize_t listener_read_single(listener_t *listener, int fd,
+                             unsigned connection_id);
+ssize_t listener_read_batch(listener_t *listener, int fd,
+                            unsigned connection_id);
 
 /**
  * @brief Callback helper function for batch reading data from listener fd.
@@ -129,7 +131,8 @@ ssize_t listener_read_batch(listener_t *listener, int fd);
  * NOTE: the function returns size_t as for TCP we might need to know how much
  * data we can consume from the socket.
  */
-ssize_t listener_read_callback(listener_t *listener, int fd, void *user_data);
+ssize_t listener_read_callback(listener_t *listener, int fd,
+                               unsigned connection_id, void *user_data);
 
 #define listener_get_forwarder(listener) (listener->forwarder)
 #define listener_get_fd(listener) (listener->fd)
diff --git a/hicn-light/src/hicn/core/mapme.c b/hicn-light/src/hicn/core/mapme.c
index b151d6cb0..fede52cae 100644
--- a/hicn-light/src/hicn/core/mapme.c
+++ b/hicn-light/src/hicn/core/mapme.c
@@ -218,7 +218,7 @@ static mapme_t mapme_default = {
 
 /******************************************************************************/
 
-int mapme_on_timeout(void *mapme_arg, int fd, void *data);
+int mapme_on_timeout(void *mapme_arg, int fd, unsigned id, void *data);
 
 mapme_t *mapme_create(void *forwarder) {
   mapme_t *mapme = malloc(sizeof(mapme_t));
@@ -575,9 +575,10 @@ int mapme_create_fib_entry(const mapme_t *mapme, const Name *name,
 }
 #endif
 
-int mapme_on_timeout(void *mapme_arg, int fd, void *data) {
+int mapme_on_timeout(void *mapme_arg, int fd, unsigned id, void *data) {
   mapme_t *mapme = mapme_arg;
   assert(mapme);
+  assert(id == 0);
   assert(!data);
   /* Timeout occurred, we have to retransmit IUs for all pending
    * prefixes having entries in TFIB
diff --git a/hicn-light/src/hicn/core/policy_stats.c b/hicn-light/src/hicn/core/policy_stats.c
index 470e74a24..8706a9244 100644
--- a/hicn-light/src/hicn/core/policy_stats.c
+++ b/hicn-light/src/hicn/core/policy_stats.c
@@ -30,9 +30,11 @@
 #define ALPHA 0.9
 #define STATS_INTERVAL 1000 /* ms */
 
-static int policy_stats_mgr_tick(void* mgr_arg, int fd, void* data) {
+static int policy_stats_mgr_tick(void* mgr_arg, int fd, unsigned id,
+                                 void* data) {
   policy_stats_mgr_t* mgr = mgr_arg;
   assert(mgr);
+  assert(id == 0);
   assert(!data);
 
   uint64_t now = ticks_now();
diff --git a/hicn-light/src/hicn/test/test-loop.cc b/hicn-light/src/hicn/test/test-loop.cc
index f783b0ada..0ebf7a22a 100644
--- a/hicn-light/src/hicn/test/test-loop.cc
+++ b/hicn-light/src/hicn/test/test-loop.cc
@@ -57,20 +57,25 @@ class LoopTest : public ::testing::Test {
     // before the destructor).
   }
 
-  static int onFirstTimerExpiration(void *owner, int fd, void *arg) {
+  static int onFirstTimerExpiration(void *owner, int fd, unsigned id,
+                                    void *arg) {
+    assert(id == 0);
     std::cout << "This function should never be called" << std::endl;
     EXPECT_TRUE(false);
     return -1;
   }
 
-  static int onSecondTimerExpiration(void *owner, int fd, void *arg) {
+  static int onSecondTimerExpiration(void *owner, int fd, unsigned id,
+                                     void *arg) {
+    assert(id == 0);
     std::cout << "First timer expired. Cancel second timer." << std::endl;
     LoopTest *test = (LoopTest *)(arg);
     loop_event_unregister(test->timer_);
     return 0;
   }
 
-  static int onTimerExpiration(void *owner, int fd, void *arg) {
+  static int onTimerExpiration(void *owner, int fd, unsigned id, void *arg) {
+    assert(id == 0);
     // Create client socket
     struct sockaddr_in addr;
     int client_socket;
@@ -103,7 +108,8 @@ class LoopTest : public ::testing::Test {
     return 0;
   }
 
-  static int onNewConnection(void *owner, int fd, void *arg) {
+  static int onNewConnection(void *owner, int fd, unsigned id, void *arg) {
+    assert(id == 0);
     LoopTest *test = (LoopTest *)arg;
     struct sockaddr_in addr;
     addr.sin_addr.s_addr = INADDR_ANY;
@@ -193,7 +199,7 @@ TEST_F(LoopTest, EventCreateAndFree) {
   loop_ = loop_create();
 
   ret = loop_fd_event_create(&event_, loop_, fd, nullptr,
-                             &LoopTest::onNewConnection, this);
+                             &LoopTest::onNewConnection, 0, this);
   EXPECT_TRUE(ret >= 0);
   EXPECT_TRUE(event_);
 
@@ -257,7 +263,7 @@ TEST_F(LoopTest, LoopDispatch) {
   loop_ = loop_create();
 
   ret = loop_fd_event_create(&event_, loop_, connection_socket_, nullptr,
-                             &LoopTest::onNewConnection, this);
+                             &LoopTest::onNewConnection, 0, this);
   EXPECT_TRUE(ret >= 0);
   EXPECT_TRUE(event_);
 
diff --git a/hicn-light/src/hicn/test/test-strategy-local-remote.cc b/hicn-light/src/hicn/test/test-strategy-local-remote.cc
index 8230b9482..413ac4080 100644
--- a/hicn-light/src/hicn/test/test-strategy-local-remote.cc
+++ b/hicn-light/src/hicn/test/test-strategy-local-remote.cc
@@ -66,7 +66,7 @@ class StrategyLocalRemoteTest : public ::testing::Test {
   }
 
   strategy_entry_t entry_;
-  nexthops_t available_nexthops_;
+  nexthops_t available_nexthops_ = NEXTHOPS_EMPTY;
   configuration_t* conf_;
   forwarder_t* fwd_;
   msgbuf_t msgbuf_;