| Message ID | 1467322039-25138-1-git-send-email-aaronorosen@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Here's the v2 version of this (Sorry this is a new email message I was hoping that this would be grouped together with the previous thread). This patch adds python3 support and hooks in the testcases with autotest. I was planning on reusing the existing test inputs that test-ovn parse-expr uses but I decided against that because I only want parse_match() to return something if there is an error otherwise just none - test-ovn parse-expr returns the normalized version of the match which we don't need. Let me know what you guys think of this patch when you get a chance. Best, Aaron On Thu, Jun 30, 2016 at 2:27 PM, Aaron Rosen <aaronorosen@gmail.com> wrote: > 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" <dev-bounces@openvswitch.org> wrote on 06/30/2016 04:27:19 PM: > From: Aaron Rosen <aaronorosen@gmail.com> > To: dev@openvswitch.org > Cc: Aaron Rosen <aaronorosen@gmail.com> > Date: 06/30/2016 04:29 PM > Subject: [ovs-dev] [PATCH] ovn: expose c validator to python > Sent by: "dev" <dev-bounces@openvswitch.org> > > 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. > --- As an FYI, if this is v2 of the patch, please indicate that in the subject line by using the -v or --reroll-count option to git-format-patch. That makes keeping track of superceded patches in the patch queue easier... Thanks in advance, Ryan
Yea sorry about that. I'll try and make sure I do that next time around. Aaron On Fri, Jul 1, 2016 at 7:39 AM, Ryan Moats <rmoats@us.ibm.com> wrote: > "dev" <dev-bounces@openvswitch.org> wrote on 06/30/2016 04:27:19 PM: > > > From: Aaron Rosen <aaronorosen@gmail.com> > > To: dev@openvswitch.org > > Cc: Aaron Rosen <aaronorosen@gmail.com> > > Date: 06/30/2016 04:29 PM > > Subject: [ovs-dev] [PATCH] ovn: expose c validator to python > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > 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. > > --- > > As an FYI, if this is v2 of the patch, please indicate that in the > subject line by using the -v or --reroll-count option to > git-format-patch. That makes keeping track of superceded patches in > the patch queue easier... > > Thanks in advance, > Ryan > > >
On Thu, Jun 30, 2016 at 02:27:19PM -0700, Aaron Rosen wrote: > 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. I'm OK with the idea of moving the code to create a symtab for testing purposes into the OVN library, but I think that it should go into expr.[ch] since that's what implements expressions, and it should have a name and comment that makes it clear that it's for testing. I don't yet understand how to use this, though. When I run a normal "make check" I get failures like this: # -*- compilation -*- 2132. test-ovn-utils.at:12: testing test-ovn-utils - Python2 ... ../../tests/test-ovn-utils.at:12: $PYTHON $srcdir/test-ovn-utils.py stderr: Traceback (most recent call last): File "../../../../tests/test-ovn-utils.py", line 17, in <module> from ovs import ovn_utils ImportError: cannot import name ovn_utils stdout: ../../tests/test-ovn-utils.at:12: exit code was 1, expected 0 2132. test-ovn-utils.at:12: 2132. test-ovn-utils - Python2 (test-ovn-utils.at:12): FAILED (test-ovn-utils.at:12) Is some extra step required? Thanks, Ben.
Thanks for the review. Response inline On Sun, Jul 3, 2016 at 12:04 PM, Ben Pfaff <blp@ovn.org> wrote: > On Thu, Jun 30, 2016 at 02:27:19PM -0700, Aaron Rosen wrote: > > 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. > > I'm OK with the idea of moving the code to create a symtab for testing > purposes into the OVN library, but I think that it should go into > expr.[ch] since that's what implements expressions, and it should have a > name and comment that makes it clear that it's for testing. > Sounds good to me I'll make this update. > > I don't yet understand how to use this, though. When I run a normal > "make check" I get failures like this: > > # -*- compilation -*- > 2132. test-ovn-utils.at:12: testing test-ovn-utils - Python2 ... > ../../tests/test-ovn-utils.at:12: $PYTHON $srcdir/test-ovn-utils.py > stderr: > Traceback (most recent call last): > File "../../../../tests/test-ovn-utils.py", line 17, in <module> > from ovs import ovn_utils > ImportError: cannot import name ovn_utils > stdout: > ../../tests/test-ovn-utils.at:12: exit code was 1, expected 0 > 2132. test-ovn-utils.at:12: 2132. test-ovn-utils - Python2 ( > test-ovn-utils.at:12): FAILED (test-ovn-utils.at:12) > > Is some extra step required? Looks like you need to run: sudo python setup.py install and sudo python3 setup.py install (if you want the python3 tests to run). inside of ovs/python first. Is there a good place were we could add a hook to make that occur? I think the other issue that we are facing before that is in order to build these C extensions it currently is relying on the openvswitch-dev package being installed on the system rather than able to use it from the current source tree: https://github.com/openvswitch/ovs/blob/master/python/setup.py#L80 I believe Terry Wilson said that he was working on a fix for that though. > Thanks, > > Ben. >
On Mon, Jul 04, 2016 at 12:17:54PM -0700, Aaron Rosen wrote: > On Sun, Jul 3, 2016 at 12:04 PM, Ben Pfaff <blp@ovn.org> wrote: > > > On Thu, Jun 30, 2016 at 02:27:19PM -0700, Aaron Rosen wrote: > > > 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. > > I don't yet understand how to use this, though. When I run a normal > > "make check" I get failures like this: > > > > # -*- compilation -*- > > 2132. test-ovn-utils.at:12: testing test-ovn-utils - Python2 ... > > ../../tests/test-ovn-utils.at:12: $PYTHON $srcdir/test-ovn-utils.py > > stderr: > > Traceback (most recent call last): > > File "../../../../tests/test-ovn-utils.py", line 17, in <module> > > from ovs import ovn_utils > > ImportError: cannot import name ovn_utils > > stdout: > > ../../tests/test-ovn-utils.at:12: exit code was 1, expected 0 > > 2132. test-ovn-utils.at:12: 2132. test-ovn-utils - Python2 ( > > test-ovn-utils.at:12): FAILED (test-ovn-utils.at:12) > > > > Is some extra step required? > > > Looks like you need to run: > > sudo python setup.py install > and > sudo python3 setup.py install (if you want the python3 tests to run). > > inside of ovs/python first. > > Is there a good place were we could add a hook to make that occur? I think > the other issue that we are facing before that is in order to build these C > extensions it currently is relying on the openvswitch-dev package being > installed on the system rather than able to use it from the current source > tree: https://github.com/openvswitch/ovs/blob/master/python/setup.py#L80 These changes are going to break the OVS build process, then. Building and testing OVS doesn't currently require root. It sounds like a step backwards to start requiring it.
Yea I totally agree. In the latest patch I sent out it skips these tests if the user doesn't have the ovs python lib installed. I'll try and see what I can do to fix this. On Wed, Jul 13, 2016 at 10:56 AM, Ben Pfaff <blp@ovn.org> wrote: > On Mon, Jul 04, 2016 at 12:17:54PM -0700, Aaron Rosen wrote: > > On Sun, Jul 3, 2016 at 12:04 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > > On Thu, Jun 30, 2016 at 02:27:19PM -0700, Aaron Rosen wrote: > > > > 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. > > > > I don't yet understand how to use this, though. When I run a normal > > > "make check" I get failures like this: > > > > > > # -*- compilation -*- > > > 2132. test-ovn-utils.at:12: testing test-ovn-utils - Python2 ... > > > ../../tests/test-ovn-utils.at:12: $PYTHON > $srcdir/test-ovn-utils.py > > > stderr: > > > Traceback (most recent call last): > > > File "../../../../tests/test-ovn-utils.py", line 17, in <module> > > > from ovs import ovn_utils > > > ImportError: cannot import name ovn_utils > > > stdout: > > > ../../tests/test-ovn-utils.at:12: exit code was 1, expected 0 > > > 2132. test-ovn-utils.at:12: 2132. test-ovn-utils - Python2 ( > > > test-ovn-utils.at:12): FAILED (test-ovn-utils.at:12) > > > > > > Is some extra step required? > > > > > > Looks like you need to run: > > > > sudo python setup.py install > > and > > sudo python3 setup.py install (if you want the python3 tests to run). > > > > inside of ovs/python first. > > > > Is there a good place were we could add a hook to make that occur? I > think > > the other issue that we are facing before that is in order to build > these C > > extensions it currently is relying on the openvswitch-dev package being > > installed on the system rather than able to use it from the current > source > > tree: https://github.com/openvswitch/ovs/blob/master/python/setup.py#L80 > > These changes are going to break the OVS build process, then. Building > and testing OVS doesn't currently require root. It sounds like a step > backwards to start requiring it. >
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])