fix(libhicnctrl): new create/face&route command fail

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

9 files changed, 169 insertions, 28 deletions

diff --git a/ctrl/libhicnctrl/src/commands/command_face.c b/ctrl/libhicnctrl/src/commands/command_face.c
index 2ea0f5cc3..f60bef18d 100644
--- a/ctrl/libhicnctrl/src/commands/command_face.c
+++ b/ctrl/libhicnctrl/src/commands/command_face.c
@@ -43,11 +43,11 @@
     .offset = offsetof(hc_face_t, remote_port),    \
   }
 
-#define interface                                                           \
-  {                                                                         \
-    .name = "interface", .help = "Interface on which to bind",              \
-    .type = TYPE_INTERFACE_NAME,                                            \
-    .offset = offsetof(hc_face_t, netdevice) + offsetof(netdevice_t, name), \
+#define interface                                              \
+  {                                                            \
+    .name = "interface", .help = "Interface on which to bind", \
+    .type = TYPE_INTERFACE_NAME,                               \
+    .offset = offsetof(hc_face_t, netdevice.name),             \
   }
 
 #define symbolic_or_id                                                \
@@ -64,6 +64,7 @@ int on_face_create(hc_face_t* face) {
   return 0;
 }
 
+#if 0
 static command_parser_t command_face_create3 = {
     .action = ACTION_CREATE,
     .object_type = OBJECT_TYPE_FACE,
@@ -73,7 +74,6 @@ static command_parser_t command_face_create3 = {
 };
 COMMAND_REGISTER(command_face_create3);
 
-#if 0
 static const command_parser_t command_face_create4 = {
     .action = ACTION_CREATE,
     .object = OBJECT_TYPE_FACE,
@@ -85,6 +85,24 @@ static const command_parser_t command_face_create4 = {
 COMMAND_REGISTER(command_face_create4);
 #endif
 
+static const command_parser_t command_face_create3 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_FACE,
+    .nparams = 3,
+    .parameters = {type_tcp_udp, remote_address, remote_port},
+    .post_hook = (parser_hook_t)on_face_create,
+};
+COMMAND_REGISTER(command_face_create3);
+
+static const command_parser_t command_face_create4 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_FACE,
+    .nparams = 4,
+    .parameters = {type_tcp_udp, remote_address, remote_port, interface},
+    .post_hook = (parser_hook_t)on_face_create,
+};
+COMMAND_REGISTER(command_face_create4);
+
 static const command_parser_t command_face_create5 = {
     .action = ACTION_CREATE,
     .object_type = OBJECT_TYPE_FACE,
@@ -127,4 +145,4 @@ static const command_parser_t command_face_remove1 = {
     .nparams = 1,
     .parameters = {symbolic_or_id},
 };
-COMMAND_REGISTER(command_face_remove1);
\ No newline at end of file
+COMMAND_REGISTER(command_face_remove1);
diff --git a/ctrl/libhicnctrl/src/commands/command_listener.c b/ctrl/libhicnctrl/src/commands/command_listener.c
index c11742bba..cfcd22f48 100644
--- a/ctrl/libhicnctrl/src/commands/command_listener.c
+++ b/ctrl/libhicnctrl/src/commands/command_listener.c
@@ -90,7 +90,16 @@ static const command_parser_t command_listener_create4 = {
 COMMAND_REGISTER(command_listener_create4);
 #endif
 
-static const command_parser_t command_listener_create6 = {
+static const command_parser_t command_listener_create4 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_LISTENER,
+    .nparams = 4,
+    .parameters = {protocol_tcp_udp, symbolic, local_address, local_port},
+    .post_hook = (parser_hook_t)on_listener_create,
+};
+COMMAND_REGISTER(command_listener_create4);
+
+static const command_parser_t command_listener_create5 = {
     .action = ACTION_CREATE,
     .object_type = OBJECT_TYPE_LISTENER,
     .nparams = 5,
@@ -98,7 +107,7 @@ static const command_parser_t command_listener_create6 = {
                    interface},
     .post_hook = (parser_hook_t)on_listener_create,
 };
-COMMAND_REGISTER(command_listener_create6);
+COMMAND_REGISTER(command_listener_create5);
 
 static const command_parser_t command_listener_list = {
     .action = ACTION_LIST,
diff --git a/ctrl/libhicnctrl/src/commands/command_route.c b/ctrl/libhicnctrl/src/commands/command_route.c
index 8e7db8192..5db710111 100644
--- a/ctrl/libhicnctrl/src/commands/command_route.c
+++ b/ctrl/libhicnctrl/src/commands/command_route.c
@@ -1,5 +1,6 @@
 #include <math.h>
 #include <hicn/ctrl/command.h>
+#include "../objects/route.h"
 
 /* Parameters */
 
@@ -27,15 +28,105 @@
     .type = TYPE_INT(1, 255), .offset = offsetof(hc_route_t, cost), \
   }
 
+/* Face parameters */
+
+#define type_tcp_udp                                                         \
+  {                                                                          \
+    .name = "type", .help = "face type [tcp | udp]",                         \
+    .type = TYPE_ENUM(face_type), .offset = offsetof(hc_route_t, face.type), \
+  }
+
+#define local_address                                                         \
+  {                                                                           \
+    .name = "local_addr", .help = "local IP address on which to bind.",       \
+    .type = TYPE_IP_ADDRESS, .offset = offsetof(hc_route_t, face.local_addr), \
+    .offset2 = offsetof(hc_route_t, face.family),                             \
+  }
+
+#define local_port                                   \
+  {                                                  \
+    .name = "local_port", .help = "Local port.",     \
+    .type = TYPE_UINT16(1, UINT16_MAX),              \
+    .offset = offsetof(hc_route_t, face.local_port), \
+  }
+
+#define remote_address                                                         \
+  {                                                                            \
+    .name = "remote_address",                                                  \
+    .help = "The IPv4 or IPv6 or hostname of the remote system.",              \
+    .type = TYPE_IP_ADDRESS, .offset = offsetof(hc_route_t, face.remote_addr), \
+    .offset2 = offsetof(hc_route_t, face.family),                              \
+  }
+
+#define remote_port                                   \
+  {                                                   \
+    .name = "remote_port", .help = "Remote port.",    \
+    .type = TYPE_UINT16(1, UINT16_MAX),               \
+    .offset = offsetof(hc_route_t, face.remote_port), \
+  }
+
+#define interface                                              \
+  {                                                            \
+    .name = "interface", .help = "Interface on which to bind", \
+    .type = TYPE_INTERFACE_NAME,                               \
+    .offset = offsetof(hc_route_t, face.netdevice.name),       \
+  }
+
 /* Commands */
+int on_route_create(hc_route_t* route) {
+  if (hc_route_has_face(route)) {
+    route->face.admin_state = FACE_STATE_UP;
+    route->face.id = INVALID_FACE_ID;
+  }
+  return 0;
+}
 
-static const command_parser_t command_route_create = {
+static const command_parser_t command_route_create3 = {
     .action = ACTION_CREATE,
     .object_type = OBJECT_TYPE_ROUTE,
     .nparams = 3,
     .parameters = {symbolic_or_id, prefix, cost},
 };
-COMMAND_REGISTER(command_route_create);
+COMMAND_REGISTER(command_route_create3);
+
+static const command_parser_t command_route_create5 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_ROUTE,
+    .nparams = 5,
+    .parameters = {prefix, cost, type_tcp_udp, remote_address, remote_port},
+    .post_hook = (parser_hook_t)on_route_create,
+};
+COMMAND_REGISTER(command_route_create5);
+
+static const command_parser_t command_route_create6 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_ROUTE,
+    .nparams = 6,
+    .parameters = {prefix, cost, type_tcp_udp, remote_address, remote_port,
+                   interface},
+    .post_hook = (parser_hook_t)on_route_create,
+};
+COMMAND_REGISTER(command_route_create6);
+
+static const command_parser_t command_route_create7 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_ROUTE,
+    .nparams = 7,
+    .parameters = {prefix, cost, type_tcp_udp, local_address, local_port,
+                   remote_address, remote_port},
+    .post_hook = (parser_hook_t)on_route_create,
+};
+COMMAND_REGISTER(command_route_create7);
+
+static const command_parser_t command_route_create8 = {
+    .action = ACTION_CREATE,
+    .object_type = OBJECT_TYPE_ROUTE,
+    .nparams = 8,
+    .parameters = {prefix, cost, type_tcp_udp, local_address, local_port,
+                   remote_address, remote_port, interface},
+    .post_hook = (parser_hook_t)on_route_create,
+};
+COMMAND_REGISTER(command_route_create8);
 
 static const command_parser_t command_route_list = {
     .action = ACTION_LIST,
diff --git a/ctrl/libhicnctrl/src/hicnctrl.c b/ctrl/libhicnctrl/src/hicnctrl.c
index c771bde69..fd028f64a 100644
--- a/ctrl/libhicnctrl/src/hicnctrl.c
+++ b/ctrl/libhicnctrl/src/hicnctrl.c
@@ -221,7 +221,7 @@ int parse_options(int argc, char *argv[], hc_command_t *command,
   command->action = ACTION_CREATE;
   int opt;
 
-  while ((opt = getopt(argc, argv, "cCdfFlLrRsShz:")) != -1) {
+  while ((opt = getopt(argc, argv, "cCdDfFlLrRsShz:")) != -1) {
     switch (opt) {
       case 'z':
         *forwarder = forwarder_type_from_str(optarg);
@@ -260,6 +260,9 @@ int parse_options(int argc, char *argv[], hc_command_t *command,
       case 'S':
         set_command(ACTION_LIST, OBJECT_TYPE_STRATEGY);
         break;
+      case 'D':
+        log_conf.log_level = LOG_DEBUG;
+        break;
       default: /* "h" */
         usage(argv[0]);
         exit(EXIT_SUCCESS);
@@ -323,7 +326,9 @@ int main(int argc, char *argv[]) {
   hc_command_t command = {0};
   char buf[MAXSZ_HC_OBJECT];
 
-  forwarder_type_t forwarder = FORWARDER_TYPE_VPP;
+  log_conf.log_level = LOG_INFO;
+
+  forwarder_type_t forwarder = FORWARDER_TYPE_HICNLIGHT;
 
   if (parse_options(argc, argv, &command, &forwarder) < 0)
     die(OPTIONS, "Bad arguments");
diff --git a/ctrl/libhicnctrl/src/modules/hicn_light.c b/ctrl/libhicnctrl/src/modules/hicn_light.c
index 9c6722975..a652602c6 100644
--- a/ctrl/libhicnctrl/src/modules/hicn_light.c
+++ b/ctrl/libhicnctrl/src/modules/hicn_light.c
@@ -394,7 +394,8 @@ static ssize_t hicnlight_prepare_generic(hc_sock_t *sock, hc_request_t *request,
   /* Dispatch to subrequest if any */
   hc_request_t *current_request = hc_request_get_current(request);
 
-  _ASSERT(!hc_request_get_data(current_request));
+  /* We discard any result from the child request */
+  hc_request_reset_data(current_request);
 
   hc_action_t action = hc_request_get_action(current_request);
   hc_object_type_t object_type = hc_request_get_object_type(current_request);
@@ -695,8 +696,8 @@ static ssize_t hicnlight_prepare_face_list(hc_sock_t *sock,
       }
       hc_data_set_complete(face_data);
 
-      hc_request_reset_data(request);
-      hc_request_set_data(request, face_data);
+      hc_request_reset_data(current_request);
+      hc_request_set_data(current_request, face_data);
 
       /* FACE/LIST could be part of FACE/GET */
       break;
@@ -750,7 +751,7 @@ static ssize_t hicnlight_prepare_face(hc_sock_t *sock, hc_request_t *request,
                                       uint8_t **buffer) {
   hc_request_t *current_request = hc_request_get_current(request);
   hc_action_t action = hc_request_get_action(current_request);
-  hc_object_type_t object_type = hc_request_get_object_type(request);
+  hc_object_type_t object_type = hc_request_get_object_type(current_request);
 
   _ASSERT(object_type == OBJECT_TYPE_FACE);
 
@@ -1027,7 +1028,6 @@ NEXT:
     case REQUEST_STATE_ROUTE_CREATE_FACE_CREATE:
       hc_request_set_state(current_request,
                            REQUEST_STATE_ROUTE_CREATE_FACE_CHECK);
-      INFO(">>>>>>subrequest create face");
       return hicnlight_prepare_subrequest(
           sock, request, ACTION_CREATE, OBJECT_TYPE_FACE,
           (hc_object_t *)&object->route.face, buffer);
diff --git a/ctrl/libhicnctrl/src/objects/connection.c b/ctrl/libhicnctrl/src/objects/connection.c
index 019e5dc22..0f8bafb10 100644
--- a/ctrl/libhicnctrl/src/objects/connection.c
+++ b/ctrl/libhicnctrl/src/objects/connection.c
@@ -45,7 +45,9 @@ int hc_connection_validate(const hc_connection_t *connection,
                            bool allow_partial) {
   int has_id = 0;
   int has_name = 0;
+#if 0
   int has_interface_name = 0;
+#endif
   int has_netdevice_type = 0;
   int has_type = 0;
   int has_family = 0;
@@ -72,6 +74,7 @@ int hc_connection_validate(const hc_connection_t *connection,
     has_name = 1;
   }
 
+#if 0
   if (!isempty(connection->interface_name)) {
     if (!IS_VALID_INTERFACE_NAME(connection->interface_name)) {
       ERROR("[hc_connection_validate] Invalid interface_name specified");
@@ -79,6 +82,7 @@ int hc_connection_validate(const hc_connection_t *connection,
     }
     has_interface_name = 1;
   }
+#endif
 
   if (connection->type != FACE_TYPE_UNDEFINED) {
     if (!IS_VALID_TYPE(connection->type)) {
@@ -128,10 +132,11 @@ int hc_connection_validate(const hc_connection_t *connection,
     has_remote_port = 1;
   }
 
+  /* Interface name is optional */
   int has_key = has_id || has_name;
-  int has_mandatory_attributes = has_interface_name && has_type && has_family &&
-                                 has_local_addr && has_local_port &&
-                                 has_remote_addr && has_remote_port;
+  int has_mandatory_attributes = has_type && has_family && has_local_addr &&
+                                 has_local_port && has_remote_addr &&
+                                 has_remote_port;
   int has_optional_attributes =
       has_netdevice_type && has_admin_state && has_state;
   has_optional_attributes = has_optional_attributes && has_priority && has_tags;
@@ -169,8 +174,10 @@ int hc_connection_cmp(const hc_connection_t *c1, const hc_connection_t *c2) {
   rc = INT_CMP(c1->family, c2->family);
   if (rc != 0) return rc;
 
-  rc = strncmp(c1->interface_name, c2->interface_name, INTERFACE_LEN);
-  if (rc != 0) return rc;
+  if (!isempty(c1->interface_name) && !isempty(c2->interface_name)) {
+    rc = strncmp(c1->interface_name, c2->interface_name, INTERFACE_LEN);
+    if (rc != 0) return rc;
+  }
 
   rc = hicn_ip_address_cmp(&c1->local_addr, &c2->local_addr);
   if (rc != 0) return rc;
diff --git a/ctrl/libhicnctrl/src/objects/listener.c b/ctrl/libhicnctrl/src/objects/listener.c
index f7da6bd9c..97a2430d8 100644
--- a/ctrl/libhicnctrl/src/objects/listener.c
+++ b/ctrl/libhicnctrl/src/objects/listener.c
@@ -42,7 +42,9 @@ int hc_listener_validate(const hc_listener_t *listener, bool allow_partial) {
 
   int has_id = 0;
   int has_name = 0;
+#if 0
   int has_interface_name = 0;
+#endif
   int has_type = 0;
   int has_family = 0;
   int has_local_addr = 0;
@@ -62,6 +64,7 @@ int hc_listener_validate(const hc_listener_t *listener, bool allow_partial) {
     has_name = 1;
   }
 
+#if 0
   if (!isempty(listener->interface_name)) {
     if (!IS_VALID_INTERFACE_NAME(listener->interface_name)) {
       ERROR("[hc_listener_validate] Invalid interface_name specified");
@@ -69,6 +72,7 @@ int hc_listener_validate(const hc_listener_t *listener, bool allow_partial) {
     }
     has_interface_name = 1;
   }
+#endif
 
   if (listener->type != FACE_TYPE_UNDEFINED) {
     if (!IS_VALID_TYPE(listener->type)) {
@@ -112,9 +116,8 @@ int hc_listener_validate(const hc_listener_t *listener, bool allow_partial) {
     else
       return -1;
   } else {
-    /* name is optional */
-    if (has_id && has_interface_name && has_type && has_family &&
-        has_local_addr && has_local_port)
+    /* name and interface are optional */
+    if (has_id && has_type && has_family && has_local_addr && has_local_port)
       return 0;
     return -1;
   }
@@ -146,7 +149,7 @@ int hc_listener_cmp(const hc_listener_t *l1, const hc_listener_t *l2) {
   rc = INT_CMP(l1->local_port, l2->local_port);
   if (rc != 0) return rc;
 
-  return rc;
+  return 0;
 }
 
 int _hc_listener_cmp(const hc_object_t *object1, const hc_object_t *object2) {
diff --git a/ctrl/libhicnctrl/src/objects/route.c b/ctrl/libhicnctrl/src/objects/route.c
index 44f39bcd7..f2d0636a7 100644
--- a/ctrl/libhicnctrl/src/objects/route.c
+++ b/ctrl/libhicnctrl/src/objects/route.c
@@ -83,7 +83,7 @@ int hc_route_validate(const hc_route_t *route, bool allow_partial) {
   }
 
   if (hc_route_has_face(route)) {
-    if (!hc_face_validate(&route->face, allow_partial)) {
+    if (hc_face_validate(&route->face, allow_partial) < 0) {
       ERROR("[hc_route_validate] Invalid face");
       return -1;
     }
diff --git a/tests/config.sh b/tests/config.sh
index 428561c7f..0665a5a8a 100755
--- a/tests/config.sh
+++ b/tests/config.sh
@@ -474,6 +474,14 @@ function test_routes() {
   echo -n "Remove non-existing route using ID: "
   command="remove route 5 $PREFIX"
   assert_cmd_failure "${command}"
+
+  echo -n "Add route and face without interface"
+  command="add route $PREFIX $COST udp $ADDRESS 9695 127.0.0.1 9695 $INTERFACE"
+  assert_cmd_success "${command}"
+
+  echo -n "Add route and face without interface"
+  command="add route $PREFIX $COST udp $ADDRESS 9695 127.0.0.1 9695"
+  assert_cmd_success "${command}"
 }
 
 declare -A ctrl_tests=(