diff options
author | Ole Troan <ot@cisco.com> | 2019-08-23 22:55:18 +0200 |
---|---|---|
committer | Andrew Yourtchenko <ayourtch@gmail.com> | 2019-09-03 20:04:13 +0000 |
commit | e5ff5a36dd126ee57dca4e0b03da2f7704e0a4f5 (patch) | |
tree | 2108baa60e8a697624288fe2c3050be29697bc88 /src | |
parent | b6fde4a8bae474c6b73d08d223028f42e396d452 (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>
Diffstat (limited to 'src')
-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 | 54 | ||||
-rw-r--r-- | src/vnet/interface.api | 10 | ||||
-rw-r--r-- | src/vnet/interface_api.c | 30 | ||||
-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 |
20 files changed, 266 insertions, 268 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 b15b38a3d19..b1f78124c2c 100644 --- a/src/plugins/map/map.api +++ b/src/plugins/map/map.api @@ -13,7 +13,7 @@ * limitations under the License. */ -option version = "4.0.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 1e50ba1b714..394ec243dbf 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 bd880a6498d..8cf26d4900d 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 ff46ae9421a..b83ea0b49f8 100644 --- a/src/plugins/nat/nat_api.c +++ b/src/plugins/nat/nat_api.c @@ -200,20 +200,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); } @@ -1208,7 +1202,6 @@ static void int rv = 0; snat_protocol_t proto; u8 *tag = 0; - u32 len = 0; if (sm->deterministic) { @@ -1216,14 +1209,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); @@ -1241,10 +1226,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, @@ -1296,19 +1279,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); @@ -1337,6 +1309,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); } @@ -1347,21 +1322,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); @@ -1382,6 +1345,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); } @@ -1442,7 +1407,6 @@ static void int rv = 0; snat_protocol_t proto = ~0; u8 *tag = 0; - u32 len = 0; if (sm->deterministic) { @@ -1461,11 +1425,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 = @@ -1507,21 +1468,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); @@ -1534,6 +1483,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); } @@ -1545,21 +1496,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); @@ -1571,6 +1510,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); } @@ -1936,9 +1877,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) { @@ -1956,15 +1895,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 = @@ -2051,28 +1983,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); @@ -2087,6 +2004,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 a64b5709c72..4d48ee494c6 100644 --- a/src/plugins/nat/test/test_nat.py +++ b/src/plugins/nat/test/test_nat.py @@ -1894,7 +1894,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 c01fa40c169..d0391cd7a37 100755 --- a/src/tools/vppapigen/vppapigen.py +++ b/src/tools/vppapigen/vppapigen.py @@ -152,6 +152,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) @@ -159,12 +178,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) @@ -208,12 +228,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) @@ -222,6 +259,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): @@ -272,16 +310,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, @@ -292,6 +333,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)) + if name in keyword.kwlist: raise ValueError("Fieldname {!r} is a python keyword and is not " "accessible via the python API. ".format(name)) @@ -521,13 +567,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 ';' @@ -540,9 +591,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, @@ -780,7 +836,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 7efdadc103b..46039371a45 100644 --- a/src/vat/api_format.c +++ b/src/vat/api_format.c @@ -974,8 +974,7 @@ static void vl_api_sw_interface_details_t_handler (vl_api_sw_interface_details_t * mp) { vat_main_t *vam = &vat_main; - u8 *s = format (0, "%s%c", - vl_api_from_api_string (&mp->interface_name), 0); + u8 *s = format (0, "%s%c", mp->interface_name, 0); hash_set_mem (vam->sw_if_index_by_interface_name, s, ntohl (mp->sw_if_index)); @@ -1027,7 +1026,7 @@ static void vl_api_sw_interface_details_t_handler_json vat_json_object_add_bytes (node, "l2_address", mp->l2_address, sizeof (mp->l2_address)); vat_json_object_add_string_copy (node, "interface_name", - mp->interface_name.buf); + mp->interface_name); vat_json_object_add_uint (node, "flags", mp->flags); vat_json_object_add_uint (node, "link_duplex", mp->link_duplex); vat_json_object_add_uint (node, "link_speed", mp->link_speed); @@ -1324,30 +1323,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; @@ -1361,22 +1340,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); @@ -20367,7 +20335,7 @@ api_sw_interface_tag_add_del (vat_main_t * vam) mp->sw_if_index = ntohl (sw_if_index); mp->is_add = enable; if (enable) - vl_api_to_api_string (strlen ((char *) tag), (char *) tag, &mp->tag); + strncpy ((char *) mp->tag, (char *) tag, ARRAY_LEN (mp->tag) - 1); vec_free (tag); S (mp); diff --git a/src/vnet/interface.api b/src/vnet/interface.api index 8a2d0797b5c..dff954438af 100644 --- a/src/vnet/interface.api +++ b/src/vnet/interface.api @@ -14,7 +14,7 @@ * limitations under the License. */ -option version = "3.0.0"; +option version = "3.1.0"; import "vnet/interface_types.api"; import "vnet/ethernet/ethernet_types.api"; @@ -185,8 +185,8 @@ define sw_interface_details u32 i_sid; /* Interface name */ - string interface_name; - string tag; + string interface_name[64]; + string tag[64]; }; /** \brief Request all or filtered subset of sw_interface_details @@ -204,7 +204,7 @@ define sw_interface_dump u32 context; vl_api_interface_index_t sw_if_index; bool name_filter_valid; - string name_filter; + string name_filter[]; }; /** \brief Set or delete one or all ip addresses on a specified interface @@ -307,7 +307,7 @@ autoreply define sw_interface_tag_add_del u32 context; bool is_add; vl_api_interface_index_t sw_if_index; - string tag; + 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 9a695ed5bdd..3448c6037b7 100644 --- a/src/vnet/interface_api.c +++ b/src/vnet/interface_api.c @@ -200,14 +200,8 @@ send_sw_interface_details (vpe_api_main_t * am, vnet_hw_interface_t *hi = vnet_get_sup_hw_interface (am->vnet_main, swif->sw_if_index); - uint32_t if_name_len = strlen ((char *) interface_name); - u8 *tag = vnet_get_sw_interface_tag (vnet_get_main (), swif->sw_if_index); - uint32_t tag_len = 0; - if (tag != NULL) - tag_len = strlen ((char *) tag); - vl_api_sw_interface_details_t *mp = - vl_msg_api_alloc (sizeof (*mp) + if_name_len + tag_len); - clib_memset (mp, 0, sizeof (*mp) + if_name_len + tag_len); + vl_api_sw_interface_details_t *mp = vl_msg_api_alloc (sizeof (*mp)); + clib_memset (mp, 0, sizeof (*mp)); mp->_vl_msg_id = ntohs (VL_API_SW_INTERFACE_DETAILS); mp->sw_if_index = ntohl (swif->sw_if_index); mp->sup_sw_if_index = ntohl (swif->sup_sw_if_index); @@ -245,6 +239,9 @@ send_sw_interface_details (vpe_api_main_t * am, mp->context = context; + strncpy ((char *) mp->interface_name, + (char *) interface_name, ARRAY_LEN (mp->interface_name) - 1); + /* Send the L2 address for ethernet physical intfcs */ if (swif->sup_sw_if_index == swif->sw_if_index && hi->hw_class_index == ethernet_hw_interface_class.index) @@ -306,12 +303,9 @@ send_sw_interface_details (vpe_api_main_t * am, mp->i_sid = i_sid; } - char *p = (char *) &mp->interface_name; - p += - vl_api_to_api_string (if_name_len, (char *) interface_name, - (vl_api_string_t *) p); - if (tag != NULL) - vl_api_to_api_string (tag_len, (char *) tag, (vl_api_string_t *) p); + u8 *tag = vnet_get_sw_interface_tag (vnet_get_main (), swif->sw_if_index); + if (tag) + strncpy ((char *) mp->tag, (char *) tag, ARRAY_LEN (mp->tag) - 1); vl_api_send_msg (rp, (u8 *) mp); } @@ -356,7 +350,8 @@ vl_api_sw_interface_dump_t_handler (vl_api_sw_interface_dump_t * mp) if (mp->name_filter_valid) { filter = - format (0, "%s%c", vl_api_from_api_string (&mp->name_filter), 0); + 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 */ @@ -884,13 +879,14 @@ static void vl_api_sw_interface_tag_add_del_t_handler if (mp->is_add) { - if (vl_api_from_api_string (&mp->tag)[0] == 0) + if (mp->tag[0] == 0) { rv = VNET_API_ERROR_INVALID_VALUE; goto out; } - tag = format (0, "%s%c", vl_api_from_api_string (&mp->tag), 0); + mp->tag[ARRAY_LEN (mp->tag) - 1] = 0; + tag = format (0, "%s%c", mp->tag, 0); vnet_set_sw_interface_tag (vnm, tag, sw_if_index); } else 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 52e050d3978..6f208ff5b0e 100644 --- a/src/vnet/session/session.api +++ b/src/vnet/session/session.api @@ -320,9 +320,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 f03507ded4f..deab6a22593 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. |