From 546f955b3dad6c0866a8ba778d0cfe1ef43d81d4 Mon Sep 17 00:00:00 2001 From: Jakub Grajciar Date: Wed, 21 Aug 2019 10:51:21 +0200 Subject: [PATCH] memif: API cleanup 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 Change-Id: I11df8b9212c40de179ee71dc9da14039b982ede5 Signed-off-by: Jakub Grajciar --- src/plugins/memif/memif.api | 62 ++++++++++++++++++++++++------------ src/plugins/memif/memif.h | 4 ++- src/plugins/memif/memif_api.c | 34 ++++++++++++++------ src/plugins/memif/memif_test.c | 18 +++++++---- src/plugins/memif/test/test_memif.py | 57 +++++++++++++++++++++++---------- src/plugins/memif/test/vpp_memif.py | 14 ++------ test/remote_test.py | 4 ++- 7 files changed, 125 insertions(+), 68 deletions(-) diff --git a/src/plugins/memif/memif.api b/src/plugins/memif/memif.api index 6a61ee110ae..4c4b587aac7 100644 --- a/src/plugins/memif/memif.api +++ b/src/plugins/memif/memif.api @@ -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"); @@ -13,7 +14,23 @@ * 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 diff --git a/src/plugins/memif/memif.h b/src/plugins/memif/memif.h index 3fbce91d68f..8539c984732 100644 --- a/src/plugins/memif/memif.h +++ b/src/plugins/memif/memif.h @@ -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; diff --git a/src/plugins/memif/memif_api.c b/src/plugins/memif/memif_api.c index 07990279fd4..5dafea34f98 100644 --- a/src/plugins/memif/memif_api.c +++ b/src/plugins/memif/memif_api.c @@ -26,6 +26,8 @@ #include #include +#include +#include /* define message IDs */ #include @@ -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); } diff --git a/src/plugins/memif/memif_test.c b/src/plugins/memif/memif_test.c index e26094b82a8..2dda6d35f2b 100644 --- a/src/plugins/memif/memif_test.c +++ b/src/plugins/memif/memif_test.c @@ -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 */ diff --git a/src/plugins/memif/test/test_memif.py b/src/plugins/memif/test/test_memif.py index 6413cfdbf20..8182a41a620 100644 --- a/src/plugins/memif/test/test_memif.py +++ b/src/plugins/memif/test/test_memif.py @@ -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() diff --git a/src/plugins/memif/test/vpp_memif.py b/src/plugins/memif/test/vpp_memif.py index befcc2840c5..ba032c5fb6f 100644 --- a/src/plugins/memif/test/vpp_memif.py +++ b/src/plugins/memif/test/vpp_memif.py @@ -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 diff --git a/test/remote_test.py b/test/remote_test.py index 9c825ccf755..1b42f8a0189 100644 --- a/test/remote_test.py +++ b/test/remote_test.py @@ -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 -- 2.16.6