From 088f042400fe104c86c86fb0de04aeb4b8013e74 Mon Sep 17 00:00:00 2001 From: Ole Troan Date: Fri, 20 Oct 2017 13:28:20 +0200 Subject: VPP-1033: Python API support arbitrary sized input parameters. Dynamically calculate the required buffer size to pack into based on message definition. Also add input parameter length checking. Change-Id: I7633bec596e4833bb328fbf63a65b866c7985de5 Signed-off-by: Ole Troan (cherry picked from commit 895b6e8b4408108a9b5cea99dcb378c3524b18b2) --- src/vpp-api/python/vpp_papi.py | 62 +++++++++++++++++++++++++++++++++--------- test/test_acl_plugin.py | 4 +-- test/test_dhcp.py | 3 +- test/test_nat.py | 17 +++++++----- test/test_papi.py | 31 +++++++++++++++++++++ test/vpp_papi_provider.py | 4 +-- 6 files changed, 96 insertions(+), 25 deletions(-) create mode 100644 test/test_papi.py diff --git a/src/vpp-api/python/vpp_papi.py b/src/vpp-api/python/vpp_papi.py index 14f367c2..7b66c0f4 100644 --- a/src/vpp-api/python/vpp_papi.py +++ b/src/vpp-api/python/vpp_papi.py @@ -129,7 +129,6 @@ class VPP(): self.messages = {} self.id_names = [] self.id_msgdef = [] - self.buffersize = 10000 self.connected = False self.header = struct.Struct('>HI') self.apifiles = [] @@ -199,35 +198,45 @@ class VPP(): if not vl: if e > 0 and t == 'u8': # Fixed byte array - return struct.Struct('>' + str(e) + 's') + s = struct.Struct('>' + str(e) + 's') + return s.size, s if e > 0: # Fixed array of base type - return [e, struct.Struct('>' + base_types[t])] + s = struct.Struct('>' + base_types[t]) + return s.size, [e, s] elif e == 0: # Old style variable array - return [-1, struct.Struct('>' + base_types[t])] + s = struct.Struct('>' + base_types[t]) + return s.size, [-1, s] else: # Variable length array - return [vl, struct.Struct('>s')] if t == 'u8' else \ - [vl, struct.Struct('>' + base_types[t])] + if t == 'u8': + s = struct.Struct('>s') + return s.size, [vl, s] + else: + s = struct.Struct('>' + base_types[t]) + return s.size, [vl, s] - return struct.Struct('>' + base_types[t]) + s = struct.Struct('>' + base_types[t]) + return s.size, s if t in self.messages: + size = self.messages[t]['sizes'][0] + # Return a list in case of array if e > 0 and not vl: - return [e, lambda self, encode, buf, offset, args: ( + return size, [e, lambda self, encode, buf, offset, args: ( self.__struct_type(encode, self.messages[t], buf, offset, args))] if vl: - return [vl, lambda self, encode, buf, offset, args: ( + return size, [vl, lambda self, encode, buf, offset, args: ( self.__struct_type(encode, self.messages[t], buf, offset, args))] elif e == 0: # Old style VLA raise NotImplementedError(1, 'No support for compound types ' + t) - return lambda self, encode, buf, offset, args: ( + return size, lambda self, encode, buf, offset, args: ( self.__struct_type(encode, self.messages[t], buf, offset, args) ) @@ -250,13 +259,14 @@ class VPP(): ' used in call to: ' + \ self.id_names[kwargs['_vl_msg_id']] + '()' ) - for k, v in vpp_iterator(msgdef['args']): off += size if k in kwargs: if type(v) is list: if callable(v[1]): e = kwargs[v[0]] if v[0] in kwargs else v[0] + if e != len(kwargs[k]): + raise (ValueError(1, 'Input list length mismatch: %s (%s != %s)' % (k, e, len(kwargs[k])))) size = 0 for i in range(e): size += v[1](self, True, buf, off + size, @@ -264,6 +274,8 @@ class VPP(): else: if v[0] in kwargs: l = kwargs[v[0]] + if l != len(kwargs[k]): + raise ValueError(1, 'Input list length mistmatch: %s (%s != %s)' % (k, l, len(kwargs[k]))) else: l = len(kwargs[k]) if v[1].size == 1: @@ -278,6 +290,8 @@ class VPP(): if callable(v): size = v(self, True, buf, off, kwargs[k]) else: + if type(kwargs[k]) is str and v.size < len(kwargs[k]): + raise ValueError(1, 'Input list length mistmatch: %s (%s < %s)' % (k, v.size, len(kwargs[k]))) v.pack_into(buf, off, kwargs[k]) size = v.size else: @@ -290,9 +304,17 @@ class VPP(): return self.messages[name] return None + def get_size(self, sizes, kwargs): + total_size = sizes[0] + for e in sizes[1]: + if e in kwargs and type(kwargs[e]) is list: + total_size += len(kwargs[e]) * sizes[1][e] + return total_size + def encode(self, msgdef, kwargs): # Make suitably large buffer - buf = bytearray(self.buffersize) + size = self.get_size(msgdef['sizes'], kwargs) + buf = bytearray(size) offset = 0 size = self.__struct_type(True, msgdef, buf, offset, kwargs) return buf[:offset + size] @@ -360,6 +382,8 @@ class VPP(): argtypes = collections.OrderedDict() fields = [] msg = {} + total_size = 0 + sizes = {} for i, f in enumerate(msgdef): if type(f) is dict and 'crc' in f: msg['crc'] = f['crc'] @@ -368,7 +392,18 @@ class VPP(): field_name = f[1] if len(f) == 3 and f[2] == 0 and i != len(msgdef) - 2: raise ValueError('Variable Length Array must be last: ' + name) - args[field_name] = self.__struct(*f) + size, s = self.__struct(*f) + args[field_name] = s + if type(s) == list and type(s[0]) == int and type(s[1]) == struct.Struct: + if s[0] < 0: + sizes[field_name] = size + else: + sizes[field_name] = size + total_size += s[0] * size + else: + sizes[field_name] = size + total_size += size + argtypes[field_name] = field_type if len(f) == 4: # Find offset to # elements field idx = list(args.keys()).index(f[3]) - i @@ -380,6 +415,7 @@ class VPP(): self.messages[name]['args'] = args self.messages[name]['argtypes'] = argtypes self.messages[name]['typeonly'] = typeonly + self.messages[name]['sizes'] = [total_size, sizes] return self.messages[name] def add_type(self, name, typedef): diff --git a/test/test_acl_plugin.py b/test/test_acl_plugin.py index 97fca1a1..cd375a2c 100644 --- a/test/test_acl_plugin.py +++ b/test/test_acl_plugin.py @@ -532,7 +532,7 @@ class TestACLplugin(VppTestCase): r[i_rule][rule_key]) # Add a deny-1234 ACL - r_deny = ({'is_permit': 0, 'is_ipv6': 0, 'proto': 17, + r_deny = [{'is_permit': 0, 'is_ipv6': 0, 'proto': 17, 'srcport_or_icmptype_first': 1234, 'srcport_or_icmptype_last': 1235, 'src_ip_prefix_len': 0, @@ -549,7 +549,7 @@ class TestACLplugin(VppTestCase): 'dstport_or_icmpcode_first': 0, 'dstport_or_icmpcode_last': 0, 'dst_ip_addr': '\x00\x00\x00\x00', - 'dst_ip_prefix_len': 0}) + 'dst_ip_prefix_len': 0}] reply = self.vapi.acl_add_replace(acl_index=4294967295, r=r_deny, tag="deny 1234;permit all") diff --git a/test/test_dhcp.py b/test/test_dhcp.py index fe97f6c9..42b80af3 100644 --- a/test/test_dhcp.py +++ b/test/test_dhcp.py @@ -19,6 +19,7 @@ from scapy.layers.dhcp6 import DHCP6, DHCP6_Solicit, DHCP6_RelayForward, \ from socket import AF_INET, AF_INET6 from scapy.utils import inet_pton, inet_ntop from scapy.utils6 import in6_ptop +from util import mactobinary DHCP4_CLIENT_PORT = 68 DHCP4_SERVER_PORT = 67 @@ -1134,7 +1135,7 @@ class TestDHCP(VppTestCase): # remove the left over ARP entry self.vapi.ip_neighbor_add_del(self.pg2.sw_if_index, - self.pg2.remote_mac, + mactobinary(self.pg2.remote_mac), self.pg2.remote_ip4, is_add=0) # diff --git a/test/test_nat.py b/test/test_nat.py index 73e9e217..44758906 100644 --- a/test/test_nat.py +++ b/test/test_nat.py @@ -16,6 +16,7 @@ from util import ppp from ipfix import IPFIX, Set, Template, Data, IPFIXDecoder from time import sleep from util import ip4_range +from util import mactobinary class MethodHolder(VppTestCase): @@ -643,7 +644,9 @@ class TestNAT44(MethodHolder): lb_sm.external_port, lb_sm.protocol, lb_sm.vrf_id, - is_add=0) + is_add=0, + local_num=0, + locals=[]) adresses = self.vapi.nat44_address_dump() for addr in adresses: @@ -1869,11 +1872,11 @@ class TestNAT44(MethodHolder): """ NAT44 interfaces without configured IP address """ self.vapi.ip_neighbor_add_del(self.pg7.sw_if_index, - self.pg7.remote_mac, + mactobinary(self.pg7.remote_mac), self.pg7.remote_ip4n, is_static=1) self.vapi.ip_neighbor_add_del(self.pg8.sw_if_index, - self.pg8.remote_mac, + mactobinary(self.pg8.remote_mac), self.pg8.remote_ip4n, is_static=1) @@ -1911,11 +1914,11 @@ class TestNAT44(MethodHolder): """ NAT44 interfaces without configured IP address - 1:1 NAT """ self.vapi.ip_neighbor_add_del(self.pg7.sw_if_index, - self.pg7.remote_mac, + mactobinary(self.pg7.remote_mac), self.pg7.remote_ip4n, is_static=1) self.vapi.ip_neighbor_add_del(self.pg8.sw_if_index, - self.pg8.remote_mac, + mactobinary(self.pg8.remote_mac), self.pg8.remote_ip4n, is_static=1) @@ -1957,11 +1960,11 @@ class TestNAT44(MethodHolder): self.icmp_id_out = 30608 self.vapi.ip_neighbor_add_del(self.pg7.sw_if_index, - self.pg7.remote_mac, + mactobinary(self.pg7.remote_mac), self.pg7.remote_ip4n, is_static=1) self.vapi.ip_neighbor_add_del(self.pg8.sw_if_index, - self.pg8.remote_mac, + mactobinary(self.pg8.remote_mac), self.pg8.remote_ip4n, is_static=1) diff --git a/test/test_papi.py b/test/test_papi.py new file mode 100644 index 00000000..1a5f6ae6 --- /dev/null +++ b/test/test_papi.py @@ -0,0 +1,31 @@ +import binascii +from framework import VppTestCase + +""" TestPAPI is a subclass of VPPTestCase classes. + +Basic test for sanity check of the Python API binding. + +""" + + +class TestPAPI(VppTestCase): + """ PAPI Test Case """ + + @classmethod + def setUpClass(cls): + super(TestPAPI, cls).setUpClass() + cls.v = cls.vapi.papi + + def test_show_version(self): + rv = self.v.show_version() + self.assertEqual(rv.retval, 0) + + def test_show_version_invalid_param(self): + self.assertRaises(ValueError, self.v.show_version, foobar='foo') + + def test_u8_array(self): + rv = self.v.get_node_index(node_name='ip4-lookup') + self.assertEqual(rv.retval, 0) + node_name = 'X' * 100 + self.assertRaises(ValueError, self.v.get_node_index, + node_name=node_name) diff --git a/test/vpp_papi_provider.py b/test/vpp_papi_provider.py index b6759ec3..2eab6c6e 100644 --- a/test/vpp_papi_provider.py +++ b/test/vpp_papi_provider.py @@ -1290,7 +1290,7 @@ class VppPapiProvider(object): protocol, vrf_id=0, local_num=0, - locals=None, + locals=[], is_add=1): """Add/delete NAT44 load balancing static mapping @@ -2004,7 +2004,7 @@ class VppPapiProvider(object): eid, eid_prefix_len=0, vni=0, - rlocs=None, + rlocs=[], rlocs_num=0, is_src_dst=0, is_add=1): -- cgit 1.2.3-korg