diff options
author | Ole Troan <ot@cisco.com> | 2019-08-23 22:55:18 +0200 |
---|---|---|
committer | Ole Trøan <otroan@employees.org> | 2019-09-09 09:59:48 +0000 |
commit | 30d28bdfd8aca4d6d3c70482ad9ebfdb753610aa (patch) | |
tree | 1ae268835577d69dd982370487215d2049f87ea1 | |
parent | 157f1cd34952759fd8c35976e68c9885537168ad (diff) |
api: enforce vla is last and fixed string type
Enforce that variable length fields are the last element of API messages.
Add a 'fixed' version of string type, since dealing with
multiple variable length strings turned out too painful
for the C language bindings.
The string type is now:
{
string name[64]; // NUL terminated C-string. Essentially decays to u8 name[64]
string name[]; // Variable length string with embedded len field (vl_api_string_t)
};
The latter notation could be made available to other types as well.
e.g.
{
vl_api_address_t addresses[];
}
instead of
{
u32 n_addr;
vl_api_address_t addresses[n_addr];
};
Type: fix
Change-Id: I18fa17ef47227633752ab50453e8d20a652a9f9b
Signed-off-by: Ole Troan <ot@cisco.com>
(cherry picked from commit e5ff5a36dd126ee57dca4e0b03da2f7704e0a4f5)
Signed-off-by: Ole Troan <ot@cisco.com>
-rw-r--r-- | src/plugins/http_static/http_static.api | 6 | ||||
-rw-r--r-- | src/plugins/http_static/http_static.c | 12 | ||||
-rw-r--r-- | src/plugins/map/map.api | 6 | ||||
-rw-r--r-- | src/plugins/map/map_api.c | 11 | ||||
-rw-r--r-- | src/plugins/nat/nat.api | 16 | ||||
-rw-r--r-- | src/plugins/nat/nat_api.c | 147 | ||||
-rw-r--r-- | src/plugins/nat/test/test_nat.py | 2 | ||||
-rwxr-xr-x | src/tools/vppapigen/vppapigen.py | 74 | ||||
-rw-r--r-- | src/tools/vppapigen/vppapigen_c.py | 11 | ||||
-rw-r--r-- | src/vat/api_format.c | 47 | ||||
-rw-r--r-- | src/vnet/interface.api | 24 | ||||
-rw-r--r-- | src/vnet/interface_api.c | 5 | ||||
-rw-r--r-- | src/vnet/ip/punt.api | 4 | ||||
-rw-r--r-- | src/vnet/session/session.api | 2 | ||||
-rwxr-xr-x | src/vpp-api/python/vpp_papi/tests/test_vpp_serializer.py | 40 | ||||
-rw-r--r-- | src/vpp-api/python/vpp_papi/vpp_serializer.py | 46 | ||||
-rwxr-xr-x | src/vpp-api/vapi/vapi_c_gen.py | 12 | ||||
-rw-r--r-- | src/vpp-api/vapi/vapi_json_parser.py | 5 | ||||
-rw-r--r-- | src/vpp/api/api.c | 28 | ||||
-rw-r--r-- | src/vpp/api/vpe.api | 18 | ||||
-rw-r--r-- | test/ext/vapi_c_test.c | 13 | ||||
-rw-r--r-- | test/ext/vapi_cpp_test.cpp | 7 |
22 files changed, 266 insertions, 270 deletions
diff --git a/src/plugins/http_static/http_static.api b/src/plugins/http_static/http_static.api index dc3dcac71ba..4d6d8bfe9b5 100644 --- a/src/plugins/http_static/http_static.api +++ b/src/plugins/http_static/http_static.api @@ -2,7 +2,7 @@ /** \file This file defines static http server control-plane API messages */ -option version = "2.0.0"; +option version = "2.1.0"; /** \brief Configure and enable the static http server @param client_index - opaque cookie to identify the sender @@ -29,7 +29,7 @@ autoreply define http_static_enable { u32 private_segment_size; /* Root of the html path */ - string www_root[limit=256]; + string www_root[256]; /* The bind URI */ - string uri[limit=256]; + string uri[256]; }; diff --git a/src/plugins/http_static/http_static.c b/src/plugins/http_static/http_static.c index 2ad4acfb0c1..6000cf62219 100644 --- a/src/plugins/http_static/http_static.c +++ b/src/plugins/http_static/http_static.c @@ -66,22 +66,16 @@ static void vl_api_http_static_enable_t_handler vl_api_http_static_enable_reply_t *rmp; http_static_main_t *hmp = &http_static_main; int rv; - u8 *www_root = 0; - u8 *uri = 0; - char *p = (char *) &mp->www_root; - www_root = vl_api_from_api_to_vec ((vl_api_string_t *) p); - p += vl_api_string_len ((vl_api_string_t *) p) + sizeof (vl_api_string_t); - uri = vl_api_from_api_to_vec ((vl_api_string_t *) p); + mp->uri[ARRAY_LEN (mp->uri) - 1] = 0; + mp->www_root[ARRAY_LEN (mp->www_root) - 1] = 0; rv = http_static_server_enable_api (ntohl (mp->fifo_size), ntohl (mp->cache_size_limit), ntohl (mp->prealloc_fifos), - ntohl (mp->private_segment_size), www_root, uri); + ntohl (mp->private_segment_size), mp->www_root, mp->uri); - vec_free (www_root); - vec_free (uri); REPLY_MACRO (VL_API_HTTP_STATIC_ENABLE_REPLY); } diff --git a/src/plugins/map/map.api b/src/plugins/map/map.api index f751f5c9ef7..61a3c22c1ba 100644 --- a/src/plugins/map/map.api +++ b/src/plugins/map/map.api @@ -13,7 +13,7 @@ * limitations under the License. */ -option version = "3.1.0"; +option version = "4.1.0"; import "vnet/ip/ip_types.api"; @@ -40,7 +40,7 @@ define map_add_domain u8 psid_offset; u8 psid_length; u16 mtu; - string tag[limit=64]; + string tag[64]; }; /** \brief Reply for MAP domain add @@ -121,7 +121,7 @@ define map_domain_details u8 psid_length; u8 flags; u16 mtu; - string tag[limit=64]; + string tag[64]; }; define map_rule_dump diff --git a/src/plugins/map/map_api.c b/src/plugins/map/map_api.c index 4a0834d786b..aa624cb1ea3 100644 --- a/src/plugins/map/map_api.c +++ b/src/plugins/map/map_api.c @@ -53,8 +53,8 @@ vl_api_map_add_domain_t_handler (vl_api_map_add_domain_t * mp) int rv = 0; u32 index; u8 flags = 0; - u8 *vtag = 0; - vtag = vl_api_from_api_to_vec (&mp->tag); + + u8 *tag = format (0, "%s", mp->tag); rv = map_create_domain ((ip4_address_t *) & mp->ip4_prefix.address, mp->ip4_prefix.len, @@ -62,9 +62,8 @@ vl_api_map_add_domain_t_handler (vl_api_map_add_domain_t * mp) mp->ip6_prefix.len, (ip6_address_t *) & mp->ip6_src.address, mp->ip6_src.len, mp->ea_bits_len, mp->psid_offset, - mp->psid_length, &index, ntohs (mp->mtu), flags, vtag); - vec_free (vtag); - + mp->psid_length, &index, ntohs (mp->mtu), flags, tag); + vec_free (tag); /* *INDENT-OFF* */ REPLY_MACRO2(VL_API_MAP_ADD_DOMAIN_REPLY, ({ @@ -140,7 +139,7 @@ vl_api_map_domain_dump_t_handler (vl_api_map_domain_dump_t * mp) rmp->flags = d->flags; rmp->mtu = htons(d->mtu); - vl_api_vec_to_api_string (de->tag, &rmp->tag ); + strncpy ((char *) rmp->tag, (char *) de->tag, ARRAY_LEN(rmp->tag)-1); vl_api_send_msg (reg, (u8 *) rmp); })); diff --git a/src/plugins/nat/nat.api b/src/plugins/nat/nat.api index b5057ce1d3c..f0c1d0bd183 100644 --- a/src/plugins/nat/nat.api +++ b/src/plugins/nat/nat.api @@ -13,7 +13,7 @@ * limitations under the License. */ -option version = "5.1.0"; +option version = "5.2.0"; import "vnet/ip/ip_types.api"; import "vnet/interface_types.api"; @@ -171,7 +171,7 @@ define nat_worker_details { u32 context; u32 worker_index; u32 lcore_id; - string name; + string name[64]; }; /** \brief Enable/disable NAT IPFIX logging @@ -678,7 +678,7 @@ autoreply define nat44_add_del_static_mapping { u16 external_port; vl_api_interface_index_t external_sw_if_index; u32 vrf_id; - string tag; + string tag[64]; }; /** \brief Dump NAT44 static mappings @@ -717,7 +717,7 @@ define nat44_static_mapping_details { u16 external_port; vl_api_interface_index_t external_sw_if_index; u32 vrf_id; - string tag; + string tag[64]; }; /** \brief Add/delete NAT44 identity mapping @@ -743,7 +743,7 @@ autoreply define nat44_add_del_identity_mapping { u16 port; vl_api_interface_index_t sw_if_index; u32 vrf_id; - string tag; + string tag[64]; }; /** \brief Dump NAT44 identity mappings @@ -773,7 +773,7 @@ define nat44_identity_mapping_details { u16 port; vl_api_interface_index_t sw_if_index; u32 vrf_id; - string tag; + string tag[64]; }; /** \brief Add/delete NAT44 pool address from specific interfce @@ -927,9 +927,9 @@ autoreply manual_endian define nat44_add_del_lb_static_mapping { u16 external_port; u8 protocol; u32 affinity; + string tag[64]; u32 local_num; vl_api_nat44_lb_addr_port_t locals[local_num]; - string tag; }; /** \brief Add/delete NAT44 load-balancing static mapping rule backend @@ -983,9 +983,9 @@ manual_endian define nat44_lb_static_mapping_details { u8 protocol; vl_api_nat_config_flags_t flags; u32 affinity; + string tag[64]; u32 local_num; vl_api_nat44_lb_addr_port_t locals[local_num]; - string tag; }; /** \brief Delete NAT44 session diff --git a/src/plugins/nat/nat_api.c b/src/plugins/nat/nat_api.c index edd5090a5f1..a4a2f7c2aa4 100644 --- a/src/plugins/nat/nat_api.c +++ b/src/plugins/nat/nat_api.c @@ -201,20 +201,14 @@ send_nat_worker_details (u32 worker_index, vl_api_registration_t * reg, snat_main_t *sm = &snat_main; vlib_worker_thread_t *w = vlib_worker_threads + worker_index + sm->first_worker_index; - u32 len = vec_len (w->name); - if (len) - --len; - - rmp = vl_msg_api_alloc (sizeof (*rmp) + len); - clib_memset (rmp, 0, sizeof (*rmp) + len); + rmp = vl_msg_api_alloc (sizeof (*rmp)); + clib_memset (rmp, 0, sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_NAT_WORKER_DETAILS + sm->msg_id_base); rmp->context = context; rmp->worker_index = htonl (worker_index); rmp->lcore_id = htonl (w->cpu_id); - - if (len) - vl_api_to_api_string (len, (char *) w->name, &rmp->name); + strncpy ((char *) rmp->name, (char *) w->name, ARRAY_LEN (rmp->name) - 1); vl_api_send_msg (reg, (u8 *) rmp); } @@ -1209,7 +1203,6 @@ static void int rv = 0; snat_protocol_t proto; u8 *tag = 0; - u32 len = 0; if (sm->deterministic) { @@ -1217,14 +1210,6 @@ static void goto send_reply; } - len = vl_api_string_len (&mp->tag); - - if (len > 64) - { - rv = VNET_API_ERROR_INVALID_VALUE; - goto send_reply; - } - memcpy (&local_addr.as_u8, mp->local_ip_address, 4); memcpy (&external_addr.as_u8, mp->external_ip_address, 4); @@ -1242,10 +1227,8 @@ static void twice_nat = TWICE_NAT; else if (mp->flags & NAT_API_IS_SELF_TWICE_NAT) twice_nat = TWICE_NAT_SELF; - - tag = vec_new (u8, len); - - memcpy (tag, mp->tag.buf, len); + mp->tag[sizeof (mp->tag) - 1] = 0; + tag = format (0, "%s", mp->tag); vec_terminate_c_string (tag); rv = snat_add_static_mapping (local_addr, external_addr, local_port, @@ -1297,19 +1280,8 @@ send_nat44_static_mapping_details (snat_static_mapping_t * m, snat_main_t *sm = &snat_main; u32 len = sizeof (*rmp); - if (m->tag) - { - len += vec_len (m->tag); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - vl_api_to_api_string (vec_len (m->tag), (char *) m->tag, &rmp->tag); - } - else - { - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - } - + rmp = vl_msg_api_alloc (len); + clib_memset (rmp, 0, len); rmp->_vl_msg_id = ntohs (VL_API_NAT44_STATIC_MAPPING_DETAILS + sm->msg_id_base); @@ -1338,6 +1310,9 @@ send_nat44_static_mapping_details (snat_static_mapping_t * m, rmp->local_port = htons (m->local_port); } + if (m->tag) + strncpy ((char *) rmp->tag, (char *) m->tag, vec_len (m->tag)); + vl_api_send_msg (reg, (u8 *) rmp); } @@ -1348,21 +1323,9 @@ send_nat44_static_map_resolve_details (snat_static_map_resolve_t * m, { vl_api_nat44_static_mapping_details_t *rmp; snat_main_t *sm = &snat_main; - u32 len = sizeof (*rmp); - - if (m->tag) - { - len += vec_len (m->tag); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - vl_api_to_api_string (vec_len (m->tag), (char *) m->tag, &rmp->tag); - } - else - { - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - } + rmp = vl_msg_api_alloc (sizeof (*rmp)); + clib_memset (rmp, 0, sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_NAT44_STATIC_MAPPING_DETAILS + sm->msg_id_base); clib_memcpy (rmp->local_ip_address, &(m->l_addr), 4); @@ -1383,6 +1346,8 @@ send_nat44_static_map_resolve_details (snat_static_map_resolve_t * m, rmp->external_port = htons (m->e_port); rmp->local_port = htons (m->l_port); } + if (m->tag) + strncpy ((char *) rmp->tag, (char *) m->tag, vec_len (m->tag)); vl_api_send_msg (reg, (u8 *) rmp); } @@ -1443,7 +1408,6 @@ static void int rv = 0; snat_protocol_t proto = ~0; u8 *tag = 0; - u32 len = 0; if (sm->deterministic) { @@ -1462,11 +1426,8 @@ static void addr.as_u32 = 0; else memcpy (&addr.as_u8, mp->ip_address, 4); - - len = vl_api_string_len (&mp->tag); - - tag = vec_new (u8, len); - memcpy (tag, mp->tag.buf, len); + mp->tag[sizeof (mp->tag) - 1] = 0; + tag = format (0, "%s", mp->tag); vec_terminate_c_string (tag); rv = @@ -1508,21 +1469,9 @@ send_nat44_identity_mapping_details (snat_static_mapping_t * m, int index, vl_api_nat44_identity_mapping_details_t *rmp; snat_main_t *sm = &snat_main; nat44_lb_addr_port_t *local = pool_elt_at_index (m->locals, index); - u32 len = sizeof (*rmp); - - if (m->tag) - { - len += vec_len (m->tag); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - vl_api_to_api_string (vec_len (m->tag), (char *) m->tag, &rmp->tag); - } - else - { - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - } + rmp = vl_msg_api_alloc (sizeof (*rmp)); + clib_memset (rmp, 0, sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_NAT44_IDENTITY_MAPPING_DETAILS + sm->msg_id_base); @@ -1535,6 +1484,8 @@ send_nat44_identity_mapping_details (snat_static_mapping_t * m, int index, rmp->vrf_id = htonl (local->vrf_id); rmp->protocol = snat_proto_to_ip_proto (m->proto); rmp->context = context; + if (m->tag) + strncpy ((char *) rmp->tag, (char *) m->tag, vec_len (m->tag)); vl_api_send_msg (reg, (u8 *) rmp); } @@ -1546,21 +1497,9 @@ send_nat44_identity_map_resolve_details (snat_static_map_resolve_t * m, { vl_api_nat44_identity_mapping_details_t *rmp; snat_main_t *sm = &snat_main; - u32 len = sizeof (*rmp); - - if (m->tag) - { - len += vec_len (m->tag); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - vl_api_to_api_string (vec_len (m->tag), (char *) m->tag, &rmp->tag); - } - else - { - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - } + rmp = vl_msg_api_alloc (sizeof (*rmp)); + clib_memset (rmp, 0, sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_NAT44_IDENTITY_MAPPING_DETAILS + sm->msg_id_base); @@ -1572,6 +1511,8 @@ send_nat44_identity_map_resolve_details (snat_static_map_resolve_t * m, rmp->vrf_id = htonl (m->vrf_id); rmp->protocol = snat_proto_to_ip_proto (m->proto); rmp->context = context; + if (m->tag) + strncpy ((char *) rmp->tag, (char *) m->tag, vec_len (m->tag)); vl_api_send_msg (reg, (u8 *) rmp); } @@ -1937,9 +1878,7 @@ static void nat44_lb_addr_port_t *locals = 0; ip4_address_t e_addr; snat_protocol_t proto; - vl_api_string_t *sp; u8 *tag = 0; - u32 len = 0; if (!sm->endpoint_dependent) { @@ -1957,15 +1896,8 @@ static void twice_nat = TWICE_NAT; else if (mp->flags & NAT_API_IS_SELF_TWICE_NAT) twice_nat = TWICE_NAT_SELF; - - sp = (void *) &mp->locals + - sizeof (vl_api_nat44_lb_addr_port_t) * - clib_net_to_host_u32 (mp->local_num); - - len = vl_api_string_len (sp); - - tag = vec_new (u8, len); - memcpy (tag, sp->buf, len); + mp->tag[sizeof (mp->tag) - 1] = 0; + tag = format (0, "%s", mp->tag); vec_terminate_c_string (tag); rv = @@ -2052,28 +1984,13 @@ send_nat44_lb_static_mapping_details (snat_static_mapping_t * m, snat_main_t *sm = &snat_main; nat44_lb_addr_port_t *ap; vl_api_nat44_lb_addr_port_t *locals; - vl_api_string_t *sp; u32 local_num = 0; - u32 len = sizeof (*rmp); - - if (m->tag) - { - len += pool_elts (m->locals) * - sizeof (nat44_lb_addr_port_t) + vec_len (m->tag); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - - sp = (void *) &rmp->locals + - sizeof (vl_api_nat44_lb_addr_port_t) * pool_elts (m->locals); - vl_api_to_api_string (vec_len (m->tag), (char *) m->tag, sp); - } - else - { - len += pool_elts (m->locals) * sizeof (nat44_lb_addr_port_t); - rmp = vl_msg_api_alloc (len); - clib_memset (rmp, 0, len); - } + rmp = + vl_msg_api_alloc (sizeof (*rmp) + + (pool_elts (m->locals) * + sizeof (nat44_lb_addr_port_t))); + clib_memset (rmp, 0, sizeof (*rmp)); rmp->_vl_msg_id = ntohs (VL_API_NAT44_LB_STATIC_MAPPING_DETAILS + sm->msg_id_base); @@ -2088,6 +2005,8 @@ send_nat44_lb_static_mapping_details (snat_static_mapping_t * m, rmp->flags |= NAT_API_IS_SELF_TWICE_NAT; if (is_out2in_only_static_mapping (m)) rmp->flags |= NAT_API_IS_OUT2IN_ONLY; + if (m->tag) + strncpy ((char *) rmp->tag, (char *) m->tag, vec_len (m->tag)); locals = (vl_api_nat44_lb_addr_port_t *) rmp->locals; /* *INDENT-OFF* */ diff --git a/src/plugins/nat/test/test_nat.py b/src/plugins/nat/test/test_nat.py index 773eb51d4ee..2e062f156c2 100644 --- a/src/plugins/nat/test/test_nat.py +++ b/src/plugins/nat/test/test_nat.py @@ -1893,7 +1893,7 @@ class TestNAT44(MethodHolder): is_add=1) sm = self.vapi.nat44_static_mapping_dump() self.assertEqual(len(sm), 1) - self.assertEqual((sm[0].tag).split(b'\0', 1)[0], b'') + self.assertEqual(sm[0].tag, '') self.assertEqual(sm[0].protocol, 0) self.assertEqual(sm[0].local_port, 0) self.assertEqual(sm[0].external_port, 0) diff --git a/src/tools/vppapigen/vppapigen.py b/src/tools/vppapigen/vppapigen.py index 8ae991c9c95..22c80e900b9 100755 --- a/src/tools/vppapigen/vppapigen.py +++ b/src/tools/vppapigen/vppapigen.py @@ -150,6 +150,25 @@ class Typedef(): self.manual_endian = True global_type_add(name, self) + self.vla = False + + for i, b in enumerate(block): + if isinstance(b, Array): + if b.length == 0: + self.vla = True + if i + 1 < len(block): + raise ValueError( + 'VLA field "{}" must be the last ' + 'field in message "{}"' + .format(b.fieldname, name)) + elif b.fieldtype == 'string': + self.vla = True + if i + 1 < len(block): + raise ValueError( + 'VLA field "{}" must be the last ' + 'field in message "{}"' + .format(b.fieldname, name)) + def __repr__(self): return self.name + str(self.flags) + str(self.block) @@ -157,12 +176,13 @@ class Typedef(): class Using(): def __init__(self, name, alias): self.name = name + self.vla = False if isinstance(alias, Array): - a = { 'type': alias.fieldtype, # noqa: E201 - 'length': alias.length } # noqa: E202 + a = {'type': alias.fieldtype, + 'length': alias.length} else: - a = { 'type': alias.fieldtype } # noqa: E201,E202 + a = {'type': alias.fieldtype} self.alias = a self.crc = str(alias).encode() global_type_add(name, self) @@ -206,12 +226,29 @@ class Define(): elif f == 'autoreply': self.autoreply = True - for b in block: + for i, b in enumerate(block): if isinstance(b, Option): if b[1] == 'singular' and b[2] == 'true': self.singular = True block.remove(b) + if isinstance(b, Array) and b.vla and i + 1 < len(block): + raise ValueError( + 'VLA field "{}" must be the last field in message "{}"' + .format(b.fieldname, name)) + elif b.fieldtype.startswith('vl_api_'): + if (global_types[b.fieldtype].vla and i + 1 < len(block)): + raise ValueError( + 'VLA field "{}" must be the last ' + 'field in message "{}"' + .format(b.fieldname, name)) + elif b.fieldtype == 'string' and b.length == 0: + if i + 1 < len(block): + raise ValueError( + 'VLA field "{}" must be the last ' + 'field in message "{}"' + .format(b.fieldname, name)) + def __repr__(self): return self.name + str(self.flags) + str(self.block) @@ -220,6 +257,7 @@ class Enum(): def __init__(self, name, block, enumtype='u32'): self.name = name self.enumtype = enumtype + self.vla = False count = 0 for i, b in enumerate(block): @@ -273,16 +311,19 @@ class Option(): class Array(): - def __init__(self, fieldtype, name, length): + def __init__(self, fieldtype, name, length, modern_vla=False): self.type = 'Array' self.fieldtype = fieldtype self.fieldname = name + self.modern_vla = modern_vla if type(length) is str: self.lengthfield = length self.length = 0 + self.vla = True else: self.length = length self.lengthfield = None + self.vla = False def __repr__(self): return str([self.fieldtype, self.fieldname, self.length, @@ -293,6 +334,11 @@ class Field(): def __init__(self, fieldtype, name, limit=None): self.type = 'Field' self.fieldtype = fieldtype + + if self.fieldtype == 'string': + raise ValueError("The string type {!r} is an " + "array type ".format(name)) + self.fieldname = name self.limit = limit @@ -517,13 +563,18 @@ class VPPAPIParser(object): if len(p) == 2: p[0] = p[1] else: - p[0] = { **p[1], **p[2] } + p[0] = {**p[1], **p[2]} def p_field_option(self, p): - '''field_option : ID '=' assignee ',' + '''field_option : ID + | ID '=' assignee ',' | ID '=' assignee + ''' - p[0] = { p[1]: p[3] } + if len(p) == 2: + p[0] = {p[1]: None} + else: + p[0] = {p[1]: p[3]} def p_declaration(self, p): '''declaration : type_specifier ID ';' @@ -536,9 +587,14 @@ class VPPAPIParser(object): self._parse_error('ERROR') self.fields.append(p[2]) + def p_declaration_array_vla(self, p): + '''declaration : type_specifier ID '[' ']' ';' ''' + p[0] = Array(p[1], p[2], 0, modern_vla=True) + def p_declaration_array(self, p): '''declaration : type_specifier ID '[' NUM ']' ';' | type_specifier ID '[' ID ']' ';' ''' + if len(p) != 7: return self._parse_error( 'array: %s' % p.value, @@ -775,7 +831,7 @@ def foldup_blocks(block, crc): try: crc = crc_block_combine(t.block, crc) return foldup_blocks(t.block, crc) - except: + except AttributeError: pass return crc diff --git a/src/tools/vppapigen/vppapigen_c.py b/src/tools/vppapigen/vppapigen_c.py index 19efc99dae0..c1bc11d4a12 100644 --- a/src/tools/vppapigen/vppapigen_c.py +++ b/src/tools/vppapigen/vppapigen_c.py @@ -153,8 +153,15 @@ def typedefs(objs, aliases, filename): if b.lengthfield: output += " %s %s[0];\n" % (api2c(b.fieldtype), b.fieldname) else: - output += " %s %s[%s];\n" % (api2c(b.fieldtype), b.fieldname, - b.length) + # Fixed length strings decay to nul terminated u8 + if b.fieldtype == 'string': + if b.modern_vla: + output += ' {} {};\n'.format(api2c(b.fieldtype), b.fieldname) + else: + output += ' u8 {}[{}];\n'.format(b.fieldname, b.length) + else: + output += " %s %s[%s];\n" % (api2c(b.fieldtype), b.fieldname, + b.length) else: raise ValueError("Error in processing array type %s" % b) diff --git a/src/vat/api_format.c b/src/vat/api_format.c index 19a7b29d01c..86457dc9218 100644 --- a/src/vat/api_format.c +++ b/src/vat/api_format.c @@ -1333,30 +1333,10 @@ static void vl_api_show_version_reply_t_handler if (retval >= 0) { - u8 *s = 0; - char *p = (char *) &mp->program; - - s = vl_api_from_api_to_vec ((vl_api_string_t *) p); - errmsg (" program: %v\n", s); - vec_free (s); - - p += - vl_api_string_len ((vl_api_string_t *) p) + sizeof (vl_api_string_t); - s = vl_api_from_api_to_vec ((vl_api_string_t *) p); - errmsg (" version: %v\n", s); - vec_free (s); - - p += - vl_api_string_len ((vl_api_string_t *) p) + sizeof (vl_api_string_t); - s = vl_api_from_api_to_vec ((vl_api_string_t *) p); - errmsg (" build date: %v\n", s); - vec_free (s); - - p += - vl_api_string_len ((vl_api_string_t *) p) + sizeof (vl_api_string_t); - s = vl_api_from_api_to_vec ((vl_api_string_t *) p); - errmsg ("build directory: %v\n", s); - vec_free (s); + errmsg (" program: %s", mp->program); + errmsg (" version: %s", mp->version); + errmsg (" build date: %s", mp->build_date); + errmsg ("build directory: %s", mp->build_directory); } vam->retval = retval; vam->result_ready = 1; @@ -1370,22 +1350,11 @@ static void vl_api_show_version_reply_t_handler_json vat_json_init_object (&node); vat_json_object_add_int (&node, "retval", ntohl (mp->retval)); - char *p = (char *) &mp->program; - vat_json_object_add_string_copy (&node, "program", - vl_api_from_api_string ((vl_api_string_t *) - p)); - p += vl_api_string_len ((vl_api_string_t *) p) + sizeof (u32); - vat_json_object_add_string_copy (&node, "version", - vl_api_from_api_string ((vl_api_string_t *) - p)); - p += vl_api_string_len ((vl_api_string_t *) p) + sizeof (u32); - vat_json_object_add_string_copy (&node, "build_date", - vl_api_from_api_string ((vl_api_string_t *) - p)); - p += vl_api_string_len ((vl_api_string_t *) p) + sizeof (u32); + vat_json_object_add_string_copy (&node, "program", mp->program); + vat_json_object_add_string_copy (&node, "version", mp->version); + vat_json_object_add_string_copy (&node, "build_date", mp->build_date); vat_json_object_add_string_copy (&node, "build_directory", - vl_api_from_api_string ((vl_api_string_t *) - p)); + mp->build_directory); vat_json_print (vam->ofp, &node); vat_json_free (&node); diff --git a/src/vnet/interface.api b/src/vnet/interface.api index d30f1c285ea..4ffc7f8801d 100644 --- a/src/vnet/interface.api +++ b/src/vnet/interface.api @@ -14,7 +14,7 @@ * limitations under the License. */ -option version = "2.3.1"; +option version = "3.1.1"; import "vnet/interface_types.api"; @@ -151,9 +151,6 @@ define sw_interface_details u32 l2_address_length; u8 l2_address[8]; - /* Interface name */ - u8 interface_name[64]; - /* 1 = up, 0 = down */ u8 admin_up_down; u8 link_up_down; @@ -192,7 +189,6 @@ define sw_interface_details u32 vtr_push_dot1q; // ethertype of first pushed tag is dot1q/dot1ad u32 vtr_tag1; // first pushed tag u32 vtr_tag2; // second pushed tag - u8 tag[64]; /* pbb tag rewrite info */ u16 outer_tag; @@ -200,6 +196,10 @@ define sw_interface_details u8 b_smac[6]; u16 b_vlanid; u32 i_sid; + + /* Interface name */ + string interface_name[64]; + string tag[64]; }; /** \brief Request all or filtered subset of sw_interface_details @@ -216,8 +216,8 @@ define sw_interface_dump u32 client_index; u32 context; vl_api_interface_index_t sw_if_index; - u8 name_filter_valid; - u8 name_filter[49]; + bool name_filter_valid; + string name_filter[]; }; /** \brief Set or delete one or all ip addresses on a specified interface @@ -319,11 +319,11 @@ autoreply define sw_interface_clear_stats */ autoreply define sw_interface_tag_add_del { - u32 client_index; - u32 context; - u8 is_add; - u32 sw_if_index; - u8 tag[64]; + u32 client_index; + u32 context; + bool is_add; + vl_api_interface_index_t sw_if_index; + string tag[64]; }; /** \brief Set an interface's MAC address diff --git a/src/vnet/interface_api.c b/src/vnet/interface_api.c index fb1982ae8b0..d170b71e07f 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -327,8 +327,9 @@ vl_api_sw_interface_dump_t_handler (vl_api_sw_interface_dump_t * mp) if (mp->name_filter_valid) { - mp->name_filter[ARRAY_LEN (mp->name_filter) - 1] = 0; - filter = format (0, "%s%c", mp->name_filter, 0); + filter = + format (0, ".*%s", vl_api_string_len (&mp->name_filter), + vl_api_from_api_string (&mp->name_filter), 0); } char *strcasestr (char *, char *); /* lnx hdr file botch */ diff --git a/src/vnet/ip/punt.api b/src/vnet/ip/punt.api index 72efc7a9557..9890385fe5a 100644 --- a/src/vnet/ip/punt.api +++ b/src/vnet/ip/punt.api @@ -13,7 +13,7 @@ * limitations under the License. */ -option version = "2.1.0"; +option version = "2.2.0"; import "vnet/ip/ip_types.api"; /** \brief The types of packets to be punted @@ -134,7 +134,7 @@ autoreply define punt_socket_deregister { typedef punt_reason { u32 id; - string name; + string name[]; }; /** \brief Dump all or one of the excpetion punt reasons diff --git a/src/vnet/session/session.api b/src/vnet/session/session.api index 533f65e85a2..8f0b7a28df2 100644 --- a/src/vnet/session/session.api +++ b/src/vnet/session/session.api @@ -263,9 +263,9 @@ autoreply define connect_sock { u8 ip[16]; u16 port; u8 proto; + u64 parent_handle; u8 hostname_len; u8 hostname[hostname_len]; - u64 parent_handle; }; /** \brief ask app to add a new cut-through registration diff --git a/src/vpp-api/python/vpp_papi/tests/test_vpp_serializer.py b/src/vpp-api/python/vpp_papi/tests/test_vpp_serializer.py index bddaa9e00c2..6ca8e6c4ff3 100755 --- a/src/vpp-api/python/vpp_papi/tests/test_vpp_serializer.py +++ b/src/vpp-api/python/vpp_papi/tests/test_vpp_serializer.py @@ -11,11 +11,47 @@ from ipaddress import * class TestLimits(unittest.TestCase): + def test_string(self): + fixed_string = VPPType('fixed_string', + [['string', 'name', 16]]) + + b = fixed_string.pack({'name': 'foobar'}) + self.assertEqual(len(b), 16) + + # Ensure string is nul terminated + self.assertEqual(b.decode('ascii')[6], '\x00') + + nt, size = fixed_string.unpack(b) + self.assertEqual(size, 16) + self.assertEqual(nt.name, 'foobar') + + # Empty string + b = fixed_string.pack({'name': ''}) + self.assertEqual(len(b), 16) + nt, size = fixed_string.unpack(b) + self.assertEqual(size, 16) + self.assertEqual(nt.name, '') + + # String too long + with self.assertRaises(VPPSerializerValueError): + b = fixed_string.pack({'name': 'foobarfoobar1234'}) + + variable_string = VPPType('variable_string', + [['string', 'name', 0]]) + b = variable_string.pack({'name': 'foobar'}) + self.assertEqual(len(b), 4 + len('foobar')) + + nt, size = variable_string.unpack(b) + self.assertEqual(size, 4 + len('foobar')) + self.assertEqual(nt.name, 'foobar') + self.assertEqual(len(nt.name), len('foobar')) + + def test_limit(self): limited_type = VPPType('limited_type_t', - [['string', 'name', {'limit': 16}]]) + [['string', 'name', 0, {'limit': 16}]]) unlimited_type = VPPType('limited_type_t', - [['string', 'name']]) + [['string', 'name', 0]]) b = limited_type.pack({'name': 'foobar'}) self.assertEqual(len(b), 10) diff --git a/src/vpp-api/python/vpp_papi/vpp_serializer.py b/src/vpp-api/python/vpp_papi/vpp_serializer.py index 9ce0287592f..ec6a06bb3d5 100644 --- a/src/vpp-api/python/vpp_papi/vpp_serializer.py +++ b/src/vpp-api/python/vpp_papi/vpp_serializer.py @@ -105,27 +105,42 @@ class BaseTypes(object): class String(object): - def __init__(self, options): - self.name = 'string' + def __init__(self, name, num, options): + self.name = name + self.num = num self.size = 1 self.length_field_packer = BaseTypes('u32') - self.limit = options['limit'] if 'limit' in options else None + self.limit = options['limit'] if 'limit' in options else num + self.fixed = True if num else False + if self.fixed and not self.limit: + raise VPPSerializerValueError( + "Invalid argument length for: {}, {} maximum {}". + format(list, len(list), self.limit)) def pack(self, list, kwargs=None): if not list: + if self.fixed: + return b"\x00" * self.limit return self.length_field_packer.pack(0) + b"" - if self.limit and len(list) > self.limit: + if self.limit and len(list) > self.limit - 1: raise VPPSerializerValueError( "Invalid argument length for: {}, {} maximum {}". - format(list, len(list), self.limit)) - - return self.length_field_packer.pack(len(list)) + list.encode('utf8') + format(list, len(list), self.limit - 1)) + if self.fixed: + return list.encode('ascii').ljust(self.limit, b'\x00') + return self.length_field_packer.pack(len(list)) + list.encode('ascii') def unpack(self, data, offset=0, result=None, ntc=False): + if self.fixed: + p = BaseTypes('u8', self.num) + s = p.unpack(data, offset) + s2 = s[0].split(b'\0', 1)[0] + return (s2.decode('ascii'), self.num) + length, length_field_size = self.length_field_packer.unpack(data, offset) if length == 0: - return b'', 0 + return '', 0 p = BaseTypes('u8', length) x, size = p.unpack(data, offset + length_field_size) x2 = x.split(b'\0', 1)[0] @@ -181,10 +196,6 @@ class FixedList_u8(object): 'Invalid array length for "{}" got {}' ' expected {}' .format(self.name, len(data[offset:]), self.num)) - if self.field_type == 'string': - s = self.packer.unpack(data, offset) - s2 = s[0].split(b'\0', 1)[0] - return (s2.decode('utf-8'), self.num) return self.packer.unpack(data, offset) @@ -473,12 +484,19 @@ class VPPType(object): if fieldlen == 3: # list list_elements = f[2] if list_elements == 0: - p = VLAList_legacy(f_name, f_type) + if f_type == 'string': + p = String(f_name, 0, self.options) + else: + p = VLAList_legacy(f_name, f_type) self.packers.append(p) - elif f_type == 'u8' or f_type == 'string': + elif f_type == 'u8': p = FixedList_u8(f_name, f_type, list_elements) self.packers.append(p) size += p.size + elif f_type == 'string': + p = String(f_name, list_elements, self.options) + self.packers.append(p) + size += p.size else: p = FixedList(f_name, f_type, list_elements) self.packers.append(p) diff --git a/src/vpp-api/vapi/vapi_c_gen.py b/src/vpp-api/vapi/vapi_c_gen.py index 381dcba7f42..b9b9aa750a8 100755 --- a/src/vpp-api/vapi/vapi_c_gen.py +++ b/src/vpp-api/vapi/vapi_c_gen.py @@ -13,10 +13,16 @@ class CField(Field): return "vapi_type_%s" % self.name def get_c_def(self): - if self.len is not None: - return "%s %s[%d];" % (self.type.get_c_name(), self.name, self.len) + if self.type.get_c_name() == 'vl_api_string_t': + if self.len: + return "u8 %s[%d];" % (self.name, self.len) + else: + return "vl_api_string_t %s;" % (self.name) else: - return "%s %s;" % (self.type.get_c_name(), self.name) + if self.len is not None: + return "%s %s[%d];" % (self.type.get_c_name(), self.name, self.len) + else: + return "%s %s;" % (self.type.get_c_name(), self.name) def get_swap_to_be_code(self, struct, var): if self.len is not None: diff --git a/src/vpp-api/vapi/vapi_json_parser.py b/src/vpp-api/vapi/vapi_json_parser.py index a9d2c8186bc..d7669365ce8 100644 --- a/src/vpp-api/vapi/vapi_json_parser.py +++ b/src/vpp-api/vapi/vapi_json_parser.py @@ -177,12 +177,15 @@ class Message(object): p = field_class(field_name=field[1], field_type=field_type) elif l == 3: - if field[2] == 0: + if field[2] == 0 and field[0] != 'string': raise ParseError( "While parsing message `%s': variable length " "array `%s' doesn't have reference to member " "containing the actual length" % ( name, field[1])) + if field[0] == 'string' and field[2] > 0: + field_type = json_parser.lookup_type_like_id('u8') + p = field_class( field_name=field[1], field_type=field_type, diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c index 801cf186904..989d867c06b 100644 --- a/src/vpp/api/api.c +++ b/src/vpp/api/api.c @@ -254,21 +254,16 @@ vl_api_show_version_t_handler (vl_api_show_version_t * mp) char *vpe_api_get_version (void); char *vpe_api_get_build_date (void); - u32 program_len = strnlen_s ("vpe", 32); - u32 version_len = strnlen_s (vpe_api_get_version (), 32); - u32 build_date_len = strnlen_s (vpe_api_get_build_date (), 32); - u32 build_directory_len = strnlen_s (vpe_api_get_build_directory (), 256); - - u32 n = program_len + version_len + build_date_len + build_directory_len; - /* *INDENT-OFF* */ - REPLY_MACRO3(VL_API_SHOW_VERSION_REPLY, n, + REPLY_MACRO2(VL_API_SHOW_VERSION_REPLY, ({ - char *p = (char *)&rmp->program; - p += vl_api_to_api_string(program_len, "vpe", (vl_api_string_t *)p); - p += vl_api_to_api_string(version_len, vpe_api_get_version(), (vl_api_string_t *)p); - p += vl_api_to_api_string(build_date_len, vpe_api_get_build_date(), (vl_api_string_t *)p); - vl_api_to_api_string(build_directory_len, vpe_api_get_build_directory(), (vl_api_string_t *)p); + strncpy ((char *) rmp->program, "vpe", ARRAY_LEN(rmp->program)-1); + strncpy ((char *) rmp->build_directory, vpe_api_get_build_directory(), + ARRAY_LEN(rmp->build_directory)-1); + strncpy ((char *) rmp->version, vpe_api_get_version(), + ARRAY_LEN(rmp->version)-1); + strncpy ((char *) rmp->build_date, vpe_api_get_build_date(), + ARRAY_LEN(rmp->build_date)-1); })); /* *INDENT-ON* */ } @@ -495,10 +490,11 @@ show_log_details (vl_api_registration_t * reg, u32 context, rmp->context = context; rmp->timestamp = clib_host_to_net_f64 (timestamp); rmp->level = htonl (*level); - char *p = (char *) &rmp->msg_class; - p += vl_api_vec_to_api_string (msg_class, (vl_api_string_t *) p); - p += vl_api_vec_to_api_string (message, (vl_api_string_t *) p); + strncpy ((char *) rmp->msg_class, (char *) msg_class, + ARRAY_LEN (rmp->msg_class) - 1); + strncpy ((char *) rmp->message, (char *) message, + ARRAY_LEN (rmp->message) - 1); vl_api_send_msg (reg, (u8 *) rmp); } diff --git a/src/vpp/api/vpe.api b/src/vpp/api/vpe.api index 9531ea5643d..487d2028b88 100644 --- a/src/vpp/api/vpe.api +++ b/src/vpp/api/vpe.api @@ -19,7 +19,7 @@ called through a shared memory interface. */ -option version = "1.5.0"; +option version = "1.6.0"; import "vpp/api/vpe_types.api"; @@ -92,7 +92,7 @@ define cli_inband { u32 client_index; u32 context; - string cmd; + string cmd[]; }; /** \brief vpe parser cli string response @@ -110,7 +110,7 @@ define cli_inband_reply { u32 context; i32 retval; - string reply; + string reply[]; }; /** \brief Get node index using name request @@ -184,10 +184,10 @@ define show_version_reply { u32 context; i32 retval; - string program [limit = 32]; - string version [limit = 32]; - string build_date [limit = 32]; - string build_directory [limit = 256]; + string program[32]; + string version[32]; + string build_date[32]; + string build_directory[256]; }; @@ -295,8 +295,8 @@ define log_details { u32 context; vl_api_timestamp_t timestamp; vl_api_log_level_t level; - string msg_class [limit=32]; - string message [limit=256]; + string msg_class[32]; + string message[256]; }; /** \brief Show the current system timestamp. diff --git a/test/ext/vapi_c_test.c b/test/ext/vapi_c_test.c index 0b22fff1706..d49f23023dd 100644 --- a/test/ext/vapi_c_test.c +++ b/test/ext/vapi_c_test.c @@ -359,14 +359,11 @@ show_version_cb (vapi_ctx_t ctx, void *caller_ctx, { ck_assert_int_eq (VAPI_OK, rv); ck_assert_int_eq (true, is_last); - ck_assert_str_eq ("vpe", (char *) vl_api_from_api_string (&p->program)); + ck_assert_str_eq ("vpe", (char *) p->program); printf ("show_version_reply: program: `%s', version: `%s', build directory: " - "`%s', build date: `%s'\n", - vl_api_from_api_string (&p->program), - vl_api_from_api_string (&p->version), - vl_api_from_api_string (&p->build_directory), - vl_api_from_api_string (&p->build_date)); + "`%s', build date: `%s'\n", p->program, p->version, p->build_directory, + p->build_date); ++*(int *) caller_ctx; return VAPI_OK; } @@ -802,9 +799,7 @@ generic_cb (vapi_ctx_t ctx, void *callback_ctx, vapi_msg_id_t id, void *msg) ck_assert_int_eq (id, vapi_msg_id_show_version_reply); ck_assert_ptr_ne (NULL, msg); vapi_msg_show_version_reply *reply = msg; - ck_assert_str_eq ("vpe", - (char *) vl_api_from_api_string (&reply-> - payload.program)); + ck_assert_str_eq ("vpe", (char *) reply->payload.program); return VAPI_OK; } diff --git a/test/ext/vapi_cpp_test.cpp b/test/ext/vapi_cpp_test.cpp index 68366af2ac8..2c2a82c8a9d 100644 --- a/test/ext/vapi_cpp_test.cpp +++ b/test/ext/vapi_cpp_test.cpp @@ -49,11 +49,8 @@ void verify_show_version_reply (const Show_version_reply &r) auto &p = r.get_payload (); printf ("show_version_reply: program: `%s', version: `%s', build directory: " "`%s', build date: `%s'\n", - vl_api_from_api_string (&p.program), - vl_api_from_api_string (&p.version), - vl_api_from_api_string (&p.build_directory), - vl_api_from_api_string (&p.build_date)); - ck_assert_str_eq ("vpe", (char *)vl_api_from_api_string (&p.program)); + p.program, p.version, p.build_directory, p.build_date); + ck_assert_str_eq ("vpe", (char *)p.program); } Connection con; |