api: improve REPLY_MACRO safety

Improve vppapigen to generate per-message #define indicating whether
said message is dynamically sized (due to VLA or string) or not. Use
these #defines in REPLY_MACROs to prevent improper usage. Fix existing
improper REPLY_MACRO* usage.

Type: improvement
Change-Id: Ia77aaf9f6cf3ed68ea21075a4cc8deda78a68651
Signed-off-by: Klement Sekera <ksekera@cisco.com>

diff --git a/src/tools/vppapigen/vppapigen_c.py b/src/tools/vppapigen/vppapigen_c.py
index d0f63fb..1e18a83 100644 (file)
--- a/src/tools/vppapigen/vppapigen_c.py
+++ b/src/tools/vppapigen/vppapigen_c.py
@@ -24,6 +24,7 @@ VAT2 tests.
 '''
 
 import datetime
+import itertools
 import os
 import time
 import sys
@@ -1256,7 +1257,7 @@ def generate_include_types(s, module, stream):
             filename = i.filename.replace('plugins/', '')
             write('#include <{}_types.h>\n'.format(filename))
 
-    for o in s['types'] + s['Define']:
+    for o in itertools.chain(s['types'], s['Define']):
         tname = o.__class__.__name__
         if tname == 'Using':
             if 'length' in o.alias:
@@ -1315,6 +1316,7 @@ def generate_include_types(s, module, stream):
                                      .format(b, o.name))
 
             write('} vl_api_%s_t;\n' % o.name)
+            write(f'#define VL_API_{o.name.upper()}_IS_CONSTANT_SIZE ({0 if o.vla else 1})\n\n')
 
     for t in s['Define']:
         write('#define VL_API_{ID}_CRC "{n}_{crc:08x}"\n'

diff --git a/src/vlibapi/api_helper_macros.h b/src/vlibapi/api_helper_macros.h
index 5c4dd9a..6b5b37a 100644 (file)
--- a/src/vlibapi/api_helper_macros.h
+++ b/src/vlibapi/api_helper_macros.h
@@ -33,20 +33,26 @@
   endian_fp = am->msg_endian_handlers[t + (REPLY_MSG_ID_BASE)];               \
   (*endian_fp) (rmp);
 
-#define REPLY_MACRO(t)                                                  \
-do {                                                                    \
-    vl_api_registration_t *rp;                                          \
-    rp = vl_api_client_index_to_registration (mp->client_index);        \
-    if (rp == 0)                                                        \
-      return;                                                           \
-                                                                        \
-    rmp = vl_msg_api_alloc (sizeof (*rmp));                             \
-    rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE));                   \
-    rmp->context = mp->context;                                         \
-    rmp->retval = ntohl(rv);                                            \
-                                                                        \
-    vl_api_send_msg (rp, (u8 *)rmp);                                    \
-} while(0);
+#define REPLY_MACRO(msg)                                                      \
+  do                                                                          \
+    {                                                                         \
+      STATIC_ASSERT (                                                         \
+       msg##_IS_CONSTANT_SIZE,                                               \
+       "REPLY_MACRO can only be used with constant size messages, "          \
+       "use REPLY_MACRO[3|4]* instead");                                     \
+      vl_api_registration_t *rp;                                              \
+      rp = vl_api_client_index_to_registration (mp->client_index);            \
+      if (rp == 0)                                                            \
+       return;                                                               \
+                                                                              \
+      rmp = vl_msg_api_alloc (sizeof (*rmp));                                 \
+      rmp->_vl_msg_id = htons (msg + (REPLY_MSG_ID_BASE));                    \
+      rmp->context = mp->context;                                             \
+      rmp->retval = ntohl (rv);                                               \
+                                                                              \
+      vl_api_send_msg (rp, (u8 *) rmp);                                       \
+    }                                                                         \
+  while (0);
 
 #define REPLY_MACRO_END(t)                                                    \
   do                                                                          \
@@ -65,20 +71,30 @@ do {                                                                    \
     }                                                                         \
   while (0);
 
-#define REPLY_MACRO2(t, body)                                           \
-do {                                                                    \
-    vl_api_registration_t *rp;                                          \
-    rp = vl_api_client_index_to_registration (mp->client_index);        \
-    if (rp == 0)                                                        \
-      return;                                                           \
-                                                                        \
-    rmp = vl_msg_api_alloc (sizeof (*rmp));                             \
-    rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE));                   \
-    rmp->context = mp->context;                                         \
-    rmp->retval = ntohl(rv);                                            \
-    do {body;} while (0);                                               \
-    vl_api_send_msg (rp, (u8 *)rmp);                                    \
-} while(0);
+#define REPLY_MACRO2(t, body)                                                 \
+  do                                                                          \
+    {                                                                         \
+      STATIC_ASSERT (                                                         \
+       t##_IS_CONSTANT_SIZE,                                                 \
+       "REPLY_MACRO2 can only be used with constant size messages, "         \
+       "use REPLY_MACRO[3|4]* instead");                                     \
+      vl_api_registration_t *rp;                                              \
+      rp = vl_api_client_index_to_registration (mp->client_index);            \
+      if (rp == 0)                                                            \
+       return;                                                               \
+                                                                              \
+      rmp = vl_msg_api_alloc (sizeof (*rmp));                                 \
+      rmp->_vl_msg_id = htons ((t) + (REPLY_MSG_ID_BASE));                    \
+      rmp->context = mp->context;                                             \
+      rmp->retval = ntohl (rv);                                               \
+      do                                                                      \
+       {                                                                     \
+         body;                                                               \
+       }                                                                     \
+      while (0);                                                              \
+      vl_api_send_msg (rp, (u8 *) rmp);                                       \
+    }                                                                         \
+  while (0);
 
 #define REPLY_MACRO2_END(t, body)                                             \
   do                                                                          \
@@ -230,20 +246,26 @@ do {                                                                    \
     }                                                                         \
   while (0);
 
-#define REPLY_MACRO3(t, n, body)                                        \
-do {                                                                    \
-    vl_api_registration_t *rp;                                          \
-    rp = vl_api_client_index_to_registration (mp->client_index);        \
-    if (rp == 0)                                                        \
-      return;                                                           \
-                                                                        \
-    rmp = vl_msg_api_alloc (sizeof (*rmp) + n);                         \
-    rmp->_vl_msg_id = htons((t)+(REPLY_MSG_ID_BASE));                   \
-    rmp->context = mp->context;                                         \
-    rmp->retval = ntohl(rv);                                            \
-    do {body;} while (0);                                               \
-    vl_api_send_msg (rp, (u8 *)rmp);                                    \
-} while(0);
+#define REPLY_MACRO3(t, n, body)                                              \
+  do                                                                          \
+    {                                                                         \
+      vl_api_registration_t *rp;                                              \
+      rp = vl_api_client_index_to_registration (mp->client_index);            \
+      if (rp == 0)                                                            \
+       return;                                                               \
+                                                                              \
+      rmp = vl_msg_api_alloc (sizeof (*rmp) + (n));                           \
+      rmp->_vl_msg_id = htons ((t) + (REPLY_MSG_ID_BASE));                    \
+      rmp->context = mp->context;                                             \
+      rmp->retval = ntohl (rv);                                               \
+      do                                                                      \
+       {                                                                     \
+         body;                                                               \
+       }                                                                     \
+      while (0);                                                              \
+      vl_api_send_msg (rp, (u8 *) rmp);                                       \
+    }                                                                         \
+  while (0);
 
 #define REPLY_MACRO3_END(t, n, body)                                          \
   do                                                                          \
@@ -619,62 +641,65 @@ bad_bd_id:                                        \
     ;                                           \
 } while (0);
 
-#define pub_sub_handler(lca,UCA)                                        \
-static void vl_api_want_##lca##_t_handler (                             \
-    vl_api_want_##lca##_t *mp)                                          \
-{                                                                       \
-    vpe_api_main_t *vam = &vpe_api_main;                                \
-    vpe_client_registration_t *rp;                                      \
-    vl_api_want_##lca##_reply_t *rmp;                                   \
-    uword *p;                                                           \
-    i32 rv = 0;                                                         \
-                                                                        \
-    p = hash_get (vam->lca##_registration_hash, mp->client_index);      \
-    if (p) {                                                            \
-        if (mp->enable_disable) {                                       \
-           clib_warning ("pid %d: already enabled...", ntohl(mp->pid)); \
-            rv = VNET_API_ERROR_INVALID_REGISTRATION;                   \
-            goto reply;                                                 \
-        } else {                                                        \
-            rp = pool_elt_at_index (vam->lca##_registrations, p[0]);    \
-            pool_put (vam->lca##_registrations, rp);                    \
-            hash_unset (vam->lca##_registration_hash,                   \
-                mp->client_index);                                      \
-            goto reply;                                                 \
-        }                                                               \
-    }                                                                   \
-    if (mp->enable_disable == 0) {                                      \
-        clib_warning ("pid %d: already disabled...", mp->pid);          \
-        rv = VNET_API_ERROR_INVALID_REGISTRATION;                       \
-        goto reply;                                                     \
-    }                                                                   \
-    pool_get (vam->lca##_registrations, rp);                            \
-    rp->client_index = mp->client_index;                                \
-    rp->client_pid = mp->pid;                                           \
-    hash_set (vam->lca##_registration_hash, rp->client_index,           \
-              rp - vam->lca##_registrations);                           \
-                                                                        \
-reply:                                                                  \
-    REPLY_MACRO (VL_API_WANT_##UCA##_REPLY);                            \
-}                                                                       \
-                                                                        \
-static clib_error_t * vl_api_want_##lca##_t_reaper (u32 client_index)   \
-{                                                                       \
-    vpe_api_main_t *vam = &vpe_api_main;                                \
-    vpe_client_registration_t *rp;                                      \
-    uword *p;                                                           \
-                                                                        \
-    p = hash_get (vam->lca##_registration_hash, client_index);          \
-    if (p)                                                              \
-      {                                                                 \
-        rp = pool_elt_at_index (vam->lca##_registrations, p[0]);        \
-        pool_put (vam->lca##_registrations, rp);                        \
-        hash_unset (vam->lca##_registration_hash, client_index);        \
-      }                                                                 \
-    return (NULL);                                                      \
-}                                                                       \
-                                                                        \
-VL_MSG_API_REAPER_FUNCTION (vl_api_want_##lca##_t_reaper);              \
+#define pub_sub_handler(lca, UCA)                                             \
+  static void vl_api_want_##lca##_t_handler (vl_api_want_##lca##_t *mp)       \
+  {                                                                           \
+    vpe_api_main_t *vam = &vpe_api_main;                                      \
+    vpe_client_registration_t *rp;                                            \
+    vl_api_want_##lca##_reply_t *rmp;                                         \
+    uword *p;                                                                 \
+    i32 rv = 0;                                                               \
+                                                                              \
+    p = hash_get (vam->lca##_registration_hash, mp->client_index);            \
+    if (p)                                                                    \
+      {                                                                       \
+       if (mp->enable_disable)                                               \
+         {                                                                   \
+           clib_warning ("pid %d: already enabled...", ntohl (mp->pid));     \
+           rv = VNET_API_ERROR_INVALID_REGISTRATION;                         \
+           goto reply;                                                       \
+         }                                                                   \
+       else                                                                  \
+         {                                                                   \
+           rp = pool_elt_at_index (vam->lca##_registrations, p[0]);          \
+           pool_put (vam->lca##_registrations, rp);                          \
+           hash_unset (vam->lca##_registration_hash, mp->client_index);      \
+           goto reply;                                                       \
+         }                                                                   \
+      }                                                                       \
+    if (mp->enable_disable == 0)                                              \
+      {                                                                       \
+       clib_warning ("pid %d: already disabled...", mp->pid);                \
+       rv = VNET_API_ERROR_INVALID_REGISTRATION;                             \
+       goto reply;                                                           \
+      }                                                                       \
+    pool_get (vam->lca##_registrations, rp);                                  \
+    rp->client_index = mp->client_index;                                      \
+    rp->client_pid = mp->pid;                                                 \
+    hash_set (vam->lca##_registration_hash, rp->client_index,                 \
+             rp - vam->lca##_registrations);                                 \
+                                                                              \
+  reply:                                                                      \
+    REPLY_MACRO (VL_API_WANT_##UCA##_REPLY);                                  \
+  }                                                                           \
+                                                                              \
+  static clib_error_t *vl_api_want_##lca##_t_reaper (u32 client_index)        \
+  {                                                                           \
+    vpe_api_main_t *vam = &vpe_api_main;                                      \
+    vpe_client_registration_t *rp;                                            \
+    uword *p;                                                                 \
+                                                                              \
+    p = hash_get (vam->lca##_registration_hash, client_index);                \
+    if (p)                                                                    \
+      {                                                                       \
+       rp = pool_elt_at_index (vam->lca##_registrations, p[0]);              \
+       pool_put (vam->lca##_registrations, rp);                              \
+       hash_unset (vam->lca##_registration_hash, client_index);              \
+      }                                                                       \
+    return (NULL);                                                            \
+  }                                                                           \
+                                                                              \
+  VL_MSG_API_REAPER_FUNCTION (vl_api_want_##lca##_t_reaper);
 
 #define foreach_registration_hash               \
 _(interface_events)                             \

diff --git a/src/vnet/session/session_api.c b/src/vnet/session/session_api.c
index 2121d20..767a24a 100644 (file)
--- a/src/vnet/session/session_api.c
+++ b/src/vnet/session/session_api.c
@@ -696,25 +696,28 @@ vl_api_app_attach_t_handler (vl_api_app_attach_t * mp)
 
 done:
   /* *INDENT-OFF* */
-  REPLY_MACRO2 (VL_API_APP_ATTACH_REPLY, ({
-    if (!rv)
-      {
-       ctrl_thread = n_workers ? 1 : 0;
-       segp = (fifo_segment_t *) a->segment;
-       rmp->app_index = clib_host_to_net_u32 (a->app_index);
-       rmp->app_mq = fifo_segment_msg_q_offset (segp, 0);
-       rmp->vpp_ctrl_mq = fifo_segment_msg_q_offset (rx_mqs_seg, ctrl_thread);
-       rmp->vpp_ctrl_mq_thread = ctrl_thread;
-       rmp->n_fds = n_fds;
-       rmp->fd_flags = fd_flags;
-       if (vec_len (segp->ssvm.name))
-         {
-           vl_api_vec_to_api_string (segp->ssvm.name, &rmp->segment_name);
-         }
-       rmp->segment_size = segp->ssvm.ssvm_size;
-       rmp->segment_handle = clib_host_to_net_u64 (a->segment_handle);
-      }
-  }));
+  REPLY_MACRO3 (
+    VL_API_APP_ATTACH_REPLY,
+    ((!rv) ? vec_len (((fifo_segment_t *) a->segment)->ssvm.name) : 0), ({
+      if (!rv)
+       {
+         ctrl_thread = n_workers ? 1 : 0;
+         segp = (fifo_segment_t *) a->segment;
+         rmp->app_index = clib_host_to_net_u32 (a->app_index);
+         rmp->app_mq = fifo_segment_msg_q_offset (segp, 0);
+         rmp->vpp_ctrl_mq =
+           fifo_segment_msg_q_offset (rx_mqs_seg, ctrl_thread);
+         rmp->vpp_ctrl_mq_thread = ctrl_thread;
+         rmp->n_fds = n_fds;
+         rmp->fd_flags = fd_flags;
+         if (vec_len (segp->ssvm.name))
+           {
+             vl_api_vec_to_api_string (segp->ssvm.name, &rmp->segment_name);
+           }
+         rmp->segment_size = segp->ssvm.ssvm_size;
+         rmp->segment_handle = clib_host_to_net_u64 (a->segment_handle);
+       }
+    }));
   /* *INDENT-ON* */
 
   if (n_fds)
@@ -780,22 +783,25 @@ vl_api_app_worker_add_del_t_handler (vl_api_app_worker_add_del_t * mp)
 
   /* *INDENT-OFF* */
 done:
-  REPLY_MACRO2 (VL_API_APP_WORKER_ADD_DEL_REPLY, ({
-    rmp->is_add = mp->is_add;
-    rmp->wrk_index = clib_host_to_net_u32 (args.wrk_map_index);
-    rmp->segment_handle = clib_host_to_net_u64 (args.segment_handle);
-    if (!rv && mp->is_add)
-      {
-       rmp->app_event_queue_address =
-         fifo_segment_msg_q_offset ((fifo_segment_t *) args.segment, 0);
-       rmp->n_fds = n_fds;
-       rmp->fd_flags = fd_flags;
-       if (vec_len (args.segment->name))
-         {
-           vl_api_vec_to_api_string (args.segment->name, &rmp->segment_name);
-         }
-      }
-  }));
+  REPLY_MACRO3 (
+    VL_API_APP_WORKER_ADD_DEL_REPLY,
+    ((!rv && mp->is_add) ? vec_len (args.segment->name) : 0), ({
+      rmp->is_add = mp->is_add;
+      rmp->wrk_index = clib_host_to_net_u32 (args.wrk_map_index);
+      rmp->segment_handle = clib_host_to_net_u64 (args.segment_handle);
+      if (!rv && mp->is_add)
+       {
+         rmp->app_event_queue_address =
+           fifo_segment_msg_q_offset ((fifo_segment_t *) args.segment, 0);
+         rmp->n_fds = n_fds;
+         rmp->fd_flags = fd_flags;
+         if (vec_len (args.segment->name))
+           {
+             vl_api_vec_to_api_string (args.segment->name,
+                                       &rmp->segment_name);
+           }
+       }
+    }));
   /* *INDENT-ON* */
 
   if (n_fds)