This patch exposes the c function expr_parse_string() to be called via python. The motivation for this is so that clients interfacing with ovn can call this method in order to validate the data they are writting to ovn.
Previously, there were several bugs in the neutron/ovn integration that went unnoticed due to the client writing invalid data. This should hopefully help catch errors like this earlier as it can now be detected on the client side and an error can be raised. --- ovn/lib/actions.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++ ovn/lib/actions.h | 3 ++ python/automake.mk | 4 +- python/ovs/_ovn-utils.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++ python/setup.py | 31 +++++++++++++- tests/automake.mk | 4 +- tests/test-ovn-utils.at | 13 ++++++ tests/test-ovn-utils.py | 33 +++++++++++++++ tests/test-ovn.c | 104 ----------------------------------------------- tests/testsuite.at | 1 + 10 files changed, 295 insertions(+), 107 deletions(-) create mode 100644 python/ovs/_ovn-utils.c create mode 100644 tests/test-ovn-utils.at create mode 100644 tests/test-ovn-utils.py diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 52c74e6..06f413d 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -1002,3 +1002,108 @@ actions_parse_string(const char *s, const struct action_params *ap, return error; } + +void +create_symtab(struct shash *symtab) +{ + shash_init(symtab); + + /* Reserve a pair of registers for the logical inport and outport. A full + * 32-bit register each is bigger than we need, but the expression code + * doesn't yet support string fields that occupy less than a full OXM. */ + expr_symtab_add_string(symtab, "inport", MFF_REG6, NULL); + expr_symtab_add_string(symtab, "outport", MFF_REG7, NULL); + + expr_symtab_add_field(symtab, "xreg0", MFF_XREG0, NULL, false); + expr_symtab_add_field(symtab, "xreg1", MFF_XREG1, NULL, false); + expr_symtab_add_field(symtab, "xreg2", MFF_XREG2, NULL, false); + + expr_symtab_add_subfield(symtab, "reg0", NULL, "xreg0[32..63]"); + expr_symtab_add_subfield(symtab, "reg1", NULL, "xreg0[0..31]"); + expr_symtab_add_subfield(symtab, "reg2", NULL, "xreg1[32..63]"); + expr_symtab_add_subfield(symtab, "reg3", NULL, "xreg1[0..31]"); + expr_symtab_add_subfield(symtab, "reg4", NULL, "xreg2[32..63]"); + expr_symtab_add_subfield(symtab, "reg5", NULL, "xreg2[0..31]"); + + expr_symtab_add_field(symtab, "eth.src", MFF_ETH_SRC, NULL, false); + expr_symtab_add_field(symtab, "eth.dst", MFF_ETH_DST, NULL, false); + expr_symtab_add_field(symtab, "eth.type", MFF_ETH_TYPE, NULL, true); + + expr_symtab_add_field(symtab, "vlan.tci", MFF_VLAN_TCI, NULL, false); + expr_symtab_add_predicate(symtab, "vlan.present", "vlan.tci[12]"); + expr_symtab_add_subfield(symtab, "vlan.pcp", "vlan.present", + "vlan.tci[13..15]"); + expr_symtab_add_subfield(symtab, "vlan.vid", "vlan.present", + "vlan.tci[0..11]"); + + expr_symtab_add_predicate(symtab, "ip4", "eth.type == 0x800"); + expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd"); + expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6"); + expr_symtab_add_field(symtab, "ip.proto", MFF_IP_PROTO, "ip", true); + expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP, "ip", false); + expr_symtab_add_field(symtab, "ip.ecn", MFF_IP_ECN, "ip", false); + expr_symtab_add_field(symtab, "ip.ttl", MFF_IP_TTL, "ip", false); + + expr_symtab_add_field(symtab, "ip4.src", MFF_IPV4_SRC, "ip4", false); + expr_symtab_add_field(symtab, "ip4.dst", MFF_IPV4_DST, "ip4", false); + + expr_symtab_add_predicate(symtab, "icmp4", "ip4 && ip.proto == 1"); + expr_symtab_add_field(symtab, "icmp4.type", MFF_ICMPV4_TYPE, "icmp4", + false); + expr_symtab_add_field(symtab, "icmp4.code", MFF_ICMPV4_CODE, "icmp4", + false); + + expr_symtab_add_field(symtab, "ip6.src", MFF_IPV6_SRC, "ip6", false); + expr_symtab_add_field(symtab, "ip6.dst", MFF_IPV6_DST, "ip6", false); + expr_symtab_add_field(symtab, "ip6.label", MFF_IPV6_LABEL, "ip6", false); + + expr_symtab_add_predicate(symtab, "icmp6", "ip6 && ip.proto == 58"); + expr_symtab_add_field(symtab, "icmp6.type", MFF_ICMPV6_TYPE, "icmp6", + true); + expr_symtab_add_field(symtab, "icmp6.code", MFF_ICMPV6_CODE, "icmp6", + true); + + expr_symtab_add_predicate(symtab, "icmp", "icmp4 || icmp6"); + + expr_symtab_add_field(symtab, "ip.frag", MFF_IP_FRAG, "ip", false); + expr_symtab_add_predicate(symtab, "ip.is_frag", "ip.frag[0]"); + expr_symtab_add_predicate(symtab, "ip.later_frag", "ip.frag[1]"); + expr_symtab_add_predicate(symtab, "ip.first_frag", "ip.is_frag && !ip.later_frag"); + + expr_symtab_add_predicate(symtab, "arp", "eth.type == 0x806"); + expr_symtab_add_field(symtab, "arp.op", MFF_ARP_OP, "arp", false); + expr_symtab_add_field(symtab, "arp.spa", MFF_ARP_SPA, "arp", false); + expr_symtab_add_field(symtab, "arp.sha", MFF_ARP_SHA, "arp", false); + expr_symtab_add_field(symtab, "arp.tpa", MFF_ARP_TPA, "arp", false); + expr_symtab_add_field(symtab, "arp.tha", MFF_ARP_THA, "arp", false); + + expr_symtab_add_predicate(symtab, "nd", "icmp6.type == {135, 136} && icmp6.code == 0"); + expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET, "nd", false); + expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL, + "nd && icmp6.type == 135", false); + expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL, + "nd && icmp6.type == 136", false); + + expr_symtab_add_predicate(symtab, "tcp", "ip.proto == 6"); + expr_symtab_add_field(symtab, "tcp.src", MFF_TCP_SRC, "tcp", false); + expr_symtab_add_field(symtab, "tcp.dst", MFF_TCP_DST, "tcp", false); + expr_symtab_add_field(symtab, "tcp.flags", MFF_TCP_FLAGS, "tcp", false); + + expr_symtab_add_predicate(symtab, "udp", "ip.proto == 17"); + expr_symtab_add_field(symtab, "udp.src", MFF_UDP_SRC, "udp", false); + expr_symtab_add_field(symtab, "udp.dst", MFF_UDP_DST, "udp", false); + + expr_symtab_add_predicate(symtab, "sctp", "ip.proto == 132"); + expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false); + expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false); + + /* For negative testing. */ + expr_symtab_add_field(symtab, "bad_prereq", MFF_XREG0, "xyzzy", false); + expr_symtab_add_field(symtab, "self_recurse", MFF_XREG0, + "self_recurse != 0", false); + expr_symtab_add_field(symtab, "mutual_recurse_1", MFF_XREG0, + "mutual_recurse_2 != 0", false); + expr_symtab_add_field(symtab, "mutual_recurse_2", MFF_XREG0, + "mutual_recurse_1 != 0", false); + expr_symtab_add_string(symtab, "big_string", MFF_XREG0, NULL); +} diff --git a/ovn/lib/actions.h b/ovn/lib/actions.h index f49e15e..aaf0082 100644 --- a/ovn/lib/actions.h +++ b/ovn/lib/actions.h @@ -111,4 +111,7 @@ char *actions_parse_string(const char *s, const struct action_params *, struct ofpbuf *ofpacts, struct expr **prereqsp) OVS_WARN_UNUSED_RESULT; +void create_symtab(struct shash *symtab); + + #endif /* ovn/actions.h */ diff --git a/python/automake.mk b/python/automake.mk index ecad39d..64aabd9 100644 --- a/python/automake.mk +++ b/python/automake.mk @@ -47,7 +47,9 @@ EXTRA_DIST += \ python/setup.py # C extension support. -EXTRA_DIST += python/ovs/_json.c +EXTRA_DIST += \ + python/ovs/_json.c \ + python/ovs/_ovn-utils.c PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) EXTRA_DIST += $(PYFILES) diff --git a/python/ovs/_ovn-utils.c b/python/ovs/_ovn-utils.c new file mode 100644 index 0000000..d1b4193 --- /dev/null +++ b/python/ovs/_ovn-utils.c @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2016 Nicira, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include <Python.h> +#include "ovn/lib/actions.h" +#include "ovn/lib/expr.h" +#include "ovn/lib/lex.h" +#include "shash.h" + +#if PY_MAJOR_VERSION >= 3 +#define IS_PY3K +#endif + +#ifdef IS_PY3K +PyMODINIT_FUNC PyInit_ovn_utils(void); +#else +void initovn_utils(void); +#endif + +static char parse_match_docs[] = + "Specify match string to validate\n"; + +static PyObject* parse_match(PyObject* self OVS_UNUSED, PyObject *args) +{ + char *match_string; + PyObject *error_string; + + if (!PyArg_ParseTuple(args, "s", &match_string)) { + return Py_BuildValue("s", "Unable to parse input"); + } + + struct shash symtab; + create_symtab(&symtab); + struct expr *expr; + char *error; + expr = expr_parse_string(match_string, &symtab, &error); + + expr_destroy(expr); + expr_symtab_destroy(&symtab); + shash_destroy(&symtab); + if(error) { +#ifdef IS_PY3K + error_string = PyUnicode_FromString(error); +#else + error_string = PyString_FromString(error); +#endif + free(error); + return error_string; + } + Py_RETURN_NONE; +} + + +static PyMethodDef ovn_utils_funcs[] = { + {"parse_match", parse_match, METH_VARARGS, parse_match_docs}, + {NULL} +}; + +#ifdef IS_PY3K +static struct PyModuleDef moduledef = { + PyModuleDef_HEAD_INIT, + "ovs.ovn_utils", /* m_name */ + "OVN helper utilities", /* m_doc */ + 0, /* m_size */ + ovn_utils_funcs, /* m_methods */ + 0, /* m_slots */ + 0, /* m_traverse */ + 0, /* m_clear */ + 0, /* m_free */ +}; +#define INITERROR return NULL +#else /* !IS_PY3K */ +#define INITERROR return +#endif + +PyMODINIT_FUNC +#ifdef IS_PY3K +PyInit_ovn_utils(void) +#else +initovn_utils(void) +#endif +{ +#ifdef IS_PY3K + PyObject *m; + m = PyModule_Create(&moduledef); + return m; +#else + Py_InitModule3("ovs.ovn_utils", ovn_utils_funcs, + "OVN helper utilities"); +#endif +} diff --git a/python/setup.py b/python/setup.py index 19c1f18..5269151 100644 --- a/python/setup.py +++ b/python/setup.py @@ -77,7 +77,36 @@ setup_args = dict( 'Programming Language :: Python :: 3.4', ], ext_modules=[setuptools.Extension("ovs._json", sources=["ovs/_json.c"], - libraries=['openvswitch'])], + libraries=['openvswitch']), + setuptools.Extension("ovs.ovn_utils", + # XXX there must be a better way + # to do this? + libraries=['openvswitch'], + extra_compile_args=([ + '-Wstrict-prototypes', + '-Wall', + '-Wextra', + '-shared', + '-Wno-sign-compare', + '-Wpointer-arith', + '-Wformat-security', + '-Wswitch-enum', + '-Wunused-parameter', + '-Wbad-function-cast', + '-Wcast-align', + '-Wmissing-prototypes', + '-Wmissing-field-initializers', + '-fno-strict-aliasing', + '-std=gnu99', + '-DHAVE_CONFIG_H']), + include_dirs=['../include/', + '../lib/', + '../ovn/lib', + '../'], + sources=["ovs/_ovn-utils.c", + "../ovn/lib/lex.c", + "../ovn/lib/expr.c", + "../ovn/lib/actions.c"])], cmdclass={'build_ext': try_build_ext}, ) diff --git a/tests/automake.mk b/tests/automake.mk index 8b24221..cf93e9f 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -92,7 +92,8 @@ TESTSUITE_AT = \ tests/ovn-nbctl.at \ tests/ovn-sbctl.at \ tests/ovn-controller.at \ - tests/ovn-controller-vtep.at + tests/ovn-controller-vtep.at \ + tests/test-ovn-utils.at SYSTEM_KMOD_TESTSUITE_AT = \ tests/system-common-macros.at \ @@ -381,6 +382,7 @@ CHECK_PYFILES = \ tests/test-jsonrpc.py \ tests/test-l7.py \ tests/test-ovsdb.py \ + tests/test-ovn-utils.py \ tests/test-reconnect.py \ tests/MockXenAPI.py \ tests/test-unix-socket.py \ diff --git a/tests/test-ovn-utils.at b/tests/test-ovn-utils.at new file mode 100644 index 0000000..10a00b9 --- /dev/null +++ b/tests/test-ovn-utils.at @@ -0,0 +1,13 @@ +AT_BANNER([Test OVN Utils]) + +m4_define([TEST_OVN_UTILS], + [AT_SETUP([test-ovn-utils - $1]) + AT_SKIP_IF([test $2 = no]) + AT_KEYWORDS([ovnutils]) + AT_CHECK([$3 $srcdir/test-ovn-utils.py], + [0],[ignore], [ignore]) + AT_CLEANUP]) + + +TEST_OVN_UTILS([Python2], [$HAVE_PYTHON], [$PYTHON]) +TEST_OVN_UTILS([Python3], [$HAVE_PYTHON3], [$PYTHON3]) diff --git a/tests/test-ovn-utils.py b/tests/test-ovn-utils.py new file mode 100644 index 0000000..fecf2c0 --- /dev/null +++ b/tests/test-ovn-utils.py @@ -0,0 +1,33 @@ +# Copyright (c) 2016 Nicira, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at: +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from ovs import ovn_utils + + +class TestOvnUtils(unittest.TestCase): + + def test_parse_match_fail(self): + expected = "Syntax error at `a' expecting field name." + result = ovn_utils.parse_match("a") + self.assertEqual(result, expected) + + def test_parse_match_success(self): + result = ovn_utils.parse_match( + 'outport == "25560992-3f36-4a8b-bc19-e3f84188ef33" && ip4 && udp') + self.assertEqual(result, None) + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 18e5aca..bcfcfa3 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -135,110 +135,6 @@ test_lex(struct ovs_cmdl_context *ctx OVS_UNUSED) ds_destroy(&output); } -static void -create_symtab(struct shash *symtab) -{ - shash_init(symtab); - - /* Reserve a pair of registers for the logical inport and outport. A full - * 32-bit register each is bigger than we need, but the expression code - * doesn't yet support string fields that occupy less than a full OXM. */ - expr_symtab_add_string(symtab, "inport", MFF_REG6, NULL); - expr_symtab_add_string(symtab, "outport", MFF_REG7, NULL); - - expr_symtab_add_field(symtab, "xreg0", MFF_XREG0, NULL, false); - expr_symtab_add_field(symtab, "xreg1", MFF_XREG1, NULL, false); - expr_symtab_add_field(symtab, "xreg2", MFF_XREG2, NULL, false); - - expr_symtab_add_subfield(symtab, "reg0", NULL, "xreg0[32..63]"); - expr_symtab_add_subfield(symtab, "reg1", NULL, "xreg0[0..31]"); - expr_symtab_add_subfield(symtab, "reg2", NULL, "xreg1[32..63]"); - expr_symtab_add_subfield(symtab, "reg3", NULL, "xreg1[0..31]"); - expr_symtab_add_subfield(symtab, "reg4", NULL, "xreg2[32..63]"); - expr_symtab_add_subfield(symtab, "reg5", NULL, "xreg2[0..31]"); - - expr_symtab_add_field(symtab, "eth.src", MFF_ETH_SRC, NULL, false); - expr_symtab_add_field(symtab, "eth.dst", MFF_ETH_DST, NULL, false); - expr_symtab_add_field(symtab, "eth.type", MFF_ETH_TYPE, NULL, true); - - expr_symtab_add_field(symtab, "vlan.tci", MFF_VLAN_TCI, NULL, false); - expr_symtab_add_predicate(symtab, "vlan.present", "vlan.tci[12]"); - expr_symtab_add_subfield(symtab, "vlan.pcp", "vlan.present", - "vlan.tci[13..15]"); - expr_symtab_add_subfield(symtab, "vlan.vid", "vlan.present", - "vlan.tci[0..11]"); - - expr_symtab_add_predicate(symtab, "ip4", "eth.type == 0x800"); - expr_symtab_add_predicate(symtab, "ip6", "eth.type == 0x86dd"); - expr_symtab_add_predicate(symtab, "ip", "ip4 || ip6"); - expr_symtab_add_field(symtab, "ip.proto", MFF_IP_PROTO, "ip", true); - expr_symtab_add_field(symtab, "ip.dscp", MFF_IP_DSCP, "ip", false); - expr_symtab_add_field(symtab, "ip.ecn", MFF_IP_ECN, "ip", false); - expr_symtab_add_field(symtab, "ip.ttl", MFF_IP_TTL, "ip", false); - - expr_symtab_add_field(symtab, "ip4.src", MFF_IPV4_SRC, "ip4", false); - expr_symtab_add_field(symtab, "ip4.dst", MFF_IPV4_DST, "ip4", false); - - expr_symtab_add_predicate(symtab, "icmp4", "ip4 && ip.proto == 1"); - expr_symtab_add_field(symtab, "icmp4.type", MFF_ICMPV4_TYPE, "icmp4", - false); - expr_symtab_add_field(symtab, "icmp4.code", MFF_ICMPV4_CODE, "icmp4", - false); - - expr_symtab_add_field(symtab, "ip6.src", MFF_IPV6_SRC, "ip6", false); - expr_symtab_add_field(symtab, "ip6.dst", MFF_IPV6_DST, "ip6", false); - expr_symtab_add_field(symtab, "ip6.label", MFF_IPV6_LABEL, "ip6", false); - - expr_symtab_add_predicate(symtab, "icmp6", "ip6 && ip.proto == 58"); - expr_symtab_add_field(symtab, "icmp6.type", MFF_ICMPV6_TYPE, "icmp6", - true); - expr_symtab_add_field(symtab, "icmp6.code", MFF_ICMPV6_CODE, "icmp6", - true); - - expr_symtab_add_predicate(symtab, "icmp", "icmp4 || icmp6"); - - expr_symtab_add_field(symtab, "ip.frag", MFF_IP_FRAG, "ip", false); - expr_symtab_add_predicate(symtab, "ip.is_frag", "ip.frag[0]"); - expr_symtab_add_predicate(symtab, "ip.later_frag", "ip.frag[1]"); - expr_symtab_add_predicate(symtab, "ip.first_frag", "ip.is_frag && !ip.later_frag"); - - expr_symtab_add_predicate(symtab, "arp", "eth.type == 0x806"); - expr_symtab_add_field(symtab, "arp.op", MFF_ARP_OP, "arp", false); - expr_symtab_add_field(symtab, "arp.spa", MFF_ARP_SPA, "arp", false); - expr_symtab_add_field(symtab, "arp.sha", MFF_ARP_SHA, "arp", false); - expr_symtab_add_field(symtab, "arp.tpa", MFF_ARP_TPA, "arp", false); - expr_symtab_add_field(symtab, "arp.tha", MFF_ARP_THA, "arp", false); - - expr_symtab_add_predicate(symtab, "nd", "icmp6.type == {135, 136} && icmp6.code == 0"); - expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET, "nd", false); - expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL, - "nd && icmp6.type == 135", false); - expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL, - "nd && icmp6.type == 136", false); - - expr_symtab_add_predicate(symtab, "tcp", "ip.proto == 6"); - expr_symtab_add_field(symtab, "tcp.src", MFF_TCP_SRC, "tcp", false); - expr_symtab_add_field(symtab, "tcp.dst", MFF_TCP_DST, "tcp", false); - expr_symtab_add_field(symtab, "tcp.flags", MFF_TCP_FLAGS, "tcp", false); - - expr_symtab_add_predicate(symtab, "udp", "ip.proto == 17"); - expr_symtab_add_field(symtab, "udp.src", MFF_UDP_SRC, "udp", false); - expr_symtab_add_field(symtab, "udp.dst", MFF_UDP_DST, "udp", false); - - expr_symtab_add_predicate(symtab, "sctp", "ip.proto == 132"); - expr_symtab_add_field(symtab, "sctp.src", MFF_SCTP_SRC, "sctp", false); - expr_symtab_add_field(symtab, "sctp.dst", MFF_SCTP_DST, "sctp", false); - - /* For negative testing. */ - expr_symtab_add_field(symtab, "bad_prereq", MFF_XREG0, "xyzzy", false); - expr_symtab_add_field(symtab, "self_recurse", MFF_XREG0, - "self_recurse != 0", false); - expr_symtab_add_field(symtab, "mutual_recurse_1", MFF_XREG0, - "mutual_recurse_2 != 0", false); - expr_symtab_add_field(symtab, "mutual_recurse_2", MFF_XREG0, - "mutual_recurse_1 != 0", false); - expr_symtab_add_string(symtab, "big_string", MFF_XREG0, NULL); -} static void create_dhcp_opts(struct hmap *dhcp_opts) diff --git a/tests/testsuite.at b/tests/testsuite.at index 6b3fb25..f3f35aa 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -75,3 +75,4 @@ m4_include([tests/ovn-nbctl.at]) m4_include([tests/ovn-sbctl.at]) m4_include([tests/ovn-controller.at]) m4_include([tests/ovn-controller-vtep.at]) +m4_include([tests/test-ovn-utils.at]) -- 2.7.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev