memif: API cleanup 27/21427/9
authorJakub Grajciar <jgrajcia@cisco.com>
Wed, 21 Aug 2019 08:51:21 +0000 (10:51 +0200)
committerOle Trøan <otroan@employees.org>
Mon, 9 Sep 2019 14:29:48 +0000 (14:29 +0000)
Use consistent API types.

memif_create now enables zero-copy by default.
Add no_zero_copy param to memif_create which
if set, disables zero copy.

Type: refactor

Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
Change-Id: I11df8b9212c40de179ee71dc9da14039b982ede5
Signed-off-by: Jakub Grajciar <jgrajcia@cisco.com>
src/plugins/memif/memif.api
src/plugins/memif/memif.h
src/plugins/memif/memif_api.c
src/plugins/memif/memif_test.c
src/plugins/memif/test/test_memif.py
src/plugins/memif/test/vpp_memif.py
test/remote_test.py

index 6a61ee1..4c4b587 100644 (file)
@@ -1,3 +1,4 @@
+/* Hey Emacs use -*- mode: C -*- */
 /*
  * Copyright (c) 2017 Cisco and/or its affiliates.
  * Licensed under the Apache License, Version 2.0 (the "License");
  * limitations under the License.
  */
 
-option version = "2.0.0";
+option version = "3.0.0";
+
+import "vnet/interface_types.api";
+import "vnet/ethernet/ethernet_types.api";
+
+enum memif_role
+{
+  MEMIF_ROLE_API_MASTER = 0,
+  MEMIF_ROLE_API_SLAVE = 1,
+};
+
+enum memif_mode
+{
+  MEMIF_MODE_API_ETHERNET = 0,
+  MEMIF_MODE_API_IP = 1,
+  MEMIF_MODE_API_PUNT_INJECT = 2,
+};
 
 /** \brief Create or remove named socket file for memif interfaces
     @param client_index - opaque cookie to identify the sender
@@ -28,9 +45,9 @@ autoreply define memif_socket_filename_add_del
 {
   u32 client_index;
   u32 context;
-  u8 is_add;           /* 0 = remove, 1 = add association */
+  bool is_add;         /* 0 = remove, 1 = add association */
   u32 socket_id;       /* unique non-0 id for given socket file name */
-  u8 socket_filename[128];     /* NUL terminated filename */
+  string socket_filename[108]; /* NUL terminated filename */
 };
 
 /** \brief Create memory interface
@@ -46,23 +63,26 @@ autoreply define memif_socket_filename_add_del
            establishment
     @param ring_size - the number of entries of RX/TX rings
     @param buffer_size - size of the buffer allocated for each ring entry
+    @param no_zero_copy - if true, disable zero copy
     @param hw_addr - interface MAC address
+    @param secret - optional, default is "", max length 24
 */
 define memif_create
 {
   u32 client_index;
   u32 context;
 
-  u8 role; /* 0 = master, 1 = slave */
-  u8 mode; /* 0 = ethernet, 1 = ip, 2 = punt/inject */
+  vl_api_memif_role_t role; /* 0 = master, 1 = slave */
+  vl_api_memif_mode_t mode; /* 0 = ethernet, 1 = ip, 2 = punt/inject */
   u8 rx_queues; /* optional, default is 1 */
   u8 tx_queues; /* optional, default is 1 */
   u32 id; /* optional, default is 0 */
   u32 socket_id; /* optional, default is 0, "/var/vpp/memif.sock" */
-  u8 secret[24]; /* optional, default is "" */
   u32 ring_size; /* optional, default is 1024 entries, must be power of 2 */
   u16 buffer_size; /* optional, default is 2048 bytes */
-  u8 hw_addr[6]; /* optional, randomly generated if not defined */
+  bool no_zero_copy; /* disable zero copy */
+  vl_api_mac_address_t hw_addr; /* optional, randomly generated if zero */
+  string secret[24]; /* optional, default is "", max length 24 */
 };
 
 /** \brief Create memory interface response
@@ -74,7 +94,7 @@ define memif_create_reply
 {
   u32 context;
   i32 retval;
-  u32 sw_if_index;
+  vl_api_interface_index_t sw_if_index;
 };
 
 /** \brief Delete memory interface
@@ -87,7 +107,7 @@ autoreply define memif_delete
   u32 client_index;
   u32 context;
 
-  u32 sw_if_index;
+  vl_api_interface_index_t sw_if_index;
 };
 
 /** \brief Memory interface details structure
@@ -99,7 +119,7 @@ define memif_socket_filename_details
 {
   u32 context;
   u32 socket_id;
-  u8 socket_filename[128];
+  string socket_filename[108];
 };
 
 /** \brief Dump the table of socket ids and corresponding filenames
@@ -115,38 +135,38 @@ define memif_socket_filename_dump
 /** \brief Memory interface details structure
     @param context - sender context, to match reply w/ request (memif_dump)
     @param sw_if_index - index of the interface
-    @param if_name - name of the interface
     @param hw_addr - interface MAC address
     @param id - id associated with the interface
     @param role - role of the interface in the connection (master/slave)
     @param mode - interface mode
+    @param zero_copy - zero copy flag present
     @param socket_id - id of the socket filename used by this interface
            to establish new connections
     @param ring_size - the number of entries of RX/TX rings
     @param buffer_size - size of the buffer allocated for each ring entry
-    @param admin_up_down - interface administrative status
-    @param link_up_down - interface link status
+    @param flags - interface_status flags
+    @param if_name - name of the interface
 
 */
 define memif_details
 {
   u32 context;
 
-  u32 sw_if_index;
-  u8 if_name[64];
-  u8 hw_addr[6];
+  vl_api_interface_index_t sw_if_index;
+  vl_api_mac_address_t hw_addr;
 
   /* memif specific parameters */
   u32 id;
-  u8 role; /* 0 = master, 1 = slave */
-  u8 mode; /* 0 = ethernet, 1 = ip, 2 = punt/inject */
+  vl_api_memif_role_t role; /* 0 = master, 1 = slave */
+  vl_api_memif_mode_t mode; /* 0 = ethernet, 1 = ip, 2 = punt/inject */
+  bool zero_copy;
   u32 socket_id;
   u32 ring_size;
   u16 buffer_size; /* optional, default is 2048 bytes */
 
-  /* 1 = up, 0 = down */
-  u8 admin_up_down;
-  u8 link_up_down;
+  vl_api_if_status_flags_t flags;
+
+  string if_name[64];
 };
 
 /** \brief Dump all memory interfaces
index 3fbce91..8539c98 100644 (file)
@@ -27,6 +27,8 @@
 #define MEMIF_VERSION_MINOR    0
 #define MEMIF_VERSION          ((MEMIF_VERSION_MAJOR << 8) | MEMIF_VERSION_MINOR)
 
+#define MEMIF_SECRET_SIZE       24
+
 /*
  *  Type definitions
  */
@@ -85,7 +87,7 @@ typedef struct __attribute__ ((packed))
   memif_version_t version;
   memif_interface_id_t id;
   memif_interface_mode_t mode:8;
-  uint8_t secret[24];
+  uint8_t secret[MEMIF_SECRET_SIZE];
   uint8_t name[32];
 } memif_msg_init_t;
 
index 0799027..5dafea3 100644 (file)
@@ -26,6 +26,8 @@
 #include <vlibapi/api.h>
 #include <vlibmemory/api.h>
 
+#include <vnet/ip/ip_types_api.h>
+#include <vnet/ethernet/ethernet_types_api.h>
 
 /* define message IDs */
 #include <memif/memif_msg_enum.h>
@@ -93,7 +95,7 @@ void
   socket_filename = 0;
   mp->socket_filename[ARRAY_LEN (mp->socket_filename) - 1] = 0;
   len = strlen ((char *) mp->socket_filename);
-  if (len > 0)
+  if (mp->is_add)
     {
       vec_validate (socket_filename, len);
       memcpy (socket_filename, mp->socket_filename, len);
@@ -122,6 +124,7 @@ vl_api_memif_create_t_handler (vl_api_memif_create_t * mp)
   u32 ring_size = MEMIF_DEFAULT_RING_SIZE;
   static const u8 empty_hw_addr[6];
   int rv = 0;
+  mac_address_t mac;
 
   /* id */
   args.id = clib_net_to_host_u32 (mp->id);
@@ -139,10 +142,12 @@ vl_api_memif_create_t_handler (vl_api_memif_create_t * mp)
     }
 
   /* role */
-  args.is_master = (mp->role == 0);
+  args.is_master = (ntohl (mp->role) == MEMIF_ROLE_API_MASTER);
 
   /* mode */
-  args.mode = mp->mode;
+  args.mode = ntohl (mp->mode);
+
+  args.is_zero_copy = mp->no_zero_copy ? 0 : 1;
 
   /* enable zero-copy */
   args.is_zero_copy = 1;
@@ -182,9 +187,10 @@ vl_api_memif_create_t_handler (vl_api_memif_create_t * mp)
     }
 
   /* MAC address */
-  if (memcmp (mp->hw_addr, empty_hw_addr, 6) != 0)
+  mac_address_decode (mp->hw_addr, &mac);
+  if (memcmp (&mac, empty_hw_addr, 6) != 0)
     {
-      memcpy (args.hw_addr, mp->hw_addr, 6);
+      memcpy (args.hw_addr, &mac, 6);
       args.hw_addr_set = 1;
     }
 
@@ -257,7 +263,7 @@ send_memif_details (vl_api_registration_t * reg,
 
   if (hwif->hw_address)
     {
-      memcpy (mp->hw_addr, hwif->hw_address, ARRAY_LEN (mp->hw_addr));
+      mac_address_encode ((mac_address_t *) hwif->hw_address, mp->hw_addr);
     }
 
   mp->id = clib_host_to_net_u32 (mif->id);
@@ -265,12 +271,22 @@ send_memif_details (vl_api_registration_t * reg,
   msf = pool_elt_at_index (mm->socket_files, mif->socket_file_index);
   mp->socket_id = clib_host_to_net_u32 (msf->socket_id);
 
-  mp->role = (mif->flags & MEMIF_IF_FLAG_IS_SLAVE) ? 1 : 0;
+  mp->role =
+    (mif->flags & MEMIF_IF_FLAG_IS_SLAVE) ? MEMIF_ROLE_API_SLAVE :
+    MEMIF_ROLE_API_MASTER;
+  mp->role = htonl (mp->role);
+  mp->mode = htonl (mif->mode);
   mp->ring_size = htonl (1 << mif->run.log2_ring_size);
   mp->buffer_size = htons (mif->run.buffer_size);
+  mp->zero_copy = (mif->flags & MEMIF_IF_FLAG_ZERO_COPY) ? 1 : 0;
+
+  mp->flags = 0;
+  mp->flags |= (swif->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) ?
+    IF_STATUS_API_FLAG_ADMIN_UP : 0;
+  mp->flags |= (hwif->flags & VNET_HW_INTERFACE_FLAG_LINK_UP) ?
+    IF_STATUS_API_FLAG_LINK_UP : 0;
+  mp->flags = htonl (mp->flags);
 
-  mp->admin_up_down = (swif->flags & VNET_SW_INTERFACE_FLAG_ADMIN_UP) ? 1 : 0;
-  mp->link_up_down = (hwif->flags & VNET_HW_INTERFACE_FLAG_LINK_UP) ? 1 : 0;
 
   vl_api_send_msg (reg, (u8 *) mp);
 }
index e26094b..2dda6d3 100644 (file)
@@ -162,12 +162,14 @@ api_memif_socket_filename_add_del (vat_main_t * vam)
       return -99;
     }
 
-  M (MEMIF_SOCKET_FILENAME_ADD_DEL, mp);
+  M2 (MEMIF_SOCKET_FILENAME_ADD_DEL, mp, strlen ((char *) socket_filename));
 
   mp->is_add = is_add;
   mp->socket_id = htonl (socket_id);
-  strncpy ((char *) mp->socket_filename,
-          (char *) socket_filename, sizeof (mp->socket_filename) - 1);
+  char *p = (char *) &mp->socket_filename;
+  p +=
+    vl_api_to_api_string (strlen ((char *) socket_filename),
+                         (char *) socket_filename, (vl_api_string_t *) p);
 
   vec_free (socket_filename);
 
@@ -259,7 +261,7 @@ api_memif_create (vat_main_t * vam)
       return -99;
     }
 
-  M (MEMIF_CREATE, mp);
+  M2 (MEMIF_CREATE, mp, strlen ((char *) secret));
 
   mp->mode = mode;
   mp->id = clib_host_to_net_u32 (id);
@@ -269,7 +271,9 @@ api_memif_create (vat_main_t * vam)
   mp->socket_id = clib_host_to_net_u32 (socket_id);
   if (secret != 0)
     {
-      strncpy ((char *) mp->secret, (char *) secret, 16);
+      char *p = (char *) &mp->secret;
+      p += vl_api_to_api_string (strlen ((char *) secret), (char *) secret,
+                                (vl_api_string_t *) p);
       vec_free (secret);
     }
   memcpy (mp->hw_addr, hw_addr, 6);
@@ -382,8 +386,8 @@ vl_api_memif_details_t_handler (vl_api_memif_details_t * mp)
           clib_net_to_host_u32 (mp->socket_id),
           mp->role ? "slave" : "master",
           ntohl (mp->ring_size), ntohs (mp->buffer_size),
-          mp->admin_up_down ? "up" : "down",
-          mp->link_up_down ? "up" : "down");
+          (mp->flags & IF_STATUS_API_FLAG_ADMIN_UP) ? "up" : "down",
+          (mp->flags & IF_STATUS_API_FLAG_LINK_UP) ? "up" : "down");
 }
 
 /* memif_socket_filename_dump API */
index 6413cfd..8182a41 100644 (file)
@@ -7,9 +7,10 @@ import six
 
 from framework import VppTestCase, VppTestRunner, running_extended_tests
 from remote_test import RemoteClass, RemoteVppTestCase
-from vpp_memif import MEMIF_MODE, MEMIF_ROLE, remove_all_memif_vpp_config, \
+from vpp_memif import remove_all_memif_vpp_config, \
     VppSocketFilename, VppMemif
 from vpp_ip_route import VppIpRoute, VppRoutePath
+from vpp_papi import VppEnum
 
 
 class TestMemif(VppTestCase):
@@ -122,7 +123,7 @@ class TestMemif(VppTestCase):
         self.assertTrue(memif.wait_for_link_up(5))
         dump = memif.query_vpp_config()
 
-        if memif.role == MEMIF_ROLE.SLAVE:
+        if memif.role == VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE:
             self.assertEqual(dump.ring_size, memif.ring_size)
             self.assertEqual(dump.buffer_size, memif.buffer_size)
         else:
@@ -145,9 +146,12 @@ class TestMemif(VppTestCase):
     def test_memif_create_delete(self):
         """ Memif create/delete interface """
 
-        memif = VppMemif(self, MEMIF_ROLE.SLAVE, MEMIF_MODE.ETHERNET)
+        memif = VppMemif(
+            self,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET)
         self._create_delete_test_one_interface(memif)
-        memif.role = MEMIF_ROLE.MASTER
+        memif.role = VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_MASTER
         self._create_delete_test_one_interface(memif)
 
     def test_memif_create_custom_socket(self):
@@ -174,34 +178,47 @@ class TestMemif(VppTestCase):
                 b"sock/memif3.sock",
                 add_default_folder=True))
 
-        memif = VppMemif(self, MEMIF_ROLE.SLAVE, MEMIF_MODE.ETHERNET)
+        memif = VppMemif(
+            self,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET)
 
         for sock in memif_sockets:
             sock.add_vpp_config()
             memif.socket_id = sock.socket_id
-            memif.role = MEMIF_ROLE.SLAVE
+            memif.role = VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE
             self._create_delete_test_one_interface(memif)
-            memif.role = MEMIF_ROLE.MASTER
+            memif.role = VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_MASTER
             self._create_delete_test_one_interface(memif)
 
     def test_memif_connect(self):
         """ Memif connect """
-        memif = VppMemif(self, MEMIF_ROLE.SLAVE,  MEMIF_MODE.ETHERNET,
-                         ring_size=1024, buffer_size=2048)
+        memif = VppMemif(
+            self,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET,
+            ring_size=1024,
+            buffer_size=2048,
+            secret="abc")
 
         remote_socket = VppSocketFilename(self.remote_test, 1,
                                           b"%s/memif.sock" % six.ensure_binary(
                                               self.tempdir, encoding='utf-8'))
         remote_socket.add_vpp_config()
 
-        remote_memif = VppMemif(self.remote_test, MEMIF_ROLE.MASTER,
-                                MEMIF_MODE.ETHERNET, socket_id=1,
-                                ring_size=1024, buffer_size=2048)
+        remote_memif = VppMemif(
+            self.remote_test,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_MASTER,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET,
+            socket_id=1,
+            ring_size=1024,
+            buffer_size=2048,
+            secret="abc")
 
         self._connect_test_interface_pair(memif, remote_memif)
 
-        memif.role = MEMIF_ROLE.MASTER
-        remote_memif.role = MEMIF_ROLE.SLAVE
+        memif.role = VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_MASTER
+        remote_memif.role = VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE
 
         self._connect_test_interface_pair(memif, remote_memif)
 
@@ -227,15 +244,21 @@ class TestMemif(VppTestCase):
     def test_memif_ping(self):
         """ Memif ping """
 
-        memif = VppMemif(self, MEMIF_ROLE.SLAVE,  MEMIF_MODE.ETHERNET)
+        memif = VppMemif(
+            self,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_SLAVE,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET)
 
         remote_socket = VppSocketFilename(self.remote_test, 1,
                                           b"%s/memif.sock" % six.ensure_binary(
                                               self.tempdir, encoding='utf-8'))
         remote_socket.add_vpp_config()
 
-        remote_memif = VppMemif(self.remote_test, MEMIF_ROLE.MASTER,
-                                MEMIF_MODE.ETHERNET, socket_id=1)
+        remote_memif = VppMemif(
+            self.remote_test,
+            VppEnum.vl_api_memif_role_t.MEMIF_ROLE_API_MASTER,
+            VppEnum.vl_api_memif_mode_t.MEMIF_MODE_API_ETHERNET,
+            socket_id=1)
 
         memif.add_vpp_config()
         memif.config_ip4()
index befcc28..ba032c5 100644 (file)
@@ -7,17 +7,6 @@ from vpp_ip import VppIpPrefix
 from vpp_papi import VppEnum
 
 
-class MEMIF_ROLE:
-    MASTER = 0
-    SLAVE = 1
-
-
-class MEMIF_MODE:
-    ETHERNET = 0
-    IP = 1
-    PUNT_INJECT = 2
-
-
 def get_if_dump(dump, sw_if_index):
     for d in dump:
         if (d.sw_if_index == sw_if_index):
@@ -127,7 +116,8 @@ class VppMemif(VppObject):
             return False
         while True:
             dump = self.query_vpp_config()
-            if dump.link_up_down == 1:
+            f = VppEnum.vl_api_if_status_flags_t.IF_STATUS_API_FLAG_LINK_UP
+            if dump.flags & f:
                 return True
             self._test.sleep(step)
             timeout -= step
index 9c825cc..1b42f8a 100644 (file)
@@ -248,12 +248,14 @@ class RemoteClass(Process):
             if name[0] == '_':
                 if name in ['__weakref__']:
                     continue
+                if name in ['__dict__']:
+                    continue
                 if not (name.startswith('__') and name.endswith('__')):
                     continue
             if callable(member) and not isinstance(member, property):
                 continue
             if not self._serializable(member):
-                continue
+                member = self._make_serializable(member)
             setattr(copy, name, member)
         return copy